check: fee distribution on failed tx

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

@@ -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,
@@ -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)
 
@@ -390,7 +390,7 @@ func TestAnteHandle(t *testing.T) {
 				}
 			},
 			RunAnte:  true,
-			RunPost:  true,
+			RunPost:  false,
 			Simulate: false,
 			ExpPass:  false,
 			ExpErr:   types.ErrNoFeeCoins,
@@ -407,18 +407,19 @@ 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,
 		},
 	}
 
 	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

@@ -1,8 +1,10 @@
 package suite
 
 import (
+	"fmt"
 	"testing"
 
+	"cosmossdk.io/math"
 	storetypes "cosmossdk.io/store/types"
 
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
@@ -106,11 +108,11 @@
 }
 
 // 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
@@ -183,11 +185,13 @@
 	StateUpdate       func(*TestSuite)
 	RunAnte           bool
 	RunPost           bool
+	MsgRunSuccess     bool
 	Simulate          bool
 	ExpPass           bool
 	ExpErr            error
 	ExpectConsumedGas uint64
 	Mock              bool
+	DistributeFees    bool
 }
 
 type TestCaseArgs struct {
@@ -207,8 +211,8 @@
 	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)
 }
 
@@ -220,7 +224,7 @@
 	// 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
@@ -241,11 +245,16 @@
 	}
 
 	if tc.RunPost && anteErr == nil {
-		newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, true)
+		newCtx, postErr = s.PostHandler(s.Ctx, tx, tc.Simulate, tc.MsgRunSuccess)
+	}
+
+	if tc.DistributeFees && !tc.Simulate && args.FeeAmount != nil {
+		postFeeBalance := s.BankKeeper.GetBalance(s.Ctx, s.AccountKeeper.GetModuleAddress(feemarkettypes.FeeCollectorName), args.FeeAmount.GetDenomByIndex(0))
+		require.True(t, postFeeBalance.Amount.Equal(math.ZeroInt()), fmt.Errorf("amounts not equal: %s, %s", postFeeBalance.Amount.String(), math.ZeroInt().String()))
 	}
 
 	if tc.ExpPass {
-		require.NoError(t, txErr)
+		require.NoError(t, txCreationErr)
 		require.NoError(t, anteErr)
 		require.NoError(t, postErr)
 		require.NotNil(t, newCtx)
@@ -258,9 +267,9 @@
 
 	} 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)
@@ -272,6 +281,11 @@
 			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")
 		}

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