From ba12f26cebce5964647cc7197f872b9e1bb6e82e Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Tue, 30 Aug 2022 12:17:04 +0200 Subject: [PATCH] feat: compute proper tx prioritization (#1290) ## Description closes: #510 --- ### Author Checklist _All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues._ I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] added appropriate labels to the PR - [ ] targeted the correct branch (see [PR Targeting](https://github.com/umee-network/umee/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist _All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items._ I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- ante/fee.go | 46 ++++++++++++++++++++++++++++++++----------- ante/fee_unit_test.go | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 12 deletions(-) create mode 100644 ante/fee_unit_test.go diff --git a/ante/fee.go b/ante/fee.go index 57ef6be854..3505826ded 100644 --- a/ante/fee.go +++ b/ante/fee.go @@ -1,12 +1,12 @@ package ante import ( - "math" - gbtypes "github.com/Gravity-Bridge/Gravity-Bridge/module/x/gravity/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" + leveragetypes "github.com/umee-network/umee/v3/x/leverage/types" oracletypes "github.com/umee-network/umee/v3/x/oracle/types" ) @@ -50,7 +50,7 @@ func FeeAndPriority(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) { } } - priority := getTxPriority(feeCoins, isOracleOrGravity) + priority := getTxPriority(isOracleOrGravity, msgs) if !chargeFees { return sdk.Coins{}, priority, nil } @@ -59,6 +59,9 @@ func FeeAndPriority(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) { // IsOracleOrGravityTx checks if all messages are oracle messages func IsOracleOrGravityTx(msgs []sdk.Msg) bool { + if len(msgs) == 0 { + return false + } for _, msg := range msgs { switch msg.(type) { case *oracletypes.MsgAggregateExchangeRatePrevote, @@ -89,23 +92,42 @@ func IsOracleOrGravityTx(msgs []sdk.Msg) bool { // getTxPriority returns naive tx priority based on the lowest fee amount (regardless of the // denom) and oracle tx check. -func getTxPriority(fee sdk.Coins, isOracleOrGravity bool) int64 { +// Dirty optimization: since we already check if msgs are oracle or gravity messages, then we +// don't recomupte it again: isOracleOrGravity flag takes a precedence over msgs check. +func getTxPriority( /*fees, gasAmount*/ isOracleOrGravity bool, msgs []sdk.Msg) int64 { var priority int64 - for _, c := range fee { - // TODO: we should better compare amounts - // https://github.com/umee-network/umee/issues/510 + /* TODO: IBC tx prioritization is not stable and we will implement a more general + * tx prioritization once that will be resolved + * https://github.com/umee-network/umee/issues/1289 + for _, c := range fees { p := int64(math.MaxInt64) - if c.Amount.IsInt64() { - p = c.Amount.Int64() + gasPrice := c.Amount.QuoRaw(gasAmount) + if gasPrice.IsInt64() { + p = gasPrice.Int64() } if priority == 0 || p < priority { priority = p } } + */ if isOracleOrGravity { - // TODO: this is a naive version. - // Proper solution will be implemented in https://github.com/umee-network/umee/issues/510 - priority += 100000 + return 100 } + for _, msg := range msgs { + var p int64 + switch msg.(type) { + case *evidencetypes.MsgSubmitEvidence: + p = 90 + case *leveragetypes.MsgLiquidate: + p = 80 + default: + // in case there is a non-prioritized mixed message, we return 0 + return 0 + } + if priority == 0 || p < priority { + priority = p + } + } + return priority } diff --git a/ante/fee_unit_test.go b/ante/fee_unit_test.go new file mode 100644 index 0000000000..2699c95be2 --- /dev/null +++ b/ante/fee_unit_test.go @@ -0,0 +1,37 @@ +package ante + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + bank "github.com/cosmos/cosmos-sdk/x/bank/types" + evidence "github.com/cosmos/cosmos-sdk/x/evidence/types" + "github.com/stretchr/testify/assert" + + leverage "github.com/umee-network/umee/v3/x/leverage/types" +) + +func TestPriority(t *testing.T) { + tcs := []struct { + name string + oracle bool + msgs []sdk.Msg + priority int64 + }{ + {"empty priority 0", false, []sdk.Msg{}, 0}, + {"when oracleOrGravity is set, then tx is max", true, []sdk.Msg{}, 100}, + {"evidence1", true, []sdk.Msg{&evidence.MsgSubmitEvidence{}}, 100}, + {"evidence2", false, []sdk.Msg{&evidence.MsgSubmitEvidence{}}, 90}, + {"evidence3", false, []sdk.Msg{&evidence.MsgSubmitEvidence{}, &evidence.MsgSubmitEvidence{}}, 90}, + {"leverage1", false, []sdk.Msg{&leverage.MsgLiquidate{}}, 80}, + {"leverage-evidence1", false, []sdk.Msg{&leverage.MsgLiquidate{}, &evidence.MsgSubmitEvidence{}}, 80}, + {"leverage-evidence2", false, []sdk.Msg{&evidence.MsgSubmitEvidence{}, &leverage.MsgLiquidate{}}, 80}, + {"mixed1", false, []sdk.Msg{&evidence.MsgSubmitEvidence{}, &leverage.MsgLiquidate{}, &bank.MsgSend{}}, 0}, + {"mixed2", false, []sdk.Msg{&bank.MsgSend{}, &evidence.MsgSubmitEvidence{}, &leverage.MsgLiquidate{}}, 0}, + } + + for _, tc := range tcs { + p := getTxPriority(tc.oracle, tc.msgs) + assert.Equal(t, tc.priority, p, tc.name) + } +}