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

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<Certificate>,
         deciding_qc: Arc<Certificate>,

light-client/src/consensus/quorum.rs

@@ -80,6 +80,65 @@ pub trait Quorum: Sync {
         cert2: &Certificate2<SeqTypes>,
     ) -> impl Send + Future<Output = Result<()>>;
 
+    /// 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<Output = Result<()>> {
+        async move {
+            match (version.major, version.minor) {
+                (0, 1) => {
+                    self.verify_next_epoch_static::<StaticVersion<0, 1>>(cert)
+                        .await
+                },
+                (0, 2) => {
+                    self.verify_next_epoch_static::<StaticVersion<0, 2>>(cert)
+                        .await
+                },
+                (0, 3) => {
+                    self.verify_next_epoch_static::<StaticVersion<0, 3>>(cert)
+                        .await
+                },
+                (0, 4) => {
+                    self.verify_next_epoch_static::<StaticVersion<0, 4>>(cert)
+                        .await
+                },
+                (0, 5) => {
+                    self.verify_next_epoch_static::<StaticVersion<0, 5>>(cert)
+                        .await
+                },
+                (0, 6) => {
+                    self.verify_next_epoch_static::<StaticVersion<0, 6>>(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<V: StaticVersionType + 'static>(
+        &self,
+        qc: &Certificate,
+    ) -> impl Send + Future<Output = Result<()>> {
+        self.verify_static::<V>(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<V: StaticVersionType + 'static>(
+        &self,
+        cert: &Certificate,
+    ) -> Result<()> {
+        let stake_table = self.membership.next_epoch_stake_table().await?;
+        stake_table
+            .verify_cert::<V, _>(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<Item = u64>) -> 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<PrivKey>, Vec<StakeTableEntry<PubKey>>);
+
+    /// Sign `msg` with every key in `keys`, aggregated over the quorum given by `entries`.
+    fn boundary_signature(
+        msg: &[u8],
+        (keys, entries): &QuorumKeys,
+    ) -> <PubKey as SignatureKey>::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::<Vec<_>>();
+        PubKey::assemble(
+            &pp,
+            &std::iter::repeat_n(true, keys.len()).collect::<BitVec>(),
+            &sigs,
+        )
+    }
+
+    fn boundary_signed_qc(
+        data: QuorumData2<SeqTypes>,
+        view: ViewNumber,
+        quorum: &QuorumKeys,
+    ) -> QuorumCertificate2<SeqTypes> {
+        let commit = VersionedVoteData::new_infallible(
+            data,
+            view,
+            &UpgradeLock::<SeqTypes>::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<SeqTypes>,
+        view: ViewNumber,
+        quorum: &QuorumKeys,
+    ) -> NextEpochQuorumCertificate2<SeqTypes> {
+        let data: NextEpochQuorumData2<SeqTypes> = data.into();
+        let commit = VersionedVoteData::new_infallible(
+            data.clone(),
+            view,
+            &UpgradeLock::<SeqTypes>::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<LeafQueryData<SeqTypes>>,
+        Certificate,
+        Certificate,
+        StakeTableQuorum<(Arc<StakeTable>, Arc<StakeTable>)>,
+    ) {
+        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), &current),
+            Some(boundary_signed_next_epoch_qc(
+                committing_data,
+                ViewNumber::new(10),
+                &next,
+            )),
+        );
+
+        let deciding_quorum = if deciding_signed_by_next {
+            &next
+        } else {
+            &current
+        };
+        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();
+    }
 }