R4R: Fix coins.IsLT() impl

PENDING.md

@@ -69,5 +69,6 @@ BUG FIXES
 * SDK
  - \#2625 [x/gov] fix AppendTag function usage error
  - \#2677 [x/stake, x/distribution] various staking/distribution fixes as found by the simulator
+ - \#2674 [types] Fix coin.IsLT() impl, coins.IsLT() impl, and renamed coins.Is\* to coins.IsAll\* (see \#2686)
 
 * Tendermint

types/coin.go

@@ -49,7 +49,7 @@ func (coin Coin) IsGTE(other Coin) bool {
 // IsLT returns true if they are the same type and the receiver is
 // a smaller value
 func (coin Coin) IsLT(other Coin) bool {
-	return !coin.IsGTE(other)
+	return coin.SameDenomAs(other) && coin.Amount.LT(other.Amount)
 }
 
 // IsEqual returns true if the two sets of Coins have the same value
@@ -142,7 +142,11 @@ func (coins Coins) Plus(coinsB Coins) Coins {
 		coinA, coinB := coins[indexA], coinsB[indexB]
 		switch strings.Compare(coinA.Denom, coinB.Denom) {
 		case -1:
-			sum = append(sum, coinA)
+			if coinA.IsZero() {
+				// ignore 0 sum coin type
+			} else {
+				sum = append(sum, coinA)
+			}
 			indexA++
 		case 0:
 			if coinA.Amount.Add(coinB.Amount).IsZero() {
@@ -153,7 +157,11 @@ func (coins Coins) Plus(coinsB Coins) Coins {
 			indexA++
 			indexB++
 		case 1:
-			sum = append(sum, coinB)
+			if coinB.IsZero() {
+				// ignore 0 sum coin type
+			} else {
+				sum = append(sum, coinB)
+			}
 			indexB++
 		}
 	}
@@ -176,25 +184,47 @@ func (coins Coins) Minus(coinsB Coins) Coins {
 	return coins.Plus(coinsB.Negative())
 }
 
-// IsGTE returns True iff coins is NonNegative(), and for every
-// currency in coinsB, the currency is present at an equal or greater
-// amount in coinsB
-func (coins Coins) IsGTE(coinsB Coins) bool {
+// IsAllGT returns True iff for every denom in coins, the denom is present at a
+// greater amount in coinsB.
+func (coins Coins) IsAllGT(coinsB Coins) bool {
+	diff := coins.Minus(coinsB)
+	if len(diff) == 0 {
+		return false
+	}
+	return diff.IsPositive()
+}
+
+// IsAllGTE returns True iff for every denom in coins, the denom is present at an
+// equal or greater amount in coinsB.
+func (coins Coins) IsAllGTE(coinsB Coins) bool {
 	diff := coins.Minus(coinsB)
 	if len(diff) == 0 {
 		return true
 	}
 	return diff.IsNotNegative()
 }
 
-// IsLT returns True iff every currency in coins, the currency is
-// present at a smaller amount in coins
-func (coins Coins) IsLT(coinsB Coins) bool {
-	return !coins.IsGTE(coinsB)
+// IsAllLT returns True iff for every denom in coins, the denom is present at
+// a smaller amount in coinsB.
+func (coins Coins) IsAllLT(coinsB Coins) bool {
+	diff := coinsB.Minus(coins)
+	if len(diff) == 0 {
+		return false
+	}
+	return diff.IsPositive()
+}
+
+// IsAllLTE returns True iff for every denom in coins, the denom is present at
+// a smaller or equal amount in coinsB.
+func (coins Coins) IsAllLTE(coinsB Coins) bool {
+	diff := coinsB.Minus(coins)
+	if len(diff) == 0 {
+		return true
+	}
+	return diff.IsNotNegative()
 }
 
-// IsZero returns true if there are no coins
-// or all coins are zero.
+// IsZero returns true if there are no coins or all coins are zero.
 func (coins Coins) IsZero() bool {
 	for _, coin := range coins {
 		if !coin.IsZero() {

types/coin_test.go

@@ -86,7 +86,10 @@ func TestIsLTCoin(t *testing.T) {
 		{NewInt64Coin("A", 1), NewInt64Coin("A", 1), false},
 		{NewInt64Coin("A", 2), NewInt64Coin("A", 1), false},
 		{NewInt64Coin("A", -1), NewInt64Coin("A", 5), true},
-		{NewInt64Coin("a", 0), NewInt64Coin("b", 1), true},
+		{NewInt64Coin("a", 0), NewInt64Coin("b", 1), false},
+		{NewInt64Coin("a", 1), NewInt64Coin("b", 1), false},
+		{NewInt64Coin("a", 1), NewInt64Coin("a", 1), false},
+		{NewInt64Coin("a", 1), NewInt64Coin("a", 2), true},
 	}
 
 	for tcIndex, tc := range cases {
@@ -245,9 +248,9 @@ func TestCoins(t *testing.T) {
 	assert.True(t, good.IsValid(), "Coins are valid")
 	assert.True(t, good.IsPositive(), "Expected coins to be positive: %v", good)
 	assert.False(t, null.IsPositive(), "Expected coins to not be positive: %v", null)
-	assert.True(t, good.IsGTE(empty), "Expected %v to be >= %v", good, empty)
-	assert.False(t, good.IsLT(empty), "Expected %v to be < %v", good, empty)
-	assert.True(t, empty.IsLT(good), "Expected %v to be < %v", empty, good)
+	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)
 	assert.False(t, neg.IsPositive(), "Expected neg coins to not be positive: %v", neg)
 	assert.Zero(t, len(sum), "Expected 0 coins")
 	assert.False(t, badSort1.IsValid(), "Coins are not sorted")
@@ -257,6 +260,60 @@ func TestCoins(t *testing.T) {
 
 }
 
+func TestCoinsGT(t *testing.T) {
+	one := NewInt(1)
+	two := NewInt(2)
+
+	assert.False(t, Coins{}.IsAllGT(Coins{}))
+	assert.True(t, Coins{{"A", one}}.IsAllGT(Coins{}))
+	assert.False(t, Coins{{"A", one}}.IsAllGT(Coins{{"A", one}}))
+	assert.False(t, Coins{{"A", one}}.IsAllGT(Coins{{"B", one}}))
+	assert.True(t, Coins{{"A", one}, {"B", one}}.IsAllGT(Coins{{"B", one}}))
+	assert.False(t, Coins{{"A", one}, {"B", one}}.IsAllGT(Coins{{"B", two}}))
+}
+
+func TestCoinsGTE(t *testing.T) {
+	one := NewInt(1)
+	two := NewInt(2)
+
+	assert.True(t, Coins{}.IsAllGTE(Coins{}))
+	assert.True(t, Coins{{"A", one}}.IsAllGTE(Coins{}))
+	assert.True(t, Coins{{"A", one}}.IsAllGTE(Coins{{"A", one}}))
+	assert.False(t, Coins{{"A", one}}.IsAllGTE(Coins{{"B", one}}))
+	assert.True(t, Coins{{"A", one}, {"B", one}}.IsAllGTE(Coins{{"B", one}}))
+	assert.False(t, Coins{{"A", one}, {"B", one}}.IsAllGTE(Coins{{"B", two}}))
+}
+
+func TestCoinsLT(t *testing.T) {
+	one := NewInt(1)
+	two := NewInt(2)
+
+	assert.False(t, Coins{}.IsAllLT(Coins{}))
+	assert.False(t, Coins{{"A", one}}.IsAllLT(Coins{}))
+	assert.False(t, Coins{{"A", one}}.IsAllLT(Coins{{"A", one}}))
+	assert.False(t, Coins{{"A", one}}.IsAllLT(Coins{{"B", one}}))
+	assert.False(t, Coins{{"A", one}, {"B", one}}.IsAllLT(Coins{{"B", one}}))
+	assert.False(t, Coins{{"A", one}, {"B", one}}.IsAllLT(Coins{{"B", two}}))
+	assert.False(t, Coins{{"A", one}, {"B", one}}.IsAllLT(Coins{{"A", one}, {"B", one}}))
+	assert.True(t, Coins{{"A", one}, {"B", one}}.IsAllLT(Coins{{"A", one}, {"B", two}}))
+	assert.True(t, Coins{}.IsAllLT(Coins{{"A", one}}))
+}
+
+func TestCoinsLTE(t *testing.T) {
+	one := NewInt(1)
+	two := NewInt(2)
+
+	assert.True(t, Coins{}.IsAllLTE(Coins{}))
+	assert.False(t, Coins{{"A", one}}.IsAllLTE(Coins{}))
+	assert.True(t, Coins{{"A", one}}.IsAllLTE(Coins{{"A", one}}))
+	assert.False(t, Coins{{"A", one}}.IsAllLTE(Coins{{"B", one}}))
+	assert.False(t, Coins{{"A", one}, {"B", one}}.IsAllLTE(Coins{{"B", one}}))
+	assert.False(t, Coins{{"A", one}, {"B", one}}.IsAllLTE(Coins{{"B", two}}))
+	assert.True(t, Coins{{"A", one}, {"B", one}}.IsAllLTE(Coins{{"A", one}, {"B", one}}))
+	assert.True(t, Coins{{"A", one}, {"B", one}}.IsAllLTE(Coins{{"A", one}, {"B", two}}))
+	assert.True(t, Coins{}.IsAllLTE(Coins{{"A", one}}))
+}
+
 func TestPlusCoins(t *testing.T) {
 	one := NewInt(1)
 	zero := NewInt(0)

x/auth/ante.go

@@ -282,7 +282,8 @@ func ensureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result {
 	// TODO: Make the gasPrice not a constant, and account for tx size.
 	requiredFees := adjustFeesByGas(ctx.MinimumFees(), stdTx.Fee.Gas)
 
-	if !ctx.MinimumFees().IsZero() && stdTx.Fee.Amount.IsLT(requiredFees) {
+	// NOTE: !A.IsAllGTE(B) is not the same as A.IsAllLT(B).
+	if !ctx.MinimumFees().IsZero() && !stdTx.Fee.Amount.IsAllGTE(requiredFees) {
 		// validators reject any tx from the mempool with less than the minimum fee per gas * gas factor
 		return sdk.ErrInsufficientFee(fmt.Sprintf(
 			"insufficient fee, got: %q required: %q", stdTx.Fee.Amount, requiredFees)).Result()

x/bank/client/cli/sendtx.go

@@ -59,7 +59,7 @@ func SendTxCmd(cdc *codec.Codec) *cobra.Command {
 			}
 
 			// ensure account has enough coins
-			if !account.GetCoins().IsGTE(coins) {
+			if !account.GetCoins().IsAllGTE(coins) {
 				return errors.Errorf("Address %s doesn't have enough coins to pay for this transaction.", from)
 			}
 

x/bank/keeper.go

@@ -190,7 +190,7 @@ func setCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s
 // HasCoins returns whether or not an account has at least amt coins.
 func hasCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) bool {
 	ctx.GasMeter().ConsumeGas(costHasCoins, "hasCoins")
-	return getCoins(ctx, am, addr).IsGTE(amt)
+	return getCoins(ctx, am, addr).IsAllGTE(amt)
 }
 
 // SubtractCoins subtracts amt from the coins at the addr.

x/gov/keeper.go

@@ -385,7 +385,7 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositerAdd
 	// Check if deposit tipped proposal into voting period
 	// Active voting period if so
 	activatedVotingPeriod := false
-	if proposal.GetStatus() == StatusDepositPeriod && proposal.GetTotalDeposit().IsGTE(keeper.GetDepositParams(ctx).MinDeposit) {
+	if proposal.GetStatus() == StatusDepositPeriod && proposal.GetTotalDeposit().IsAllGTE(keeper.GetDepositParams(ctx).MinDeposit) {
 		keeper.activateVotingPeriod(ctx, proposal)
 		activatedVotingPeriod = true
 	}