From 9a5f8fc5dd5cc4d3a32ce4023e130f0511d56e99 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 31 Aug 2023 22:48:02 -0400 Subject: [PATCH] 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 | 41 +-------------- coordinator/src/main.rs | 72 ++++++++++---------------- coordinator/src/tests/tributary/mod.rs | 6 +-- coordinator/src/tributary/handle.rs | 16 +++--- coordinator/src/tributary/mod.rs | 19 ++++--- coordinator/src/tributary/scanner.rs | 6 +-- docs/coordinator/Tributary.md | 18 +++---- 7 files changed, 58 insertions(+), 120 deletions(-) diff --git a/coordinator/src/db.rs b/coordinator/src/db.rs index 6f309d46..c2eaefb3 100644 --- a/coordinator/src/db.rs +++ b/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 { - Self::main_key(b"batches_in_block", (network, block).encode()) - } - pub fn batches_in_block( - 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 { Self::main_key(b"first_preprocess", id) } diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index 6743acfb..04489680 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -180,7 +180,7 @@ pub async fn scan_tributaries< D: Db, Pro: Processors, P: P2p, - FRid: Future>, + FRid: Future, RID: Clone + Fn(NetworkId, [u8; 32], RecognizedIdType, [u8; 32]) -> FRid, >( raw_db: D, @@ -590,19 +590,17 @@ pub async fn handle_processors( 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::::save_first_preprocess(&mut txn, id.id, preprocess); - MainDb::::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( 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::::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( } }; - 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::::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; } } }; diff --git a/coordinator/src/tests/tributary/mod.rs b/coordinator/src/tests/tributary/mod.rs index 4724d752..4ed87fd8 100644 --- a/coordinator/src/tests/tributary/mod.rs +++ b/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())); diff --git a/coordinator/src/tributary/handle.rs b/coordinator/src/tributary/handle.rs index 9f9cda79..7355b457 100644 --- a/coordinator/src/tributary/handle.rs +++ b/coordinator/src/tributary/handle.rs @@ -229,7 +229,7 @@ pub async fn handle_application_tx< Pro: Processors, FPst: Future, PST: Clone + Fn(ValidatorSet, Encoded) -> FPst, - FRid: Future>, + FRid: Future, 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::::recognize_id(txn, Zone::Batch.label(), genesis, id); - } + Transaction::Batch(batch) => { + // Because this Batch has achieved synchrony, its batch ID should be authorized + TributaryDb::::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::::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; } } diff --git a/coordinator/src/tributary/mod.rs b/coordinator/src/tributary/mod.rs index 2c4f4d63..3329b7e7 100644 --- a/coordinator/src/tributary/mod.rs +++ b/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, diff --git a/coordinator/src/tributary/scanner.rs b/coordinator/src/tributary/scanner.rs index 0dd07a02..a8e9588e 100644 --- a/coordinator/src/tributary/scanner.rs +++ b/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, PST: Clone + Fn(ValidatorSet, Encoded) -> FPst, - FRid: Future>, + FRid: Future, 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, PST: Clone + Fn(ValidatorSet, Encoded) -> FPst, - FRid: Future>, + FRid: Future, RID: Clone + Fn(NetworkId, [u8; 32], RecognizedIdType, [u8; 32]) -> FRid, P: P2p, >( diff --git a/docs/coordinator/Tributary.md b/docs/coordinator/Tributary.md index 43686598..4bd53654 100644 --- a/docs/coordinator/Tributary.md +++ b/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.