Skip to content

perf(debug-trace-server): thread resolved (number, hash) through the fetch pipeline - #168

Open
flyq wants to merge 2 commits into
mainfrom
liquan/perf/thread-resolved-block-meta
Open

perf(debug-trace-server): thread resolved (number, hash) through the fetch pipeline#168
flyq wants to merge 2 commits into
mainfrom
liquan/perf/thread-resolved-block-meta

Conversation

@flyq

@flyq flyq commented Aug 1, 2026

Copy link
Copy Markdown
Member

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 new get_block_data_with_known_number entry (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: Latest burned eth_blockNumber and Finalized/Safe fetched a full header but kept only the number — then resolve_canonical_hash re-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_data Step 1 re-discovered the block number via eth_getHeaderByHash even when the caller had just resolved number and hash one step earlier.

Fix

  • resolve_block_number returns (u64, Option<B256>) (data_provider.rs:744): numeric/Earliest(n, None); Latest/Finalized/Safe → one hash-verified header fetch (Latest switches off eth_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_tip feeding is unchanged.
  • lookup_block_by_number uses the tag-supplied hash when present, else resolve_canonical_hash as before, and fetches via get_block_data_with_known_number.
  • do_fetch_block_data takes known_number: Option<u64>; Step 1 runs only when it is None (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_tx passes tx.block_number (mined tx objects carry it next to the hash; None falls 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 one eth_getHeaderByNumber, zero eth_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 extended start_mock_rpc mock (now serves tag params and eth_getHeaderByHash, counting block_number/header_by_number/header_by_hash).
  • Full workspace: fmt/clippy/cargo-sort clean, all test targets green (108 bin unit tests).

Notes

  • The tx-path branch (tx.block_number Some 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.
  • Upstream compatibility: Finalized/Safe already used tag-param eth_getHeaderByNumber in production; Latest now uses the same call shape.

🤖 Generated with Claude Code

…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>
@mega-maxwell

mega-maxwell Bot commented Aug 1, 2026

Copy link
Copy Markdown

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; reply to a finding in its own review thread, and answer an open question in a reply on this PR. The next review round reconciles your answer.

🛠️ Review did not finish

Attempted head 78490c59 · updated 2026-08-01T15:15:18+00:00

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant