From 9ffe64dc1eea9721bd1d0a0fb4377f50283823d8 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 7 Mar 2019 09:02:40 +0100 Subject: [PATCH 1/6] failing test --- types/coin_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/types/coin_test.go b/types/coin_test.go index 50dd8bb52bf6..6971e67503c0 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -519,3 +519,43 @@ func TestCoinsIsAnyGTE(t *testing.T) { assert.True(t, Coins{{testDenom1, one}, {testDenom2, one}}.IsAnyGTE(Coins{{testDenom1, one}, {testDenom2, two}})) 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}})) +} From 71ca0e55357b2a82895dd730f8687d25fb5bc2f0 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 8 Mar 2019 00:23:12 +0000 Subject: [PATCH 2/6] Port IsAllGT from safe-coins PR --- types/coin.go | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/types/coin.go b/types/coin.go index 95b30a890932..cf6e4d122e73 100644 --- a/types/coin.go +++ b/types/coin.go @@ -269,6 +269,23 @@ func (coins Coins) safeAdd(coinsB Coins) Coins { } } +// ContainsDenomsOf returns true if coinsB' denom set +// is subset of the receiver's denoms. +func (coins Coins) ContainsDenomsOf(coinsB Coins) bool { + // more denoms in B than in receiver + if len(coinsB) > len(coins) { + return false + } + + for _, coinB := range coinsB { + if coins.AmountOf(coinB.Denom).IsZero() { + return false + } + } + + return true +} + // Sub subtracts a set of coins from another. // // e.g. @@ -297,12 +314,26 @@ func (coins Coins) SafeSub(coinsB Coins) (Coins, bool) { // IsAllGT returns true if for every denom in coins, the denom is present at a // greater amount in coinsB. 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 !coins.ContainsDenomsOf(coinsB) { + 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 From 862cc436f334570932e6efe1730d9083d74cacdd Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 8 Mar 2019 00:24:05 +0000 Subject: [PATCH 3/6] Fix tests --- types/coin_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/types/coin_test.go b/types/coin_test.go index b29b5ece6be4..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}})) } From 38fde7ddaca128f841235f576861513ea13b8811 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 8 Mar 2019 23:15:16 +0000 Subject: [PATCH 4/6] Update PENDING.md --- PENDING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PENDING.md b/PENDING.md index 338afcbacda9..d60c27fdbf8d 100644 --- a/PENDING.md +++ b/PENDING.md @@ -45,6 +45,8 @@ ### SDK +* [\#3820] Make Coins.IsAllGT() more robust and consistent. + ### Tendermint From 3049862325c22bf74c8f23fd00375ea8130374a8 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Mon, 11 Mar 2019 15:26:46 +0100 Subject: [PATCH 5/6] ContainsDenomsOf -> DenomsSubsetOf --- types/coin.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/types/coin.go b/types/coin.go index cf6e4d122e73..740a00d5e71d 100644 --- a/types/coin.go +++ b/types/coin.go @@ -269,16 +269,16 @@ func (coins Coins) safeAdd(coinsB Coins) Coins { } } -// ContainsDenomsOf returns true if coinsB' denom set +// DenomsSubsetOf returns true if coinsB' denom set // is subset of the receiver's denoms. -func (coins Coins) ContainsDenomsOf(coinsB Coins) bool { +func (coins Coins) DenomsSubsetOf(coinsB Coins) bool { // more denoms in B than in receiver - if len(coinsB) > len(coins) { + if len(coins) > len(coinsB) { return false } - for _, coinB := range coinsB { - if coins.AmountOf(coinB.Denom).IsZero() { + for _, coin := range coins { + if coinsB.AmountOf(coin.Denom).IsZero() { return false } } @@ -322,7 +322,7 @@ func (coins Coins) IsAllGT(coinsB Coins) bool { return true } - if !coins.ContainsDenomsOf(coinsB) { + if !coinsB.DenomsSubsetOf(coins) { return false } From e200a2bf35830447814479e48d48f2b3d2dae732 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Tue, 12 Mar 2019 16:02:18 +0100 Subject: [PATCH 6/6] Fix comments --- types/coin.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/types/coin.go b/types/coin.go index 740a00d5e71d..7d9b78e81c49 100644 --- a/types/coin.go +++ b/types/coin.go @@ -269,8 +269,8 @@ func (coins Coins) safeAdd(coinsB Coins) Coins { } } -// DenomsSubsetOf returns true if coinsB' denom set -// is subset of the receiver's denoms. +// 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) { @@ -311,8 +311,8 @@ 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 { if len(coins) == 0 { return false