From 2f6fb93f877ec4bfbcacc4eff74615ed3ecd7dbf Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 1 Dec 2023 23:31:18 -0500 Subject: [PATCH] Bridge the gap between the prior two commits --- coordinator/src/main.rs | 12 +- coordinator/src/tests/tributary/dkg.rs | 6 +- coordinator/src/tests/tributary/mod.rs | 53 ++++++--- coordinator/src/tests/tributary/tx.rs | 2 +- coordinator/src/tributary/handle.rs | 2 +- coordinator/src/tributary/mod.rs | 112 +++++++++++------- coordinator/src/tributary/scanner.rs | 4 +- .../tributary/src/tests/transaction/mod.rs | 6 + 8 files changed, 123 insertions(+), 74 deletions(-) diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs index 7ba1a825..98bb126f 100644 --- a/coordinator/src/main.rs +++ b/coordinator/src/main.rs @@ -36,9 +36,7 @@ use ::tributary::{ }; mod tributary; -use crate::tributary::{ - TributarySpec, SignData, Transaction, scanner::RecognizedIdType, PlanIds, -}; +use crate::tributary::{TributarySpec, SignData, Transaction, scanner::RecognizedIdType, PlanIds}; mod db; use db::MainDb; @@ -135,14 +133,14 @@ async fn publish_signed_transaction( ) { log::debug!("publishing transaction {}", hex::encode(tx.hash())); - let signer = if let TransactionKind::Signed(signed) = tx.kind() { + let (order, signer) = if let TransactionKind::Signed(order, signed) = tx.kind() { let signer = signed.signer; // Safe as we should deterministically create transactions, meaning if this is already on-disk, // it's what we're saving now MainDb::::save_signed_transaction(txn, signed.nonce, tx); - signer + (order, signer) } else { panic!("non-signed transaction passed to publish_signed_transaction"); }; @@ -152,7 +150,7 @@ async fn publish_signed_transaction( while let Some(tx) = MainDb::::take_signed_transaction( txn, tributary - .next_nonce(signer) + .next_nonce(&signer, &order) .await .expect("we don't have a nonce, meaning we aren't a participant on this tributary"), ) { @@ -697,7 +695,7 @@ async fn handle_processor_message( Err(e) => panic!("created an invalid unsigned transaction: {e:?}"), } } - TransactionKind::Signed(_) => { + TransactionKind::Signed(_, _) => { tx.sign(&mut OsRng, genesis, key); publish_signed_transaction(&mut txn, tributary, tx).await; } diff --git a/coordinator/src/tests/tributary/dkg.rs b/coordinator/src/tests/tributary/dkg.rs index b8bf5129..2126dcf8 100644 --- a/coordinator/src/tests/tributary/dkg.rs +++ b/coordinator/src/tests/tributary/dkg.rs @@ -50,7 +50,7 @@ async fn dkg_test() { let mut tx = Transaction::DkgCommitments(attempt, vec![commitments], Transaction::empty_signed()); - tx.sign(&mut OsRng, spec.genesis(), key, 0); + tx.sign(&mut OsRng, spec.genesis(), key); txs.push(tx); } @@ -177,7 +177,7 @@ async fn dkg_test() { confirmation_nonces: crate::tributary::dkg_confirmation_nonces(key, &spec, 0), signed: Transaction::empty_signed(), }; - tx.sign(&mut OsRng, spec.genesis(), key, 1); + tx.sign(&mut OsRng, spec.genesis(), key); txs.push(tx); } @@ -296,7 +296,7 @@ async fn dkg_test() { txn.commit(); let mut tx = Transaction::DkgConfirmed(attempt, share, Transaction::empty_signed()); - tx.sign(&mut OsRng, spec.genesis(), key, 2); + tx.sign(&mut OsRng, spec.genesis(), key); txs.push(tx); } let block_before_tx = tributaries[0].1.tip().await; diff --git a/coordinator/src/tests/tributary/mod.rs b/coordinator/src/tests/tributary/mod.rs index c17db906..0b10597d 100644 --- a/coordinator/src/tests/tributary/mod.rs +++ b/coordinator/src/tests/tributary/mod.rs @@ -5,7 +5,7 @@ use rand_core::{RngCore, OsRng}; use scale::{Encode, Decode}; use processor_messages::coordinator::SubstrateSignableId; -use tributary::{ReadWrite, tests::random_signed}; +use tributary::{ReadWrite, tests::random_signed_with_nonce}; use crate::tributary::{SignData, Transaction}; @@ -34,6 +34,7 @@ fn random_vec(rng: &mut R, limit: usize) -> Vec { fn random_sign_data( rng: &mut R, plan: Id, + nonce: u32, ) -> SignData { SignData { plan, @@ -47,7 +48,7 @@ fn random_sign_data(value: SignData) { + let mut buf = vec![]; + value.write(&mut buf).unwrap(); + assert_eq!(value, SignData::read(&mut buf.as_slice(), value.signed.nonce).unwrap()) + } + let mut plan = [0; 3]; OsRng.fill_bytes(&mut plan); - test_read_write(random_sign_data::<_, _>(&mut OsRng, plan)); + test_read_write(random_sign_data::<_, _>( + &mut OsRng, + plan, + u32::try_from(OsRng.next_u64() >> 32).unwrap(), + )); let mut plan = [0; 5]; OsRng.fill_bytes(&mut plan); - test_read_write(random_sign_data::<_, _>(&mut OsRng, plan)); + test_read_write(random_sign_data::<_, _>( + &mut OsRng, + plan, + u32::try_from(OsRng.next_u64() >> 32).unwrap(), + )); let mut plan = [0; 8]; OsRng.fill_bytes(&mut plan); - test_read_write(random_sign_data::<_, _>(&mut OsRng, plan)); + test_read_write(random_sign_data::<_, _>( + &mut OsRng, + plan, + u32::try_from(OsRng.next_u64() >> 32).unwrap(), + )); let mut plan = [0; 24]; OsRng.fill_bytes(&mut plan); - test_read_write(random_sign_data::<_, _>(&mut OsRng, plan)); + test_read_write(random_sign_data::<_, _>( + &mut OsRng, + plan, + u32::try_from(OsRng.next_u64() >> 32).unwrap(), + )); } #[test] @@ -114,7 +137,7 @@ fn serialize_transaction() { test_read_write(Transaction::DkgCommitments( random_u32(&mut OsRng), commitments, - random_signed(&mut OsRng), + random_signed_with_nonce(&mut OsRng, 0), )); } @@ -145,7 +168,7 @@ fn serialize_transaction() { OsRng.fill_bytes(&mut nonces); nonces }, - signed: random_signed(&mut OsRng), + signed: random_signed_with_nonce(&mut OsRng, 1), }); } @@ -165,7 +188,7 @@ fn serialize_transaction() { } else { Some(random_vec(&mut OsRng, 500)).filter(|blame| !blame.is_empty()) }, - signed: random_signed(&mut OsRng), + signed: random_signed_with_nonce(&mut OsRng, 2), }); } @@ -176,7 +199,7 @@ fn serialize_transaction() { OsRng.fill_bytes(&mut share); share }, - random_signed(&mut OsRng), + random_signed_with_nonce(&mut OsRng, 2), )); { @@ -200,6 +223,7 @@ fn serialize_transaction() { test_read_write(Transaction::SubstratePreprocess(random_sign_data( &mut OsRng, SubstrateSignableId::Batch(plan), + 0, ))); } { @@ -208,18 +232,19 @@ fn serialize_transaction() { test_read_write(Transaction::SubstrateShare(random_sign_data( &mut OsRng, SubstrateSignableId::Batch(plan), + 1, ))); } { let mut plan = [0; 32]; OsRng.fill_bytes(&mut plan); - test_read_write(Transaction::SignPreprocess(random_sign_data(&mut OsRng, plan))); + test_read_write(Transaction::SignPreprocess(random_sign_data(&mut OsRng, plan, 0))); } { let mut plan = [0; 32]; OsRng.fill_bytes(&mut plan); - test_read_write(Transaction::SignShare(random_sign_data(&mut OsRng, plan))); + test_read_write(Transaction::SignShare(random_sign_data(&mut OsRng, plan, 1))); } { @@ -230,8 +255,8 @@ fn serialize_transaction() { test_read_write(Transaction::SignCompleted { plan, tx_hash, - first_signer: random_signed(&mut OsRng).signer, - signature: random_signed(&mut OsRng).signature, + first_signer: random_signed_with_nonce(&mut OsRng, 2).signer, + signature: random_signed_with_nonce(&mut OsRng, 2).signature, }); } } diff --git a/coordinator/src/tests/tributary/tx.rs b/coordinator/src/tests/tributary/tx.rs index c1e9e22c..cfe1bab8 100644 --- a/coordinator/src/tests/tributary/tx.rs +++ b/coordinator/src/tests/tributary/tx.rs @@ -41,7 +41,7 @@ async fn tx_test() { let block_before_tx = tributaries[sender].1.tip().await; let mut tx = Transaction::DkgCommitments(attempt, vec![commitments.clone()], Transaction::empty_signed()); - tx.sign(&mut OsRng, spec.genesis(), &key, 0); + tx.sign(&mut OsRng, spec.genesis(), &key); assert_eq!(tributaries[sender].1.add_transaction(tx.clone()).await, Ok(true)); let included_in = wait_for_tx_inclusion(&tributaries[sender].1, block_before_tx, tx.hash()).await; diff --git a/coordinator/src/tributary/handle.rs b/coordinator/src/tributary/handle.rs index 4012c798..7887df75 100644 --- a/coordinator/src/tributary/handle.rs +++ b/coordinator/src/tributary/handle.rs @@ -153,7 +153,7 @@ pub(crate) async fn handle_application_tx< // Don't handle transactions from fatally slashed participants // TODO: Because fatally slashed participants can still publish onto the blockchain, they have // a notable DoS ability - if let TransactionKind::Signed(signed) = tx.kind() { + if let TransactionKind::Signed(_, signed) = tx.kind() { if FatallySlashed::get(txn, genesis, signed.signer.to_bytes()).is_some() { return; } diff --git a/coordinator/src/tributary/mod.rs b/coordinator/src/tributary/mod.rs index 28c7fb39..064002d2 100644 --- a/coordinator/src/tributary/mod.rs +++ b/coordinator/src/tributary/mod.rs @@ -189,7 +189,7 @@ impl Debug for SignData SignData { - fn read(reader: &mut R, nonce: u32) -> io::Result { + pub(crate) fn read(reader: &mut R, nonce: u32) -> io::Result { let plan = Id::decode(&mut scale::IoReader(&mut *reader)) .map_err(|_| io::Error::other("invalid plan in SignData"))?; @@ -219,7 +219,7 @@ impl SignData { Ok(SignData { plan, attempt, data, signed }) } - fn write(&self, writer: &mut W) -> io::Result<()> { + pub(crate) fn write(&self, writer: &mut W) -> io::Result<()> { writer.write_all(&self.plan.encode())?; writer.write_all(&self.attempt.to_le_bytes())?; @@ -661,28 +661,44 @@ impl TransactionTrait for Transaction { match self { Transaction::RemoveParticipant(_) => TransactionKind::Provided("remove"), - Transaction::DkgCommitments(attempt, _, signed) => TransactionKind::Signed((b"dkg", attempt).encode(), signed), - Transaction::DkgShares { attempt, signed, .. } => TransactionKind::Signed((b"dkg", attempt).encode(), signed), - Transaction::InvalidDkgShare { attempt, signed, .. } => TransactionKind::Signed((b"dkg", attempt).encode(), signed), - Transaction::DkgConfirmed(attempt, _, signed) => TransactionKind::Signed((b"dkg", attempt).encode(), signed), + Transaction::DkgCommitments(attempt, _, signed) => { + TransactionKind::Signed((b"dkg", attempt).encode(), signed) + } + Transaction::DkgShares { attempt, signed, .. } => { + TransactionKind::Signed((b"dkg", attempt).encode(), signed) + } + Transaction::InvalidDkgShare { attempt, signed, .. } => { + TransactionKind::Signed((b"dkg", attempt).encode(), signed) + } + Transaction::DkgConfirmed(attempt, _, signed) => { + TransactionKind::Signed((b"dkg", attempt).encode(), signed) + } Transaction::CosignSubstrateBlock(_) => TransactionKind::Provided("cosign"), Transaction::Batch(_, _) => TransactionKind::Provided("batch"), Transaction::SubstrateBlock(_) => TransactionKind::Provided("serai"), - Transaction::SubstratePreprocess(data) => TransactionKind::Signed((b"substrate", data.0.plan, data.0.attempt).encode(), &data.signed), - Transaction::SubstrateShare(data) => TransactionKind::Signed((b"substrate", data.0.plan, data.0.attempt).encode(), &data.signed), + Transaction::SubstratePreprocess(data) => { + TransactionKind::Signed((b"substrate", data.plan, data.attempt).encode(), &data.signed) + } + Transaction::SubstrateShare(data) => { + TransactionKind::Signed((b"substrate", data.plan, data.attempt).encode(), &data.signed) + } - Transaction::SignPreprocess(data) => TransactionKind::Signed((b"sign", data.0.plan, data.0.attempt).encode(), &data.signed), - Transaction::SignShare(data) => TransactionKind::Signed((b"sign", data.0.plan, data.0.attempt).encode(), &data.signed), + Transaction::SignPreprocess(data) => { + TransactionKind::Signed((b"sign", data.plan, data.attempt).encode(), &data.signed) + } + Transaction::SignShare(data) => { + TransactionKind::Signed((b"sign", data.plan, data.attempt).encode(), &data.signed) + } Transaction::SignCompleted { .. } => TransactionKind::Unsigned, } } fn hash(&self) -> [u8; 32] { let mut tx = self.serialize(); - if let TransactionKind::Signed(signed) = self.kind() { + if let TransactionKind::Signed(_, signed) = self.kind() { // Make sure the part we're cutting off is the signature assert_eq!(tx.drain((tx.len() - 64) ..).collect::>(), signed.signature.serialize()); } @@ -728,57 +744,61 @@ impl Transaction { genesis: [u8; 32], key: &Zeroizing<::F>, ) { - fn signed(tx: &mut Transaction) -> &mut Signed { - match tx { + fn signed(tx: &mut Transaction) -> (u32, &mut Signed) { + let nonce = match tx { Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"), - Transaction::DkgCommitments(_, _, ref mut signed) => signed, - Transaction::DkgShares { ref mut signed, .. } => signed, - Transaction::InvalidDkgShare { ref mut signed, .. } => signed, - Transaction::DkgConfirmed(_, _, ref mut signed) => signed, + Transaction::DkgCommitments(_, _, _) => 0, + Transaction::DkgShares { .. } => 1, + Transaction::InvalidDkgShare { .. } => 2, + Transaction::DkgConfirmed(_, _, _) => 2, Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"), Transaction::Batch(_, _) => panic!("signing Batch"), Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"), - Transaction::SubstratePreprocess(ref mut data) => &mut data.signed, - Transaction::SubstrateShare(ref mut data) => &mut data.signed, + Transaction::SubstratePreprocess(_) => 0, + Transaction::SubstrateShare(_) => 1, - Transaction::SignPreprocess(ref mut data) => &mut data.signed, - Transaction::SignShare(ref mut data) => &mut data.signed, + Transaction::SignPreprocess(_) => 0, + Transaction::SignShare(_) => 1, Transaction::SignCompleted { .. } => panic!("signing SignCompleted"), - } + }; + + ( + nonce, + match tx { + Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"), + + Transaction::DkgCommitments(_, _, ref mut signed) => signed, + Transaction::DkgShares { ref mut signed, .. } => signed, + Transaction::InvalidDkgShare { ref mut signed, .. } => signed, + Transaction::DkgConfirmed(_, _, ref mut signed) => signed, + + Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"), + + Transaction::Batch(_, _) => panic!("signing Batch"), + Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"), + + Transaction::SubstratePreprocess(ref mut data) => &mut data.signed, + Transaction::SubstrateShare(ref mut data) => &mut data.signed, + + Transaction::SignPreprocess(ref mut data) => &mut data.signed, + Transaction::SignShare(ref mut data) => &mut data.signed, + Transaction::SignCompleted { .. } => panic!("signing SignCompleted"), + }, + ) } - let signed_ref = signed(self); + let (nonce, signed_ref) = signed(self); signed_ref.signer = Ristretto::generator() * key.deref(); - - signed_ref.nonce = match tx { - Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"), - - Transaction::DkgCommitments(_, _, _) => 0, - Transaction::DkgShares { .. } => 1, - Transaction::InvalidDkgShare { .. } => 2, - Transaction::DkgConfirmed(_, _, _) => 2, - - Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"), - - Transaction::Batch(_, _) => panic!("signing Batch"), - Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"), - - Transaction::SubstratePreprocess(_) => 0, - Transaction::SubstrateShare(_) => 1, - - Transaction::SignPreprocess(_) => 0, - Transaction::SignShare(_) => 1, - Transaction::SignCompleted { .. } => panic!("signing SignCompleted"), - }; + signed_ref.nonce = nonce; let sig_nonce = Zeroizing::new(::F::random(rng)); - signed(self).signature.R = ::generator() * sig_nonce.deref(); + signed(self).1.signature.R = ::generator() * sig_nonce.deref(); let sig_hash = self.sig_hash(genesis); - signed(self).signature = SchnorrSignature::::sign(key, sig_nonce, sig_hash); + signed(self).1.signature = SchnorrSignature::::sign(key, sig_nonce, sig_hash); } pub fn sign_completed_challenge(&self) -> ::F { diff --git a/coordinator/src/tributary/scanner.rs b/coordinator/src/tributary/scanner.rs index f7f6df79..23c33ed6 100644 --- a/coordinator/src/tributary/scanner.rs +++ b/coordinator/src/tributary/scanner.rs @@ -38,8 +38,8 @@ pub(crate) trait RIDTrait: Clone + Fn(ValidatorSet, [u8; 32], RecognizedIdType, Vec) -> FRid { } -impl) -> FRid> - RIDTrait for F +impl) -> FRid> RIDTrait + for F { } diff --git a/coordinator/tributary/src/tests/transaction/mod.rs b/coordinator/tributary/src/tests/transaction/mod.rs index 806b6139..1f85947a 100644 --- a/coordinator/tributary/src/tests/transaction/mod.rs +++ b/coordinator/tributary/src/tests/transaction/mod.rs @@ -42,6 +42,12 @@ pub fn random_signed(rng: &mut R) -> Signed { } } +pub fn random_signed_with_nonce(rng: &mut R, nonce: u32) -> Signed { + let mut signed = random_signed(rng); + signed.nonce = nonce; + signed +} + #[derive(Clone, PartialEq, Eq, Debug)] pub struct ProvidedTransaction(pub Vec);