refactor(core): remove the unused EIP-3155 trace writer - #195
Merged
Merged
Conversation
Deletes the second sources of truth and the dead parameters/accessors the /simplify review found, with no observable change on canonical-block paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CHVpMMX9N69sUNbuKgpBVY
Deletes the `writer` parameter threaded through `validate_block`, `validate_block_deriving_updates`, `replay_block` and `verify_and_replay`, and with it EIP-3155 trace output. A feature deletion, not a cleanup. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CHVpMMX9N69sUNbuKgpBVY
Contributor
Claude review status
✅ Review clean Last reviewed: New this round: 0 finding(s), 0 question(s) · Resolved this round: 0 · Open questions: 0 |
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
flyq
marked this pull request as draft
September 4, 2026 00:28
Troublor
approved these changes
Sep 7, 2026
Base automatically changed from
liquan/refactor/cleanups-behavior-preserving
to
main
September 8, 2026 01:54
PR #194 was squash-merged as 2675337; this branch carried its pre-squash snapshot, which had drifted from the final form during review. Every conflicted file is untouched by this PR's own commit, so each was resolved to main's version. The merge result differs from main by exactly this PR's diff (executor.rs + chain_sync.rs). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196vFR5tf9XpXMe3CuDyuiv
…o replay_block The helper was split out so the same transaction loop could serve both the plain executor and the EIP-3155 tracer executor; with the tracer arm gone it has one caller and one executor type, so its generic where-clause (an HRTB and two trait imports that existed only for the bounds) advertised a generality nothing used. replay_block is now one straight-line body. The last cfg-gated import (std::time::Instant) is spelled at its single std-only use in timed() instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196vFR5tf9XpXMe3CuDyuiv
flyq
marked this pull request as ready for review
September 8, 2026 03:22
vincent-k2026
approved these changes
Sep 8, 2026
flyq
added a commit
that referenced
this pull request
Sep 8, 2026
…code PR #195 was squash-merged as 7ca2253; this branch carried the pre-squash snapshots of #194 and #195, which had drifted from their final form during review. Ten of the twelve conflicted files are untouched by this PR's own commit and were resolved to main's version. The other two (executor.rs, rpc_client.rs) were taken from main with this PR's hunks re-applied on top: #195's final form inlined `execute_transactions` back into `replay_block`, so the `gas_used` change now lives there. The merge result differs from main by exactly this PR's diff. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 8, 2026
flyq
added a commit
that referenced
this pull request
Sep 17, 2026
Resolves what main's squash-merges of #194, #195 and #196 left conflicting against this branch's own copies of that work. Every file outside this PR's two commits is taken from main verbatim, so the branch keeps only the reviewed form of those refactors and the PR diff narrows to the perf work. - verify_block_integrity: main's #196 form wins outright. It is this branch's "encode each transaction once" rewrite plus the per-transaction keccak check and its forged-hash test, so the PR no longer carries that hunk at all. The blocking-pool wrapper around it is unchanged. - executor.rs: main's inlined replay body, with this branch's `execution_result.gas_used` + `debug_assert_eq!` re-applied on top; the `execute_transactions` extraction is dropped, as it was during #194's review. - rpc_client.rs test imports: union of main's `TestFixtures` and this branch's `consistent_header`. The merged tree is origin/main plus exactly the deltas of e62e3e1 and 432e550. Cargo.lock is untouched; check, clippy, fmt, sort and the full test suite are green.
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
3 participants
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.
Summary
PR 2/6 of the #170 split, following #194 (merged). Removes the
writerparameter threaded throughvalidate_block/validate_block_deriving_updates/replay_block/verify_and_replay, and with it EIP-3155 trace output.Called out separately because it is what vincent's review of #170 said it is: "which removes EIP-3155 trace output entirely — a feature deletion, not a cleanup." It gets its own PR and its own changelog line rather than riding in a 30-file cleanup.
Why delete it
No caller in this repo, and none in mega-reth, ever passed
Some. The parameter existed only to beNone, and it dragged along a#[cfg(feature = "std")]split (TracerEip3155,io::Write) plus arun_plainclosure that existed solely so the tracer branch and the no-std branch could share a body. Theexecute_transactionshelper goes the same way: it was split out so the plain and tracer executors could share one transaction loop, and with a single executor type left its generic where-clause (an HRTB plus two bound-only imports) advertised a generality nothing used, so it folds back intoreplay_block. With both gone,replay_blockis one straight-line body from executor construction to receipts root.The tracing that debug/trace serving actually uses is unaffected — that path is
tracing_executor.rsin the trace server, not this one. Deliberately not taken: routingreplay_blockthroughstart_executor_with_inspector(NoOpInspector)to share the executor prologue, which would add two clones per block on the validator hot path for no gain.Testing
cargo fmt --all --check,cargo clippy --workspace --all-targets --all-features(0 warnings),cargo sort --check, full workspace suite 480 passed / 0 failed,cargo test -p stateless-core --no-default-features --lib --no-runclean. The mainnet fixture replays (validate_block_mainnet_fixtures,validate_block_deriving_updates_mainnet_fixtures) cover the rewrittenreplay_blockbody.Breaking at the next tag
mega-reth's two call sites —
validate_blockandvalidate_block_deriving_updatesincrates/megaeth/validator/src/processor.rs— drop a trailingNonewhen it next bumps its pinned tag.replay_block's signature changes as well, but nothing outside this repo calls it. Nothing breaks at the pinned tag.