From ed2b6bd9a730a0558f2740f09587b272d42ca893 Mon Sep 17 00:00:00 2001 From: Zaki Manian Date: Tue, 8 Jan 2019 08:21:54 -0800 Subject: [PATCH] Multiple fees in min fees to OR instead of AND (#3239) --- cmd/gaia/cli_test/cli_test.go | 23 ++++++++++++-- types/coin.go | 60 ++++++++++++++++++++++++++++++++++- types/coin_test.go | 22 +++++++++++++ x/auth/ante.go | 2 +- 4 files changed, 103 insertions(+), 4 deletions(-) diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index 15ebb14d82..93e411a7c3 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -40,7 +40,7 @@ func TestGaiaCLIMinimumFees(t *testing.T) { flags := fmt.Sprintf("--home=%s --node=%v --chain-id=%v", gaiacliHome, servAddr, chainID) // start gaiad server with minimum fees - proc := tests.GoExecuteTWithStdout(t, fmt.Sprintf("gaiad start --home=%s --rpc.laddr=%v --p2p.laddr=%v --minimum_fees=2feetoken", gaiadHome, servAddr, p2pAddr)) + proc := tests.GoExecuteTWithStdout(t, fmt.Sprintf("gaiad start --home=%s --rpc.laddr=%v --p2p.laddr=%v --minimum_fees=2%s,2feetoken", gaiadHome, servAddr, p2pAddr, stakeTypes.DefaultBondDenom)) defer proc.Stop(false) tests.WaitForTMStart(port) @@ -52,9 +52,28 @@ func TestGaiaCLIMinimumFees(t *testing.T) { fooAcc := executeGetAccount(t, fmt.Sprintf("gaiacli query account %s %v", fooAddr, flags)) require.Equal(t, int64(50), fooAcc.GetCoins().AmountOf(stakeTypes.DefaultBondDenom).Int64()) + // Ensure that a tx with no fees fails success := executeWrite(t, fmt.Sprintf( "gaiacli tx send %v --amount=10%s --to=%s --from=foo", flags, stakeTypes.DefaultBondDenom, barAddr), app.DefaultKeyPass) require.False(t, success) + + // Ensure tx with correct fees (stake) pass + success = executeWrite(t, fmt.Sprintf( + "gaiacli tx send %v --amount=10%s --to=%s --from=foo --fees=23%s", flags, stakeTypes.DefaultBondDenom, barAddr, stakeTypes.DefaultBondDenom), app.DefaultKeyPass) + require.True(t, success) + tests.WaitForNextNBlocksTM(1, port) + + // Ensure tx with correct fees (feetoken) pass + success = executeWrite(t, fmt.Sprintf( + "gaiacli tx send %v --amount=10feetoken --to=%s --from=foo --fees=23feetoken", flags, barAddr), app.DefaultKeyPass) + require.True(t, success) + tests.WaitForNextNBlocksTM(1, port) + + // Ensure tx with improper fees fails + success = executeWrite(t, fmt.Sprintf( + "gaiacli tx send %v --amount=10%s --to=%s --from=foo --fees=2footoken", flags, stakeTypes.DefaultBondDenom, barAddr), app.DefaultKeyPass) + require.False(t, success) + cleanupDirs(gaiadHome, gaiacliHome) } @@ -732,7 +751,7 @@ func initializeFixtures(t *testing.T) (chainID, servAddr, port, gaiadHome, gaiac chainID = executeInit(t, fmt.Sprintf("gaiad init -o --moniker=foo --home=%s", gaiadHome)) executeWriteCheckErr(t, fmt.Sprintf( - "gaiad add-genesis-account %s 150%s,1000footoken --home=%s", fooAddr, stakeTypes.DefaultBondDenom, gaiadHome)) + "gaiad add-genesis-account %s 150%s,1000footoken,1000feetoken --home=%s", fooAddr, stakeTypes.DefaultBondDenom, gaiadHome)) executeWrite(t, fmt.Sprintf("cat %s%sconfig%sgenesis.json", gaiadHome, string(os.PathSeparator), string(os.PathSeparator))) executeWriteCheckErr(t, fmt.Sprintf( "gaiad gentx --name=foo --home=%s --home-client=%s", gaiadHome, gaiacliHome), app.DefaultKeyPass) diff --git a/types/coin.go b/types/coin.go index 8c504a83b2..c3f2ecfbf5 100644 --- a/types/coin.go +++ b/types/coin.go @@ -265,7 +265,29 @@ func (coins Coins) SafeMinus(coinsB Coins) (Coins, bool) { return diff, !diff.IsNotNegative() } -// IsAllGT returns true iff for every denom in coins, the denom is present at a +// IsAnyGT returns true if coins contains at least one denom +// that is present at a smaller amount in coinsB; it +// returns false otherwise. +func (coins Coins) IsAnyGT(coinsB Coins) bool { + intersection := coins.intersect(coinsB) + if intersection.Empty() { + return false + } + + diff, _ := intersection.SafeMinus(coinsB) + if len(diff) == 0 { + return false + } + + for _, coin := range diff { + if coin.IsPositive() { + return true + } + } + return false +} + +// 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.SafeMinus(coinsB) @@ -276,6 +298,28 @@ func (coins Coins) IsAllGT(coinsB Coins) bool { return diff.IsPositive() } +// IsAnyGT returns true if coins contains at least one denom +// that is present at a smaller or equal amount in coinsB; it +// returns false otherwise. +func (coins Coins) IsAnyGTE(coinsB Coins) bool { + intersection := coins.intersect(coinsB) + if intersection.Empty() { + return false + } + + diff, _ := intersection.SafeMinus(coinsB) + if len(diff) == 0 || len(diff) < len(intersection) { // zero diff is removed from the diff set + return true + } + + for _, coin := range diff { + if coin.IsNotNegative() { + return true + } + } + return false +} + // 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 { @@ -430,6 +474,20 @@ func removeZeroCoins(coins Coins) Coins { return coins[:i] } +func (coins Coins) intersect(coinsB Coins) Coins { + intersection := Coins{} + for _, coin := range coins { + for _, bCoin := range coinsB { + if coin.Denom == bCoin.Denom { + intersection = append(intersection, coin) + break + } + } + } + + return intersection +} + //----------------------------------------------------------------------------- // Sort interface diff --git a/types/coin_test.go b/types/coin_test.go index 3f69ad2ccf..67ae4437ca 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -368,6 +368,28 @@ func TestCoinsLTE(t *testing.T) { assert.True(t, Coins{}.IsAllLTE(Coins{{"a", one}})) } +func TestCoinsIsAnyGT(t *testing.T) { + one := NewInt(1) + two := NewInt(2) + assert.False(t, Coins{}.IsAnyGT(Coins{})) + assert.False(t, Coins{{"a", one}}.IsAnyGT(Coins{})) + assert.False(t, Coins{}.IsAnyGT(Coins{{"a", one}})) + assert.False(t, Coins{{"a", one}}.IsAnyGT(Coins{{"a", one}})) + assert.True(t, Coins{{"a", one}, {"b", two}}.IsAnyGT(Coins{{"a", one}, {"b", one}})) + assert.True(t, Coins{{"a", two}, {"b", one}}.IsAnyGT(Coins{{"a", one}, {"b", two}})) +} + +func TestCoinsIsAnyGTE(t *testing.T) { + one := NewInt(1) + two := NewInt(2) + assert.False(t, Coins{}.IsAnyGTE(Coins{})) + assert.False(t, Coins{{"a", one}}.IsAnyGTE(Coins{})) + assert.False(t, Coins{}.IsAnyGTE(Coins{{"a", one}})) + assert.True(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", one}})) + assert.True(t, Coins{{"a", one}, {"b", two}}.IsAnyGTE(Coins{{"a", one}, {"b", one}})) + assert.True(t, Coins{{"a", one}, {"b", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}})) +} + func TestParse(t *testing.T) { one := NewInt(1) diff --git a/x/auth/ante.go b/x/auth/ante.go index 60c50f3e69..7470f3198e 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -294,7 +294,7 @@ func EnsureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result { requiredFees := adjustFeesByGas(ctx.MinimumFees(), stdTx.Fee.Gas) // NOTE: !A.IsAllGTE(B) is not the same as A.IsAllLT(B). - if !ctx.MinimumFees().IsZero() && !stdTx.Fee.Amount.IsAllGTE(requiredFees) { + if !ctx.MinimumFees().IsZero() && !stdTx.Fee.Amount.IsAnyGTE(requiredFees) { // validators reject any tx from the mempool with less than the minimum fee per gas * gas factor return sdk.ErrInsufficientFee( fmt.Sprintf(