Replace ExternalBlock with Batch

The initial TODO was simply to use one ExternalBlock per all batches in the
block. This would require publishing ExternalBlock after the last batch,
requiring knowing the last batch. While we could add such a pipeline, it'd
require:

1) Initial preprocesses using a distinct message from BatchPreprocess
2) An additional message sent after all BatchPreprocess are sent

Unfortunately, both would require tweaks to the SubstrateSigner which aren't
worth the complexity compared to the solution here, at least, not at this time.

While this will cause, if a Tributary is signing a block whose total batch data
exceeds 25 kB, to use multiple transactions which could be optimized out by
'better' local data pipelining, that's an extreme edge case. Given the temporal
nature of each Tributary, it's also an acceptable edge.

This does no longer achieve synchrony over external blocks accordingly. While
signed batches have synchrony, as they embed their block hash, batches being
signed don't have cryptographic synchrony on their contents. This means
validators who are eclipsed may produce invalid shares, as they sign a
different batch. This will be introduced in a follow-up commit.

coordinator/src/db.rs

@@ -1,8 +1,5 @@
 use scale::{Encode, Decode};
-use serai_client::{
-  primitives::{NetworkId, BlockHash},
-  in_instructions::primitives::SignedBatch,
-};
+use serai_client::{primitives::NetworkId, in_instructions::primitives::SignedBatch};
 
 pub use serai_db::*;
 
@@ -48,42 +45,6 @@ impl<'a, D: Db> MainDb<'a, D> {
     txn.commit();
   }
 
-  fn batches_in_block_key(network: NetworkId, block: [u8; 32]) -> Vec<u8> {
-    Self::main_key(b"batches_in_block", (network, block).encode())
-  }
-  pub fn batches_in_block<G: Get>(
-    getter: &G,
-    network: NetworkId,
-    block: [u8; 32],
-  ) -> Vec<[u8; 32]> {
-    getter
-      .get(Self::batches_in_block_key(network, block))
-      .expect("asking for batches in block for block without batches")
-      .chunks(32)
-      .map(|id| id.try_into().unwrap())
-      .collect()
-  }
-  pub fn add_batch_to_block(
-    txn: &mut D::Transaction<'_>,
-    network: NetworkId,
-    block: BlockHash,
-    id: [u8; 32],
-  ) {
-    let key = Self::batches_in_block_key(network, block.0);
-    let Some(mut existing) = txn.get(&key) else {
-      txn.put(&key, id);
-      return;
-    };
-
-    if existing.chunks(32).any(|existing_id| existing_id == id) {
-      // TODO: Is this an invariant?
-      return;
-    }
-
-    existing.extend(id);
-    txn.put(&key, existing);
-  }
-
   fn first_preprocess_key(id: [u8; 32]) -> Vec<u8> {
     Self::main_key(b"first_preprocess", id)
   }

coordinator/src/main.rs

@@ -180,7 +180,7 @@ pub async fn scan_tributaries<
   D: Db,
   Pro: Processors,
   P: P2p,
