Refactor DeliverTx hook so that panics can be handled

sei-protocol · Oct 4, 2024 · 111dabc · 111dabc
1 parent d73f5bc
commit 111dabc
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 26 deletions.
diff --git a/baseapp/abci.go b/baseapp/abci.go
@@ -305,32 +305,7 @@ func (app *BaseApp) DeliverTx(ctx sdk.Context, req abci.RequestDeliverTx, tx sdk
 		}
 		return sdkerrors.ResponseDeliverTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, sdk.MarkEventsToIndex(anteEvents, app.indexEvents), app.trace)
 	}
-
-	res = abci.ResponseDeliverTx{
-		GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
-		GasUsed:   int64(gInfo.GasUsed),   // TODO: Should type accept unsigned ints?
-		Log:       result.Log,
-		Data:      result.Data,
-		Events:    sdk.MarkEventsToIndex(result.Events, app.indexEvents),
-	}
-	if resCtx.IsEVM() {
-		res.EvmTxInfo = &abci.EvmTxInfo{
-			SenderAddress: resCtx.EVMSenderAddress(),
-			Nonce:         resCtx.EVMNonce(),
-			TxHash:        resCtx.EVMTxHash(),
-			VmError:       result.EvmError,
-		}
-		// TODO: populate error data for EVM err
-		if result.EvmError != "" {
-			evmErr := sdkerrors.Wrap(sdkerrors.ErrEVMVMError, result.EvmError)
-			res.Codespace, res.Code, res.Log = sdkerrors.ABCIInfo(evmErr, app.trace)
-			resultStr = "failed"
-			return
-		}
-	}
-	for _, hook := range app.deliverTxHooks {
-		hook(ctx, tx, checksum, res)
-	}
+	res = app.getDeliverTxResponse(resCtx, gInfo, result, &resultStr)
 	return
 }
 

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
@@ -1007,6 +1007,11 @@ func (app *BaseApp) runTx(ctx sdk.Context, mode runTxMode, tx sdk.Tx, checksum [
 	if ctx.CheckTxCallback() != nil {
 		ctx.CheckTxCallback()(ctx, err)
 	}
+	resultStr := ""
+	res := app.getDeliverTxResponse(ctx, gInfo, result, &resultStr)
+	for _, hook := range app.deliverTxHooks {
+		hook(ctx, tx, checksum, res)
+	}
 	return gInfo, result, anteEvents, priority, pendingTxChecker, expireHandler, ctx, err
 }
 
@@ -1217,3 +1222,28 @@ func (app *BaseApp) GetCheckCtx() sdk.Context {
 func (app *BaseApp) RegisterDeliverTxHook(hook DeliverTxHook) {
 	app.deliverTxHooks = append(app.deliverTxHooks, hook)
 }
+
+func (app *BaseApp) getDeliverTxResponse(ctx sdk.Context, gInfo sdk.GasInfo, result *sdk.Result, resultStr *string) abci.ResponseDeliverTx {
+	res := abci.ResponseDeliverTx{
+		GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
+		GasUsed:   int64(gInfo.GasUsed),   // TODO: Should type accept unsigned ints?
+		Log:       result.Log,
+		Data:      result.Data,
+		Events:    sdk.MarkEventsToIndex(result.Events, app.indexEvents),
+	}
+	if ctx.IsEVM() {
+		res.EvmTxInfo = &abci.EvmTxInfo{
+			SenderAddress: ctx.EVMSenderAddress(),
+			Nonce:         ctx.EVMNonce(),
+			TxHash:        ctx.EVMTxHash(),
+			VmError:       result.EvmError,
+		}
+		// TODO: populate error data for EVM err
+		if result.EvmError != "" {
+			evmErr := sdkerrors.Wrap(sdkerrors.ErrEVMVMError, result.EvmError)
+			res.Codespace, res.Code, res.Log = sdkerrors.ABCIInfo(evmErr, app.trace)
+			*resultStr = "failed"
+		}
+	}
+	return res
+}
diff --git a/baseapp/deliver_tx_test.go b/baseapp/deliver_tx_test.go
@@ -1646,6 +1646,48 @@ func TestDeliverTx(t *testing.T) {
 	}
 }
 
+func TestDeliverTxHooks(t *testing.T) {
+	anteOpt := func(*BaseApp) {}
+	routerOpt := func(bapp *BaseApp) {
+		r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) { return &sdk.Result{}, nil })
+		bapp.Router().AddRoute(r)
+	}
+
+	app := setupBaseApp(t, anteOpt, routerOpt)
+	app.InitChain(context.Background(), &abci.RequestInitChain{})
+
+	// Create same codec used in txDecoder
+	codec := codec.NewLegacyAmino()
+	registerTestCodec(codec)
+
+	header := tmproto.Header{Height: 1}
+	app.setDeliverState(header)
+	app.BeginBlock(app.deliverState.ctx, abci.RequestBeginBlock{Header: header})
+
+	// every even i is an evm tx
+	counter := int64(1)
+	tx := newTxCounter(counter, counter)
+
+	txBytes, err := codec.Marshal(tx)
+	require.NoError(t, err)
+
+	decoded, _ := app.txDecoder(txBytes)
+
+	ctx := app.deliverState.ctx
+
+	// register noop hook
+	app.RegisterDeliverTxHook(func(ctx sdk.Context, tx sdk.Tx, b [32]byte, rdt abci.ResponseDeliverTx) {})
+	res := app.DeliverTx(ctx, abci.RequestDeliverTx{Tx: txBytes}, decoded, sha256.Sum256(txBytes))
+	require.True(t, res.IsOK(), fmt.Sprintf("%v", res))
+
+	// register panic hook (should be captured by recover() middleware)
+	app.RegisterDeliverTxHook(func(ctx sdk.Context, tx sdk.Tx, b [32]byte, rdt abci.ResponseDeliverTx) { panic(1) })
+	require.NotPanics(t, func() {
+		res = app.DeliverTx(ctx, abci.RequestDeliverTx{Tx: txBytes}, decoded, sha256.Sum256(txBytes))
+	})
+	require.False(t, res.IsOK(), fmt.Sprintf("%v", res))
+}
+
 func TestOptionFunction(t *testing.T) {
 	logger := defaultLogger()
 	db := dbm.NewMemDB()