BFT-496: Return at most one quorum for now.

node/actors/network/src/gossip/batch_votes.rs

@@ -38,7 +38,7 @@ impl BatchUpdateStats {
 pub(crate) struct BatchVotes {
     /// The latest vote received from each attester. We only keep the last one
     /// for now, hoping that with 1 minute batches there's plenty of time for
-    /// the quorum to be reached, but eventually we'll have to allow multiple
+    /// the quorum to be reached, but eventually we might have to allow multiple
     /// votes across different heights.
     pub(crate) votes: im::HashMap<attester::PublicKey, Arc<attester::Signed<attester::Batch>>>,
 
@@ -51,6 +51,10 @@ pub(crate) struct BatchVotes {
         im::OrdMap<attester::BatchNumber, im::HashMap<attester::BatchHash, attester::Weight>>,
 
     /// The minimum batch number for which we are still interested in votes.
+    ///
+    /// Because we only store 1 vote per attester the memory is very much bounded,
+    /// but this extra pruning mechanism can ge used to get rid of votes of attesters
+    /// who rotated out of the committee (which currently requires a re-genesis, but still).
     pub(crate) min_batch_number: attester::BatchNumber,
 }
 
@@ -131,20 +135,19 @@ impl BatchVotes {
 
     /// Check if we have achieved quorum for any of the batch hashes.
     ///
-    /// The return value is a vector because eventually we will be potentially waiting for
-    /// quorums on multiple heights simultaneously.
+    /// Returns the first quorum it finds, after which we expect that the state of the main node or L1
+    /// will indicate that attestation on the next height can happen, which will either naturally move
+    /// the QC, or we can do so by increasing the `min_batch_number`.
     ///
-    /// For repeated queries we can supply a skip list of heights for which we already saved the QC.
-    pub(super) fn find_quorums(
+    /// While we only store 1 vote per attester we'll only ever have at most one quorum anyway.
+    pub(super) fn find_quorum(
         &self,
         attesters: &attester::Committee,
         genesis: &attester::GenesisHash,
-        skip: impl Fn(attester::BatchNumber) -> bool,
-    ) -> Vec<attester::BatchQC> {
+    ) -> Option<attester::BatchQC> {
         let threshold = attesters.threshold();
         self.support
             .iter()
-            .filter(|(number, _)| !skip(**number))
             .flat_map(|(number, candidates)| {
                 candidates
                     .iter()
@@ -170,7 +173,7 @@ impl BatchVotes {
                         }
                     })
             })
-            .collect()
+            .next()
     }
 
     /// Set the minimum batch number for which we admit votes.

node/actors/network/src/gossip/mod.rs

@@ -157,15 +157,12 @@ impl Network {
         let genesis = self.genesis().hash();
         let mut sub = self.batch_votes.subscribe();
         loop {
-            // In the future when we might be gossiping about multiple batches at the same time,
-            // we can collect the ones we submitted into a skip list until we see them confirmed
-            // on L1 and we can finally increase the minimum as well.
-            let quorums = {
+            let quorum_opt = {
                 let votes = sync::changed(ctx, &mut sub).await?;
-                votes.find_quorums(attesters, &genesis, |_| false)
+                votes.find_quorum(attesters, &genesis)
             };
 
-            for qc in quorums {
+            if let Some(qc) = quorum_opt {
                 // In the future this should come from confirmations, but for now it's best effort, so we can forget ASAP.
                 // TODO: An initial value could be looked up in the database even now.
                 let next_batch_number = qc.message.number.next();

node/actors/network/src/gossip/tests/mod.rs

@@ -673,27 +673,21 @@ fn test_batch_votes_quorum() {
 
             // Check that as soon as we have quorum it's found.
             if batches[b].1 >= attesters.threshold() {
-                let qs = votes.find_quorums(&attesters, &genesis, |_| false);
-                assert!(!qs.is_empty(), "should find quorum");
-                assert!(qs[0].message == *batch);
-                assert!(qs[0].signatures.keys().count() > 0);
+                let qc = votes
+                    .find_quorum(&attesters, &genesis)
+                    .expect("should find quorum");
+                assert!(qc.message == *batch);
+                assert!(qc.signatures.keys().count() > 0);
             }
         }
 
-        if let Some(quorum) = batches
+        if batches
             .iter()
             .find(|b| b.1 >= attesters.threshold())
-            .map(|(b, _)| b)
+            .is_none()
         {
-            // Check that a quorum can be skipped
-            assert!(votes
-                .find_quorums(&attesters, &genesis, |b| b == quorum.number)
-                .is_empty());
-        } else {
             // Check that if there was no quoroum then we don't find any.
-            assert!(votes
-                .find_quorums(&attesters, &genesis, |_| false)
-                .is_empty());
+            assert!(votes.find_quorum(&attesters, &genesis).is_none());
         }
 
         // Check that the minimum batch number prunes data.