diff --git a/PENDING.md b/PENDING.md index 9796c00881e4..c399a5099809 100644 --- a/PENDING.md +++ b/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 diff --git a/types/coin.go b/types/coin.go index d7484a66997d..31f0e98a4239 100644 --- a/types/coin.go +++ b/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,10 +184,19 @@ 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 @@ -187,14 +204,27 @@ func (coins Coins) IsGTE(coinsB Coins) bool { 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() { diff --git a/types/coin_test.go b/types/coin_test.go index bc0441279155..ac9fcb391bf7 100644 --- a/types/coin_test.go +++ b/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) diff --git a/x/auth/ante.go b/x/auth/ante.go index 9d5bc5059787..e88a20a43499 100644 --- a/x/auth/ante.go +++ b/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() diff --git a/x/bank/client/cli/sendtx.go b/x/bank/client/cli/sendtx.go index c72ed6fcb956..bae2179766b8 100644 --- a/x/bank/client/cli/sendtx.go +++ b/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) } diff --git a/x/bank/keeper.go b/x/bank/keeper.go index ac7a484d5e37..67c05d32068f 100644 --- a/x/bank/keeper.go +++ b/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. diff --git a/x/gov/keeper.go b/x/gov/keeper.go index 48d9000d8187..862db82744ac 100644 --- a/x/gov/keeper.go +++ b/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 }