Out of gas stuck escrow fee

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() {

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{

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)
 

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,

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)

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

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.

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
 }

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)
+		})
+	}
+}

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)
 

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"
 )