execution/stagedsync, execution/exec: dedup exec3 block-task construction and AA execution#22174
Draft
yperbasis wants to merge 2 commits into
Draft
execution/stagedsync, execution/exec: dedup exec3 block-task construction and AA execution#22174yperbasis wants to merge 2 commits into
yperbasis wants to merge 2 commits into
Conversation
HistoricalTraceWorker.execAATxn duplicated the AA-bundle validation and execution logic of TxTask.executeAA line for line. Replace the copy with a call to executeAA, keeping the TraceFroms/TraceTos filling (success path only, as before) at the historical call site. Behavior-preserving except one log-only delta: the historical copy's 'validated AA bundle' log printed endIdx-startIdx+1 while executeAA still prints startIdx-endIdx; the shared site now wins. That expression is fixed separately (yperbasis/dedup-audit-fixes, b3b25f8), which after this dedup fixes both callers at once.
serialExecutor.exec and txExecutor.executeBlocks duplicated the per-block ingest sequence: read-ahead ping, canonical-hash lookup, block fetch with readAheader fallback, EVM block context construction, and the TxTask build loop. Extract txExecutor.buildBlockTasks and call it from both. Deliberate deltas parameterized or kept at the call sites: - getHash fn: serial passes its mutex-wrapped se.getHeader; parallel passes the per-worker-overridden placeholder reading via blockTx. - BlockStateCache: parallel passes a fresh per-block cache; serial nil. - BAL read/decode/validate stays parallel-only, now after the helper. - Amsterdam guard and accumulator.StartChange stay serial-only. - Snapshot step-misalignment check stays parallel-only, untouched. The helper skips already-executed txns before constructing the task (parallel's form; serial previously constructed then discarded, with no side effects from the discarded task) and reports partialBlock, which serial folds into havePartialBlock. Serial tasks now carry Config and Engine from construction; executeBlock reassigns the same values before use. Behavior-preserving. The similar loop in exec.CustomTraceMapReduce is left alone: package exec cannot import stagedsync, and it differs structurally (streams tasks straight to a queue, no readAheader cache or skip logic, inline hash warming, HistoryExecution always true).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces duplication in the execution pipeline by centralizing (1) AA bundle validation/execution and (2) exec3 block-ingest + TxTask construction so serial and parallel paths share the same task-building logic while keeping intentional per-path differences at the call sites.
Changes:
- Added
(*txExecutor).buildBlockTasksand switched both parallelexecuteBlocksand the serial executor to use it for block read +TxTaskconstruction. - Replaced the historical trace worker’s inlined AA bundle logic with a thin wrapper over
TxTask.executeAA, keeping tracer-specificTraceFroms/TraceToshandling at the call site.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| execution/stagedsync/exec3.go | Introduces buildBlockTasks and uses it from the parallel exec3 loop to deduplicate per-block task construction. |
| execution/stagedsync/exec3_serial.go | Switches serial exec3 to use the shared buildBlockTasks helper and removes the now-duplicated local task-construction loop. |
| execution/exec/historical_trace_worker.go | Replaces the historical trace worker’s AA execution/validation copy with a wrapper calling TxTask.executeAA. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+257
to
+259
| aaTxn := txTask.Tx().(*types.AccountAbstractionTransaction) // type cast checked by the caller | ||
| result := txTask.executeAA(aaTxn, rw.evm, rw.taskGasPool, rw.ibs, rw.execArgs.ChainConfig) | ||
| if result.Err != nil { |
| if len(result.ValidationResults) == 0 { | ||
| result.Err = fmt.Errorf("found RIP-7560 but no remaining validation results, txIndex %d", txTask.TxIndex) | ||
| aaTxn := txTask.Tx().(*types.AccountAbstractionTransaction) // type cast checked by the caller | ||
| result := txTask.executeAA(aaTxn, rw.evm, rw.taskGasPool, rw.ibs, rw.execArgs.ChainConfig) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two consolidations in the execution pipeline, one commit each:
execAATxn→TxTask.executeAA. The historical trace worker carried a ~70-line copy of the AA-bundle validation/execution logic; it is now an 8-line wrapper over the shared method, with its tracer-specificTraceFroms/TraceTosfilling kept at the call site. The single semantic delta between the copies is the "validated AA bundle" log length, fixed separately in cl, db, execution, p2p, cmd: fix copy-paste drift defects #22165 — that line is left byte-identical here so both PRs land conflict-free, after which the fix covers both callers through the shared path. (Pre-existing behavior preserved, worth knowing: the soleexecAATxncaller discards the AA path's Logs/TraceFroms — unchanged.)buildBlockTasks. The serial and parallel executors carried the same ~70-line block-ingest +TxTask-construction loop and had already drifted (the parallel path gained the step-misalignment check; serial didn't). One helper on the embeddedtxExecutornow builds tasks for both; deliberate per-path differences stay at the call sites, parameterized: getHash closure (serial mutex-wrapped vs parallel per-worker placeholder),BlockStateCache(parallel-only), BAL read (parallel-only), Amsterdam guard + accumulatorStartChange(serial-only, still ahead of execution), partial-block tracking (serial-only), step-misalignment check untouched and NOT added to serial. The historicalCustomTraceMapReducecopy is not converted (import cycle; structurally different — streams to a queue).Reorderings are limited to moving pure task construction ahead of side-effect-free reads (verified
GetHashFn/NewEVMBlockContextare lazy); execution ordering unchanged.Net −89 lines. Verified: short + full non-short
execution/stagedsync+execution/execsuites green before and after, scopedgolangci-lintclean (×2),make erigon integration.Part of a dedup series; siblings #22165–#22173.