Skip to content

Commit

Permalink
Refactor DeliverTx hook so that panics can be handled (#543)
Browse files Browse the repository at this point in the history
## Describe your changes and provide context
Panics in tx handler is usually handled by a defer statement containing
a `recover()` clause. Previously the call stack looks like:
```
func DeliverTx:
    func runTx:
        deferred recover
        (actual processing)
    DeliverTx hook
```
In the above structure, DeliverTx hooks are run on `DeliverTx` level, so
they are outside the deferred recover clause which is within `runTx`
level.

This PR changes it to be:
```
func DeliverTx:
    func runTx:
        deferred recover
        (actual processing)
        DeliverTx hook
```
so that the hook can be recovered as well

## Testing performed to validate your change
unit test
  • Loading branch information
codchen authored Oct 17, 2024
1 parent d73f5bc commit 4e7d467
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 4 deletions.
3 changes: 0 additions & 3 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,6 @@ func (app *BaseApp) DeliverTx(ctx sdk.Context, req abci.RequestDeliverTx, tx sdk
return
}
}
for _, hook := range app.deliverTxHooks {
hook(ctx, tx, checksum, res)
}
return
}

Expand Down
21 changes: 20 additions & 1 deletion baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ type (
// (or removed a substore) between two versions of the software.
StoreLoader func(ms sdk.CommitMultiStore) error

DeliverTxHook func(sdk.Context, sdk.Tx, [32]byte, abci.ResponseDeliverTx)
DeliverTxHook func(sdk.Context, sdk.Tx, [32]byte, sdk.DeliverTxHookInput)
)

// BaseApp reflects the ABCI application implementation.
Expand Down Expand Up @@ -1007,6 +1007,25 @@ func (app *BaseApp) runTx(ctx sdk.Context, mode runTxMode, tx sdk.Tx, checksum [
if ctx.CheckTxCallback() != nil {
ctx.CheckTxCallback()(ctx, err)
}
var evmTxInfo *abci.EvmTxInfo
if ctx.IsEVM() {
evmTxInfo = &abci.EvmTxInfo{
SenderAddress: ctx.EVMSenderAddress(),
Nonce: ctx.EVMNonce(),
TxHash: ctx.EVMTxHash(),
VmError: result.EvmError,
}
}
var events []abci.Event = []abci.Event{}
if result != nil {
events = sdk.MarkEventsToIndex(result.Events, app.indexEvents)
}
for _, hook := range app.deliverTxHooks {
hook(ctx, tx, checksum, sdk.DeliverTxHookInput{
EvmTxInfo: evmTxInfo,
Events: events,
})
}
return gInfo, result, anteEvents, priority, pendingTxChecker, expireHandler, ctx, err
}

Expand Down
42 changes: 42 additions & 0 deletions baseapp/deliver_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 sdk.DeliverTxHookInput) {})
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 sdk.DeliverTxHookInput) { 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()
Expand Down
5 changes: 5 additions & 0 deletions types/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,8 @@ func WrapServiceResult(ctx Context, res proto.Message, err error) (*Result, erro
}
return sdkRes, nil
}

type DeliverTxHookInput struct {
EvmTxInfo *abci.EvmTxInfo
Events []abci.Event
}

0 comments on commit 4e7d467

Please sign in to comment.