-  FRid: Future<Output = Vec<[u8; 32]>>,
+  FRid: Future<Output = ()>,
   RID: Clone + Fn(NetworkId, [u8; 32], RecognizedIdType, [u8; 32]) -> FRid,
 >(
   raw_db: D,
@@ -590,19 +590,17 @@ pub async fn handle_processors<D: Db, Pro: Processors, P: P2p>(
             id.attempt,
             hex::encode(block),
           );
-          // If this is the first attempt instance, synchronize around the block first
+          // If this is the first attempt instance, wait until we synchronize around the batch
+          // first
           if id.attempt == 0 {
             // Save the preprocess to disk so we can publish it later
             // This is fine to use its own TX since it's static and just needs to be written
             // before this message finishes it handling (or with this message's finished handling)
             let mut txn = db.txn();
             MainDb::<D>::save_first_preprocess(&mut txn, id.id, preprocess);
-            MainDb::<D>::add_batch_to_block(&mut txn, msg.network, block, id.id);
             txn.commit();
 
-            // TODO: This will publish one ExternalBlock per Batch. We should only publish one per
-            // all batches within a block
-            Some(Transaction::ExternalBlock(block.0))
+            Some(Transaction::Batch(id.id))
           } else {
             Some(Transaction::BatchPreprocess(SignData {
               plan: id.id,
@@ -787,11 +785,11 @@ pub async fn run<D: Db, Pro: Processors, P: P2p>(
       async move {
         // SubstrateBlockAck is fired before Preprocess, creating a race between Tributary ack
         // of the SubstrateBlock and the sending of all Preprocesses
-        // A similar race condition exists when multiple Batches are present in a block
         // This waits until the necessary preprocess is available
         let get_preprocess = |raw_db, id| async move {
           loop {
             let Some(preprocess) = MainDb::<D>::first_preprocess(raw_db, id) else {
+              assert_eq!(id_type, RecognizedIdType::Plan);
               sleep(Duration::from_millis(100)).await;
               continue;
             };
@@ -799,53 +797,37 @@ pub async fn run<D: Db, Pro: Processors, P: P2p>(
           }
         };
 
-        let (ids, txs) = match id_type {
-          RecognizedIdType::Block => {
-            let block = id;
+        let mut tx = match id_type {
+          RecognizedIdType::Batch => Transaction::BatchPreprocess(SignData {
+            plan: id,
+            attempt: 0,
+            data: get_preprocess(&raw_db, id).await,
+            signed: Transaction::empty_signed(),
+          }),
 
-            let ids = MainDb::<D>::batches_in_block(&raw_db, network, block);
-            let mut txs = vec![];
-            for id in &ids {
-              txs.push(Transaction::BatchPreprocess(SignData {
-                plan: *id,
-                attempt: 0,
-                data: get_preprocess(&raw_db, *id).await,
-                signed: Transaction::empty_signed(),
-              }));
-            }
-            (ids, txs)
-          }
-
-          RecognizedIdType::Plan => (
-            vec![id],
-            vec![Transaction::SignPreprocess(SignData {
-              plan: id,
-              attempt: 0,
-              data: get_preprocess(&raw_db, id).await,
-              signed: Transaction::empty_signed(),
-            })],
-          ),
+          RecognizedIdType::Plan => Transaction::SignPreprocess(SignData {
+            plan: id,
+            attempt: 0,
+            data: get_preprocess(&raw_db, id).await,
+            signed: Transaction::empty_signed(),
+          }),
         };
 
         let tributaries = tributaries.read().await;
         let Some(tributary) = tributaries.get(&genesis) else {
-          panic!("tributary we don't have came to consensus on an ExternalBlock");
+          panic!("tributary we don't have came to consensus on an Batch");
         };
         let tributary = tributary.tributary.read().await;
 
-        for mut tx in txs {
-          // TODO: Same note as prior nonce acquisition
-          log::trace!("getting next nonce for Tributary TX containing Batch signing data");
-          let nonce = tributary
-            .next_nonce(Ristretto::generator() * key.deref())
-            .await
-            .expect("publishing a TX to a tributary we aren't in");
-          tx.sign(&mut OsRng, genesis, &key, nonce);
+        // TODO: Same note as prior nonce acquisition
+        log::trace!("getting next nonce for Tributary TX containing Batch signing data");
+        let nonce = tributary
+          .next_nonce(Ristretto::generator() * key.deref())
+          .await
+          .expect("publishing a TX to a tributary we aren't in");
+        tx.sign(&mut OsRng, genesis, &key, nonce);
 
-          publish_transaction(&tributary, tx).await;
-        }
-
-        ids
+        publish_transaction(&tributary, tx).await;
       }
     }
   };

coordinator/src/tests/tributary/mod.rs

@@ -103,9 +103,9 @@ fn serialize_transaction() {
   ));
 
   {
-    let mut ext_block = [0; 32];
-    OsRng.fill_bytes(&mut ext_block);
-    test_read_write(Transaction::ExternalBlock(ext_block));
+    let mut batch = [0; 32];
+    OsRng.fill_bytes(&mut batch);
+    test_read_write(Transaction::Batch(batch));
   }
   test_read_write(Transaction::SubstrateBlock(OsRng.next_u64()));
 

coordinator/src/tributary/handle.rs

@@ -229,7 +229,7 @@ pub async fn handle_application_tx<
   Pro: Processors,
   FPst: Future<Output = ()>,
   PST: Clone + Fn(ValidatorSet, Encoded) -> FPst,
-  FRid: Future<Output = Vec<[u8; 32]>>,
+  FRid: Future<Output = ()>,
   RID: Clone + Fn(NetworkId, [u8; 32], RecognizedIdType, [u8; 32]) -> FRid,
 >(
   tx: Transaction,
@@ -443,11 +443,10 @@ pub async fn handle_application_tx<
       }
     }
 
-    Transaction::ExternalBlock(block) => {
-      // Because this external block has been finalized, its batch IDs should be authorized
-      for id in recognized_id(spec.set().network, genesis, RecognizedIdType::Block, block).await {
-        TributaryDb::<D>::recognize_id(txn, Zone::Batch.label(), genesis, id);
-      }
+    Transaction::Batch(batch) => {
+      // Because this Batch has achieved synchrony, its batch ID should be authorized
+      TributaryDb::<D>::recognize_id(txn, Zone::Batch.label(), genesis, batch);
+      recognized_id(spec.set().network, genesis, RecognizedIdType::Batch, batch).await;
     }
 
     Transaction::SubstrateBlock(block) => {
@@ -458,10 +457,7 @@ pub async fn handle_application_tx<
 
       for id in plan_ids {
         TributaryDb::<D>::recognize_id(txn, Zone::Sign.label(), genesis, id);
-        assert_eq!(
-          recognized_id(spec.set().network, genesis, RecognizedIdType::Plan, id).await,
-          vec![id]
-        );
+        recognized_id(spec.set().network, genesis, RecognizedIdType::Plan, id).await;
       }
     }
 

coordinator/src/tributary/mod.rs

@@ -231,9 +231,8 @@ pub enum Transaction {
   },
   DkgConfirmed(u32, [u8; 32], Signed),
 
-  // When an external block is finalized, we can allow the associated batch IDs
-  // Commits to the full block so eclipsed nodes don't continue on their eclipsed state
-  ExternalBlock([u8; 32]),
+  // When we have synchrony on a batch, we can allow signing it
+  Batch([u8; 32]),
   // When a Serai block is finalized, with the contained batches, we can allow the associated plan
   // IDs
   SubstrateBlock(u64),
@@ -332,9 +331,9 @@ impl ReadWrite for Transaction {
       }
 
       3 => {
-        let mut block = [0; 32];
-        reader.read_exact(&mut block)?;
-        Ok(Transaction::ExternalBlock(block))
+        let mut batch = [0; 32];
+        reader.read_exact(&mut batch)?;
+        Ok(Transaction::Batch(batch))
       }
 
       4 => {
@@ -431,9 +430,9 @@ impl ReadWrite for Transaction {
         signed.write(writer)
       }
 
-      Transaction::ExternalBlock(block) => {
+      Transaction::Batch(batch) => {
         writer.write_all(&[3])?;
-        writer.write_all(block)
+        writer.write_all(batch)
       }
 
       Transaction::SubstrateBlock(block) => {
@@ -476,7 +475,7 @@ impl TransactionTrait for Transaction {
       Transaction::DkgShares { signed, .. } => TransactionKind::Signed(signed),
       Transaction::DkgConfirmed(_, _, signed) => TransactionKind::Signed(signed),
 
-      Transaction::ExternalBlock(_) => TransactionKind::Provided("external"),
+      Transaction::Batch(_) => TransactionKind::Provided("batch"),
       Transaction::SubstrateBlock(_) => TransactionKind::Provided("serai"),
 
       Transaction::BatchPreprocess(data) => TransactionKind::Signed(&data.signed),
@@ -535,7 +534,7 @@ impl Transaction {
         Transaction::DkgShares { ref mut signed, .. } => signed,
         Transaction::DkgConfirmed(_, _, ref mut signed) => signed,
 
-        Transaction::ExternalBlock(_) => panic!("signing ExternalBlock"),
+        Transaction::Batch(_) => panic!("signing Batch"),
         Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"),
 
         Transaction::BatchPreprocess(ref mut data) => &mut data.signed,

coordinator/src/tributary/scanner.rs

@@ -28,7 +28,7 @@ use crate::{
 
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum RecognizedIdType {
-  Block,
+  Batch,
   Plan,
 }
 
@@ -39,7 +39,7 @@ async fn handle_block<
   Pro: Processors,
   FPst: Future<Output = ()>,
   PST: Clone + Fn(ValidatorSet, Encoded) -> FPst,
-  FRid: Future<Output = Vec<[u8; 32]>>,
+  FRid: Future<Output = ()>,
   RID: Clone + Fn(NetworkId, [u8; 32], RecognizedIdType, [u8; 32]) -> FRid,
   P: P2p,
 >(
@@ -107,7 +107,7 @@ pub async fn handle_new_blocks<
   Pro: Processors,
   FPst: Future<Output = ()>,
   PST: Clone + Fn(ValidatorSet, Encoded) -> FPst,
-  FRid: Future<Output = Vec<[u8; 32]>>,
+  FRid: Future<Output = ()>,
   RID: Clone + Fn(NetworkId, [u8; 32], RecognizedIdType, [u8; 32]) -> FRid,
   P: P2p,
 >(

docs/coordinator/Tributary.md

@@ -46,14 +46,14 @@ by Substrate.
 Note that the keys are confirmed when Substrate emits a `KeyGen` event,
 regardless of if the Tributary has the expected `DkgConfirmed` transactions.
 
-### External Block
+### Batch
 
-When *TODO*, a `ExternalBlock` transaction is provided. This is used to have
-the group acknowledge and synchronize around the block, without the overhead of
-voting in its acknowledgment.
+When *TODO*, a `Batch` transaction is provided. This is used to have the group
+acknowledge and synchronize around a batch, without the overhead of voting in
+its acknowledgment.
 
-When a `ExternalBlock` transaction is included, participants are allowed to
-publish transactions to produce a threshold signature for the block's `Batch`.
+When a `Batch` transaction is included, participants are allowed to publish
+transactions to produce a threshold signature for the batch synchronized over.
 
 ### Substrate Block
 
@@ -66,8 +66,8 @@ publish transactions for the signing protocols it causes.
 ### Batch Preprocess
 
 `BatchPreprocess` is created when a processor sends the coordinator
-`coordinator::ProcessorMessage::BatchPreprocess` and an `ExternalBlock`
-transaction allowing the batch to be signed has already been included on chain.
+`coordinator::ProcessorMessage::BatchPreprocess` and an `Batch` transaction
+allowing the batch to be signed has already been included on chain.
 
 When `t` validators have published `BatchPreprocess` transactions, if the
 coordinator represents one of the first `t` validators to do so, a
@@ -77,7 +77,7 @@ excluding the processor's own preprocess.
 ### Batch Share
 
 `BatchShare` is created when a processor sends the coordinator
-`coordinator::ProcessorMessage::BatchShare`. The relevant `ExternalBlock`
+`coordinator::ProcessorMessage::BatchShare`. The relevant `Batch`
 transaction having already been included on chain follows from
 `coordinator::ProcessorMessage::BatchShare` being a response to a message which
 also has that precondition.