perf(debug-trace-server): thread resolved (number, hash) through the fetch pipeline - #168
Open
flyq wants to merge 2 commits into
Open
perf(debug-trace-server): thread resolved (number, hash) through the fetch pipeline#168flyq wants to merge 2 commits into
flyq wants to merge 2 commits into
Conversation
…fetch pipeline Every resolved lookup was paying a redundant upstream header round trip: tag requests (Latest via eth_blockNumber, Finalized/Safe via a header fetch that kept only the number) re-resolved the same header again by number in resolve_canonical_hash, and every cold fetch re-discovered the block number by hash in do_fetch_block_data Step 1 even when the caller had just resolved it. Tag resolution now binds number -> hash in its single header fetch (hash-verified, the same trust bar as the numeric upstream path; the binding comes from one atomic response, so reorg safety only improves) and Latest switches from eth_blockNumber to that header fetch. resolve_block_number returns (number, Option<hash>); a tag-supplied hash skips resolve_canonical_hash entirely. The by-number prelude, the tag path, and the transaction lookup (tx objects already carry block_number) all hand the resolved pair to get_block_data_with_known_number, whose fetch future skips Step 1's number-discovery header fetch — the number is routing input only (witness-source selection, deadline tightening); everything stays keyed and fetched by hash. Raw by-hash requests keep the discovery fetch. Saves one serial upstream RTT on every tag request and every cold resolved miss. start_mock_rpc grows per-method counters (block_number / header_by_number / header_by_hash) plus tag and by-hash handlers; pinned by tag_resolution_binds_hash_in_its_single_round_trip and known_number_skips_header_discovery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude review status
🛠️ Review did not finish Attempted head This round did not publish: MODEL_ACTION_FAILED in phase review_retry. Anything listed below is from the last round that did. Re-run the workflow or push a new commit to try again. |
/simplify pass over the PR: - collapse get_block_data_by_hash_with_deadline / get_block_data_with_known_number / get_block_data_inner into one pub(crate) get_block_data(hash, known_number, deadline); the tx path passes tx.block_number directly instead of an 8-line match that existed only to pick a wrapper name - move number discovery into the witness arm of the fetch join: the full-block fetch is keyed purely by hash, so it now starts immediately instead of idling behind the discovery header round trip on cold raw by-hash misses (up to one header RTT saved on the one path that still pays discovery); header time is still reported separately in the slow-stage log - parse the mock's eth_getHeaderByNumber param as BlockNumberOrTag so the test wire format cannot drift from the production client's encoding Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
Every resolved lookup was paying a redundant serial upstream header round trip. Tag resolution now binds number → hash in its one header fetch and skips
resolve_canonical_hash; the by-number prelude, the tag path, and the transaction lookup hand the resolved(number, hash)pair to a newget_block_data_with_known_numberentry (data_provider.rs:612), whose fetch future skips the pipeline's number-discovery header fetch. One upstream RTT saved on every tag request and every cold resolved miss; raw by-hash requests are unchanged.Root cause
Two shapes of the same waste. Tag requests:
Latestburnedeth_blockNumberandFinalized/Safefetched a full header but kept only the number — thenresolve_canonical_hashre-fetched the same header by number (shallow heights are never memoized, so this second trip was paid on every tag request, before the cache check). Cold misses:do_fetch_block_dataStep 1 re-discovered the block number viaeth_getHeaderByHasheven when the caller had just resolved number and hash one step earlier.Fix
resolve_block_numberreturns(u64, Option<B256>)(data_provider.rs:744): numeric/Earliest→(n, None);Latest/Finalized/Safe→ one hash-verified header fetch (Latestswitches offeth_blockNumber), returning the atomically-bound hash. Same trust bar as the numeric upstream path; the binding comes from one response, so reorg safety only improves.observe_tipfeeding is unchanged.lookup_block_by_numberuses the tag-supplied hash when present, elseresolve_canonical_hashas before, and fetches viaget_block_data_with_known_number.do_fetch_block_datatakesknown_number: Option<u64>; Step 1 runs only when it isNone(raw by-hash requests). The number is routing input only (witness-source selection, deadline tightening) — every fetch stays keyed by hash, so a wrong number could at worst misroute, never mis-serve.get_block_data_for_txpassestx.block_number(mined tx objects carry it next to the hash;Nonefalls back to discovery) — covers the tx-driven majority of historical traffic.Testing
tag_resolution_binds_hash_in_its_single_round_trip(data_provider.rs:1935): per tag exactly oneeth_getHeaderByNumber, zeroeth_blockNumber, hash returned, tip hint fed; numeric tags stay local.known_number_skips_header_discovery(data_provider.rs:1974): with a known number the fetch pipeline makes zero header calls; the raw by-hash entry still makes exactly one — pinned via per-method counters on the extendedstart_mock_rpcmock (now serves tag params andeth_getHeaderByHash, countingblock_number/header_by_number/header_by_hash).Notes
tx.block_numberSome vs None) has no dedicated unit test — both arms funnel into the same tested inner path; constructing a full OP tx JSON mock wasn't worth the churn.Finalized/Safealready used tag-parameth_getHeaderByNumberin production;Latestnow uses the same call shape.🤖 Generated with Claude Code