diff --git a/Cargo.lock b/Cargo.lock index eb3c9b4f..fcb9b442 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8996,6 +8996,7 @@ dependencies = [ name = "serai-processor-signers" version = "0.1.0" dependencies = [ + "blake2", "borsh", "ciphersuite", "frost-schnorrkel", diff --git a/processor/signers/Cargo.toml b/processor/signers/Cargo.toml index 07e42052..ddd295d3 100644 --- a/processor/signers/Cargo.toml +++ b/processor/signers/Cargo.toml @@ -24,6 +24,7 @@ workspace = true rand_core = { version = "0.6", default-features = false } zeroize = { version = "1", default-features = false, features = ["std"] } +blake2 = { version = "0.10", default-features = false, features = ["std"] } ciphersuite = { path = "../../crypto/ciphersuite", default-features = false, features = ["std"] } frost = { package = "modular-frost", path = "../../crypto/frost", default-features = false } frost-schnorrkel = { path = "../../crypto/schnorrkel", default-features = false } diff --git a/processor/signers/src/batch/db.rs b/processor/signers/src/batch/db.rs index a895e0bb..8d9bc605 100644 --- a/processor/signers/src/batch/db.rs +++ b/processor/signers/src/batch/db.rs @@ -5,8 +5,9 @@ use serai_db::{Get, DbTxn, create_db}; create_db! { SignersBatch { - ActiveSigningProtocols: (session: Session) -> Vec, - Batches: (id: u32) -> Batch, + ActiveSigningProtocols: (session: Session) -> Vec<[u8; 32]>, + BatchHash: (id: u32) -> [u8; 32], + Batches: (hash: [u8; 32]) -> Batch, SignedBatches: (id: u32) -> SignedBatch, LastAcknowledgedBatch: () -> u32, } diff --git a/processor/signers/src/batch/mod.rs b/processor/signers/src/batch/mod.rs index b8ad7ccb..c791f4e0 100644 --- a/processor/signers/src/batch/mod.rs +++ b/processor/signers/src/batch/mod.rs @@ -1,9 +1,12 @@ use core::future::Future; use std::collections::HashSet; +use blake2::{digest::typenum::U32, Digest, Blake2b}; use ciphersuite::{group::GroupEncoding, Ristretto}; use frost::dkg::ThresholdKeys; +use scale::Encode; + use serai_validator_sets_primitives::Session; use serai_in_instructions_primitives::{SignedBatch, batch_message}; @@ -40,7 +43,7 @@ pub(crate) struct BatchSignerTask { external_key: E, keys: Vec>, - active_signing_protocols: HashSet, + active_signing_protocols: HashSet<[u8; 32]>, attempt_manager: AttemptManager, } @@ -63,7 +66,6 @@ impl BatchSignerTask { active_signing_protocols.insert(id); let batch = Batches::get(&db, id).unwrap(); - assert_eq!(batch.id, id); let mut machines = Vec::with_capacity(keys.len()); for keys in &keys { @@ -90,19 +92,21 @@ impl ContinuallyRan for BatchSignerTask { iterated = true; // Save this to the database as a transaction to sign - self.active_signing_protocols.insert(batch.id); + let batch_hash = <[u8; 32]>::from(Blake2b::::digest(batch.encode())); + self.active_signing_protocols.insert(batch_hash); ActiveSigningProtocols::set( &mut txn, self.session, &self.active_signing_protocols.iter().copied().collect(), ); - Batches::set(&mut txn, batch.id, &batch); + BatchHash::set(&mut txn, batch.id, &batch_hash); + Batches::set(&mut txn, batch_hash, &batch); let mut machines = Vec::with_capacity(self.keys.len()); for keys in &self.keys { machines.push(WrappedSchnorrkelMachine::new(keys.clone(), batch_message(&batch))); } - for msg in self.attempt_manager.register(VariantSignId::Batch(batch.id), machines) { + for msg in self.attempt_manager.register(VariantSignId::Batch(batch_hash), machines) { BatchSignerToCoordinatorMessages::send(&mut txn, self.session, &msg); } @@ -112,48 +116,57 @@ impl ContinuallyRan for BatchSignerTask { // Check for acknowledged Batches (meaning we should no longer sign for these Batches) loop { let mut txn = self.db.txn(); - let Some(id) = AcknowledgedBatches::try_recv(&mut txn, &self.external_key) else { - break; - }; + let batch_hash = { + let Some(batch_id) = AcknowledgedBatches::try_recv(&mut txn, &self.external_key) else { + break; + }; + /* + We may have yet to register this signing protocol. + + While `BatchesToSign` is populated before `AcknowledgedBatches`, we could theoretically + have `BatchesToSign` populated with a new batch _while iterating over + `AcknowledgedBatches`_, and then have `AcknowledgedBatched` populated. In that edge + case, we will see the acknowledgement notification before we see the transaction. + + In such a case, we break (dropping the txn, re-queueing the acknowledgement + notification). On the task's next iteration, we'll process the Batch from + `BatchesToSign` and be able to make progress. + */ + let Some(batch_hash) = BatchHash::take(&mut txn, batch_id) else { + drop(txn); + break; + }; + batch_hash + }; + let batch = + Batches::take(&mut txn, batch_hash).expect("BatchHash populated but not Batches"); + + iterated = true; + + // Update the last acknowledged Batch { let last_acknowledged = LastAcknowledgedBatch::get(&txn); - if Some(id) > last_acknowledged { - LastAcknowledgedBatch::set(&mut txn, &id); + if Some(batch.id) > last_acknowledged { + LastAcknowledgedBatch::set(&mut txn, &batch.id); } } - /* - We may have yet to register this signing protocol. - - While `BatchesToSign` is populated before `AcknowledgedBatches`, we could theoretically - have `BatchesToSign` populated with a new batch _while iterating over - `AcknowledgedBatches`_, and then have `AcknowledgedBatched` populated. In that edge case, - we will see the acknowledgement notification before we see the transaction. - - In such a case, we break (dropping the txn, re-queueing the acknowledgement notification). - On the task's next iteration, we'll process the Batch from `BatchesToSign` and be - able to make progress. - */ - if !self.active_signing_protocols.remove(&id) { - break; - } - iterated = true; - - // Since it was, remove this as an active signing protocol + // Remove this as an active signing protocol + assert!(self.active_signing_protocols.remove(&batch_hash)); ActiveSigningProtocols::set( &mut txn, self.session, &self.active_signing_protocols.iter().copied().collect(), ); - // Clean up the database - Batches::del(&mut txn, id); - SignedBatches::del(&mut txn, id); + + // Clean up SignedBatches + SignedBatches::del(&mut txn, batch.id); // We retire with a txn so we either successfully flag this Batch as acknowledged, and // won't re-register it (making this retire safe), or we don't flag it, meaning we will // re-register it, yet that's safe as we have yet to retire it - self.attempt_manager.retire(&mut txn, VariantSignId::Batch(id)); + self.attempt_manager.retire(&mut txn, VariantSignId::Batch(batch_hash)); txn.commit(); } diff --git a/processor/signers/src/coordinator/mod.rs b/processor/signers/src/coordinator/mod.rs index b57742a5..319f098c 100644 --- a/processor/signers/src/coordinator/mod.rs +++ b/processor/signers/src/coordinator/mod.rs @@ -143,7 +143,7 @@ impl ContinuallyRan for CoordinatorTask { // the prior Batch(es) (and accordingly didn't publish them) let last_batch = crate::batch::last_acknowledged_batch(&txn).max(db::LastPublishedBatch::get(&txn)); - let mut next_batch = last_batch.map_or(0, |id| id + 1); + let mut next_batch = last_batch.map(|id| id + 1).unwrap_or(0); while let Some(batch) = crate::batch::signed_batch(&txn, next_batch) { iterated = true; db::LastPublishedBatch::set(&mut txn, &batch.batch.id);