From 8b28bd31ce9a276e977c7b8b2d2eb5b965ac52c5 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Fri, 17 Jul 2026 14:43:24 -0400 Subject: [PATCH 1/3] 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 --- light-client/src/consensus/leaf.rs | 4 +- light-client/src/consensus/quorum.rs | 282 ++++++++++++++++++++++++++- 2 files changed, 284 insertions(+), 2 deletions(-) diff --git a/light-client/src/consensus/leaf.rs b/light-client/src/consensus/leaf.rs index 6861f602cbf..5accc490c7f 100644 --- a/light-client/src/consensus/leaf.rs +++ b/light-client/src/consensus/leaf.rs @@ -31,7 +31,9 @@ pub enum FinalityProof { /// * `committing_qc.leaf_commit() == leaf.commit()` /// * `committing_qc.view_number() == leaf.view_number()` /// * `deciding_qc.view_number() == committing_qc.view_number() + 1` - /// * Both QCs have a valid threshold signature given a stake table + /// * Both QCs have a valid threshold signature given a stake table. When the leaf is the last + /// leaf of an epoch, the deciding QC is produced in the subsequent epoch and is checked + /// against the next epoch's stake table. HotStuff2 { committing_qc: Arc, deciding_qc: Arc, diff --git a/light-client/src/consensus/quorum.rs b/light-client/src/consensus/quorum.rs index 4c872be9782..f14fd55e176 100644 --- a/light-client/src/consensus/quorum.rs +++ b/light-client/src/consensus/quorum.rs @@ -80,6 +80,65 @@ pub trait Quorum: Sync { cert2: &Certificate2, ) -> impl Send + Future>; + /// Check a threshold signature on a certificate signed by the epoch after this quorum's. + /// + /// A QC chain proving the last leaf of an epoch necessarily contains certificates produced + /// in the subsequent epoch, signed by that epoch's quorum. + fn verify_next_epoch( + &self, + cert: &Certificate, + version: Version, + ) -> impl Send + Future> { + async move { + match (version.major, version.minor) { + (0, 1) => { + self.verify_next_epoch_static::>(cert) + .await + }, + (0, 2) => { + self.verify_next_epoch_static::>(cert) + .await + }, + (0, 3) => { + self.verify_next_epoch_static::>(cert) + .await + }, + (0, 4) => { + self.verify_next_epoch_static::>(cert) + .await + }, + (0, 5) => { + self.verify_next_epoch_static::>(cert) + .await + }, + (0, 6) => { + self.verify_next_epoch_static::>(cert) + .await + }, + _ => { + const { + assert!(MAX_SUPPORTED_VERSION.major == 0); + assert!(MAX_SUPPORTED_VERSION.minor == 6); + } + bail!("unsupported version {version}"); + }, + } + } + } + + /// Same as [`verify_next_epoch`](Self::verify_next_epoch), but with the version as a + /// type-level parameter. + /// + /// The default delegates to [`verify_static`](Self::verify_static), which is only appropriate + /// for quorums that do not distinguish epochs (such as test mocks). Implementations backed by + /// per-epoch stake tables must override this to verify against the next epoch's quorum. + fn verify_next_epoch_static( + &self, + qc: &Certificate, + ) -> impl Send + Future> { + self.verify_static::(qc) + } + /// Verify that QCs are signed, form a chain starting from `leaf`, with a particular protocol /// version. /// @@ -135,8 +194,32 @@ pub trait Quorum: Sync { _ => version, }; + // Which epoch's quorum do we expect to have signed this certificate? A chain + // proving the last leaf of an epoch necessarily contains certificates produced in + // the subsequent epoch, signed by the next epoch's quorum. Each certificate commits + // to its epoch in the signed payload, so dispatching on it is sound: a certificate + // claiming the wrong epoch will fail signature verification. + let leaf_epoch = first.unwrap_or(cert).epoch(); + let next_epoch = match (leaf_epoch, cert.epoch()) { + // Certificates from before the epochs upgrade are always checked against the + // quorum supplied for this proof. + (None, _) | (_, None) => false, + (Some(leaf_epoch), Some(cert_epoch)) if cert_epoch == leaf_epoch => false, + (Some(leaf_epoch), Some(cert_epoch)) if cert_epoch == leaf_epoch + 1 => true, + (Some(leaf_epoch), Some(cert_epoch)) => { + bail!( + "certificate from epoch {cert_epoch} cannot justify a leaf in epoch \ + {leaf_epoch}" + ); + }, + }; + // Check the signature. - self.verify(cert, version).await?; + if next_epoch { + self.verify_next_epoch(cert, version).await?; + } else { + self.verify(cert, version).await?; + } // Check chaining. if let Some(prev) = curr { @@ -308,10 +391,43 @@ where .await .context("verifying cert2") } + + async fn verify_next_epoch_static( + &self, + cert: &Certificate, + ) -> Result<()> { + let stake_table = self.membership.next_epoch_stake_table().await?; + stake_table + .verify_cert::(cert.qc()) + .await + .context("verifying QC against next epoch quorum")?; + + // A certificate from the epoch after the proof's epoch is never itself part of an epoch + // transition (transition certificates belong to the epoch that is ending), so there is no + // second next-epoch QC to check. We could not check one anyway: that would require the + // stake table two epochs ahead of the proof's. + ensure!( + cert.next_epoch_qc().is_none(), + "unexpected next-epoch QC on a certificate from the next epoch" + ); + + Ok(()) + } } #[cfg(test)] mod test { + use bitvec::vec::BitVec; + use committable::Commitment; + use espresso_types::PrivKey; + use hotshot_query_service_types::availability::LeafQueryData; + use hotshot_types::{ + data::{EpochNumber, ViewNumber}, + simple_certificate::{NextEpochQuorumCertificate2, QuorumCertificate2}, + simple_vote::{NextEpochQuorumData2, QuorumData2, VersionedVoteData}, + traits::signature_key::SignatureKey, + vote::Certificate as _, + }; use pretty_assertions::assert_eq; use super::*; @@ -481,4 +597,168 @@ mod test { .unwrap(); assert_eq!(version, leaves[0].header().version()); } + + const BOUNDARY_EPOCH_HEIGHT: u64 = 10; + + /// Generate BLS keys and stake table entries for validators with the given seeds. + fn boundary_quorum(seeds: impl IntoIterator) -> QuorumKeys { + seeds + .into_iter() + .map(|i| { + let (stake_key, priv_key) = + PubKey::generated_from_seed_indexed(Default::default(), i); + ( + priv_key, + StakeTableEntry { + stake_key, + stake_amount: U256::from(1), + }, + ) + }) + .unzip() + } + + type QuorumKeys = (Vec, Vec>); + + /// Sign `msg` with every key in `keys`, aggregated over the quorum given by `entries`. + fn boundary_signature( + msg: &[u8], + (keys, entries): &QuorumKeys, + ) -> ::QcType { + let total = entries + .iter() + .fold(U256::ZERO, |acc, entry| acc + entry.stake_amount); + let pp = PubKey::public_parameter(entries, supermajority_threshold(total)); + let sigs = keys + .iter() + .map(|key| PubKey::sign(key, msg).unwrap()) + .collect::>(); + PubKey::assemble( + &pp, + &std::iter::repeat_n(true, keys.len()).collect::(), + &sigs, + ) + } + + fn boundary_signed_qc( + data: QuorumData2, + view: ViewNumber, + quorum: &QuorumKeys, + ) -> QuorumCertificate2 { + let commit = VersionedVoteData::new_infallible( + data, + view, + &UpgradeLock::::new(Upgrade::trivial(EPOCH_VERSION)), + ) + .commit(); + let sig = boundary_signature(commit.as_ref(), quorum); + QuorumCertificate2::create_signed_certificate(commit, data, sig, view) + } + + fn boundary_signed_next_epoch_qc( + data: QuorumData2, + view: ViewNumber, + quorum: &QuorumKeys, + ) -> NextEpochQuorumCertificate2 { + let data: NextEpochQuorumData2 = data.into(); + let commit = VersionedVoteData::new_infallible( + data.clone(), + view, + &UpgradeLock::::new(Upgrade::trivial(EPOCH_VERSION)), + ) + .commit(); + let commit_bytes: [u8; 32] = commit.into(); + let sig = boundary_signature(commit.as_ref(), quorum); + NextEpochQuorumCertificate2::new( + data, + Commitment::from_raw(commit_bytes), + view, + Some(sig), + Default::default(), + ) + } + + /// Build a 2-chain proving the last leaf of epoch 1 (block 10), where epochs 1 and 2 have + /// disjoint quorums. This mirrors the switch from the genesis stake table to the first + /// contract-sourced one on a real network. + /// + /// The deciding QC is produced in epoch 2. If `deciding_signed_by_next` it is correctly signed + /// by epoch 2's quorum; otherwise it is (invalidly) signed by epoch 1's quorum. + async fn epoch_boundary_fixture( + deciding_signed_by_next: bool, + ) -> ( + Vec>, + Certificate, + Certificate, + StakeTableQuorum<(Arc, Arc)>, + ) { + let current = boundary_quorum(0..5); + let next = boundary_quorum(5..10); + + // Leaf 10 is the last leaf of epoch 1; leaf 11 is the first leaf of epoch 2. + let leaves = leaf_chain(9..=11, EPOCH_VERSION).await; + + // Block 10 is an epoch transition block, so its QC is dual-signed by both quorums. + let committing_data = QuorumData2 { + leaf_commit: Committable::commit(leaves[1].leaf()), + epoch: Some(EpochNumber::new(1)), + block_number: Some(10), + }; + let committing_qc = Certificate::new( + boundary_signed_qc(committing_data, ViewNumber::new(10), ¤t), + Some(boundary_signed_next_epoch_qc( + committing_data, + ViewNumber::new(10), + &next, + )), + ); + + let deciding_quorum = if deciding_signed_by_next { + &next + } else { + ¤t + }; + let deciding_qc = Certificate::non_epoch_change(boundary_signed_qc( + QuorumData2 { + leaf_commit: Committable::commit(leaves[2].leaf()), + epoch: Some(EpochNumber::new(2)), + block_number: Some(11), + }, + ViewNumber::new(11), + deciding_quorum, + )); + + let quorum = StakeTableQuorum::new( + ( + Arc::new(StakeTable::from(current.1)), + Arc::new(StakeTable::from(next.1)), + ), + BOUNDARY_EPOCH_HEIGHT, + ); + (leaves, committing_qc, deciding_qc, quorum) + } + + /// A 2-chain proving the last leaf of an epoch includes a deciding QC signed by the next + /// epoch's quorum; it must be verified against that quorum, not the quorum of the epoch of the + /// leaf under proof. + #[test_log::test(tokio::test(flavor = "multi_thread"))] + async fn test_epoch_boundary_quorum_change() { + let (leaves, committing_qc, deciding_qc, quorum) = epoch_boundary_fixture(true).await; + let version = quorum + .verify_qc_chain_and_get_version(leaves[1].leaf(), [&committing_qc, &deciding_qc]) + .await + .unwrap(); + assert_eq!(version, leaves[1].header().version()); + } + + /// A deciding QC claiming to be from the next epoch but signed by the current epoch's quorum + /// must fail verification. + #[test_log::test(tokio::test(flavor = "multi_thread"))] + async fn test_epoch_boundary_deciding_qc_wrong_quorum() { + let (leaves, committing_qc, deciding_qc, quorum) = epoch_boundary_fixture(false).await; + quorum + .verify_qc_chain_and_get_version(leaves[1].leaf(), [&committing_qc, &deciding_qc]) + .await + .unwrap_err(); + } } From b5d9596dbf7af6d3ea8d14285cd8122d20ba8252 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Fri, 17 Jul 2026 16:56:36 -0400 Subject: [PATCH 2/3] fix mid epoch check --- light-client/src/consensus/quorum.rs | 125 ++++++++++++++++++++++----- 1 file changed, 102 insertions(+), 23 deletions(-) diff --git a/light-client/src/consensus/quorum.rs b/light-client/src/consensus/quorum.rs index f14fd55e176..46364b43b3b 100644 --- a/light-client/src/consensus/quorum.rs +++ b/light-client/src/consensus/quorum.rs @@ -81,9 +81,6 @@ pub trait Quorum: Sync { ) -> impl Send + Future>; /// Check a threshold signature on a certificate signed by the epoch after this quorum's. - /// - /// A QC chain proving the last leaf of an epoch necessarily contains certificates produced - /// in the subsequent epoch, signed by that epoch's quorum. fn verify_next_epoch( &self, cert: &Certificate, @@ -199,13 +196,20 @@ pub trait Quorum: Sync { // the subsequent epoch, signed by the next epoch's quorum. Each certificate commits // to its epoch in the signed payload, so dispatching on it is sound: a certificate // claiming the wrong epoch will fail signature verification. - let leaf_epoch = first.unwrap_or(cert).epoch(); - let next_epoch = match (leaf_epoch, cert.epoch()) { + let committing = first.unwrap_or(cert); + let next_epoch = match (committing.epoch(), cert.epoch()) { // Certificates from before the epochs upgrade are always checked against the // quorum supplied for this proof. (None, _) | (_, None) => false, (Some(leaf_epoch), Some(cert_epoch)) if cert_epoch == leaf_epoch => false, - (Some(leaf_epoch), Some(cert_epoch)) if cert_epoch == leaf_epoch + 1 => true, + (Some(leaf_epoch), Some(cert_epoch)) if cert_epoch == leaf_epoch + 1 => { + // Only the last leaf of an epoch is justified by the next epoch's quorum, + // and its committing QC is dual-signed: it carries a next-epoch QC that + // `verify` has already checked against the next epoch's quorum. Requiring + // that here stops the next epoch's quorum from finalizing an interior leaf + // of the previous epoch, which it never co-signed. + committing.next_epoch_qc().is_some() + }, (Some(leaf_epoch), Some(cert_epoch)) => { bail!( "certificate from epoch {cert_epoch} cannot justify a leaf in epoch \ @@ -368,13 +372,22 @@ where if version(V::MAJOR, V::MINOR) >= EPOCH_VERSION { // If this certificate is part of an epoch change, also check that the next epoch's - // quorum has signed. - if let Some(next_epoch_qc) = cert.verify_next_epoch_qc(self.epoch_height)? { - let stake_table = self.membership.next_epoch_stake_table().await?; - stake_table - .verify_cert::(next_epoch_qc) - .await - .context("verifying next epoch QC")?; + // quorum has signed. Reject a next-epoch QC attached to any other certificate: it would + // otherwise go unverified, yet the epoch dispatch in `verify_qc_chain_and_get_version` + // treats a next-epoch QC on the committing certificate as proof that the next epoch + // co-signed the leaf. + match cert.verify_next_epoch_qc(self.epoch_height)? { + Some(next_epoch_qc) => { + let stake_table = self.membership.next_epoch_stake_table().await?; + stake_table + .verify_cert::(next_epoch_qc) + .await + .context("verifying next epoch QC")?; + }, + None => ensure!( + cert.next_epoch_qc().is_none(), + "certificate carries a next-epoch QC but is not an epoch transition" + ), } } @@ -404,8 +417,7 @@ where // A certificate from the epoch after the proof's epoch is never itself part of an epoch // transition (transition certificates belong to the epoch that is ending), so there is no - // second next-epoch QC to check. We could not check one anyway: that would require the - // stake table two epochs ahead of the proof's. + // second next-epoch QC to check. ensure!( cert.next_epoch_qc().is_none(), "unexpected next-epoch QC on a certificate from the next epoch" @@ -600,7 +612,6 @@ mod test { const BOUNDARY_EPOCH_HEIGHT: u64 = 10; - /// Generate BLS keys and stake table entries for validators with the given seeds. fn boundary_quorum(seeds: impl IntoIterator) -> QuorumKeys { seeds .into_iter() @@ -620,7 +631,6 @@ mod test { type QuorumKeys = (Vec, Vec>); - /// Sign `msg` with every key in `keys`, aggregated over the quorum given by `entries`. fn boundary_signature( msg: &[u8], (keys, entries): &QuorumKeys, @@ -679,11 +689,9 @@ mod test { } /// Build a 2-chain proving the last leaf of epoch 1 (block 10), where epochs 1 and 2 have - /// disjoint quorums. This mirrors the switch from the genesis stake table to the first - /// contract-sourced one on a real network. - /// - /// The deciding QC is produced in epoch 2. If `deciding_signed_by_next` it is correctly signed - /// by epoch 2's quorum; otherwise it is (invalidly) signed by epoch 1's quorum. + /// disjoint quorums. The deciding QC is produced in epoch 2. If `deciding_signed_by_next` it + /// is correctly signed by epoch 2's quorum; otherwise it is (invalidly) signed by epoch 1's + /// quorum. async fn epoch_boundary_fixture( deciding_signed_by_next: bool, ) -> ( @@ -695,7 +703,6 @@ mod test { let current = boundary_quorum(0..5); let next = boundary_quorum(5..10); - // Leaf 10 is the last leaf of epoch 1; leaf 11 is the first leaf of epoch 2. let leaves = leaf_chain(9..=11, EPOCH_VERSION).await; // Block 10 is an epoch transition block, so its QC is dual-signed by both quorums. @@ -761,4 +768,76 @@ mod test { .await .unwrap_err(); } + + /// Build a 2-chain over an interior (non-boundary) leaf of epoch 1 whose deciding QC is forged + /// by epoch 2's quorum, mimicking a next epoch trying to finalize a leaf the current epoch + /// never decided. If `disguise_as_boundary`, the committing QC also carries a next-epoch QC to + /// imitate a genuine epoch-transition committing QC. + async fn mid_epoch_forgery_fixture( + disguise_as_boundary: bool, + ) -> ( + Vec>, + Certificate, + Certificate, + StakeTableQuorum<(Arc, Arc)>, + ) { + let current = boundary_quorum(0..5); + let next = boundary_quorum(5..10); + + // Block 5 is in the interior of epoch 1 (epoch height 10), not an epoch boundary. + let leaves = leaf_chain(4..=6, EPOCH_VERSION).await; + + let committing_data = QuorumData2 { + leaf_commit: Committable::commit(leaves[1].leaf()), + epoch: Some(EpochNumber::new(1)), + block_number: Some(5), + }; + let committing_qc = Certificate::new( + boundary_signed_qc(committing_data, ViewNumber::new(5), ¤t), + disguise_as_boundary + .then(|| boundary_signed_next_epoch_qc(committing_data, ViewNumber::new(5), &next)), + ); + + let deciding_qc = Certificate::non_epoch_change(boundary_signed_qc( + QuorumData2 { + leaf_commit: Committable::commit(leaves[2].leaf()), + epoch: Some(EpochNumber::new(2)), + block_number: Some(6), + }, + ViewNumber::new(6), + &next, + )); + + let quorum = StakeTableQuorum::new( + ( + Arc::new(StakeTable::from(current.1)), + Arc::new(StakeTable::from(next.1)), + ), + BOUNDARY_EPOCH_HEIGHT, + ); + (leaves, committing_qc, deciding_qc, quorum) + } + + /// The next epoch's quorum must not be able to finalize an interior leaf of the previous epoch: + /// only the last leaf of an epoch is justified by the next epoch, so a deciding QC signed by + /// the next epoch over a non-boundary leaf must be rejected. + #[test_log::test(tokio::test(flavor = "multi_thread"))] + async fn test_mid_epoch_next_epoch_forgery_rejected() { + let (leaves, committing_qc, deciding_qc, quorum) = mid_epoch_forgery_fixture(false).await; + quorum + .verify_qc_chain_and_get_version(leaves[1].leaf(), [&committing_qc, &deciding_qc]) + .await + .unwrap_err(); + } + + /// The same forgery must be rejected even when the committing QC is dressed up with a + /// next-epoch QC to imitate a genuine epoch-transition committing QC. + #[test_log::test(tokio::test(flavor = "multi_thread"))] + async fn test_mid_epoch_next_epoch_forgery_with_fake_transition_rejected() { + let (leaves, committing_qc, deciding_qc, quorum) = mid_epoch_forgery_fixture(true).await; + quorum + .verify_qc_chain_and_get_version(leaves[1].leaf(), [&committing_qc, &deciding_qc]) + .await + .unwrap_err(); + } } From 896a9e3b7a6e6eab833fbd261e837f5e2c752490 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Fri, 17 Jul 2026 19:53:08 -0400 Subject: [PATCH 3/3] 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 --- light-client/src/testing.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/light-client/src/testing.rs b/light-client/src/testing.rs index 5baf6814906..be620217012 100644 --- a/light-client/src/testing.rs +++ b/light-client/src/testing.rs @@ -513,19 +513,32 @@ impl InnerTestClient { { assert!(block_header.set_next_stake_table_hash(self.stake_table_hash(*epoch + 1))); } - let next_epoch_justify_qc = if i > 0 && is_epoch_transition(i as u64, epoch_height) { + // As in production, a leaf carries a next-epoch justify QC exactly when the block + // justified by its QC (the parent) is part of an epoch transition, and that QC is + // signed by the epoch after the parent's. New-protocol leaves never carry one: + // epoch changes there are justified by `Certificate2`, and the `Leaf2` + // representation always has `next_epoch_justify_qc: None`. + let next_epoch_justify_qc = if i > 0 + && self.version_at(i as u64) < NEW_PROTOCOL_VERSION + && is_epoch_transition(i as u64 - 1, epoch_height) + { let parent_qc = self.leaves[i - 1].qc().clone(); + let parent_epoch = epoch_from_block_number(i as u64 - 1, epoch_height); + let parent_upgrade = Upgrade::trivial(self.version_at(i as u64 - 1)); let data: NextEpochQuorumData2 = parent_qc.data.into(); let versioned_commit = VersionedVoteData::new_infallible( data.clone(), parent_qc.view_number, - &UpgradeLock::::new(upgrade), + &UpgradeLock::::new(parent_upgrade), ) .commit(); let commit_bytes: [u8; 32] = versioned_commit.into(); - let (next_keys, next_validators): (Vec<_>, Vec<_>) = - self.quorum_for_epoch(*epoch + 1).iter().cloned().unzip(); + let (next_keys, next_validators): (Vec<_>, Vec<_>) = self + .quorum_for_epoch(parent_epoch + 1) + .iter() + .cloned() + .unzip(); let mut next_entries = vec![]; let mut next_total = U256::ZERO; for validator in &next_validators {