From 83978758b3b75aeed2d5a8325105be1dbd3699e5 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Thu, 11 Jul 2024 10:09:51 -0400 Subject: [PATCH 1/5] small updates --- x/feemarket/ante/fee.go | 24 +++++++++++++----------- x/feemarket/post/fee.go | 3 ++- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 1e6c6dc..dadc362 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -104,7 +104,8 @@ func (dfd feeMarketCheckDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simula var feeCoin sdk.Coin if simulate && len(feeCoins) == 0 { - feeCoin = sdk.NewCoin(params.FeeDenom, sdkmath.ZeroInt()) + // if simulating and user did not provider a fee - create a dummy value for them + feeCoin = sdk.NewCoin(params.FeeDenom, sdkmath.OneInt()) } else { feeCoin = feeCoins[0] } @@ -128,19 +129,20 @@ func (dfd feeMarketCheckDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simula if err != nil { return ctx, errorsmod.Wrapf(err, "error checking fee") } + } - 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 - } + priorityFee, err := dfd.resolveTxPriorityCoins(ctx, feeCoin, params.FeeDenom) + if err != nil { + return ctx, errorsmod.Wrapf(err, "error resolving fee priority") + } - ctx = ctx.WithPriority(GetTxPriority(priorityFee, int64(gas), baseGasPrice)) + 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) } diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 0433d0b..1b59755 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -85,7 +85,8 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul var feeCoin sdk.Coin if simulate && len(feeCoins) == 0 { - feeCoin = sdk.NewCoin(params.FeeDenom, math.ZeroInt()) + // if simulating and user did not provider a fee - create a dummy value for them + feeCoin = sdk.NewCoin(params.FeeDenom, math.OneInt()) } else { feeCoin = feeCoins[0] } From be995b07fd1b0985796220769955679c77283a45 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Thu, 11 Jul 2024 10:19:41 -0400 Subject: [PATCH 2/5] protect --- x/feemarket/ante/fee.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index dadc362..737951d 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -222,6 +222,16 @@ const ( // 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 { + // protections from dividing by 0 + + if gasLimit == 0 { + return 0 + } + + if currentGasPrice.IsZero() { + return fee.Amount.Int64() + } + effectiveGasPrice := fee.Amount.ToLegacyDec().QuoInt64(gasLimit) normalizedGasPrice := effectiveGasPrice.Quo(currentGasPrice.Amount) scaledGasPrice := normalizedGasPrice.MulInt64(int64(math.Pow10(gasPricePrecision))) From c566b3c6decba09d730aefdebc21078f7bd7240a Mon Sep 17 00:00:00 2001 From: aljo242 Date: Thu, 11 Jul 2024 10:37:55 -0400 Subject: [PATCH 3/5] update ante --- x/feemarket/ante/fee.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feemarket/ante/fee.go b/x/feemarket/ante/fee.go index 737951d..dac2e86 100644 --- a/x/feemarket/ante/fee.go +++ b/x/feemarket/ante/fee.go @@ -223,11 +223,11 @@ const ( // 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 { // protections from dividing by 0 - if gasLimit == 0 { return 0 } + // if the gas price is 0, just use a raw amount if currentGasPrice.IsZero() { return fee.Amount.Int64() } From 0f371c63b8bd406868a65de5c0b953b6af6052f0 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Thu, 11 Jul 2024 10:38:01 -0400 Subject: [PATCH 4/5] testing --- x/feemarket/ante/suite/suite.go | 21 ++++-- x/feemarket/post/fee_test.go | 118 +++++++++++++++++--------------- 2 files changed, 78 insertions(+), 61 deletions(-) diff --git a/x/feemarket/ante/suite/suite.go b/x/feemarket/ante/suite/suite.go index 3489583..595e4eb 100644 --- a/x/feemarket/ante/suite/suite.go +++ b/x/feemarket/ante/suite/suite.go @@ -133,13 +133,14 @@ func (s *TestSuite) SetupHandlers(mock bool) { // TestCase represents a test case used in test tables. type TestCase struct { - Name string - Malleate func(*TestSuite) TestCaseArgs - RunAnte bool - RunPost bool - Simulate bool - ExpPass bool - ExpErr error + Name string + Malleate func(*TestSuite) TestCaseArgs + RunAnte bool + RunPost bool + Simulate bool + ExpPass bool + ExpErr error + ExpectConsumedGas uint64 } type TestCaseArgs struct { @@ -185,6 +186,7 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { if tc.RunPost { newCtx, handleErr = s.PostHandler(s.Ctx, tx, tc.Simulate, true) + } if tc.ExpPass { @@ -193,6 +195,11 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { require.NotNil(t, newCtx) s.Ctx = newCtx + if tc.RunPost { + consumedGas := newCtx.GasMeter().GasConsumed() + require.Equal(t, tc.ExpectConsumedGas, consumedGas) + } + } else { switch { case txErr != nil: diff --git a/x/feemarket/post/fee_test.go b/x/feemarket/post/fee_test.go index 4a6d5a0..4ed1f0e 100644 --- a/x/feemarket/post/fee_test.go +++ b/x/feemarket/post/fee_test.go @@ -124,12 +124,12 @@ func TestSendTip(t *testing.T) { func TestPostHandle(t *testing.T) { // Same data for every test case const ( - baseDenom = "stake" - resolvableDenom = "atom" + baseDenom = "stake" + resolvableDenom = "atom" + expectedConsumedGas = 33339 + gasLimit = expectedConsumedGas ) - // 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())) @@ -204,11 +204,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFee, } }, - RunAnte: true, - RunPost: true, - Simulate: true, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "signer has enough funds, should pass, no tip", @@ -223,11 +224,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFee, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "signer has enough funds, should pass with tip", @@ -242,11 +244,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFeeWithTip, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "signer has enough funds, should pass with tip - simulate", @@ -261,11 +264,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFeeWithTip, } }, - RunAnte: true, - RunPost: true, - Simulate: true, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "signer has enough funds, should pass, no tip - resolvable denom", @@ -280,11 +284,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validResolvableFee, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "signer has enough funds, should pass, no tip - resolvable denom - simulate", @@ -299,11 +304,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validResolvableFee, } }, - RunAnte: true, - RunPost: true, - Simulate: true, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "signer has enough funds, should pass with tip - resolvable denom", @@ -318,11 +324,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validResolvableFeeWithTip, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "signer has enough funds, should pass with tip - resolvable denom - simulate", @@ -337,11 +344,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validResolvableFeeWithTip, } }, - RunAnte: true, - RunPost: true, - Simulate: true, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: true, + Simulate: true, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "0 gas given should pass in simulate - no fee", @@ -354,11 +362,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: nil, } }, - RunAnte: true, - RunPost: false, - Simulate: true, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: false, + Simulate: true, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "0 gas given should pass in simulate - fee", @@ -371,11 +380,12 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFee, } }, - RunAnte: true, - RunPost: false, - Simulate: true, - ExpPass: true, - ExpErr: nil, + RunAnte: true, + RunPost: false, + Simulate: true, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, }, { Name: "no fee - fail", From eb691e434f56b2c13a184bcc2c67c901f41bbde2 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Thu, 11 Jul 2024 10:50:28 -0400 Subject: [PATCH 5/5] lint fix --- .github/workflows/ictest.yml | 2 +- .github/workflows/lint.yml | 2 +- .github/workflows/test.yml | 6 +++--- x/feemarket/ante/suite/suite.go | 1 - 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ictest.yml b/.github/workflows/ictest.yml index c5a7c23..9c17ac7 100644 --- a/.github/workflows/ictest.yml +++ b/.github/workflows/ictest.yml @@ -12,7 +12,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v4 with: - go-version: 1.22.3 + go-version: 1.22.5 cache: true cache-dependency-path: go.sum - uses: technote-space/get-diff-action@v6.1.2 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d12397a..ed6676d 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -18,7 +18,7 @@ jobs: steps: - uses: actions/setup-go@v4 with: - go-version: 1.22.3 + go-version: 1.22.5 - uses: actions/checkout@v4 - name: golangci-lint uses: golangci/golangci-lint-action@v3 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 83a648c..4c352cf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,7 +20,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v4 with: - go-version: 1.22.3 + go-version: 1.22.5 cache: true cache-dependency-path: go.sum - uses: technote-space/get-diff-action@v6.1.2 @@ -40,7 +40,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v4 with: - go-version: 1.22.3 + go-version: 1.22.5 cache: true cache-dependency-path: go.sum - uses: technote-space/get-diff-action@v6.1.2 @@ -60,7 +60,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v4 with: - go-version: 1.22.3 + go-version: 1.22.5 cache: true cache-dependency-path: go.sum - uses: technote-space/get-diff-action@v6.1.2 diff --git a/x/feemarket/ante/suite/suite.go b/x/feemarket/ante/suite/suite.go index 595e4eb..d996eef 100644 --- a/x/feemarket/ante/suite/suite.go +++ b/x/feemarket/ante/suite/suite.go @@ -186,7 +186,6 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { if tc.RunPost { newCtx, handleErr = s.PostHandler(s.Ctx, tx, tc.Simulate, true) - } if tc.ExpPass {