actually test

skip-mev · Oct 19, 2024 · 901431f · 901431f
1 parent 3676549
commit 901431f
Show file tree

Hide file tree

Showing 5 changed files with 142 additions and 93 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,
 		},