Skip to content

Commit

Permalink
feat: Change the default priority mechanism to be based on gas price (#…
Browse files Browse the repository at this point in the history 
…12953)

(cherry picked from commit befd816)

# Conflicts:
#	CHANGELOG.md
#	x/auth/ante/fee_test.go
  • Loading branch information
@yihuang @mergify
yihuang authored and mergify[bot] committed Aug 23, 2022
1 parent afb3def commit 5bf06a0
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 5 deletions.
57 changes: 57 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -47,6 +47,63 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/bank) [#12674](https://github.com/cosmos/cosmos-sdk/pull/12674) Add convenience function `CreatePrefixedAccountStoreKey()` to construct key to access account's balance for a given denom.
* [#12877](https://github.com/cosmos/cosmos-sdk/pull/12877) Bumped cosmossdk.io/math to v1.0.0-beta.3
* [#12693](https://github.com/cosmos/cosmos-sdk/pull/12693) Make sure the order of each node is consistent when emitting proto events.
<<<<<<< HEAD
=======
* [#12455](https://github.com/cosmos/cosmos-sdk/pull/12455) Show attempts count in error for signing.
* [#12886](https://github.com/cosmos/cosmos-sdk/pull/12886) Amortize cost of processing cache KV store
* [#12953](https://github.com/cosmos/cosmos-sdk/pull/12953) Change the default priority mechanism to be based on gas price.

### State Machine Breaking

* (x/bank) [#12610](https://github.com/cosmos/cosmos-sdk/pull/12610) `MsgMultiSend` now allows only a single input.
* (x/bank) [#12630](https://github.com/cosmos/cosmos-sdk/pull/12630) Migrate `x/bank` to self-managed parameters and deprecate its usage of `x/params`.
* (x/auth) [#12475](https://github.com/cosmos/cosmos-sdk/pull/12475) Migrate `x/auth` to self-managed parameters and deprecate its usage of `x/params`.
* (x/slashing) [#12399](https://github.com/cosmos/cosmos-sdk/pull/12399) Migrate `x/slashing` to self-managed parameters and deprecate its usage of `x/params`.
* (x/mint) [#12363](https://github.com/cosmos/cosmos-sdk/pull/12363) Migrate `x/mint` to self-managed parameters and deprecate it's usage of `x/params`.
* (x/distribution) [#12434](https://github.com/cosmos/cosmos-sdk/pull/12434) Migrate `x/distribution` to self-managed parameters and deprecate it's usage of `x/params`.
* (x/crisis) [#12445](https://github.com/cosmos/cosmos-sdk/pull/12445) Migrate `x/crisis` to self-managed parameters and deprecate it's usage of `x/params`.
* (x/gov) [#12631](https://github.com/cosmos/cosmos-sdk/pull/12631) Migrate `x/gov` to self-managed parameters and deprecate it's usage of `x/params`.
* (x/staking) [#12409](https://github.com/cosmos/cosmos-sdk/pull/12409) Migrate `x/staking` to self-managed parameters and deprecate it's usage of `x/params`.
* (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) Move the SendEnabled information out of the Params and into the state store directly.
* (x/gov) [#12771](https://github.com/cosmos/cosmos-sdk/pull/12771) Initial deposit requirement for proposals at submission time.

### API Breaking Changes

* (x/bank) [#12706](https://github.com/cosmos/cosmos-sdk/pull/12706) Removed the `testutil` package from the `x/bank/client` package.
* (simapp) [#12747](https://github.com/cosmos/cosmos-sdk/pull/12747) Remove `simapp.MakeTestEncodingConfig`. Please use `moduletestutil.MakeTestEncodingConfig` (`types/module/testutil`) in tests instead.
* (x/bank) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) `NewSendAuthorization` takes a new argument of an optional list of addresses allowed to receive bank assests via authz MsgSend grant. You can pass `nil` for the same behavior as before, i.e. any recipient is allowed.
* (x/bank) [#12593](https://github.com/cosmos/cosmos-sdk/pull/12593) Add `SpendableCoin` method to `BaseViewKeeper`
* (x/slashing) [#12581](https://github.com/cosmos/cosmos-sdk/pull/12581) Remove `x/slashing` legacy querier.
* (types) [#12355](https://github.com/cosmos/cosmos-sdk/pull/12355) Remove the compile-time `types.DBbackend` variable. Removes usage of the same in server/util.go
* (x/gov) [#12368](https://github.com/cosmos/cosmos-sdk/pull/12369) Gov keeper is now passed by reference instead of copy to make post-construction mutation of Hooks and Proposal Handlers possible at a framework level.
* (simapp) [#12270](https://github.com/cosmos/cosmos-sdk/pull/12270) Remove `invCheckPeriod uint` attribute from `SimApp` struct as per migration of `x/crisis` to app wiring
* (simapp) [#12334](https://github.com/cosmos/cosmos-sdk/pull/12334) Move `simapp.ConvertAddrsToValAddrs` and `simapp.CreateTestPubKeys ` to respectively `simtestutil.ConvertAddrsToValAddrs` and `simtestutil.CreateTestPubKeys` (`testutil/sims`)
* (simapp) [#12312](https://github.com/cosmos/cosmos-sdk/pull/12312) Move `simapp.EmptyAppOptions` to `simtestutil.EmptyAppOptions` (`testutil/sims`)
* (simapp) [#12312](https://github.com/cosmos/cosmos-sdk/pull/12312) Remove `skipUpgradeHeights map[int64]bool` and `homePath string` from `NewSimApp` constructor as per migration of `x/upgrade` to app-wiring.
* (testutil) [#12278](https://github.com/cosmos/cosmos-sdk/pull/12278) Move all functions from `simapp/helpers` to `testutil/sims`
* (testutil) [#12233](https://github.com/cosmos/cosmos-sdk/pull/12233) Move `simapp.TestAddr` to `simtestutil.TestAddr` (`testutil/sims`)
* (x/staking) [#12102](https://github.com/cosmos/cosmos-sdk/pull/12102) Staking keeper now is passed by reference instead of copy. Keeper's SetHooks no longer returns keeper. It updates the keeper in place instead.
* (linting) [#12141](https://github.com/cosmos/cosmos-sdk/pull/12141) Fix usability related linting for database. This means removing the infix Prefix from `prefix.NewPrefixWriter` and such so that it is `prefix.NewWriter` and making `db.DBConnection` and such into `db.Connection`
* (x/distribution) [#12434](https://github.com/cosmos/cosmos-sdk/pull/12434) `x/distribution` module `SetParams` keeper method definition is now updated to return `error`.
* (x/staking) [#12409](https://github.com/cosmos/cosmos-sdk/pull/12409) `x/staking` module `SetParams` keeper method definition is now updated to return `error`.
* (x/crisis) [#12445](https://github.com/cosmos/cosmos-sdk/pull/12445) `x/crisis` module `SetConstantFee` keeper method definition is now updated to return `error`.
* (x/gov) [#12631](https://github.com/cosmos/cosmos-sdk/pull/12631) `x/gov` module refactored to use `Params` as single struct instead of `DepositParams`, `TallyParams` & `VotingParams`.
* (x/gov) [#12631](https://github.com/cosmos/cosmos-sdk/pull/12631) Migrate `x/gov` to self-managed parameters and deprecate it's usage of `x/params`.
* (x/bank) [#12630](https://github.com/cosmos/cosmos-sdk/pull/12630) `x/bank` module `SetParams` keeper method definition is now updated to return `error`.
* (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) Move the SendEnabled information out of the Params and into the state store directly.
The information can now be accessed using the BankKeeper.
Setting can be done using MsgSetSendEnabled as a governance proposal.
A SendEnabled query has been added to both GRPC and CLI.
* (appModule) Remove `Route`, `QuerierRoute` and `LegacyQuerierHandler` from AppModule Interface.
* (x/modules) Remove all LegacyQueries and related code from modules
* (store) [#11825](https://github.com/cosmos/cosmos-sdk/pull/11825) Make extension snapshotter interface safer to use, renamed the util function `WriteExtensionItem` to `WriteExtensionPayload`.
* (codec) [#12964](https://github.com/cosmos/cosmos-sdk/pull/12964) `ProtoCodec.MarshalInterface` now returns an error when serializing unregistered types and a subsequent `ProtoCodec.UnmarshalInterface` would fail.
* (x/staking) [#12973](https://github.com/cosmos/cosmos-sdk/pull/12973) Removed `stakingkeeper.RandomValidator`. Use `testutil.RandSliceElem(r, sk.GetAllValidators(ctx))` instead.

### CLI Breaking Changes

* /
>>>>>>> befd8162e (feat: Change the default priority mechanism to be based on gas price (#12953))
### Bug Fixes

Expand Down
16 changes: 16 additions & 0 deletions x/auth/ante/fee_test.go
View file
Expand Up @@ -57,8 +57,13 @@ func (s *AnteTestSuite) TestEnsureMempoolFees() {
// msg and signatures
msg := testdata.NewTestMsg(addr1)
feeAmount := testdata.NewTestFeeAmount()
<<<<<<< HEAD
gasLimit := testdata.NewTestGasLimit()
s.Require().NoError(s.txBuilder.SetMsgs(msg))
=======
gasLimit := uint64(15)
require.NoError(t, s.txBuilder.SetMsgs(msg))
>>>>>>> befd8162e (feat: Change the default priority mechanism to be based on gas price (#12953))
s.txBuilder.SetFeeAmount(feeAmount)
s.txBuilder.SetGasLimit(gasLimit)

Expand All @@ -67,7 +72,11 @@ func (s *AnteTestSuite) TestEnsureMempoolFees() {
s.Require().NoError(err)

// Set high gas price so standard test fee fails
<<<<<<< HEAD
atomPrice := sdk.NewDecCoinFromDec("atom", sdk.NewDec(200).Quo(sdk.NewDec(100000)))
=======
atomPrice := sdk.NewDecCoinFromDec("atom", math.LegacyNewDec(20))
>>>>>>> befd8162e (feat: Change the default priority mechanism to be based on gas price (#12953))
highGasPrice := []sdk.DecCoin{atomPrice}
s.ctx = s.ctx.WithMinGasPrices(highGasPrice)

Expand Down Expand Up @@ -98,10 +107,17 @@ func (s *AnteTestSuite) TestEnsureMempoolFees() {
s.ctx = s.ctx.WithMinGasPrices(lowGasPrice)

newCtx, err := antehandler(s.ctx, tx, false)
<<<<<<< HEAD
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())
=======
require.Nil(t, err, "Decorator should not have errored on fee higher than local gasPrice")
// Priority is the smallest gas price amount in any denom. Since we have only 1 gas price
// of 10atom, the priority here is 10.
require.Equal(t, int64(10), newCtx.Priority())
>>>>>>> befd8162e (feat: Change the default priority mechanism to be based on gas price (#12953))
}

func (s *AnteTestSuite) TestDeductFees() {
Expand Down
13 changes: 8 additions & 5 deletions x/auth/ante/validator_tx_fee.go
View file
Expand Up @@ -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
Expand Down

0 comments on commit 5bf06a0

Please sign in to comment.