From a74335d457a2b50380bf2e100af61fa3a0688366 Mon Sep 17 00:00:00 2001 From: freeelancer Date: Thu, 17 Oct 2024 15:00:52 +0800 Subject: [PATCH 1/9] added tests for stuck escrow fee --- tests/integration/integration_test.go | 2 +- testutils/keeper/keeper.go | 6 +- x/feemarket/ante/fee_test.go | 4 +- x/feemarket/ante/feegrant_test.go | 2 +- x/feemarket/ante/suite/suite.go | 11 ++- x/feemarket/keeper/keeper_test.go | 2 +- x/feemarket/post/fee_distribute_test.go | 121 ++++++++++++++++++++++++ x/feemarket/post/fee_test.go | 10 +- 8 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 x/feemarket/post/fee_distribute_test.go diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 16358c5..8eb84b4 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -37,7 +37,7 @@ func TestIntegrationTestSuite(t *testing.T) { func (s *IntegrationTestSuite) SetupTest() { s.encCfg = MakeTestEncodingConfig() - s.ctx, s.TestKeepers, s.TestMsgServers = testkeeper.NewTestSetup(s.T()) + s.ctx, s.TestKeepers, s.TestMsgServers = testkeeper.NewTestSetup(s.T(), false) } func (s *IntegrationTestSuite) TestState() { diff --git a/testutils/keeper/keeper.go b/testutils/keeper/keeper.go index 6f15b49..f1f4bdb 100644 --- a/testutils/keeper/keeper.go +++ b/testutils/keeper/keeper.go @@ -36,7 +36,7 @@ var additionalMaccPerms = map[string][]string{ } // NewTestSetup returns initialized instances of all the keepers and message servers of the modules -func NewTestSetup(t testing.TB, options ...testkeeper.SetupOption) (sdk.Context, TestKeepers, TestMsgServers) { +func NewTestSetup(t testing.TB, distributeFees bool, options ...testkeeper.SetupOption) (sdk.Context, TestKeepers, TestMsgServers) { options = append(options, testkeeper.WithAdditionalModuleAccounts(additionalMaccPerms)) _, tk, tms := testkeeper.NewTestSetup(t, options...) @@ -55,7 +55,9 @@ func NewTestSetup(t testing.TB, options ...testkeeper.SetupOption) (sdk.Context, err := feeMarketKeeper.SetState(ctx, feemarkettypes.DefaultState()) require.NoError(t, err) - err = feeMarketKeeper.SetParams(ctx, feemarkettypes.DefaultParams()) + params := feemarkettypes.DefaultParams() + params.DistributeFees = distributeFees + err = feeMarketKeeper.SetParams(ctx, params) require.NoError(t, err) testKeepers := TestKeepers{ diff --git a/x/feemarket/ante/fee_test.go b/x/feemarket/ante/fee_test.go index b90971a..4f6224b 100644 --- a/x/feemarket/ante/fee_test.go +++ b/x/feemarket/ante/fee_test.go @@ -197,7 +197,7 @@ func TestAnteHandleMock(t *testing.T) { for _, tc := range testCases { t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) { - s := antesuite.SetupTestSuite(t, tc.Mock) + s := antesuite.SetupTestSuite(t, tc.Mock, false) s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() args := tc.Malleate(s) @@ -418,7 +418,7 @@ func TestAnteHandle(t *testing.T) { for _, tc := range testCases { t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) { - s := antesuite.SetupTestSuite(t, tc.Mock) + s := antesuite.SetupTestSuite(t, tc.Mock, false) s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() args := tc.Malleate(s) diff --git a/x/feemarket/ante/feegrant_test.go b/x/feemarket/ante/feegrant_test.go index 6456416..7106ccf 100644 --- a/x/feemarket/ante/feegrant_test.go +++ b/x/feemarket/ante/feegrant_test.go @@ -126,7 +126,7 @@ func TestEscrowFunds(t *testing.T) { for name, stc := range cases { tc := stc // to make scopelint happy t.Run(name, func(t *testing.T) { - s := antesuite.SetupTestSuite(t, true) + s := antesuite.SetupTestSuite(t, true, false) protoTxCfg := tx.NewTxConfig(codec.NewProtoCodec(s.EncCfg.InterfaceRegistry), tx.DefaultSignModes) // this just tests our handler dfd := feemarketante.NewFeeMarketCheckDecorator(s.AccountKeeper, s.MockBankKeeper, s.MockFeeGrantKeeper, diff --git a/x/feemarket/ante/suite/suite.go b/x/feemarket/ante/suite/suite.go index 5113539..0a77ff1 100644 --- a/x/feemarket/ante/suite/suite.go +++ b/x/feemarket/ante/suite/suite.go @@ -3,6 +3,7 @@ package suite import ( "testing" + "cosmossdk.io/math" storetypes "cosmossdk.io/store/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" @@ -106,11 +107,11 @@ func (s *TestSuite) SetAccountBalances(accounts []TestAccountBalance) { } // SetupTestSuite setups a new test, with new app, context, and anteHandler. -func SetupTestSuite(t *testing.T, mock bool) *TestSuite { +func SetupTestSuite(t *testing.T, mock, distributeFees bool) *TestSuite { s := &TestSuite{} s.EncCfg = MakeTestEncodingConfig() - ctx, testKeepers, _ := testkeeper.NewTestSetup(t) + ctx, testKeepers, _ := testkeeper.NewTestSetup(t, distributeFees) s.Ctx = ctx s.AccountKeeper = testKeepers.AccountKeeper @@ -188,6 +189,7 @@ type TestCase struct { ExpErr error ExpectConsumedGas uint64 Mock bool + DistributeFees bool } type TestCaseArgs struct { @@ -244,6 +246,11 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, true) } + if tc.DistributeFees && !tc.Simulate && args.FeeAmount != nil { + postFeeBalance := s.BankKeeper.GetBalance(s.Ctx, s.AccountKeeper.GetModuleAddress(feemarkettypes.FeeCollectorName), args.FeeAmount.GetDenomByIndex(0)) + require.Equal(t, postFeeBalance.Amount, math.ZeroInt()) + } + if tc.ExpPass { require.NoError(t, txErr) require.NoError(t, anteErr) diff --git a/x/feemarket/keeper/keeper_test.go b/x/feemarket/keeper/keeper_test.go index e9b9b5e..dbfdacd 100644 --- a/x/feemarket/keeper/keeper_test.go +++ b/x/feemarket/keeper/keeper_test.go @@ -48,7 +48,7 @@ func (s *KeeperTestSuite) SetupTest() { s.encCfg = MakeTestEncodingConfig() s.authorityAccount = authtypes.NewModuleAddress(govtypes.ModuleName) s.accountKeeper = mocks.NewAccountKeeper(s.T()) - ctx, tk, tm := testkeeper.NewTestSetup(s.T()) + ctx, tk, tm := testkeeper.NewTestSetup(s.T(), false) s.ctx = ctx s.feeMarketKeeper = tk.FeeMarketKeeper diff --git a/x/feemarket/post/fee_distribute_test.go b/x/feemarket/post/fee_distribute_test.go new file mode 100644 index 0000000..206daa4 --- /dev/null +++ b/x/feemarket/post/fee_distribute_test.go @@ -0,0 +1,121 @@ +package post_test + +import ( + "fmt" + "testing" + + "cosmossdk.io/math" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/mock" + + authtypes "github.com/cosmos/cosmos-sdk/x/auth/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" +) + +func TestPostHandleDistributeFeesMock(t *testing.T) { + // Same data for every test case + const ( + baseDenom = "stake" + resolvableDenom = "atom" + expectedConsumedGas = 11730 + expectedConsumedSimGas = expectedConsumedGas + post.BankSendGasConsumption + gasLimit = expectedConsumedSimGas + ) + + validFeeAmount := types.DefaultMinBaseGasPrice.MulInt64(int64(gasLimit)) + validFeeAmountWithTip := validFeeAmount.Add(math.LegacyNewDec(100)) + validFeeWithTip := sdk.NewCoins(sdk.NewCoin(baseDenom, validFeeAmountWithTip.TruncateInt())) + + testCases := []antesuite.TestCase{ + { + Name: "signer has enough funds, should pass with tip", + Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := s.CreateTestAccounts(1) + s.MockBankKeeper.On("SendCoinsFromAccountToModule", mock.Anything, accs[0].Account.GetAddress(), + types.FeeCollectorName, mock.Anything).Return(nil).Once() + s.MockBankKeeper.On("SendCoinsFromModuleToModule", mock.Anything, types.FeeCollectorName, authtypes.FeeCollectorName, mock.Anything).Return(nil).Once() + s.MockBankKeeper.On("SendCoinsFromModuleToAccount", mock.Anything, types.FeeCollectorName, mock.Anything, mock.Anything).Return(nil).Once() + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: gasLimit, + FeeAmount: validFeeWithTip, + } + }, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: expectedConsumedGas, + Mock: true, + DistributeFees: true, + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) { + s := antesuite.SetupTestSuite(t, tc.Mock, true) + s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() + args := tc.Malleate(s) + + s.RunTestCase(t, tc, args) + }) + } +} + +func TestPostHandleDistributeFees(t *testing.T) { + // Same data for every test case + const ( + baseDenom = "stake" + resolvableDenom = "atom" + expectedConsumedGas = 54500 + + gasLimit = 100000 + ) + + validFeeAmount := types.DefaultMinBaseGasPrice.MulInt64(int64(gasLimit)) + validFeeAmountWithTip := validFeeAmount.Add(math.LegacyNewDec(100)) + validFeeWithTip := sdk.NewCoins(sdk.NewCoin(baseDenom, validFeeAmountWithTip.TruncateInt())) + + testCases := []antesuite.TestCase{ + { + Name: "signer has enough funds, gaslimit is not enough to complete entire transaction, should pass", + Malleate: func(s *antesuite.TestSuite) antesuite.TestCaseArgs { + accs := s.CreateTestAccounts(1) + + balance := antesuite.TestAccountBalance{ + TestAccount: accs[0], + Coins: validFeeWithTip, + } + s.SetAccountBalances([]antesuite.TestAccountBalance{balance}) + + return antesuite.TestCaseArgs{ + Msgs: []sdk.Msg{testdata.NewTestMsg(accs[0].Account.GetAddress())}, + GasLimit: 35188, + FeeAmount: validFeeWithTip, + } + }, + RunAnte: true, + RunPost: true, + Simulate: false, + ExpPass: true, + ExpErr: nil, + ExpectConsumedGas: 65558, + Mock: false, + DistributeFees: true, + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) { + s := antesuite.SetupTestSuite(t, tc.Mock, true) + s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() + args := tc.Malleate(s) + + s.RunTestCase(t, tc, args) + }) + } +} diff --git a/x/feemarket/post/fee_test.go b/x/feemarket/post/fee_test.go index af67585..e682e55 100644 --- a/x/feemarket/post/fee_test.go +++ b/x/feemarket/post/fee_test.go @@ -63,7 +63,7 @@ func TestDeductCoins(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - s := antesuite.SetupTestSuite(t, true) + s := antesuite.SetupTestSuite(t, true, tc.distributeFees) if tc.distributeFees { s.MockBankKeeper.On("SendCoinsFromModuleToModule", s.Ctx, types.FeeCollectorName, authtypes.FeeCollectorName, @@ -101,7 +101,7 @@ func TestDeductCoinsAndDistribute(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - s := antesuite.SetupTestSuite(t, true) + s := antesuite.SetupTestSuite(t, true, false) s.MockBankKeeper.On("SendCoinsFromModuleToModule", s.Ctx, types.FeeCollectorName, authtypes.FeeCollectorName, tc.coins).Return(nil).Once() @@ -136,7 +136,7 @@ func TestSendTip(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { - s := antesuite.SetupTestSuite(t, true) + s := antesuite.SetupTestSuite(t, true, false) accs := s.CreateTestAccounts(2) s.MockBankKeeper.On("SendCoinsFromModuleToAccount", s.Ctx, types.FeeCollectorName, mock.Anything, tc.coins).Return(nil).Once() @@ -525,7 +525,7 @@ func TestPostHandleMock(t *testing.T) { for _, tc := range testCases { t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) { - s := antesuite.SetupTestSuite(t, tc.Mock) + s := antesuite.SetupTestSuite(t, tc.Mock, false) s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() args := tc.Malleate(s) @@ -983,7 +983,7 @@ func TestPostHandle(t *testing.T) { for _, tc := range testCases { t.Run(fmt.Sprintf("Case %s", tc.Name), func(t *testing.T) { - s := antesuite.SetupTestSuite(t, tc.Mock) + s := antesuite.SetupTestSuite(t, tc.Mock, false) s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder() args := tc.Malleate(s) From 9b9134e7811fb01da13af8f0c23db85699c5a3e8 Mon Sep 17 00:00:00 2001 From: freeelancer Date: Thu, 17 Oct 2024 15:17:50 +0800 Subject: [PATCH 2/9] fix to distribute stuck escrowed fees --- x/feemarket/post/expected_keeper.go | 1 + x/feemarket/post/fee.go | 38 ++++++++++++++++++++--------- x/feemarket/types/keys.go | 11 +++++---- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/x/feemarket/post/expected_keeper.go b/x/feemarket/post/expected_keeper.go index a759656..f71ca3c 100644 --- a/x/feemarket/post/expected_keeper.go +++ b/x/feemarket/post/expected_keeper.go @@ -31,6 +31,7 @@ type BankKeeper interface { SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error SendCoinsFromModuleToModule(ctx context.Context, senderModule, recipientModule string, amt sdk.Coins) error SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error + GetAllBalances(ctx context.Context, addr sdk.AccAddress) sdk.Coins } // FeeMarketKeeper defines the expected feemarket keeper. diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 108af3e..c80e19f 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -159,18 +159,8 @@ func (dfd FeeMarketDeductDecorator) PayOutFeeAndTip(ctx sdk.Context, fee, tip sd var events sdk.Events // deduct the fees and tip - if !fee.IsNil() { - err := DeductCoins(dfd.bankKeeper, ctx, sdk.NewCoins(fee), params.DistributeFees) - if err != nil { - return err - } - - events = append(events, sdk.NewEvent( - feemarkettypes.EventTypeFeePay, - sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), - )) - } - + // fees from previous transactions may be stuck in escrow if transaction is out of gas + // distribute the stuck escrow fees based on the distribute fees parameter proposer := sdk.AccAddress(ctx.BlockHeader().ProposerAddress) if !tip.IsNil() { err := SendTip(dfd.bankKeeper, ctx, proposer, sdk.NewCoins(tip)) @@ -185,6 +175,30 @@ func (dfd FeeMarketDeductDecorator) PayOutFeeAndTip(ctx sdk.Context, fee, tip sd )) } + if params.DistributeFees { + feemarketCollector := dfd.accountKeeper.GetModuleAccount(ctx, feemarkettypes.FeeCollectorName) + feesToDistribute := dfd.bankKeeper.GetAllBalances(ctx, feemarketCollector.GetAddress()) + if !feesToDistribute.IsZero() { + err := DeductCoins(dfd.bankKeeper, ctx, feesToDistribute, params.DistributeFees) + if err != nil { + return err + } + if feesToDistribute.AmountOf(fee.Denom).GT(fee.Amount) { + events = append(events, sdk.NewEvent( + feemarkettypes.EventTypeDistributeStuckEscrowFees, + sdk.NewAttribute(sdk.AttributeKeyFee, feesToDistribute.Sub(fee).String()), + )) + } + } + } + + if !fee.IsNil() { + events = append(events, sdk.NewEvent( + feemarkettypes.EventTypeFeePay, + sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), + )) + } + ctx.EventManager().EmitEvents(events) return nil } diff --git a/x/feemarket/types/keys.go b/x/feemarket/types/keys.go index 8bbd54c..62d9387 100644 --- a/x/feemarket/types/keys.go +++ b/x/feemarket/types/keys.go @@ -26,9 +26,10 @@ var ( // KeyEnabledHeight is the store key for the feemarket module's enabled height. KeyEnabledHeight = []byte{prefixEnableHeight} - EventTypeFeePay = "fee_pay" - EventTypeTipPay = "tip_pay" - AttributeKeyTip = "tip" - AttributeKeyTipPayer = "tip_payer" - AttributeKeyTipPayee = "tip_payee" + EventTypeFeePay = "fee_pay" + EventTypeTipPay = "tip_pay" + EventTypeDistributeStuckEscrowFees = "distribute_stuck_escrow_fees" + AttributeKeyTip = "tip" + AttributeKeyTipPayer = "tip_payer" + AttributeKeyTipPayee = "tip_payee" ) From 6fbf825acb28bfe5bf13f267c816049b4bab9f0c Mon Sep 17 00:00:00 2001 From: aljo242 Date: Sat, 19 Oct 2024 17:24:13 -0400 Subject: [PATCH 3/9] try to revert --- x/feemarket/post/fee.go | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index c80e19f..108af3e 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -159,43 +159,29 @@ func (dfd FeeMarketDeductDecorator) PayOutFeeAndTip(ctx sdk.Context, fee, tip sd var events sdk.Events // deduct the fees and tip - // fees from previous transactions may be stuck in escrow if transaction is out of gas - // distribute the stuck escrow fees based on the distribute fees parameter - proposer := sdk.AccAddress(ctx.BlockHeader().ProposerAddress) - if !tip.IsNil() { - err := SendTip(dfd.bankKeeper, ctx, proposer, sdk.NewCoins(tip)) + if !fee.IsNil() { + err := DeductCoins(dfd.bankKeeper, ctx, sdk.NewCoins(fee), params.DistributeFees) if err != nil { return err } events = append(events, sdk.NewEvent( - feemarkettypes.EventTypeTipPay, - sdk.NewAttribute(feemarkettypes.AttributeKeyTip, tip.String()), - sdk.NewAttribute(feemarkettypes.AttributeKeyTipPayee, proposer.String()), + feemarkettypes.EventTypeFeePay, + sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), )) } - if params.DistributeFees { - feemarketCollector := dfd.accountKeeper.GetModuleAccount(ctx, feemarkettypes.FeeCollectorName) - feesToDistribute := dfd.bankKeeper.GetAllBalances(ctx, feemarketCollector.GetAddress()) - if !feesToDistribute.IsZero() { - err := DeductCoins(dfd.bankKeeper, ctx, feesToDistribute, params.DistributeFees) - if err != nil { - return err - } - if feesToDistribute.AmountOf(fee.Denom).GT(fee.Amount) { - events = append(events, sdk.NewEvent( - feemarkettypes.EventTypeDistributeStuckEscrowFees, - sdk.NewAttribute(sdk.AttributeKeyFee, feesToDistribute.Sub(fee).String()), - )) - } + proposer := sdk.AccAddress(ctx.BlockHeader().ProposerAddress) + if !tip.IsNil() { + err := SendTip(dfd.bankKeeper, ctx, proposer, sdk.NewCoins(tip)) + if err != nil { + return err } - } - if !fee.IsNil() { events = append(events, sdk.NewEvent( - feemarkettypes.EventTypeFeePay, - sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), + feemarkettypes.EventTypeTipPay, + sdk.NewAttribute(feemarkettypes.AttributeKeyTip, tip.String()), + sdk.NewAttribute(feemarkettypes.AttributeKeyTipPayee, proposer.String()), )) } From c06077bfc23d72157e69d99a339fee1547efc38d Mon Sep 17 00:00:00 2001 From: aljo242 Date: Sat, 19 Oct 2024 17:49:34 -0400 Subject: [PATCH 4/9] repro --- x/feemarket/ante/suite/suite.go | 7 ++++--- x/feemarket/post/fee_distribute_test.go | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/x/feemarket/ante/suite/suite.go b/x/feemarket/ante/suite/suite.go index 0a77ff1..373d4ea 100644 --- a/x/feemarket/ante/suite/suite.go +++ b/x/feemarket/ante/suite/suite.go @@ -1,6 +1,7 @@ package suite import ( + "fmt" "testing" "cosmossdk.io/math" @@ -242,13 +243,13 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) { tc.StateUpdate(s) } - if tc.RunPost && anteErr == nil { - newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, true) + if tc.RunPost { + newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, anteErr == nil) } if tc.DistributeFees && !tc.Simulate && args.FeeAmount != nil { postFeeBalance := s.BankKeeper.GetBalance(s.Ctx, s.AccountKeeper.GetModuleAddress(feemarkettypes.FeeCollectorName), args.FeeAmount.GetDenomByIndex(0)) - require.Equal(t, postFeeBalance.Amount, math.ZeroInt()) + require.True(t, postFeeBalance.Amount.Equal(math.ZeroInt()), fmt.Errorf("amounts not equal: %s, %s", postFeeBalance.Amount.String(), math.ZeroInt().String())) } if tc.ExpPass { diff --git a/x/feemarket/post/fee_distribute_test.go b/x/feemarket/post/fee_distribute_test.go index 206daa4..c1aad21 100644 --- a/x/feemarket/post/fee_distribute_test.go +++ b/x/feemarket/post/fee_distribute_test.go @@ -2,6 +2,7 @@ package post_test import ( "fmt" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "testing" "cosmossdk.io/math" @@ -71,7 +72,7 @@ func TestPostHandleDistributeFees(t *testing.T) { const ( baseDenom = "stake" resolvableDenom = "atom" - expectedConsumedGas = 54500 + expectedConsumedGas = 65558 gasLimit = 100000 ) @@ -101,8 +102,8 @@ func TestPostHandleDistributeFees(t *testing.T) { RunAnte: true, RunPost: true, Simulate: false, - ExpPass: true, - ExpErr: nil, + ExpPass: false, + ExpErr: sdkerrors.ErrOutOfGas, ExpectConsumedGas: 65558, Mock: false, DistributeFees: true, From d71f3dec97562a67b01c39fca08124869d38b63f Mon Sep 17 00:00:00 2001 From: aljo242 Date: Sat, 19 Oct 2024 17:53:46 -0400 Subject: [PATCH 5/9] mock and format --- x/feemarket/ante/mocks/mock_account_keeper.go | 2 +- x/feemarket/ante/mocks/mock_bank_keeper.go | 2 +- x/feemarket/ante/mocks/mock_feegrant_keeper.go | 10 ++++------ x/feemarket/ante/mocks/mock_feemarket_keeper.go | 2 +- x/feemarket/post/expected_keeper.go | 1 - x/feemarket/post/fee_distribute_test.go | 4 +++- x/feemarket/post/mocks/mock_account_keeper.go | 2 +- x/feemarket/post/mocks/mock_bank_keeper.go | 2 +- x/feemarket/post/mocks/mock_feemarket_keeper.go | 2 +- x/feemarket/types/mocks/mock_account_keeper.go | 2 +- 10 files changed, 14 insertions(+), 15 deletions(-) diff --git a/x/feemarket/ante/mocks/mock_account_keeper.go b/x/feemarket/ante/mocks/mock_account_keeper.go index 088b40f..e60fda4 100644 --- a/x/feemarket/ante/mocks/mock_account_keeper.go +++ b/x/feemarket/ante/mocks/mock_account_keeper.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks diff --git a/x/feemarket/ante/mocks/mock_bank_keeper.go b/x/feemarket/ante/mocks/mock_bank_keeper.go index 3251f31..9ec1b09 100644 --- a/x/feemarket/ante/mocks/mock_bank_keeper.go +++ b/x/feemarket/ante/mocks/mock_bank_keeper.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks diff --git a/x/feemarket/ante/mocks/mock_feegrant_keeper.go b/x/feemarket/ante/mocks/mock_feegrant_keeper.go index 2f92225..82722ab 100644 --- a/x/feemarket/ante/mocks/mock_feegrant_keeper.go +++ b/x/feemarket/ante/mocks/mock_feegrant_keeper.go @@ -1,14 +1,12 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks import ( context "context" - proto "github.com/cosmos/gogoproto/proto" - mock "github.com/stretchr/testify/mock" - types "github.com/cosmos/cosmos-sdk/types" + mock "github.com/stretchr/testify/mock" ) // FeeGrantKeeper is an autogenerated mock type for the FeeGrantKeeper type @@ -17,7 +15,7 @@ type FeeGrantKeeper struct { } // UseGrantedFees provides a mock function with given fields: ctx, granter, grantee, fee, msgs -func (_m *FeeGrantKeeper) UseGrantedFees(ctx context.Context, granter types.AccAddress, grantee types.AccAddress, fee types.Coins, msgs []proto.Message) error { +func (_m *FeeGrantKeeper) UseGrantedFees(ctx context.Context, granter types.AccAddress, grantee types.AccAddress, fee types.Coins, msgs []types.Msg) error { ret := _m.Called(ctx, granter, grantee, fee, msgs) if len(ret) == 0 { @@ -25,7 +23,7 @@ func (_m *FeeGrantKeeper) UseGrantedFees(ctx context.Context, granter types.AccA } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, types.AccAddress, types.AccAddress, types.Coins, []proto.Message) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, types.AccAddress, types.AccAddress, types.Coins, []types.Msg) error); ok { r0 = rf(ctx, granter, grantee, fee, msgs) } else { r0 = ret.Error(0) diff --git a/x/feemarket/ante/mocks/mock_feemarket_keeper.go b/x/feemarket/ante/mocks/mock_feemarket_keeper.go index c767bb5..8726a16 100644 --- a/x/feemarket/ante/mocks/mock_feemarket_keeper.go +++ b/x/feemarket/ante/mocks/mock_feemarket_keeper.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks diff --git a/x/feemarket/post/expected_keeper.go b/x/feemarket/post/expected_keeper.go index f71ca3c..a759656 100644 --- a/x/feemarket/post/expected_keeper.go +++ b/x/feemarket/post/expected_keeper.go @@ -31,7 +31,6 @@ type BankKeeper interface { SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error SendCoinsFromModuleToModule(ctx context.Context, senderModule, recipientModule string, amt sdk.Coins) error SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error - GetAllBalances(ctx context.Context, addr sdk.AccAddress) sdk.Coins } // FeeMarketKeeper defines the expected feemarket keeper. diff --git a/x/feemarket/post/fee_distribute_test.go b/x/feemarket/post/fee_distribute_test.go index c1aad21..2bc73c5 100644 --- a/x/feemarket/post/fee_distribute_test.go +++ b/x/feemarket/post/fee_distribute_test.go @@ -2,15 +2,17 @@ package post_test import ( "fmt" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "testing" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/mock" authtypes "github.com/cosmos/cosmos-sdk/x/auth/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" diff --git a/x/feemarket/post/mocks/mock_account_keeper.go b/x/feemarket/post/mocks/mock_account_keeper.go index 1882847..5bf381b 100644 --- a/x/feemarket/post/mocks/mock_account_keeper.go +++ b/x/feemarket/post/mocks/mock_account_keeper.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks diff --git a/x/feemarket/post/mocks/mock_bank_keeper.go b/x/feemarket/post/mocks/mock_bank_keeper.go index 19cfc10..af2dc10 100644 --- a/x/feemarket/post/mocks/mock_bank_keeper.go +++ b/x/feemarket/post/mocks/mock_bank_keeper.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks diff --git a/x/feemarket/post/mocks/mock_feemarket_keeper.go b/x/feemarket/post/mocks/mock_feemarket_keeper.go index 0d69365..db252e9 100644 --- a/x/feemarket/post/mocks/mock_feemarket_keeper.go +++ b/x/feemarket/post/mocks/mock_feemarket_keeper.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks diff --git a/x/feemarket/types/mocks/mock_account_keeper.go b/x/feemarket/types/mocks/mock_account_keeper.go index 84cdcf1..5a961d7 100644 --- a/x/feemarket/types/mocks/mock_account_keeper.go +++ b/x/feemarket/types/mocks/mock_account_keeper.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package mocks From 3676549922158cab10dc35d78cbe4c162462be92 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Sat, 19 Oct 2024 17:54:20 -0400 Subject: [PATCH 6/9] remove --- x/feemarket/types/keys.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/x/feemarket/types/keys.go b/x/feemarket/types/keys.go index 62d9387..8bbd54c 100644 --- a/x/feemarket/types/keys.go +++ b/x/feemarket/types/keys.go @@ -26,10 +26,9 @@ var ( // KeyEnabledHeight is the store key for the feemarket module's enabled height. KeyEnabledHeight = []byte{prefixEnableHeight} - EventTypeFeePay = "fee_pay" - EventTypeTipPay = "tip_pay" - EventTypeDistributeStuckEscrowFees = "distribute_stuck_escrow_fees" - AttributeKeyTip = "tip" - AttributeKeyTipPayer = "tip_payer" - AttributeKeyTipPayee = "tip_payee" + EventTypeFeePay = "fee_pay" + EventTypeTipPay = "tip_pay" + AttributeKeyTip = "tip" + AttributeKeyTipPayer = "tip_payer" + AttributeKeyTipPayee = "tip_payee" ) From 901431ff7d156de0a62abbe2ec38b7516ae4b073 Mon Sep 17 00:00:00 2001 From: aljo242 Date: Sat, 19 Oct 2024 18:36:04 -0400 Subject: [PATCH 7/9] actually test --- x/feemarket/ante/fee_test.go | 19 +-- x/feemarket/ante/suite/suite.go | 24 ++-- x/feemarket/post/fee.go | 28 +++-- x/feemarket/post/fee_distribute_test.go | 13 +- x/feemarket/post/fee_test.go | 151 +++++++++++++++--------- 5 files changed, 142 insertions(+), 93 deletions(-) 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, }, } From 083bd5d46d73f1a6df579c2bb11ae356c582d43e Mon Sep 17 00:00:00 2001 From: aljo242 Date: Sat, 19 Oct 2024 18:39:03 -0400 Subject: [PATCH 8/9] ok --- x/feemarket/post/fee.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/x/feemarket/post/fee.go b/x/feemarket/post/fee.go index 8706ba9..d54ea2f 100644 --- a/x/feemarket/post/fee.go +++ b/x/feemarket/post/fee.go @@ -96,15 +96,17 @@ 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 + /* + // 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) } - - return next(ctx, tx, simulate, success) - } + */ feeGas := int64(feeTx.GetGas()) From 5609bd6b3df82207c47014885c41eff43619832e Mon Sep 17 00:00:00 2001 From: aljo242 Date: Sat, 19 Oct 2024 18:40:35 -0400 Subject: [PATCH 9/9] gosimple --- x/feemarket/ante/suite/suite.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feemarket/ante/suite/suite.go b/x/feemarket/ante/suite/suite.go index a7b6df2..da4bf6c 100644 --- a/x/feemarket/ante/suite/suite.go +++ b/x/feemarket/ante/suite/suite.go @@ -281,7 +281,7 @@ 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: + case !tc.MsgRunSuccess: // message failed to run but ante and post should succeed require.NoError(t, anteErr) require.NoError(t, postErr)