Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use sdk.Dec for quantities #168

Merged
merged 32 commits into from
Aug 7, 2023
Merged

refactor: use sdk.Dec for quantities #168

merged 32 commits into from
Aug 7, 2023

Conversation

kingcre
Copy link
Contributor

@kingcre kingcre commented Jul 19, 2023

Description

Use sdk.Dec instead of sdk.Int(also sdk.DecCoin instead of sdk.Coin) in x/exchange. This will reduce errors caused by decimal truncation.

Tasks

  • Refactor codebase
  • Remove QueueSendCoins and ExecuteSendCoins, instead use new Escrow type
  • Write tests for decimal quantities

@kingcre kingcre self-assigned this Jul 19, 2023
@kingcre kingcre marked this pull request as ready for review July 19, 2023 11:40
@crypin
Copy link
Member

crypin commented Jul 31, 2023

Please let me know when it ready for review

@kingcre
Copy link
Contributor Author

kingcre commented Jul 31, 2023

@crypin It's now ready

x/amm/keeper/invariants.go Show resolved Hide resolved
x/amm/simulation/operations.go Show resolved Hide resolved
Comment on lines 170 to +171
} else if amtInDiff.IsNegative() { // sanity check
//panic(amtInDiff)
panic(amtInDiff)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this part is likely to happen, I will share the detailed scenario again as soon as it is sorted out

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

@kingcre
Copy link
Contributor Author

kingcre commented Aug 7, 2023

Merging this PR and the remaining issues will be handled in separate issues.

@kingcre kingcre merged commit 5b2a644 into amm-module Aug 7, 2023
17 checks passed
@kingcre kingcre deleted the dec-matching-2 branch August 22, 2023 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants