diff --git a/PENDING.md b/PENDING.md index b6e159544..fb41e1438 100644 --- a/PENDING.md +++ b/PENDING.md @@ -21,6 +21,7 @@ BREAKING CHANGES * Gaia * [\#3457](https://github.com/cosmos/cosmos-sdk/issues/3457) Changed governance tally validatorGovInfo to use sdk.Int power instead of sdk.Dec + * Reintroduce OR semantics for tx fees * SDK * \#2513 Tendermint updates are adjusted by 10^-6 relative to staking tokens, diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index d94d064bf..fcea83893 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -89,13 +89,13 @@ func TestGaiaCLIMinimumFees(t *testing.T) { tests.WaitForNextNBlocksTM(1, f.Port) // Ensure tx w/ correct fees pass - txFees := fmt.Sprintf("--fees=%s,%s", sdk.NewInt64Coin(feeDenom, 2), sdk.NewInt64Coin(fee2Denom, 2)) + txFees := fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 2)) success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fee2Denom, 10), txFees) require.True(f.T, success) tests.WaitForNextNBlocksTM(1, f.Port) // Ensure tx w/ improper fees fails - txFees = fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 5)) + txFees = fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 1)) success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, 10), txFees) require.False(f.T, success) diff --git a/docs/gaia/gaiacli.md b/docs/gaia/gaiacli.md index 381c52f0e..b43f638be 100644 --- a/docs/gaia/gaiacli.md +++ b/docs/gaia/gaiacli.md @@ -137,7 +137,7 @@ the transaction being included in the ledger. Validator's have a minimum gas price (multi-denom) configuration and they use this value when when determining if they should include the transaction in a block during `CheckTx`, where `gasPrices >= minGasPrices`. Note, your transaction must -supply fees that match all the denominations the validator requires. +supply fees that are greater than or equal to __any__ of the denominations the validator requires. __Note__: With such a mechanism in place, validators may start to prioritize txs by `gasPrice` in the mempool, so providing higher fees or gas prices may yield diff --git a/docs/spec/auth/gas_fees.md b/docs/spec/auth/gas_fees.md index 533ccb002..309f590ce 100644 --- a/docs/spec/auth/gas_fees.md +++ b/docs/spec/auth/gas_fees.md @@ -13,7 +13,7 @@ signature verification, as well as costs proportional to the tx size. Operators should set minimum gas prices when starting their nodes. They must set the unit costs of gas in each token denomination they wish to support: -`gaiad start ... --minimum-gas-prices=0.00001steak,0.05photinos` +`gaiad start ... --minimum-gas-prices=0.00001steak;0.05photinos` When adding transactions to mempool or gossipping transactions, validators check if the transaction's gas prices, which are determined by the provided fees, meet diff --git a/server/config/config.go b/server/config/config.go index dc9314729..6afde66f0 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -13,8 +13,8 @@ const ( // BaseConfig defines the server's basic configuration type BaseConfig struct { // The minimum gas prices a validator is willing to accept for processing a - // transaction. A transaction's fees must meet the minimum of each denomination - // specified in this config (e.g. 0.01photino,0.0001stake). + // transaction. A transaction's fees must meet the minimum of any denomination + // specified in this config (e.g. 0.01photino;0.0001stake). MinGasPrices string `mapstructure:"minimum-gas-prices"` } diff --git a/server/config/toml.go b/server/config/toml.go index 7486e171f..1841393ab 100644 --- a/server/config/toml.go +++ b/server/config/toml.go @@ -14,8 +14,8 @@ const defaultConfigTemplate = `# This is a TOML config file. ##### main base config options ##### # The minimum gas prices a validator is willing to accept for processing a -# transaction. A transaction's fees must meet the minimum of each denomination -# specified in this config (e.g. 0.01photino,0.0001stake). +# transaction. A transaction's fees must meet the minimum of any denomination +# specified in this config (e.g. 0.01photino;0.0001stake). minimum-gas-prices = "{{ .BaseConfig.MinGasPrices }}" ` diff --git a/server/start.go b/server/start.go index 0f09f7736..f0718e673 100644 --- a/server/start.go +++ b/server/start.go @@ -50,7 +50,7 @@ func StartCmd(ctx *Context, appCreator AppCreator) *cobra.Command { cmd.Flags().String(flagPruning, "syncable", "Pruning strategy: syncable, nothing, everything") cmd.Flags().String( FlagMinGasPrices, "", - "Minimum gas prices to accept for transactions; All fees in a tx must meet this minimum (e.g. 0.01photino,0.0001stake)", + "Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)", ) // add support for all Tendermint-specific command line options diff --git a/types/coin.go b/types/coin.go index d3d63f2aa..627febca6 100644 --- a/types/coin.go +++ b/types/coin.go @@ -299,6 +299,26 @@ func (coins Coins) IsAllLTE(coinsB Coins) bool { return coinsB.IsAllGTE(coins) } +// IsAnyGTE returns true iff coins contains at least one denom that is present +// at a greater or equal amount in coinsB; it returns false otherwise. +// +// NOTE: IsAnyGTE operates under the invariant that both coin sets are sorted +// by denominations and there exists no zero coins. +func (coins Coins) IsAnyGTE(coinsB Coins) bool { + if len(coinsB) == 0 { + return false + } + + for _, coin := range coins { + amt := coinsB.AmountOf(coin.Denom) + if coin.Amount.GTE(amt) { + return true + } + } + + return false +} + // IsZero returns true if there are no coins or all coins are zero. func (coins Coins) IsZero() bool { for _, coin := range coins { diff --git a/types/coin_test.go b/types/coin_test.go index 3f69ad2cc..52bdc54e9 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -508,3 +508,22 @@ func TestAmountOf(t *testing.T) { assert.Panics(t, func() { cases[0].coins.AmountOf("Invalid") }) } + +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.False(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", two}})) + assert.True(t, Coins{{"a", one}, {"b", two}}.IsAnyGTE(Coins{{"a", two}, {"b", one}})) + assert.True(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", one}})) + assert.True(t, Coins{{"a", two}}.IsAnyGTE(Coins{{"a", one}})) + assert.True(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}})) + assert.True(t, Coins{{"b", two}}.IsAnyGTE(Coins{{"a", one}, {"b", two}})) + assert.False(t, Coins{{"b", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}})) + 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}})) + assert.True(t, Coins{{"x", one}, {"y", one}}.IsAnyGTE(Coins{{"b", one}, {"c", one}, {"y", one}, {"z", one}})) +} diff --git a/types/dec_coin.go b/types/dec_coin.go index cc29c921c..e3222ae32 100644 --- a/types/dec_coin.go +++ b/types/dec_coin.go @@ -2,6 +2,7 @@ package types import ( "fmt" + "regexp" "sort" "strings" @@ -375,7 +376,8 @@ func ParseDecCoins(coinsStr string) (coins DecCoins, err error) { return nil, nil } - coinStrs := strings.Split(coinsStr, ",") + splitRe := regexp.MustCompile(",|;") + coinStrs := splitRe.Split(coinsStr, -1) for _, coinStr := range coinStrs { coin, err := ParseDecCoin(coinStr) if err != nil { diff --git a/types/int.go b/types/int.go index 33654c48a..0ac8737a5 100644 --- a/types/int.go +++ b/types/int.go @@ -16,6 +16,8 @@ func equal(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) == 0 } func gt(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) == 1 } +func gte(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) >= 0 } + func lt(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) == -1 } func lte(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) <= 0 } @@ -188,6 +190,12 @@ func (i Int) GT(i2 Int) bool { return gt(i.i, i2.i) } +// GTE returns true if receiver Int is greater than or equal to the parameter +// Int. +func (i Int) GTE(i2 Int) bool { + return gte(i.i, i2.i) +} + // LT returns true if first Int is lesser than second func (i Int) LT(i2 Int) bool { return lt(i.i, i2.i) diff --git a/x/auth/ante.go b/x/auth/ante.go index 37f159536..90ed43f69 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -345,7 +345,7 @@ func EnsureSufficientMempoolFees(ctx sdk.Context, stdFee StdFee) sdk.Result { requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) } - if !stdFee.Amount.IsAllGTE(requiredFees) { + if !stdFee.Amount.IsAnyGTE(requiredFees) { return sdk.ErrInsufficientFee( fmt.Sprintf( "insufficient fees; got: %q required: %q", stdFee.Amount, requiredFees, diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index 21e9f50cd..9a554080a 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -710,8 +710,8 @@ func TestEnsureSufficientMempoolFees(t *testing.T) { input := setupTestInput() ctx := input.ctx.WithMinGasPrices( sdk.DecCoins{ - sdk.NewDecCoinFromDec("photino", sdk.NewDecWithPrec(1000000, sdk.Precision)), // 0.0001photino - sdk.NewDecCoinFromDec("stake", sdk.NewDecWithPrec(10000, sdk.Precision)), // 0.000001stake + sdk.NewDecCoinFromDec("photino", sdk.NewDecWithPrec(50000000000000, sdk.Precision)), // 0.0001photino + sdk.NewDecCoinFromDec("stake", sdk.NewDecWithPrec(10000000000000, sdk.Precision)), // 0.000001stake }, ) @@ -719,14 +719,16 @@ func TestEnsureSufficientMempoolFees(t *testing.T) { input StdFee expectedOK bool }{ + {NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("photino", 5)}), false}, {NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("stake", 1)}), false}, - {NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("photino", 20)}), false}, + {NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("stake", 2)}), true}, + {NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("photino", 10)}), true}, { NewStdFee( 200000, sdk.Coins{ - sdk.NewInt64Coin("photino", 20), - sdk.NewInt64Coin("stake", 1), + sdk.NewInt64Coin("photino", 10), + sdk.NewInt64Coin("stake", 2), }, ), true, @@ -735,9 +737,9 @@ func TestEnsureSufficientMempoolFees(t *testing.T) { NewStdFee( 200000, sdk.Coins{ - sdk.NewInt64Coin("atom", 2), - sdk.NewInt64Coin("photino", 20), - sdk.NewInt64Coin("stake", 1), + sdk.NewInt64Coin("atom", 5), + sdk.NewInt64Coin("photino", 10), + sdk.NewInt64Coin("stake", 2), }, ), true,