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

Coin.IsGTE bad logic #27

Closed
rigelrozanski opened this issue Feb 10, 2017 · 0 comments · Fixed by #26
Closed

Coin.IsGTE bad logic #27

rigelrozanski opened this issue Feb 10, 2017 · 0 comments · Fixed by #26
Labels

Comments

@rigelrozanski
Copy link
Contributor

https://github.com/tendermint/basecoin/blob/master/types/coin.go#L96-L102

assumptions

  • IsGTE means Greater Than Or Equal (>=)
  • A wallet which contains only 0 of coin 'X' is equal to an empty wallet (AKA have zero of a coin is like not having any reference to that coin in your wallet)

Orig code:

func (coinsA Coins) IsGTE(coinsB Coins) bool {
	diff := coinsA.Minus(coinsB)
	if len(diff) == 0 {
		return true
	}
	return diff.IsPositive()
}

This function will incorrectly return false under the circumstance that coinA and coinB contain different coins but of an amount of zero. Under this circumstance diff != 0 because there are different coins, however diff.IsPositive() returns false because it's not positive it is zero. The fix is to simply replace diff.IsPositive() with diff.IsNonnegative()

sahith-narahari pushed a commit to vitwit/cosmos-sdk that referenced this issue Apr 29, 2020
faddat referenced this issue in notional-labs/cosmos-sdk Dec 3, 2021
* Significantly lower number of heap allocations that should be required by events

* Use EmitEvent instead of EmitEvents
joeabbey referenced this issue in joeabbey/cosmos-sdk Jan 21, 2022
* Significantly lower number of heap allocations that should be required by events

* Use EmitEvent instead of EmitEvents
Raumo0 pushed a commit to mapofzones/cosmos-sdk that referenced this issue Feb 13, 2022
Thunnini referenced this issue in Thunnini/cosmos-sdk Feb 28, 2022
* Significantly lower number of heap allocations that should be required by events

* Use EmitEvent instead of EmitEvents
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants