fix: tx prioritization (#101)

* fix

* beauty

* fix

* fix

* fix

* test

* lint

* fix

tests/e2e/consumer.go

@@ -34,16 +34,17 @@ func CCVChainConstructor(t *testing.T, spec *interchaintest.ChainSpec) []*cosmos
 				Version:       providerVersion,
 				NumValidators: &providerNumValidators,
 				ChainConfig: ibc.ChainConfig{
-					GasPrices:      "1uatom",
-					GasAdjustment:  1.5,
+					GasPrices:      "0uatom",
+					GasAdjustment:  0.0,
 					ChainID:        providerChainID,
 					TrustingPeriod: "336h",
 					ModifyGenesis: cosmos.ModifyGenesis(
 						[]cosmos.GenesisKV{
 							cosmos.NewGenesisKV("app_state.provider.params.blocks_per_epoch", "1"),
 						},
 					),
-				}},
+				},
+			},
 		},
 	)
 

x/feemarket/ante/fee.go

@@ -113,23 +113,50 @@ func (dfd feeMarketCheckDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simula
 	ctx = ctx.WithMinGasPrices(sdk.NewDecCoins(minGasPrice))
 
 	if !simulate {
-		_, tip, err := CheckTxFee(ctx, minGasPrice, feeCoin, feeGas, true)
+		_, _, err := CheckTxFee(ctx, minGasPrice, feeCoin, feeGas, true)
 		if err != nil {
 			return ctx, errorsmod.Wrapf(err, "error checking fee")
 		}
-		ctx = ctx.WithPriority(getTxPriority(tip, int64(gas)))
+
+		priorityFee, err := dfd.resolveTxPriorityCoins(ctx, feeCoin, params.FeeDenom)
+		if err != nil {
+			return ctx, errorsmod.Wrapf(err, "error resolving fee priority")
+		}
+
+		baseGasPrice, err := dfd.feemarketKeeper.GetMinGasPrice(ctx, params.FeeDenom)
+		if err != nil {
+			return ctx, err
+		}
+
+		ctx = ctx.WithPriority(GetTxPriority(priorityFee, int64(gas), baseGasPrice))
 		return next(ctx, tx, simulate)
 	}
 	return next(ctx, tx, simulate)
 }
 
+// resolveTxPriorityCoins converts the coins to the proper denom used for tx prioritization calculation.
+func (dfd feeMarketCheckDecorator) resolveTxPriorityCoins(ctx sdk.Context, fee sdk.Coin, baseDenom string) (sdk.Coin, error) {
+	if fee.Denom == baseDenom {
+		return fee, nil
+	}
+
+	feeDec := sdk.NewDecCoinFromCoin(fee)
+	convertedDec, err := dfd.feemarketKeeper.GetDenomResolver().ConvertToDenom(ctx, feeDec, baseDenom)
+	if err != nil {
+		return sdk.Coin{}, err
+	}
+
+	// truncate down
+	return sdk.NewCoin(baseDenom, convertedDec.Amount.TruncateInt()), nil
+}
+
 // CheckTxFee implements the logic for the fee market to check if a Tx has provided sufficient
 // fees given the current state of the fee market. Returns an error if insufficient fees.
