cosmos-sdk: Coins.AmountOf bug

When trying to run gentx on the gaia-10k @zmanian and I encountered an issue where unless the bondDenom was the last token in the array the AmountOf function didn’t return the proper amount (it returned 0). The binary search is not working correctly, it seems like it doesn’t find values in position 0:

midIdx := len(coins) / 2 // 2:1, 3:1, 4:2
coin := coins[midIdx]

if denom < coin.Denom {
  return coins[:midIdx].AmountOf(denom)
} else if denom == coin.Denom {
  return coin.Amount
} else {
  return coins[midIdx+1:].AmountOf(denom)
}

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 17 (14 by maintainers)

Commits related to this issue

Implement genesis file sanitization Add GenesisState.Sanitize(). It sorts genesis accounts and coin sets to ensure genesis state passes validation. gaia app calls GenesisState.Sanitize() on initFrom... — committed to cosmos/cosmos-sdk by deleted user 5 years ago

Most upvoted comments

Let me think on it. My only immediate suggestion would be to add something like “Coins must be sorted by denom” to the godoc for all functions that depend on coins being sorted, like HasCoins() in bank keeper, and AmountOf().

shanev on Feb 1, 2019

Agreed the documentation should be more clear and updated (although I think sorted by denom is implicit and somewhat obvious). However, if we can find a low performance cost way of further guaranteeing safety and avoiding such a situation altogether, I’m all for it. Do you have suggestions? Perhaps we can can implement radix sort on denoms for near constant time.

alexanderbez on Jan 31, 2019

great! I can think of a few options here:

Add a Sanitize method to GenesisState. For now, all this method will do is sort coins. This will be called right after it’s loaded from file and right before accountInGenesis is called.
Or, do as you suggest and manually call Sort in accountInGenesis.

I think the former might be more useful.

alexanderbez on Jan 25, 2019