From e36dc4682f0e045b1094c7bde32f9034fa90e036 Mon Sep 17 00:00:00 2001 From: Matthieu Vachon Date: Thu, 15 May 2025 14:51:24 -0400 Subject: [PATCH] Avoid calling `OnBlockEnd` on panic This avoid problem where the tracer receives `OnBlockEnd(nil)` and thus cannot know that an error happen, in which case it could flush data out. Technically, shouldn't be a big problem as since there is a panic, the block will not be committed to geth storage and will be replayed correctly. The other possibility would be to still call the tracer but passing the recovered error to `OnBlockEnd`, but this would require transforming the recovered `any` type into an `error`. --- core/blockchain.go | 6 +++++- core/blockchain_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/core/blockchain.go b/core/blockchain.go index b98c2d43aa..6036f24d5c 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1882,7 +1882,11 @@ func (bc *BlockChain) processBlock(block *types.Block, statedb *state.StateDB, s } if bc.logger != nil && bc.logger.OnBlockEnd != nil { defer func() { - bc.logger.OnBlockEnd(blockEndErr) + if recovered := recover(); recovered != nil { + panic(recovered) + } else { + bc.logger.OnBlockEnd(blockEndErr) + } }() } diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 04dfa87b8b..af4dfbb468 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -35,6 +35,7 @@ import ( "github.com/ethereum/go-ethereum/consensus/ethash" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/state" + "github.com/ethereum/go-ethereum/core/tracing" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/core/vm/program" @@ -4265,3 +4266,43 @@ func TestEIP7702(t *testing.T) { t.Fatalf("addr2 storage wrong: expected %d, got %d", fortyTwo, actual) } } + +func TestTracingBlockEndNotCalledOnPanic(t *testing.T) { + genDb, _, blockchain, err := newCanonical(ethash.NewFaker(), 0, true, "path") + if err != nil { + t.Fatalf("failed to create pristine chain: %v", err) + } + defer blockchain.Stop() + + blockEndCalled := false + hooks := &tracing.Hooks{ + // This simulates a panic in the tracer + OnBalanceChange: func(addr common.Address, prev, new *big.Int, reason tracing.BalanceChangeReason) { + panic("panic") + }, + OnBlockEnd: func(err error) { + blockEndCalled = true + }, + } + + blockchain.logger = hooks + blockchain.vmConfig.Tracer = hooks + + blocks := makeBlockChain(blockchain.chainConfig, blockchain.GetBlockByHash(blockchain.CurrentBlock().Hash()), 1, ethash.NewFullFaker(), genDb, 0) + + func() { + defer func() { + if r := recover(); r == nil { + t.Fatalf("Expected panic, but got none, ensure that OnBalanceChange hook is called correctly to generate the panic") + } + }() + + if _, err := blockchain.InsertChain(blocks); err != nil { + t.Fatalf("Failed to insert block: %v", err) + } + }() + + if blockEndCalled { + t.Fatalf("OnBlockEnd should not be called on panic within the tracer") + } +}