Skip to content

[Backport release-ff] Light client fixes and V5→V6 upgrade tests#4742

Merged
bfish713 merged 3 commits into
release-fffrom
backport-light-client-to-release-ff
Jul 22, 2026
Merged

[Backport release-ff] Light client fixes and V5→V6 upgrade tests#4742
bfish713 merged 3 commits into
release-fffrom
backport-light-client-to-release-ff

Conversation

@bfish713

Copy link
Copy Markdown
Contributor

Backports the three light-client PRs that landed on main after the last release-ff sync (#4712):

This PR:

This PR does not:

Key places to review:

Things tested

  • cargo nextest run -p light-client: 96/96 pass.
  • cargo check -p light-client -p espresso-node --tests clean.
  • The new test_light_client_new_protocol_upgrade integration test is left to CI (30-min budget).

🤖 Generated with Claude Code

bfish713 and others added 3 commits July 22, 2026 12:28
#4713)

* fix(light-client): verify boundary 2-chain QCs against their own epoch

A 2-chain proving the last leaf of an epoch includes a deciding QC
produced in the subsequent epoch, signed by the next epoch's quorum.
verify_qc_chain_and_get_version checked every certificate against the
single quorum of the leaf's epoch, so fetching the last leaf of an
epoch failed with "invalid threshold signature: Pairing check failed"
whenever adjacent epochs have different stake tables. First observed
when fetching the last leaf signed by the genesis stake table on a
fresh node, where the next epoch uses the first contract-sourced
stake table.

Dispatch each certificate on the epoch committed in its signed
payload: certificates from the leaf's epoch are checked against the
proof's quorum, certificates from the following epoch against the
next epoch's stake table. Any other epoch is rejected. The epoch
number is part of the signed vote data, so a certificate claiming the
wrong epoch fails signature verification.

Regression tests (real BLS signatures over disjoint per-epoch
quorums; both fail without the fix):
- test_epoch_boundary_quorum_change
- test_epoch_boundary_deciding_qc_wrong_quorum

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix mid epoch check

* fix(light-client): align mock next-epoch QC placement with production

The test client attached next_epoch_justify_qc to a leaf when the
leaf's own block was in epoch transition. Production attaches it when
the block justified by the leaf's QC (the parent) is in transition, so
the mock dual-signed certificates one block too early and never
dual-signed the QC for the last block of an epoch. The stricter
verification introduced in this branch (rejecting a next-epoch QC on a
non-transition certificate) correctly rejected these mock chains,
failing every state::test stake table catchup test.

Attach the QC based on the parent block, sign it with the quorum of
the epoch after the parent's (which differs from the leaf's epoch + 1
at the epoch boundary), and commit the vote data under the parent's
protocol version. Also gate the QC to pre-0.6 leaves: the new protocol
justifies epoch changes with Certificate2 and always stores
next_epoch_justify_qc: None in the Leaf2 representation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit f8cd09d)
* fix(light-client): report error cause when all fallback clients fail

* refactor(light-client): use structured err field in fetch logs

* refactor(light-client): use %err in fetch logs

(cherry picked from commit 845a6e2)
* fix(light-client): reject HotStuff2 finality proofs across the cutover

The HotStuff2 2-chain commit rule is unsound for new-protocol (0.6) leaves:
consecutive phase-1 certificates exist even for leaves that never finalize,
so finality there requires a Certificate2. A malicious query server could
package two real phase-1 certs into a `HotStuff2` proof and convince a light
client that a non-finalized 0.6 leaf is final.

Tighten `LeafProof::verify`'s HotStuff2 arm to require both the proven leaf
and every certificate in the chain to predate the cutover:

- reject when the proven leaf is >= NEW_PROTOCOL_VERSION, and
- reject when any QC in the chain (i.e. the deciding QC) was verified at
  >= NEW_PROTOCOL_VERSION, which happens when the chain spans the boundary.

To support the second check, `verify_qc_chain_and_get_version` now returns
`ChainVersions { leaf, max_cert }` exposing the highest version any cert was
verified under. The honest server never emits a spanning 2-chain (it switches
to the cert2 path at the first new-protocol leaf), so this costs no liveness.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(light-client): stop prover emitting 2-chains that span the cutover

