diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index 4f6224b..d04287f 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -169,7 +169,7 @@ func TestAnteHandleMock(t *testing.T) { } }, RunAnte: true, - RunPost: true, + RunPost: false, Simulate: false, ExpPass: false, ExpErr: types.ErrNoFeeCoins, @@ -187,7 +187,7 @@ func TestAnteHandleMock(t *testing.T) { } }, RunAnte: true, - RunPost: true, + RunPost: false, Simulate: false, ExpPass: false, ExpErr: sdkerrors.ErrOutOfGas, @@ -390,7 +390,7 @@ func TestAnteHandle(t *testing.T) { } }, RunAnte: true, - RunPost: true, + RunPost: false, Simulate: false, ExpPass: false, ExpErr: types.ErrNoFeeCoins, @@ -407,12 +407,13 @@ func TestAnteHandle(t *testing.T) { FeeAmount: nil, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: sdkerrors.ErrOutOfGas, - Mock: false, + RunAnte: true, + RunPost: false, + Simulate: false, + MsgRunSuccess: true, + ExpPass: false, + ExpErr: sdkerrors.ErrOutOfGas, + Mock: false, }, } diff --git a/x/feemarket/ante/suite/suite.go b/x/feemarket/ante/suite/suite.go index 373d4ea..a7b6df2 100644 --- a/x/feemarket/ante/suite/suite.go +++ b/x/feemarket/ante/suite/suite.go @@ -185,6 +185,7 @@ type TestCase struct { StateUpdate func(*TestSuite) RunAnte bool RunPost bool + MsgRunSuccess bool Simulate bool ExpPass bool ExpErr error @@ -210,8 +211,8 @@ func (s *TestSuite) DeliverMsgs(t *testing.T, privs []cryptotypes.PrivKey, msgs s.TxBuilder.SetFeeAmount(feeAmount) s.TxBuilder.SetGasLimit(gasLimit) - tx, txErr := s.CreateTestTx(privs, accNums, accSeqs, chainID) - require.NoError(t, txErr) + tx, txCreationErr := s.CreateTestTx(privs, accNums, accSeqs, chainID) + require.NoError(t, txCreationErr) return s.AnteHandler(s.Ctx, tx, simulate) } @@ -223,7 +224,7 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { // Theoretically speaking, ante handler unit tests should only test // ante handlers, but here we sometimes also test the tx creation // process. - tx, txErr := s.CreateTestTx(args.Privs, args.AccNums, args.AccSeqs, args.ChainID) + tx, txCreationErr := s.CreateTestTx(args.Privs, args.AccNums, args.AccSeqs, args.ChainID) var ( newCtx sdk.Context @@ -243,8 +244,8 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { tc.StateUpdate(s) } - if tc.RunPost { - newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, anteErr == nil) + if tc.RunPost && anteErr == nil { + newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, tc.MsgRunSuccess) } if tc.DistributeFees && !tc.Simulate && args.FeeAmount != nil { @@ -253,7 +254,7 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { } if tc.ExpPass { - require.NoError(t, txErr) + require.NoError(t, txCreationErr) require.NoError(t, anteErr) require.NoError(t, postErr) require.NotNil(t, newCtx) @@ -266,9 +267,9 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { } else { switch { - case txErr != nil: - require.Error(t, txErr) - require.ErrorIs(t, txErr, tc.ExpErr) + case txCreationErr != nil: + require.Error(t, txCreationErr) + require.ErrorIs(t, txCreationErr, tc.ExpErr) case anteErr != nil: require.Error(t, anteErr) @@ -280,6 +281,11 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { require.Error(t, postErr) require.ErrorIs(t, postErr, tc.ExpErr) + case tc.MsgRunSuccess == false: + // message failed to run but ante and post should succeed + require.NoError(t, anteErr) + require.NoError(t, postErr) + default: t.Fatal("expected one of txErr, handleErr to be an error") } diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 108af3e..8706ba9 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -56,6 +56,9 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } + feeCoins := feeTx.GetFee() + gas := ctx.GasMeter().GasConsumed() // use context gas consumed + // update fee market params params, err := dfd.feemarketKeeper.GetParams(ctx) if err != nil { @@ -77,15 +80,6 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul return next(ctx, tx, simulate, success) } - // update fee market state - state, err := dfd.feemarketKeeper.GetState(ctx) - if err != nil { - return ctx, errorsmod.Wrapf(err, "unable to get fee market state") - } - - feeCoins := feeTx.GetFee() - gas := ctx.GasMeter().GasConsumed() // use context gas consumed - if len(feeCoins) == 0 && !simulate { return ctx, errorsmod.Wrapf(feemarkettypes.ErrNoFeeCoins, "got length %d", len(feeCoins)) } @@ -102,6 +96,16 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul payCoin = feeCoins[0] } + // if the tx failed, deal with escrowed funds and return early + if !success && !simulate { + err := DeductCoins(dfd.bankKeeper, ctx, sdk.NewCoins(payCoin), params.DistributeFees) + if err != nil { + return ctx, err + } + + return next(ctx, tx, simulate, success) + } + feeGas := int64(feeTx.GetGas()) minGasPrice, err := dfd.feemarketKeeper.GetMinGasPrice(ctx, payCoin.GetDenom()) @@ -130,6 +134,12 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, err } + // update fee market state + state, err := dfd.feemarketKeeper.GetState(ctx) + if err != nil { + return ctx, errorsmod.Wrapf(err, "unable to get fee market state") + } + err = state.Update(gas, params) if err != nil { return ctx, errorsmod.Wrapf(err, "unable to update fee market state") diff --git a/x/feemarket/post/fee_distribute_test.go b/x/feemarket/post/fee_distribute_test.go index 2bc73c5..a461d6f 100644 --- a/x/feemarket/post/fee_distribute_test.go +++ b/x/feemarket/post/fee_distribute_test.go @@ -22,8 +22,7 @@ func TestPostHandleDistributeFeesMock(t *testing.T) { // Same data for every test case const ( baseDenom = "stake" - resolvableDenom = "atom" - expectedConsumedGas = 11730 + expectedConsumedGas = 11700 expectedConsumedSimGas = expectedConsumedGas + post.BankSendGasConsumption gasLimit = expectedConsumedSimGas ) @@ -51,6 +50,7 @@ func TestPostHandleDistributeFeesMock(t *testing.T) { RunPost: true, Simulate: false, ExpPass: true, + MsgRunSuccess: true, ExpErr: nil, ExpectConsumedGas: expectedConsumedGas, Mock: true, @@ -73,10 +73,8 @@ func TestPostHandleDistributeFees(t *testing.T) { // Same data for every test case const ( baseDenom = "stake" - resolvableDenom = "atom" expectedConsumedGas = 65558 - - gasLimit = 100000 + gasLimit = 100000 ) validFeeAmount := types.DefaultMinBaseGasPrice.MulInt64(int64(gasLimit)) @@ -97,16 +95,17 @@ func TestPostHandleDistributeFees(t *testing.T) { return antesuite.TestCaseArgs{ Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, - GasLimit: 35188, + GasLimit: gasLimit, FeeAmount: validFeeWithTip, } }, RunAnte: true, RunPost: true, Simulate: false, + MsgRunSuccess: false, ExpPass: false, ExpErr: sdkerrors.ErrOutOfGas, - ExpectConsumedGas: 65558, + ExpectConsumedGas: expectedConsumedGas, Mock: false, DistributeFees: true, }, diff --git a/x/feemarket/post/fee_test.go b/x/feemarket/post/fee_test.go index e682e55..7cfc796 100644 --- a/x/feemarket/post/fee_test.go +++ b/x/feemarket/post/fee_test.go @@ -153,7 +153,7 @@ func TestPostHandleMock(t *testing.T) { const ( baseDenom = "stake" resolvableDenom = "atom" - expectedConsumedGas = 10631 + expectedConsumedGas = 10601 expectedConsumedSimGas = expectedConsumedGas + post.BankSendGasConsumption gasLimit = expectedConsumedSimGas ) @@ -179,12 +179,13 @@ func TestPostHandleMock(t *testing.T) { FeeAmount: validFee, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: sdkerrors.ErrInsufficientFunds, - Mock: true, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrInsufficientFunds, + Mock: true, }, { Name: "signer has no funds - simulate", @@ -217,12 +218,13 @@ func TestPostHandleMock(t *testing.T) { FeeAmount: validFee, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: sdkerrors.ErrOutOfGas, - Mock: true, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrOutOfGas, + Mock: true, }, { Name: "0 gas given should pass - simulate", @@ -240,6 +242,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -262,6 +265,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, @@ -284,6 +288,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, @@ -306,6 +311,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -350,6 +356,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, @@ -373,6 +380,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, @@ -395,6 +403,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -417,6 +426,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, @@ -439,6 +449,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -459,6 +470,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: false, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -479,6 +491,7 @@ func TestPostHandleMock(t *testing.T) { }, RunAnte: true, RunPost: false, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -496,12 +509,13 @@ func TestPostHandleMock(t *testing.T) { FeeAmount: nil, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: types.ErrNoFeeCoins, - Mock: true, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: types.ErrNoFeeCoins, + Mock: true, }, { Name: "no gas limit - fail", @@ -514,12 +528,13 @@ func TestPostHandleMock(t *testing.T) { FeeAmount: nil, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: sdkerrors.ErrOutOfGas, - Mock: true, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrOutOfGas, + Mock: true, }, } @@ -539,9 +554,9 @@ func TestPostHandle(t *testing.T) { const ( baseDenom = "stake" resolvableDenom = "atom" - expectedConsumedGas = 36650 + expectedConsumedGas = 36620 - expectedConsumedGasResolve = 36524 // slight difference due to denom resolver + expectedConsumedGasResolve = 36494 // slight difference due to denom resolver gasLimit = 100000 ) @@ -565,12 +580,13 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFee, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: sdkerrors.ErrInsufficientFunds, - Mock: false, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrInsufficientFunds, + Mock: false, }, { Name: "signer has no funds - simulate - pass", @@ -585,6 +601,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -602,12 +619,13 @@ func TestPostHandle(t *testing.T) { FeeAmount: validFee, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: sdkerrors.ErrOutOfGas, - Mock: false, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrOutOfGas, + Mock: false, }, { Name: "0 gas given should pass - simulate", @@ -622,6 +640,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -647,10 +666,11 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, - ExpectConsumedGas: 36650, + ExpectConsumedGas: expectedConsumedGas, Mock: false, }, { @@ -696,10 +716,11 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, - ExpectConsumedGas: 36650, + ExpectConsumedGas: expectedConsumedGas, Mock: false, }, { @@ -715,6 +736,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -762,6 +784,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, @@ -787,6 +810,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, @@ -812,6 +836,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -832,6 +857,7 @@ func TestPostHandle(t *testing.T) { RunAnte: true, RunPost: true, Simulate: true, + MsgRunSuccess: true, ExpPass: true, ExpErr: nil, ExpectConsumedGas: expectedConsumedGas, @@ -854,12 +880,13 @@ func TestPostHandle(t *testing.T) { FeeAmount: validResolvableFeeWithTip, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: sdkerrors.ErrInsufficientFunds, - Mock: false, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrInsufficientFunds, + Mock: false, }, { Name: "signer has enough funds, should pass with tip - resolvable denom", @@ -880,6 +907,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: false, ExpPass: true, ExpErr: nil, @@ -899,6 +927,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: true, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -918,6 +947,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: false, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -937,6 +967,7 @@ func TestPostHandle(t *testing.T) { }, RunAnte: true, RunPost: false, + MsgRunSuccess: true, Simulate: true, ExpPass: true, ExpErr: nil, @@ -954,12 +985,13 @@ func TestPostHandle(t *testing.T) { FeeAmount: nil, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: types.ErrNoFeeCoins, - Mock: false, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: types.ErrNoFeeCoins, + Mock: false, }, { Name: "no gas limit - fail", @@ -972,12 +1004,13 @@ func TestPostHandle(t *testing.T) { FeeAmount: nil, } }, - RunAnte: true, - RunPost: true, - Simulate: false, - ExpPass: false, - ExpErr: sdkerrors.ErrOutOfGas, - Mock: false, + RunAnte: true, + RunPost: true, + MsgRunSuccess: true, + Simulate: false, + ExpPass: false, + ExpErr: sdkerrors.ErrOutOfGas, + Mock: false, }, }