-func CheckTxFee(ctx sdk.Context, minGasPrice sdk.DecCoin, feeCoin sdk.Coin, feeGas int64, isAnte bool) (payCoin sdk.Coin, tip sdk.Coin, err error) {
+func CheckTxFee(ctx sdk.Context, gasPrice sdk.DecCoin, feeCoin sdk.Coin, feeGas int64, isAnte bool) (payCoin sdk.Coin, tip sdk.Coin, err error) {
 	payCoin = feeCoin
 
 	// Ensure that the provided fees meet the minimum
-	if !minGasPrice.IsZero() {
+	if !gasPrice.IsZero() {
 		var (
 			requiredFee sdk.Coin
 			consumedFee sdk.Coin
@@ -141,50 +168,58 @@ func CheckTxFee(ctx sdk.Context, minGasPrice sdk.DecCoin, feeCoin sdk.Coin, feeG
 		gcDec := sdkmath.LegacyNewDec(gasConsumed)
 		glDec := sdkmath.LegacyNewDec(feeGas)
 
-		consumedFeeAmount := minGasPrice.Amount.Mul(gcDec)
-		limitFee := minGasPrice.Amount.Mul(glDec)
-		consumedFee = sdk.NewCoin(minGasPrice.Denom, consumedFeeAmount.Ceil().RoundInt())
-		requiredFee = sdk.NewCoin(minGasPrice.Denom, limitFee.Ceil().RoundInt())
+		consumedFeeAmount := gasPrice.Amount.Mul(gcDec)
+		limitFee := gasPrice.Amount.Mul(glDec)
+
+		consumedFee = sdk.NewCoin(gasPrice.Denom, consumedFeeAmount.Ceil().RoundInt())
+		requiredFee = sdk.NewCoin(gasPrice.Denom, limitFee.Ceil().RoundInt())
 
 		if !payCoin.IsGTE(requiredFee) {
 			return sdk.Coin{}, sdk.Coin{}, sdkerrors.ErrInsufficientFee.Wrapf(
 				"got: %s required: %s, minGasPrice: %s, gas: %d",
 				payCoin,
 				requiredFee,
-				minGasPrice,
+				gasPrice,
 				gasConsumed,
 			)
 		}
 
 		if isAnte {
 			tip = payCoin.Sub(requiredFee)
-			//  set fee coins to be required amount if checking
 			payCoin = requiredFee
 		} else {
-			// tip is the difference between payCoin and the required fee
-			tip = payCoin.Sub(requiredFee)
-			// set fee coin to be ONLY the consumed amount if we are calculated consumed fee to deduct
+			tip = payCoin.Sub(consumedFee)
 			payCoin = consumedFee
 		}
 	}
 
 	return payCoin, tip, nil
 }
 
-// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price
-// provided in a transaction.
-// NOTE: This implementation should be used with a great consideration as it opens potential attack vectors
-// where txs with multiple coins could not be prioritized as expected.
-func getTxPriority(fee sdk.Coin, gas int64) int64 {
-	var priority int64
-	p := int64(math.MaxInt64)
-	gasPrice := fee.Amount.QuoRaw(gas)
-	if gasPrice.IsInt64() {
-		p = gasPrice.Int64()
-	}
-	if p < priority {
-		priority = p
+const (
+	// gasPricePrecision is the amount of digit precision to scale the gas prices to.
+	gasPricePrecision = 6
+)
+
+// GetTxPriority returns a naive tx priority based on the amount of gas price provided in a transaction.
+//
+// The fee amount is divided by the gasLimit to calculate "Effective Gas Price".
+// This value is then normalized and scaled into an integer, so it can be used as a priority.
+//
+//	effectiveGasPrice = feeAmount / gas limit (denominated in fee per gas)
+//	normalizedGasPrice = effectiveGasPrice / currentGasPrice (floor is 1.  The minimum effective gas price can ever be is current gas price)
+//	scaledGasPrice = normalizedGasPrice * 10 ^ gasPricePrecision (amount of decimal places in the normalized gas price to consider when converting to int64).
+func GetTxPriority(fee sdk.Coin, gasLimit int64, currentGasPrice sdk.DecCoin) int64 {
+	effectiveGasPrice := fee.Amount.ToLegacyDec().QuoInt64(gasLimit)
+	normalizedGasPrice := effectiveGasPrice.Quo(currentGasPrice.Amount)
+	scaledGasPrice := normalizedGasPrice.MulInt64(int64(math.Pow10(gasPricePrecision)))
+
+	// overflow panic protection
+	if scaledGasPrice.GTE(sdkmath.LegacyNewDec(math.MaxInt64)) {
+		return math.MaxInt64
+	} else if scaledGasPrice.LTE(sdkmath.LegacyOneDec()) {
+		return 0
 	}
 
-	return priority
+	return scaledGasPrice.TruncateInt64()
 }

x/feemarket/fuzz/tx_priority_test.go

@@ -0,0 +1,52 @@
+package fuzz_test
+
+import (
+	"math"
+	"testing"
+
+	sdkmath "cosmossdk.io/math"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/require"
+	"pgregory.net/rapid"
+
+	"github.com/skip-mev/feemarket/x/feemarket/ante"
+)
+
+type input struct {
+	payFee          sdk.Coin
+	gasLimit        int64
+	currentGasPrice sdk.DecCoin
+}
+
+// TestGetTxPriority ensures that tx priority is properly bounded
+func TestGetTxPriority(t *testing.T) {
+	rapid.Check(t, func(t *rapid.T) {
+		inputs := createRandomInput(t)
+
+		priority := ante.GetTxPriority(inputs.payFee, inputs.gasLimit, inputs.currentGasPrice)
+		require.GreaterOrEqual(t, priority, int64(0))
+		require.LessOrEqual(t, priority, int64(math.MaxInt64))
+	})
+}
+
+// CreateRandomInput returns a random inputs to the priority function.
+func createRandomInput(t *rapid.T) input {
+	denom := "skip"
+
+	price := rapid.Int64Range(1, 1_000_000_000).Draw(t, "gas price")
+	priceDec := sdkmath.LegacyNewDecWithPrec(price, 6)
+
+	gasLimit := rapid.Int64Range(1_000_000, 1_000_000_000_000).Draw(t, "gas limit")
+
+	if priceDec.MulInt64(gasLimit).GTE(sdkmath.LegacyNewDec(math.MaxInt64)) {
+		t.Fatalf("not supposed to happen")
+	}
+
+	payFeeAmt := rapid.Int64Range(priceDec.MulInt64(gasLimit).TruncateInt64(), math.MaxInt64).Draw(t, "fee amount")
+
+	return input{
+		payFee:          sdk.NewCoin(denom, sdkmath.NewInt(payFeeAmt)),
+		gasLimit:        gasLimit,
+		currentGasPrice: sdk.NewDecCoinFromDec(denom, priceDec),
+	}
+}

x/feemarket/post/fee_test.go

@@ -5,14 +5,12 @@ import (
 	"testing"
 
 	"cosmossdk.io/math"
-
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/stretchr/testify/mock"
 
-	sdk "github.com/cosmos/cosmos-sdk/types"
-
 	antesuite "github.com/skip-mev/feemarket/x/feemarket/ante/suite"
 	"github.com/skip-mev/feemarket/x/feemarket/post"
 	"github.com/skip-mev/feemarket/x/feemarket/types"
@@ -154,7 +152,8 @@ func TestPostHandle(t *testing.T) {
 		resolvableDenom = "atom"
 	)
 
-	gasLimit := antesuite.NewTestGasLimit()
+	// exact cost of transaction
+	gasLimit := uint64(27284)
 	validFeeAmount := types.DefaultMinBaseGasPrice.MulInt64(int64(gasLimit))
 	validFeeAmountWithTip := validFeeAmount.Add(math.LegacyNewDec(100))
 	validFee := sdk.NewCoins(sdk.NewCoin(baseDenom, validFeeAmount.TruncateInt()))