LeafProof::push gated HotStuff2 completion only on the proven leaf's
version, but the deciding QC signs the following leaf. When proving the
last legacy leaf, that leaf is the first new-protocol leaf, so push
assembled a spanning 2-chain whose deciding QC is a post-cutover phase-1
certificate - exactly the proof the verifier now rejects - and the
server preferred it over the cert2 it had already fetched.

Gate completion on the deciding QC's leaf instead (which subsumes the
old check, since versions are non-decreasing along the chain), so the
prover falls through to a cert2 proof at the boundary.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(light-client): trim verbose comments

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* verify light client proofs for v5 to v6

* test(light-client): cover upgrade_height-2 finality on both sides of the cutover

---------

Co-authored-by: Brendon Fish <bfish713@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 81eccae)
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

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


Review: backport of light-client fixes + V5→V6 upgrade tests

  • Gather context (diff vs origin/release-ff, the 3 backported commits)
  • Review the quorum.rs conflict resolution
  • Review the rest of the diff
  • Post review

Verdict: LGTM. This is a faithful backport of #4713, #4726, and #4724. The three commits carry clean (cherry picked from …) back-references, and the one non-trivial piece — the quorum.rs conflict resolution — reconciles correctly. No blocking issues found.

light-client/src/consensus/quorum.rs (the flagged conflict)

The resolution restores main's #4722 code (ChainVersions, verify_next_epoch/verify_next_epoch_static) and composes it with #4713's epoch-boundary dispatch. Walking the merged logic:

  • verify_qc_chain_and_get_version (lines 198–230) computes next_epoch per certificate from the signed epoch fields, and only treats a deciding QC as next-epoch-signed when (a) cert_epoch == leaf_epoch + 1 and (b) the committing QC carries a next_epoch_qc(). That closes the interior-leaf forgery: a next epoch cannot finalize a non-boundary leaf it never co-signed. The cert_epoch > leaf_epoch + 1 arm bails, and the pre-epochs (None) case falls back to the single supplied quorum. Sound.
  • verify (lines 391–412) now rejects a stray next_epoch_qc on a non-transition certificate (ensure!(cert.next_epoch_qc().is_none(), …)), which is exactly what makes the dispatch above trustworthy — otherwise an attached-but-unverified next-epoch QC could masquerade as a genuine transition.
  • The four new tests (test_epoch_boundary_quorum_change, …_deciding_qc_wrong_quorum, test_mid_epoch_next_epoch_forgery_rejected, …_with_fake_transition_rejected) exercise both the happy path and both forgery variants, including the disguised-transition case. Good coverage of the security-relevant branches.

Other files (all applied cleanly per the PR description; confirmed)

  • client.rs — the fetch<T> helper de-duplicates the GET+deserialize boilerplate, and the get_any_recursive change now threads prev_err so the final error is the last real client failure rather than a generic "no clients" message. The prev_err is propagated (not overwritten) through the timeout branch and set to Some(err) on the eager-failure branch — correct.
  • leaf.rs / testing.rs — doc + test-fixture updates; the testing.rs change correctly bases the next-epoch justify QC on the parent's epoch/version and suppresses it for new-protocol leaves.
  • api.rs / api/light_client.rs — test-only: extraction of check_light_client_proofs / check_light_client_stake_table helpers and the new test_light_client_new_protocol_upgrade + test_leaf_before_cutover_uses_{hotstuff2,cert2} tests. No production-code changes here.
  • .config/nextest.toml — adds a slow-timeout/threads-required override for the new integration test.

Notes / non-blocking

  • I could not independently re-verify the "byte-identical to main" claim — fetching origin/main in this environment requires an approval I don't have. The claim is verifiable locally with git diff origin/main -- light-client/ crates/espresso/node/src/api{,/light_client}.rs .config/nextest.toml (should be empty), and the diff I reviewed against origin/release-ff is internally consistent with it.
  • The new test_light_client_new_protocol_upgrade is left to CI (30-min budget), as stated. Worth confirming it's green before merge, since it's the main behavioral coverage for the V5→V6 boundary.
  • Minor: in that test, heights = (upgrade_height-3..=upgrade_height+1).chain(epoch_change_height-1..=max_block) — if the two boundaries are close the ranges could overlap and re-check a height, which is harmless (idempotent assertions), just slightly redundant.
    · backport-light-client-to-release-ff

@bfish713
bfish713 merged commit 7914b7f into release-ff Jul 22, 2026
144 checks passed
@bfish713
bfish713 deleted the backport-light-client-to-release-ff branch July 22, 2026 20:46
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.

2 participants