Skip to content

fix(light-client): bound leaf proof construction to prevent OOM#4716

Merged
imabdulbasit merged 6 commits into
mainfrom
brendon/light-client-range-protection
Jul 20, 2026
Merged

fix(light-client): bound leaf proof construction to prevent OOM#4716
imabdulbasit merged 6 commits into
mainfrom
brendon/light-client-range-protection

Conversation

@bfish713

Copy link
Copy Markdown
Contributor

Problem

Restarting a wiped archival query node on the new-protocol devnet OOM'd every other query node. The wiped node's backfill requests leaf proofs with a finalized hint far from the requested leaf (its only cached anchor is near the tip), and for leaves with version >= 0.6 LeafProof::push can never terminate a chain — both of its chain-detection branches require pre-0.6 versions. Each serving node therefore materialized the entire leaf chain from the requested height to the hint (~600k leaves on the devnet) in memory per request, and the light client's 300ms no-cancel fallback fanned the same request out to every peer.

Fix

Server (crates/espresso/node/src/api/light_client.rs):

  • New get_leaf_proof routing function shared by the tide-disco endpoint and the axum/v1 API: a nearby finalized hint keeps the cheap assumption path; a distant hint falls through to a bounded cert2/QC-chain proof instead of walking an unbounded range, so old clients that send far hints keep working.
  • All three proof builders enforce a new leaf_proof_chain_limit (default 500, matching small_object_range_limit): the QC-chain walk fails with 404 rather than walking to the tip, the cert2 path refuses a cert2 further than the limit, and the assumption path rejects hints beyond the limit with 400.
  • New shared helper complete_proof_with_cert2: when a QC-chain walk crosses into v0.6 leaves, it completes the proof with the nearest cert2 instead of walking forever. get_leaf_proof_with_cert2 now delegates to it.

Client (light-client/src/state.rs):

  • fetch_leaf only sends the cached upper bound as the finalized hint when it is within 500 blocks of the requested leaf. This also protects peers still running old server code: without the distant hint, an unpatched server takes the cert2 path, which is naturally short.

Docs (light-client.toml): document the proof-size bound, the distant-hint fallback, and the previously-undocumented NewProtocol proof variant.

Testing

  • 4 new tests in api::light_client: cert2 walk bounded and refused when cert2 is too far, QC-chain walk bounded, distant hint rejected at function level, and a chain crossing the v0.6 cutover with non-consecutive views terminating via cert2 (the shape that OOM'd the devnet).
  • New client test test_fetch_leaf_distant_upper_bound verifies a distant cached anchor is not sent as the hint, against a TestClient that rejects distant hints like a patched server.
  • All 14 api::light_client tests and 84 light-client tests pass. The 5 client::test TestNetwork tests are flaky when run concurrently, but fail identically on main and pass individually with these changes.

Reviewer focus

complete_proof_with_cert2 — the interplay with LeafProof::push at the protocol cutover boundary (push may legitimately complete a HotStuff2 chain, in which case the cert2 is not attached), and the remaining budget threading in get_leaf_proof_with_qc_chain.

Follow-up (not in this PR)

FallbackClient's 300ms no-cancel fan-out still lands one slow request on every peer; a longer delay or cancellation would reduce redundant load during mass backfill.

🤖 Generated with Claude Code

@gemini-code-assist

Copy link
Copy Markdown
Contributor
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bfish713's task in 2m 58s —— View job


Code Review

I reviewed the full diff against origin/main. This is a well-structured, well-reasoned fix — the bounding is threaded correctly through all three proof builders, the remaining budget handoff from the QC-chain walk into complete_proof_with_cert2 is correct, and the push-may-complete-a-HotStuff-chain edge at the cutover is handled defensively. Below are the things worth a second look, ordered by importance. None are blocking.

1. The core fallback path (get_leaf_proof routing) has no test

The scenario that OOM'd the devnet is: an old client sends a distant finalized hint, and the server must ignore it and fall through to a bounded cert2/QC-chain proof. That routing decision lives in get_leaf_proof (light_client.rs:45-66), but that function is only invoked from the endpoint at light_client.rs:606 — no test calls it.

The 4 new tests all exercise the sub-builders directly:

  • test_finalized_hint_too_far calls get_leaf_proof_with_finalized_assumption directly and asserts BAD_REQUEST — but via the router a distant hint never reaches that function; it falls through instead. So this test validates a branch that production routing makes unreachable.
  • No test asserts that get_leaf_proof(state, leaf, Some(distant), …) succeeds by producing a bounded QC-chain/cert2 proof.

A test that drives get_leaf_proof with a far hint and asserts it returns a valid (non-Assumption) bounded proof would directly cover the regression this PR exists to prevent. Fix this →

2. Client MAX_FINALIZED_HINT_DISTANCE (500) is hard-coded but the server limit is configurable

light-client/src/state.rs:70 hard-codes MAX_FINALIZED_HINT_DISTANCE = 500 to match the server default (leaf_proof_chain_limit = small_object_range_limit = 500). The boundaries line up exactly today (both use <= 500 / > 500).

But leaf_proof_chain_limit is an operator-configurable Options field. If an operator lowers it below 500, clients will keep sending hints in the (server_limit, 500] gap, and fetch_leaf_from_server (state.rs:779-796) propagates the server's 400/404 as a hard error with no retry-without-hint fallback — so those fetches fail outright instead of degrading gracefully.

Two mild concerns bundled here: (a) the two constants are coupled by convention only, and (b) there's no client-side "on distant-hint rejection, retry without the hint" safety net. Given defaults match, this is low-severity, but a comment cross-referencing the server default (and/or a note that lowering the server limit below the client constant breaks hinted fetches) would prevent a future footgun.

3. Behavior change worth calling out: far-from-tip pre-0.6 leaves can now 404

Previously get_leaf_proof_with_qc_chain always walked to latest_height and appended the final 2-chain (light_client.rs:133-136) — that unbounded walk was the OOM. Now the walk is capped at chain_limit, so a pre-0.6 leaf that is more than chain_limit below the tip and never forms a QC 3-chain within that window (pathological view skew) will return chain_too_long (404) instead of succeeding via the tip's 2-chain. In practice pre-0.6 chains form a 3-chain within a few consecutive views, so this is almost certainly fine — but it is a real behavioral narrowing over the previous "always resolvable at the tip" guarantee. Worth a one-line note in the code or PR description so it's not surprising later.

Minor / nits

  • get_leaf_proof_with_finalized_assumption's internal distance check (light_client.rs:285-295) is now dead via the router (the router only calls it when distance <= chain_limit). Keeping it as defense-in-depth is reasonable; just flagging that test_finalized_hint_too_far is the only thing exercising it.
  • complete_proof_with_cert2: the if !proof.push(leaf) guard at light_client.rs:250 (and the early-return push at :196) can only return true for pre-0.6 leaves, and every leaf here is ≥0.6, so the "push completed the chain, skip the cert2" branch is effectively unreachable for pure new-protocol chains. Correct and harmless — just confirming the reviewer-flagged interplay is sound.

Docs / tests

  • light-client.toml changes are accurate — the new NewProtocol variant and the distant-hint-fallback note match the code.
  • reject_distant_finalized_hints test helper and test_fetch_leaf_distant_upper_bound are a good simulation of a patched server; the client-side omission logic is correctly verified.

Overall: solid fix with correct bounding math. The main gap is test coverage of the actual routing fallback (#1); the rest are hardening/documentation suggestions.

@imabdulbasit
imabdulbasit merged commit 8682dc5 into main Jul 20, 2026
141 checks passed
@imabdulbasit
imabdulbasit deleted the brendon/light-client-range-protection branch July 20, 2026 09:13
@github-actions

Copy link
Copy Markdown
Contributor

Successfully created backport PR for release-ff:

bfish713 added a commit that referenced this pull request Jul 20, 2026
…n to prevent OOM (#4718)

fix(light-client): bound leaf proof construction to prevent OOM (#4716)

* limit range of leaf requests

* send smallest proof possible

* comments

* test(light-client): distant hint falls through to bounded proof



---------



(cherry picked from commit 8682dc5)

Co-authored-by: Brendon Fish <bfish713@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Abdul Basit <45506001+imabdulbasit@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants