From f99a91b34d7c9ec68b9c720209371c0139818ffa Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 20 Apr 2023 14:24:49 -0400 Subject: [PATCH] Slash on unrecognized ID --- coordinator/src/tests/tributary.rs | 6 +- coordinator/src/tributary/db.rs | 20 +++++++ coordinator/src/tributary/mod.rs | 11 ++-- coordinator/src/tributary/scanner.rs | 88 +++++++++++++++++++++------- 4 files changed, 98 insertions(+), 27 deletions(-) diff --git a/coordinator/src/tests/tributary.rs b/coordinator/src/tests/tributary.rs index a1e8413b..088cde53 100644 --- a/coordinator/src/tests/tributary.rs +++ b/coordinator/src/tests/tributary.rs @@ -71,7 +71,11 @@ fn serialize_transaction() { )); } - test_read_write(Transaction::ExternalBlock(OsRng.next_u64())); + { + let mut ext_block = [0; 32]; + OsRng.fill_bytes(&mut ext_block); + test_read_write(Transaction::ExternalBlock(ext_block)); + } test_read_write(Transaction::SeraiBlock(OsRng.next_u64())); test_read_write(Transaction::BatchPreprocess(random_sign_data(&mut OsRng))); diff --git a/coordinator/src/tributary/db.rs b/coordinator/src/tributary/db.rs index 0858908a..aa7fd006 100644 --- a/coordinator/src/tributary/db.rs +++ b/coordinator/src/tributary/db.rs @@ -25,6 +25,26 @@ impl TributaryDb { self.0.get(Self::block_key(genesis)).unwrap_or(genesis.to_vec()).try_into().unwrap() } + fn recognized_id_key(label: &'static str, genesis: [u8; 32], id: [u8; 32]) -> Vec { + Self::tributary_key(b"recognized", [label.as_bytes(), genesis.as_ref(), id.as_ref()].concat()) + } + pub fn recognized_id( + getter: &G, + label: &'static str, + genesis: [u8; 32], + id: [u8; 32], + ) -> bool { + getter.get(Self::recognized_id_key(label, genesis, id)).is_some() + } + pub fn recognize_id( + txn: &mut D::Transaction<'_>, + label: &'static str, + genesis: [u8; 32], + id: [u8; 32], + ) { + txn.put(Self::recognized_id_key(label, genesis, id), []) + } + fn attempt_key(genesis: [u8; 32], id: [u8; 32]) -> Vec { let genesis_ref: &[u8] = genesis.as_ref(); Self::tributary_key(b"attempt", [genesis_ref, id.as_ref()].concat()) diff --git a/coordinator/src/tributary/mod.rs b/coordinator/src/tributary/mod.rs index 9fce99eb..3ff10dd2 100644 --- a/coordinator/src/tributary/mod.rs +++ b/coordinator/src/tributary/mod.rs @@ -158,7 +158,8 @@ pub enum Transaction { DkgShares(u32, HashMap>, Signed), // When an external block is finalized, we can allow the associated batch IDs - ExternalBlock(u64), + // Commits to the full block so eclipsed nodes don't continue on their eclipsed state + ExternalBlock([u8; 32]), // When a Serai block is finalized, with the contained batches, we can allow the associated plan // IDs SeraiBlock(u64), @@ -223,9 +224,9 @@ impl ReadWrite for Transaction { } 2 => { - let mut block = [0; 8]; + let mut block = [0; 32]; reader.read_exact(&mut block)?; - Ok(Transaction::ExternalBlock(u64::from_le_bytes(block))) + Ok(Transaction::ExternalBlock(block)) } 3 => { @@ -287,7 +288,7 @@ impl ReadWrite for Transaction { Transaction::ExternalBlock(block) => { writer.write_all(&[2])?; - writer.write_all(&block.to_le_bytes()) + writer.write_all(block) } Transaction::SeraiBlock(block) => { @@ -343,8 +344,6 @@ impl TransactionTrait for Transaction { } fn verify(&self) -> Result<(), TransactionError> { - // TODO: Augment with checks that the Vecs can be deser'd and are for recognized IDs - if let Transaction::BatchShare(data) = self { if data.data.len() != 32 { Err(TransactionError::InvalidContent)?; diff --git a/coordinator/src/tributary/scanner.rs b/coordinator/src/tributary/scanner.rs index 76a39408..4b9b3168 100644 --- a/coordinator/src/tributary/scanner.rs +++ b/coordinator/src/tributary/scanner.rs @@ -33,11 +33,37 @@ async fn handle_block( let hash = block.hash(); let mut event_id = 0; + #[allow(clippy::explicit_counter_loop)] // event_id isn't TX index. It just currently lines up for tx in block.transactions { if !TributaryDb::::handled_event(&db.0, hash, event_id) { let mut txn = db.0.txn(); - let mut handle = |label, needed, id, attempt, mut bytes: Vec, signed: Signed| { + // Used to determine if an ID is acceptable + #[derive(Clone, Copy, PartialEq, Eq, Debug)] + enum Zone { + Dkg, + Batch, + Sign, + } + + let mut handle = |zone, label, needed, id, attempt, mut bytes: Vec, signed: Signed| { + if zone == Zone::Dkg { + // Since Dkg doesn't have an ID, solely attempts, this should just be [0; 32] + assert_eq!(id, [0; 32], "DKG, which shouldn't have IDs, had a non-0 ID"); + } else if !TributaryDb::::recognized_id( + &txn, + match zone { + Zone::Dkg => panic!("zone was Dkg despite prior if clause handling Dkg"), + Zone::Batch => "batch", + Zone::Sign => "sign", + }, + tributary.genesis(), + id, + ) { + // TODO: Full slash + todo!(); + } + // If they've already published a TX for this attempt, slash if let Some(data) = TributaryDb::::data(label, &txn, tributary.genesis(), id, attempt, &signed.signer) @@ -100,7 +126,7 @@ async fn handle_block( match tx { Transaction::DkgCommitments(attempt, bytes, signed) => { if let Some(commitments) = - handle(b"dkg_commitments", spec.n(), [0; 32], attempt, bytes, signed) + handle(Zone::Dkg, b"dkg_commitments", spec.n(), [0; 32], attempt, bytes, signed) { processor .send(CoordinatorMessage::KeyGen(key_gen::CoordinatorMessage::Commitments { @@ -114,7 +140,7 @@ async fn handle_block( Transaction::DkgShares(attempt, mut shares, signed) => { if shares.len() != usize::from(spec.n()) { // TODO: Full slash - continue; + todo!(); } let bytes = shares @@ -125,7 +151,9 @@ async fn handle_block( ) .unwrap(); - if let Some(shares) = handle(b"dkg_shares", spec.n(), [0; 32], attempt, bytes, signed) { + if let Some(shares) = + handle(Zone::Dkg, b"dkg_shares", spec.n(), [0; 32], attempt, bytes, signed) + { processor .send(CoordinatorMessage::KeyGen(key_gen::CoordinatorMessage::Shares { id: KeyGenId { set: spec.set(), attempt }, @@ -140,10 +168,15 @@ async fn handle_block( Transaction::SeraiBlock(..) => todo!(), Transaction::BatchPreprocess(data) => { - // TODO: Validate data.plan - if let Some(preprocesses) = - handle(b"batch_preprocess", spec.t(), data.plan, data.attempt, data.data, data.signed) - { + if let Some(preprocesses) = handle( + Zone::Batch, + b"batch_preprocess", + spec.t(), + data.plan, + data.attempt, + data.data, + data.signed, + ) { processor .send(CoordinatorMessage::Coordinator( coordinator::CoordinatorMessage::BatchPreprocesses { @@ -155,10 +188,15 @@ async fn handle_block( } } Transaction::BatchShare(data) => { - // TODO: Validate data.plan - if let Some(shares) = - handle(b"batch_share", spec.t(), data.plan, data.attempt, data.data, data.signed) - { + if let Some(shares) = handle( + Zone::Batch, + b"batch_share", + spec.t(), + data.plan, + data.attempt, + data.data, + data.signed, + ) { processor .send(CoordinatorMessage::Coordinator(coordinator::CoordinatorMessage::BatchShares { id: SignId { key: todo!(), id: data.plan, attempt: data.attempt }, @@ -172,10 +210,15 @@ async fn handle_block( } Transaction::SignPreprocess(data) => { - // TODO: Validate data.plan - if let Some(preprocesses) = - handle(b"sign_preprocess", spec.t(), data.plan, data.attempt, data.data, data.signed) - { + if let Some(preprocesses) = handle( + Zone::Sign, + b"sign_preprocess", + spec.t(), + data.plan, + data.attempt, + data.data, + data.signed, + ) { processor .send(CoordinatorMessage::Sign(sign::CoordinatorMessage::Preprocesses { id: SignId { key: todo!(), id: data.plan, attempt: data.attempt }, @@ -185,10 +228,15 @@ async fn handle_block( } } Transaction::SignShare(data) => { - // TODO: Validate data.plan - if let Some(shares) = - handle(b"sign_share", spec.t(), data.plan, data.attempt, data.data, data.signed) - { + if let Some(shares) = handle( + Zone::Sign, + b"sign_share", + spec.t(), + data.plan, + data.attempt, + data.data, + data.signed, + ) { processor .send(CoordinatorMessage::Sign(sign::CoordinatorMessage::Shares { id: SignId { key: todo!(), id: data.plan, attempt: data.attempt },