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

Unexpected "negative coin amount" in DecCoin's Sub, Suggest sort before sub. #5408

Closed
4 tasks
readygo586 opened this issue Dec 15, 2019 · 1 comment · Fixed by #5410
Closed
4 tasks

Unexpected "negative coin amount" in DecCoin's Sub, Suggest sort before sub. #5408

readygo586 opened this issue Dec 15, 2019 · 1 comment · Fixed by #5410
Assignees

Comments

@readygo586
Copy link

Summary of Bug

Upexpected negative coin amount in DecCoin's Sub

When call DecCoins's Sub, if not sort the coins before, an unexpected "negative coin amount" happen.

Version

Cosmos-sdk v0.37.2

Steps to Reproduce

Run the following test code, a panic happens,.

func TestSubDecCoins(t *testing.T){
	decCoins1 := NewDecCoins(Coins{NewCoin("mytoken", NewInt(10)),  NewCoin("btc", NewInt(20)), NewCoin("eth", NewInt(30))})
	decCoins2 := NewDecCoins(Coins{NewCoin("btc", NewInt(10)),  NewCoin("eth", NewInt(15)), NewCoin("mytoken", NewInt(5))})
	diff := decCoins1.Sub(decCoins2)
	require.Equal(t, decCoins2, diff)
}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

Thanks @readygo586!

Unfortunately, our decimal coins implementation didn't receive as much TLC as our coins implementation did. All arithmetic assumes the invariant that the coins are valid and sorted. We need to update documentation and update NewDecCoins.

@fedekunze fedekunze mentioned this issue Dec 16, 2019
11 tasks
@fedekunze fedekunze changed the title Upexpected "negative coin amount" in DecCoin's Sub, Suggest sort before sub. Unexpected "negative coin amount" in DecCoin's Sub, Suggest sort before sub. Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants