eth/tracers: refactor tracers to deduplicate code, use same underlying block/tx tracing methods for all code paths #31748
Conversation
jwasinger
changed the title
standardTraceBlockToFile, standardTraceBadBlockToFile. refactor standardTrace* and native/js tracers, deduplicating the shared logic between them
jwasinger
changed the title
| @@ -333,7 +324,7 @@ func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed | |||
| close(resCh) | |||
| }() | |||
| // Feed all the blocks both into the tracer, as well as fast process concurrently | |||
| for number = start.NumberU64(); number < end.NumberU64(); number++ { | |||
| for number = start.NumberU64() + 1; number <= end.NumberU64(); number++ { | |||
There was a problem hiding this comment.
This test case has been changed: number is now the block that we are scheduling a tracing task for. The reason for this change is that I separate block prestate retrieval into a separate method that is reused in other areas of this file, and this block number takes the block that we want to retrieve a prestate for (parent block is no longer explicitly needed in this function).
| // Insert block's parent beacon block root in the state | ||
| // as per EIP-4788. | ||
| context := core.NewEVMBlockContext(next.Header(), api.chainContext(ctx), nil) | ||
| evm := vm.NewEVM(context, statedb, api.backend.ChainConfig(), vm.Config{}) | ||
| if beaconRoot := next.BeaconRoot(); beaconRoot != nil { | ||
| core.ProcessBeaconBlockRoot(*beaconRoot, evm) | ||
| } | ||
| // Insert parent hash in history contract. | ||
| if api.backend.ChainConfig().IsPrague(next.Number(), next.Time()) { | ||
| core.ProcessParentBlockHash(next.ParentHash(), evm) | ||
| } |
There was a problem hiding this comment.
moved this into traceBlockWithState
… parallel for TraceChain
| type traceExecOptions struct { | ||
| txTimeout time.Duration | ||
| reexec uint64 | ||
| parallel bool | ||
| } |
There was a problem hiding this comment.
traceBlock and traceTx (now execTx) are now agnostic to the type of tracer (call vs standard-trace vs js/native-trace). Now, we extract the trace execution parameters from the config type in the outer method call (StdTrace, TraceConfig, TraceCallConfig) into a traceExecOptions instance which is used by the internal tracing methods.
This reduces the size of some method signatures and prevents duplication of config instantiation logic.
| } | ||
|
|
||
| execOpt := standardTraceExecOpt(config) | ||
| execOpt.txTimeout = 100 * time.Hour |
There was a problem hiding this comment.
The timeout value is too long.
There was a problem hiding this comment.
We don't currently have a timeout for the standardTrace methods. I think the reasoning is that these are usually used for debugging, and we want to ensure that the trace completes regardless of how long it takes.
There was a problem hiding this comment.
It's right to consider it from debug point.
| execOpt := standardTraceExecOpt(config) | ||
| execOpt.txTimeout = 100 * time.Hour | ||
| if _, err := api.traceBlock(ctx, block, execOpt, chainConfig, instantiateTracer); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
If appear error, we also need to return the other successful traces.
There was a problem hiding this comment.
Good catch! also, traceBlock will not emit the traces if an error occurs midway through the trace.
There was a problem hiding this comment.
So return dumps[:len(dumps)-1], err if len(dumps) > 1.
Co-authored-by: maskpp <[email protected]>
Currently, tracers duplicate transaction execution logic three times. This PR modifies the implementation of the tracers so that block/transaction trace logic is only implemented once, and used by all tracers.