diff --git a/PENDING.md b/PENDING.md index c8734670917a..f3669357c84d 100644 --- a/PENDING.md +++ b/PENDING.md @@ -47,6 +47,7 @@ ### SDK +* [\#3820] Make Coins.IsAllGT() more robust and consistent. * #3801 `baseapp` saftey improvements ### Tendermint diff --git a/types/coin.go b/types/coin.go index 95b30a890932..7d9b78e81c49 100644 --- a/types/coin.go +++ b/types/coin.go @@ -269,6 +269,23 @@ func (coins Coins) safeAdd(coinsB Coins) Coins { } } +// DenomsSubsetOf returns true if receiver's denom set +// is subset of coinsB's denoms. +func (coins Coins) DenomsSubsetOf(coinsB Coins) bool { + // more denoms in B than in receiver + if len(coins) > len(coinsB) { + return false + } + + for _, coin := range coins { + if coinsB.AmountOf(coin.Denom).IsZero() { + return false + } + } + + return true +} + // Sub subtracts a set of coins from another. // // e.g. @@ -294,15 +311,29 @@ func (coins Coins) SafeSub(coinsB Coins) (Coins, bool) { return diff, diff.IsAnyNegative() } -// IsAllGT returns true if for every denom in coins, the denom is present at a -// greater amount in coinsB. +// IsAllGT returns true if for every denom in coinsB, +// the denom is present at a greater amount in coins. func (coins Coins) IsAllGT(coinsB Coins) bool { - diff, _ := coins.SafeSub(coinsB) - if len(diff) == 0 { + if len(coins) == 0 { return false } - return diff.IsAllPositive() + if len(coinsB) == 0 { + return true + } + + if !coinsB.DenomsSubsetOf(coins) { + return false + } + + for _, coinB := range coinsB { + amountA, amountB := coins.AmountOf(coinB.Denom), coinB.Amount + if !amountA.GT(amountB) { + return false + } + } + + return true } // IsAllGTE returns true iff for every denom in coins, the denom is present at diff --git a/types/coin_test.go b/types/coin_test.go index 643d94127fe5..f9268b0190ae 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -276,10 +276,7 @@ func TestCoins(t *testing.T) { mixedCase3 := Coins{ {"gAs", NewInt(1)}, } - empty := Coins{ - {"gold", NewInt(0)}, - } - null := Coins{} + empty := NewCoins() badSort1 := Coins{ {"tree", NewInt(1)}, {"gas", NewInt(1)}, @@ -312,7 +309,7 @@ func TestCoins(t *testing.T) { assert.False(t, mixedCase2.IsValid(), "First Coins denoms contain upper case characters") assert.False(t, mixedCase3.IsValid(), "Single denom in Coins contains upper case characters") assert.True(t, good.IsAllPositive(), "Expected coins to be positive: %v", good) - assert.False(t, null.IsAllPositive(), "Expected coins to not be positive: %v", null) + assert.False(t, empty.IsAllPositive(), "Expected coins to not be positive: %v", empty) assert.True(t, good.IsAllGTE(empty), "Expected %v to be >= %v", good, empty) assert.False(t, good.IsAllLT(empty), "Expected %v to be < %v", good, empty) assert.True(t, empty.IsAllLT(good), "Expected %v to be < %v", empty, good) @@ -331,7 +328,7 @@ func TestCoinsGT(t *testing.T) { assert.True(t, Coins{{testDenom1, one}}.IsAllGT(Coins{})) assert.False(t, Coins{{testDenom1, one}}.IsAllGT(Coins{{testDenom1, one}})) assert.False(t, Coins{{testDenom1, one}}.IsAllGT(Coins{{testDenom2, one}})) - assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllGT(Coins{{testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, two}}.IsAllGT(Coins{{testDenom2, one}})) assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllGT(Coins{{testDenom2, two}})) } @@ -358,7 +355,7 @@ func TestCoinsLT(t *testing.T) { assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom2, one}})) assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom2, two}})) assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom1, one}, {testDenom2, one}})) - assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllLT(Coins{{testDenom1, two}, {testDenom2, two}})) assert.True(t, Coins{}.IsAllLT(Coins{{testDenom1, one}})) } @@ -520,6 +517,46 @@ func TestCoinsIsAnyGTE(t *testing.T) { assert.True(t, Coins{{"xxx", one}, {"yyy", one}}.IsAnyGTE(Coins{{testDenom2, one}, {"ccc", one}, {"yyy", one}, {"zzz", one}})) } +func TestCoinsIsAllGT(t *testing.T) { + one := NewInt(1) + two := NewInt(2) + + assert.False(t, Coins{}.IsAllGT(Coins{})) + assert.True(t, Coins{{testDenom1, one}}.IsAllGT(Coins{})) + assert.False(t, Coins{}.IsAllGT(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGT(Coins{{testDenom1, two}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGT(Coins{{testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, two}}.IsAllGT(Coins{{testDenom1, two}, {testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGT(Coins{{testDenom1, one}})) + assert.True(t, Coins{{testDenom1, two}}.IsAllGT(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGT(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.False(t, Coins{{testDenom2, two}}.IsAllGT(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.False(t, Coins{{testDenom2, one}}.IsAllGT(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, two}}.IsAllGT(Coins{{testDenom1, one}, {testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllGT(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.False(t, Coins{{"xxx", one}, {"yyy", one}}.IsAllGT(Coins{{testDenom2, one}, {"ccc", one}, {"yyy", one}, {"zzz", one}})) +} + +func TestCoinsIsAllGTE(t *testing.T) { + one := NewInt(1) + two := NewInt(2) + + assert.True(t, Coins{}.IsAllGTE(Coins{})) + assert.True(t, Coins{{testDenom1, one}}.IsAllGTE(Coins{})) + assert.False(t, Coins{}.IsAllGTE(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGTE(Coins{{testDenom1, two}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGTE(Coins{{testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, two}}.IsAllGTE(Coins{{testDenom1, two}, {testDenom2, one}})) + assert.True(t, Coins{{testDenom1, one}}.IsAllGTE(Coins{{testDenom1, one}})) + assert.True(t, Coins{{testDenom1, two}}.IsAllGTE(Coins{{testDenom1, one}})) + assert.False(t, Coins{{testDenom1, one}}.IsAllGTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.False(t, Coins{{testDenom2, two}}.IsAllGTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.False(t, Coins{{testDenom2, one}}.IsAllGTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.True(t, Coins{{testDenom1, one}, {testDenom2, two}}.IsAllGTE(Coins{{testDenom1, one}, {testDenom2, one}})) + assert.False(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAllGTE(Coins{{testDenom1, one}, {testDenom2, two}})) + assert.False(t, Coins{{"xxx", one}, {"yyy", one}}.IsAllGTE(Coins{{testDenom2, one}, {"ccc", one}, {"yyy", one}, {"zzz", one}})) +} + func TestNewCoins(t *testing.T) { tenatom := NewInt64Coin("atom", 10) tenbtc := NewInt64Coin("btc", 10)