BFT-474: Removed min batch number query

core/lib/dal/src/consensus_dal.rs

@@ -461,40 +461,6 @@ impl ConsensusDal<'_, '_> {
             .number
             .map(|number| attester::BatchNumber(number as u64)))
     }
-
-    /// Get the earliest of L1 batch which does not have a corresponding quorum certificate
-    /// and need signatures to be gossiped and collected.
-    ///
-    /// On the main node this means every L1 batch, because we need QC over all of them to be
-    /// able to submit them to L1. Replicas don't necessarily have to have the QC, because once
-    /// the batch is on L1 and it's final, they can get the batch from there and don't need the
-    /// attestations. The caller will have to choose the `min_batch_number` accordingly.
-    pub async fn earliest_batch_number_to_sign(
-        &mut self,
-        min_batch_number: attester::BatchNumber,
-    ) -> DalResult<Option<attester::BatchNumber>> {
-        let row = sqlx::query!(
-            r#"
-            SELECT
-                MIN(b.number) AS "number"
-            FROM
-                l1_batches b
-                LEFT JOIN l1_batches_consensus c ON b.number = c.l1_batch_number
-            WHERE
-                b.number >= $1
-                AND c.l1_batch_number IS NULL
-            "#,
-            min_batch_number.0 as i64
-        )
-        .instrument("earliest_batch_number_to_sign")
-        .report_latency()
-        .fetch_one(self.storage)
-        .await?;
-
-        Ok(row
-            .number
-            .map(|number| attester::BatchNumber(number as u64)))
-    }
 }
 
 #[cfg(test)]
@@ -634,24 +600,5 @@ mod tests {
             .insert_mock_l1_batch(&create_l1_batch_header(batch_number + 1))
             .await
             .unwrap();
-
-        // Check the earliest batch number to be signed at various points.
-        for (i, (min_l1_batch_number, want_earliest)) in [
-            (0, Some(batch_number - 2)), // We inserted 2 unsigned batches before the first cert
-            (batch_number, Some(batch_number + 1)), // This one has the corresponding cert
-            (batch_number + 1, Some(batch_number + 1)), // This is the one we inserted later without a cert
-            (batch_number + 2, None),                   // Querying beyond the last one
-        ]
-        .into_iter()
-        .enumerate()
-        {
-            let min_batch_number = attester::BatchNumber(u64::from(min_l1_batch_number));
-            let earliest = conn
-                .consensus_dal()
-                .earliest_batch_number_to_sign(min_batch_number)
-                .await
-                .unwrap();
-            assert_eq!(earliest.map(|n| n.0 as u32), want_earliest, "test case {i}");
-        }
     }
 }

core/node/consensus/src/storage/connection.rs

@@ -430,20 +430,4 @@ impl<'a> Connection<'a> {
             last,
         })
     }
-
-    /// Wrapper for `consensus_dal().earliest_batch_number_to_sign()`.
-    pub async fn earliest_batch_number_to_sign(
-        &mut self,
-        ctx: &ctx::Ctx,
-        min_batch_number: attester::BatchNumber,
-    ) -> ctx::Result<Option<attester::BatchNumber>> {
-        Ok(ctx
-            .wait(
-                self.0
-                    .consensus_dal()
-                    .earliest_batch_number_to_sign(min_batch_number),
-            )
-            .await?
-            .context("earliest_batch_number_to_sign()")?)
-    }
 }

core/node/consensus/src/storage/store.rs

@@ -446,26 +446,33 @@ impl storage::PersistentBatchStore for Store {
         self.batches_persisted.clone()
     }
 
-    /// Get the earliest L1 batch for which there is no corresponding L1 batch quorum certificate,
-    /// and thus it potentially needs to be signed by attesters.
+    /// Get the earliest L1 batch number which has to be (re)signed by a node.
+    ///
+    /// Ideally we would make this decision by looking up the last batch submitted to L1,
+    /// and so it might require a quorum of attesters to sign a certificate for it.
     async fn earliest_batch_number_to_sign(
         &self,
         ctx: &ctx::Ctx,
     ) -> ctx::Result<Option<attester::BatchNumber>> {
-        // TODO: In the future external nodes will be able to ask the main node which L1 batch should be considered final.
-        // Later when we're fully decentralized the nodes will have to look at L1 instead.
-        // For now we make a best effort at gossiping votes, and have no way to tell what has been finalized, so we can
-        // just pick a reasonable maximum number of batches for which we might have to re-submit our signatures.
+        // This is the rough roadmap of how this logic will evolve:
+        // 1. Make best effort at gossiping and collecting votes; the `BatchVotes` in consensus only considers the last vote per attesters.
+        //    Still, we can re-sign more than the last batch, anticipating step 2.
+        // 2. Change `BatchVotes` to handle multiple pending batch numbers, anticipating that batch intervals might decrease dramatically.
+        // 3. Ask the Main Node what is the earliest batch number that it still expects votes for (ie. what is the last submission + 1).
+        // 4. Look at L1 to figure out what is the last submssion, and sign after that.
+
+        // Originally this method returned all unsigned batch numbers by doing a DAL query, but we decided it shoudl be okay and cheap
+        // to resend signatures for already signed batches, and we don't have to worry about skipping them. Because of that, we also
+        // didn't think it makes sense to query the database for the earliest unsigned batch *after* the submission, because we might
+        // as well just re-sign everything. Until we have a way to argue about the "last submission" we just re-sign the last 10 to
+        // try to produce as many QCs as the voting register allows, within reason.
+
         let Some(last_batch_number) = self.last_batch(ctx).await? else {
             return Ok(None);
         };
-        let min_batch_number = attester::BatchNumber(last_batch_number.0.saturating_sub(10));
-
-        self.conn(ctx)
-            .await?
-            .earliest_batch_number_to_sign(ctx, min_batch_number)
-            .await
-            .wrap("earliest_batch_number_to_sign")
+        Ok(Some(attester::BatchNumber(
+            last_batch_number.0.saturating_sub(10),
+        )))
     }
 
     /// Get the highest L1 batch number from storage.