From 60e6274d0fdaeb86da4521f7ee8b8b2178a845b5 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 24 Aug 2022 16:45:44 +0200 Subject: [PATCH] feat: Change the default priority mechanism to be based on gas price (backport #12953) (#13006) * feat: Change the default priority mechanism to be based on gas price (#12953) (cherry picked from commit befd8162ee25bec8e231b52d3646430c61c3dafc) # Conflicts: # CHANGELOG.md # x/auth/ante/fee_test.go * fix conflict Co-authored-by: yihuang Co-authored-by: Julien Robert --- CHANGELOG.md | 1 + x/auth/ante/fee_test.go | 10 +++++----- x/auth/ante/validator_tx_fee.go | 13 ++++++++----- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecdf8a1c4f39..846712e58dd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* [#12953](https://github.com/cosmos/cosmos-sdk/pull/12953) Change the default priority mechanism to be based on gas price. * [#12981](https://github.com/cosmos/cosmos-sdk/pull/12981) Return proper error when parsing telemetry configuration. * [#12969](https://github.com/cosmos/cosmos-sdk/pull/12969) Bump Tendermint to `v0.34.21` and IAVL to `v0.19.1`. * [#12886](https://github.com/cosmos/cosmos-sdk/pull/12886) Amortize cost of processing cache KV store. diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index a182b4fa15b1..c63706137e0a 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -57,7 +57,7 @@ func (s *AnteTestSuite) TestEnsureMempoolFees() { // msg and signatures msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() - gasLimit := testdata.NewTestGasLimit() + gasLimit := uint64(15) s.Require().NoError(s.txBuilder.SetMsgs(msg)) s.txBuilder.SetFeeAmount(feeAmount) s.txBuilder.SetGasLimit(gasLimit) @@ -67,7 +67,7 @@ func (s *AnteTestSuite) TestEnsureMempoolFees() { s.Require().NoError(err) // Set high gas price so standard test fee fails - atomPrice := sdk.NewDecCoinFromDec("atom", sdk.NewDec(200).Quo(sdk.NewDec(100000))) + atomPrice := sdk.NewDecCoinFromDec("atom", sdk.NewDec(20)) highGasPrice := []sdk.DecCoin{atomPrice} s.ctx = s.ctx.WithMinGasPrices(highGasPrice) @@ -99,9 +99,9 @@ func (s *AnteTestSuite) TestEnsureMempoolFees() { newCtx, err := antehandler(s.ctx, tx, false) s.Require().Nil(err, "Decorator should not have errored on fee higher than local gasPrice") - // Priority is the smallest amount in any denom. Since we have only 1 fee - // of 150atom, the priority here is 150. - s.Require().Equal(feeAmount.AmountOf("atom").Int64(), newCtx.Priority()) + // Priority is the smallest gas price amount in any denom. Since we have only 1 gas price + // of 10atom, the priority here is 10. + s.Require().Equal(int64(10), newCtx.Priority()) } func (s *AnteTestSuite) TestDeductFees() { diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index b1725d62ddc0..dbb13988806b 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -40,18 +40,21 @@ func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, } } - priority := getTxPriority(feeCoins) + priority := getTxPriority(feeCoins, int64(gas)) return feeCoins, priority, nil } -// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the fee +// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price // provided in a transaction. -func getTxPriority(fee sdk.Coins) int64 { +// NOTE: This implementation should be used with a great consideration as it opens potential attack vectors +// where txs with multiple coins could not be prioritize as expected. +func getTxPriority(fee sdk.Coins, gas int64) int64 { var priority int64 for _, c := range fee { p := int64(math.MaxInt64) - if c.Amount.IsInt64() { - p = c.Amount.Int64() + gasPrice := c.Amount.QuoRaw(gas) + if gasPrice.IsInt64() { + p = gasPrice.Int64() } if priority == 0 || p < priority { priority = p