From 1ca66b846a67667b8b1b92ffc0da2f201d0ddf7c Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 1 Dec 2023 12:09:22 -0500 Subject: [PATCH] Use multiple nonces in the Tributary --- coordinator/tributary/src/block.rs | 23 ++-- coordinator/tributary/src/blockchain.rs | 112 +++++++++++------ coordinator/tributary/src/lib.rs | 8 +- coordinator/tributary/src/mempool.rs | 115 ++++++++++-------- coordinator/tributary/src/provided.rs | 2 +- coordinator/tributary/src/tests/block.rs | 15 ++- coordinator/tributary/src/tests/blockchain.rs | 10 +- coordinator/tributary/src/tests/mempool.rs | 61 ++++------ .../tributary/src/tests/transaction/mod.rs | 8 +- .../tributary/src/tests/transaction/signed.rs | 41 ++----- coordinator/tributary/src/transaction.rs | 26 ++-- 11 files changed, 221 insertions(+), 200 deletions(-) diff --git a/coordinator/tributary/src/block.rs b/coordinator/tributary/src/block.rs index 06b3a5a3..f931b5b7 100644 --- a/coordinator/tributary/src/block.rs +++ b/coordinator/tributary/src/block.rs @@ -7,13 +7,12 @@ use thiserror::Error; use blake2::{Digest, Blake2s256}; -use ciphersuite::{Ciphersuite, Ristretto}; - use tendermint::ext::{Network, Commit}; use crate::{ transaction::{ - TransactionError, Signed, TransactionKind, Transaction as TransactionTrait, verify_transaction, + TransactionError, Signed, TransactionKind, Transaction as TransactionTrait, GAIN, + verify_transaction, }, BLOCK_SIZE_LIMIT, ReadWrite, merkle, Transaction, tendermint::tx::verify_tendermint_tx, @@ -122,7 +121,7 @@ impl Block { let mut unsigned = vec![]; for tx in mempool { match tx.kind() { - TransactionKind::Signed(_) => signed.push(tx), + TransactionKind::Signed(_, _) => signed.push(tx), TransactionKind::Unsigned => unsigned.push(tx), TransactionKind::Provided(_) => panic!("provided transaction entered mempool"), } @@ -135,7 +134,7 @@ impl Block { // Check TXs are sorted by nonce. let nonce = |tx: &Transaction| { - if let TransactionKind::Signed(Signed { nonce, .. }) = tx.kind() { + if let TransactionKind::Signed(_, Signed { nonce, .. }) = tx.kind() { *nonce } else { 0 @@ -169,12 +168,12 @@ impl Block { } #[allow(clippy::too_many_arguments)] - pub(crate) fn verify( + pub(crate) fn verify( &self, genesis: [u8; 32], last_block: [u8; 32], mut locally_provided: HashMap<&'static str, VecDeque>, - mut next_nonces: HashMap<::G, u32>, + get_and_increment_nonce: &mut G, schema: N::SignatureScheme, commit: impl Fn(u32) -> Option>, unsigned_in_chain: impl Fn([u8; 32]) -> bool, @@ -259,10 +258,12 @@ impl Block { Err(e) => Err(BlockError::TransactionError(e))?, } } - Transaction::Application(tx) => match verify_transaction(tx, genesis, &mut next_nonces) { - Ok(()) => {} - Err(e) => Err(BlockError::TransactionError(e))?, - }, + Transaction::Application(tx) => { + match verify_transaction(tx, genesis, get_and_increment_nonce) { + Ok(()) => {} + Err(e) => Err(BlockError::TransactionError(e))?, + } + } } } diff --git a/coordinator/tributary/src/blockchain.rs b/coordinator/tributary/src/blockchain.rs index 3be658e0..71767f30 100644 --- a/coordinator/tributary/src/blockchain.rs +++ b/coordinator/tributary/src/blockchain.rs @@ -1,8 +1,8 @@ -use std::collections::{VecDeque, HashMap}; +use std::collections::{VecDeque, HashSet}; use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto}; -use serai_db::{DbTxn, Db}; +use serai_db::{Get, DbTxn, Db}; use scale::Decode; @@ -20,7 +20,7 @@ pub(crate) struct Blockchain { block_number: u32, tip: [u8; 32], - next_nonces: HashMap<::G, u32>, + participants: HashSet<::G>, provided: ProvidedTransactions, mempool: Mempool, @@ -53,11 +53,15 @@ impl Blockchain { fn provided_included_key(genesis: &[u8], hash: &[u8; 32]) -> Vec { D::key(b"tributary_blockchain", b"provided_included", [genesis, hash].concat()) } - fn next_nonce_key(&self, signer: &::G) -> Vec { + fn next_nonce_key( + genesis: &[u8; 32], + signer: &::G, + order: &[u8], + ) -> Vec { D::key( b"tributary_blockchain", b"next_nonce", - [self.genesis.as_ref(), signer.to_bytes().as_ref()].concat(), + [genesis.as_ref(), signer.to_bytes().as_ref(), order].concat(), ) } @@ -66,18 +70,13 @@ impl Blockchain { genesis: [u8; 32], participants: &[::G], ) -> Self { - let mut next_nonces = HashMap::new(); - for participant in participants { - next_nonces.insert(*participant, 0); - } - let mut res = Self { db: Some(db.clone()), genesis, + participants: participants.iter().cloned().collect(), block_number: 0, tip: genesis, - next_nonces, provided: ProvidedTransactions::new(db.clone(), genesis), mempool: Mempool::new(db, genesis), @@ -93,12 +92,6 @@ impl Blockchain { res.tip.copy_from_slice(&tip); } - for participant in participants { - if let Some(next_nonce) = res.db.as_ref().unwrap().get(res.next_nonce_key(participant)) { - res.next_nonces.insert(*participant, u32::from_le_bytes(next_nonce.try_into().unwrap())); - } - } - res } @@ -179,27 +172,58 @@ impl Blockchain { let unsigned_in_chain = |hash: [u8; 32]| db.get(Self::unsigned_included_key(&self.genesis, &hash)).is_some(); - self.mempool.add::(&self.next_nonces, internal, tx, schema, unsigned_in_chain, commit) + self.mempool.add::( + |signer, order| { + if self.participants.contains(&signer) { + Some( + db.get(Self::next_nonce_key(&self.genesis, &signer, &order)) + .map(|bytes| u32::from_le_bytes(bytes.try_into().unwrap())) + .unwrap_or(0), + ) + } else { + None + } + }, + internal, + tx, + schema, + unsigned_in_chain, + commit, + ) } pub(crate) fn provide_transaction(&mut self, tx: T) -> Result<(), ProvidedError> { self.provided.provide(tx) } - /// Returns the next nonce for signing, or None if they aren't a participant. - pub(crate) fn next_nonce(&self, key: ::G) -> Option { - Some(self.next_nonces.get(&key).cloned()?.max(self.mempool.next_nonce(&key).unwrap_or(0))) + pub(crate) fn next_nonce( + &self, + signer: &::G, + order: &[u8], + ) -> Option { + if let Some(next_nonce) = self.mempool.next_nonce_in_mempool(signer, order.to_vec()) { + return Some(next_nonce); + } + if self.participants.contains(signer) { + Some( + self + .db + .as_ref() + .unwrap() + .get(Self::next_nonce_key(&self.genesis, signer, order)) + .map(|bytes| u32::from_le_bytes(bytes.try_into().unwrap())) + .unwrap_or(0), + ) + } else { + None + } } pub(crate) fn build_block(&mut self, schema: N::SignatureScheme) -> Block { - let db = self.db.as_ref().unwrap(); - let unsigned_in_chain = - |hash: [u8; 32]| db.get(Self::unsigned_included_key(&self.genesis, &hash)).is_some(); - let block = Block::new( self.tip, self.provided.transactions.values().flatten().cloned().collect(), - self.mempool.block(&self.next_nonces, unsigned_in_chain), + self.mempool.block(), ); // build_block should not return invalid blocks self.verify_block::(&block, schema, false).unwrap(); @@ -222,17 +246,34 @@ impl Blockchain { // commit has to be valid if it is coming from our db Some(Commit::::decode(&mut commit.as_ref()).unwrap()) }; - block.verify::( + + let mut txn_db = db.clone(); + let mut txn = txn_db.txn(); + let res = block.verify::( self.genesis, self.tip, self.provided.transactions.clone(), - self.next_nonces.clone(), + &mut |signer, order| { + if self.participants.contains(signer) { + let key = Self::next_nonce_key(&self.genesis, signer, order); + let next = txn + .get(&key) + .map(|next_nonce| u32::from_le_bytes(next_nonce.try_into().unwrap())) + .unwrap_or(0); + txn.put(key, (next + 1).to_le_bytes()); + Some(next) + } else { + None + } + }, schema, &commit, unsigned_in_chain, provided_in_chain, allow_non_local_provided, - ) + ); + drop(txn); + res } /// Add a block. @@ -285,18 +326,9 @@ impl Blockchain { // remove from the mempool self.mempool.remove(&hash); } - TransactionKind::Signed(Signed { signer, nonce, .. }) => { + TransactionKind::Signed(order, Signed { signer, nonce, .. }) => { let next_nonce = nonce + 1; - let prev = self - .next_nonces - .insert(*signer, next_nonce) - .expect("block had signed transaction from non-participant"); - if prev != *nonce { - panic!("verified block had an invalid nonce"); - } - - txn.put(self.next_nonce_key(signer), next_nonce.to_le_bytes()); - + txn.put(Self::next_nonce_key(&self.genesis, signer, &order), next_nonce.to_le_bytes()); self.mempool.remove(&tx.hash()); } } diff --git a/coordinator/tributary/src/lib.rs b/coordinator/tributary/src/lib.rs index 752178c3..a3d1bd70 100644 --- a/coordinator/tributary/src/lib.rs +++ b/coordinator/tributary/src/lib.rs @@ -254,8 +254,12 @@ impl Tributary { self.network.blockchain.write().await.provide_transaction(tx) } - pub async fn next_nonce(&self, signer: ::G) -> Option { - self.network.blockchain.read().await.next_nonce(signer) + pub async fn next_nonce( + &self, + signer: &::G, + order: &[u8], + ) -> Option { + self.network.blockchain.read().await.next_nonce(signer, order) } // Returns Ok(true) if new, Ok(false) if an already present unsigned, or the error. diff --git a/coordinator/tributary/src/mempool.rs b/coordinator/tributary/src/mempool.rs index 00d182a4..084d1730 100644 --- a/coordinator/tributary/src/mempool.rs +++ b/coordinator/tributary/src/mempool.rs @@ -20,8 +20,9 @@ pub(crate) struct Mempool { db: D, genesis: [u8; 32], + last_nonce_in_mempool: HashMap<(::G, Vec), u32>, txs: HashMap<[u8; 32], Transaction>, - next_nonces: HashMap<::G, u32>, + txs_per_signer: HashMap<::G, u32>, } impl Mempool { @@ -58,7 +59,13 @@ impl Mempool { } pub(crate) fn new(db: D, genesis: [u8; 32]) -> Self { - let mut res = Mempool { db, genesis, txs: HashMap::new(), next_nonces: HashMap::new() }; + let mut res = Mempool { + db, + genesis, + last_nonce_in_mempool: HashMap::new(), + txs: HashMap::new(), + txs_per_signer: HashMap::new(), + }; let current_mempool = res.db.get(res.current_mempool_key()).unwrap_or(vec![]); @@ -73,21 +80,24 @@ impl Mempool { Transaction::Tendermint(tx) => { res.txs.insert(hash, Transaction::Tendermint(tx)); } - Transaction::Application(tx) => { - match tx.kind() { - TransactionKind::Signed(Signed { signer, nonce, .. }) => { - if let Some(prev) = res.next_nonces.insert(*signer, nonce + 1) { - // These mempool additions should've been ordered - debug_assert!(prev < *nonce); - } - res.txs.insert(hash, Transaction::Application(tx)); + Transaction::Application(tx) => match tx.kind() { + TransactionKind::Signed(order, Signed { signer, nonce, .. }) => { + let amount = *res.txs_per_signer.get(signer).unwrap_or(&0) + 1; + res.txs_per_signer.insert(*signer, amount); + + if let Some(prior_nonce) = + res.last_nonce_in_mempool.insert((*signer, order.clone()), *nonce) + { + assert_eq!(prior_nonce, nonce - 1); } - TransactionKind::Unsigned => { - res.txs.insert(hash, Transaction::Application(tx)); - } - _ => panic!("mempool database had a provided transaction"), + + res.txs.insert(hash, Transaction::Application(tx)); } - } + TransactionKind::Unsigned => { + res.txs.insert(hash, Transaction::Application(tx)); + } + _ => panic!("mempool database had a provided transaction"), + }, } } @@ -95,9 +105,12 @@ impl Mempool { } // Returns Ok(true) if new, Ok(false) if an already present unsigned, or the error. - pub(crate) fn add( + pub(crate) fn add< + N: Network, + F: FnOnce(::G, Vec) -> Option, + >( &mut self, - blockchain_next_nonces: &HashMap<::G, u32>, + blockchain_next_nonce: F, internal: bool, tx: Transaction, schema: N::SignatureScheme, @@ -119,32 +132,31 @@ impl Mempool { } Transaction::Application(app_tx) => { match app_tx.kind() { - TransactionKind::Signed(Signed { signer, nonce, .. }) => { + TransactionKind::Signed(order, Signed { signer, .. }) => { // Get the nonce from the blockchain - let Some(blockchain_next_nonce) = blockchain_next_nonces.get(signer).cloned() else { + let Some(blockchain_next_nonce) = blockchain_next_nonce(*signer, order.clone()) else { // Not a participant Err(TransactionError::InvalidSigner)? }; + let mut next_nonce = blockchain_next_nonce; - // If the blockchain's nonce is greater than the mempool's, use it - // Default to true so if the mempool hasn't tracked this nonce yet, it'll be inserted - let mut blockchain_is_greater = true; - if let Some(mempool_next_nonce) = self.next_nonces.get(signer) { - blockchain_is_greater = blockchain_next_nonce > *mempool_next_nonce; - } - - if blockchain_is_greater { - self.next_nonces.insert(*signer, blockchain_next_nonce); + if let Some(mempool_last_nonce) = + self.last_nonce_in_mempool.get(&(*signer, order.clone())) + { + assert!(*mempool_last_nonce >= blockchain_next_nonce); + next_nonce = *mempool_last_nonce + 1; } // If we have too many transactions from this sender, don't add this yet UNLESS we are // this sender - if !internal && (nonce >= &(blockchain_next_nonce + ACCOUNT_MEMPOOL_LIMIT)) { + let amount_in_pool = *self.txs_per_signer.get(signer).unwrap_or(&0) + 1; + if !internal && (amount_in_pool > ACCOUNT_MEMPOOL_LIMIT) { Err(TransactionError::TooManyInMempool)?; } - verify_transaction(app_tx, self.genesis, &mut self.next_nonces)?; - debug_assert_eq!(self.next_nonces[signer], nonce + 1); + verify_transaction(app_tx, self.genesis, &mut |_, _| Some(next_nonce))?; + self.last_nonce_in_mempool.insert((*signer, order.clone()), next_nonce); + self.txs_per_signer.insert(*signer, amount_in_pool); } TransactionKind::Unsigned => { // check we have the tx in the pool/chain @@ -165,38 +177,26 @@ impl Mempool { } // Returns None if the mempool doesn't have a nonce tracked. - pub(crate) fn next_nonce(&self, signer: &::G) -> Option { - self.next_nonces.get(signer).cloned() + pub(crate) fn next_nonce_in_mempool( + &self, + signer: &::G, + order: Vec, + ) -> Option { + self.last_nonce_in_mempool.get(&(*signer, order)).cloned().map(|nonce| nonce + 1) } /// Get transactions to include in a block. - pub(crate) fn block( - &mut self, - blockchain_next_nonces: &HashMap<::G, u32>, - unsigned_in_chain: impl Fn([u8; 32]) -> bool, - ) -> Vec> { + pub(crate) fn block(&mut self) -> Vec> { let mut unsigned = vec![]; let mut signed = vec![]; for hash in self.txs.keys().cloned().collect::>() { let tx = &self.txs[&hash]; - // Verify this hasn't gone stale match tx.kind() { - TransactionKind::Signed(Signed { signer, nonce, .. }) => { - if blockchain_next_nonces[signer] > *nonce { - self.remove(&hash); - continue; - } - - // Since this TX isn't stale, include it + TransactionKind::Signed(_, Signed { .. }) => { signed.push(tx.clone()); } TransactionKind::Unsigned => { - if unsigned_in_chain(hash) { - self.remove(&hash); - continue; - } - unsigned.push(tx.clone()); } _ => panic!("provided transaction entered mempool"), @@ -205,7 +205,7 @@ impl Mempool { // Sort signed by nonce let nonce = |tx: &Transaction| { - if let TransactionKind::Signed(Signed { nonce, .. }) = tx.kind() { + if let TransactionKind::Signed(_, Signed { nonce, .. }) = tx.kind() { *nonce } else { unreachable!() @@ -242,7 +242,16 @@ impl Mempool { } txn.commit(); - self.txs.remove(tx); + if let Some(tx) = self.txs.remove(tx) { + if let TransactionKind::Signed(order, Signed { signer, nonce, .. }) = tx.kind() { + let amount = *self.txs_per_signer.get(signer).unwrap() - 1; + self.txs_per_signer.insert(*signer, amount); + + if self.last_nonce_in_mempool.get(&(*signer, order.clone())) == Some(nonce) { + self.last_nonce_in_mempool.remove(&(*signer, order)); + } + } + } } #[cfg(test)] diff --git a/coordinator/tributary/src/provided.rs b/coordinator/tributary/src/provided.rs index e4e8193c..82212016 100644 --- a/coordinator/tributary/src/provided.rs +++ b/coordinator/tributary/src/provided.rs @@ -89,7 +89,7 @@ impl ProvidedTransactions { pub(crate) fn provide(&mut self, tx: T) -> Result<(), ProvidedError> { let TransactionKind::Provided(order) = tx.kind() else { Err(ProvidedError::NotProvided)? }; - match verify_transaction(&tx, self.genesis, &mut HashMap::new()) { + match verify_transaction(&tx, self.genesis, &mut |_, _| None) { Ok(()) => {} Err(e) => Err(ProvidedError::InvalidProvided(e))?, } diff --git a/coordinator/tributary/src/tests/block.rs b/coordinator/tributary/src/tests/block.rs index 67b2c27a..2e16f660 100644 --- a/coordinator/tributary/src/tests/block.rs +++ b/coordinator/tributary/src/tests/block.rs @@ -61,7 +61,7 @@ impl ReadWrite for NonceTransaction { impl TransactionTrait for NonceTransaction { fn kind(&self) -> TransactionKind<'_> { - TransactionKind::Signed(&self.2) + TransactionKind::Signed(vec![], &self.2) } fn hash(&self) -> [u8; 32] { @@ -84,11 +84,11 @@ fn empty_block() { let unsigned_in_chain = |_: [u8; 32]| false; let provided_in_chain = |_: [u8; 32]| false; Block::::new(LAST, vec![], vec![]) - .verify::( + .verify::( GENESIS, LAST, HashMap::new(), - HashMap::new(), + &mut |_, _| None, validators, commit, unsigned_in_chain, @@ -119,11 +119,16 @@ fn duplicate_nonces() { let unsigned_in_chain = |_: [u8; 32]| false; let provided_in_chain = |_: [u8; 32]| false; - let res = Block::new(LAST, vec![], mempool).verify::( + let mut last_nonce = 0; + let res = Block::new(LAST, vec![], mempool).verify::( GENESIS, LAST, HashMap::new(), - HashMap::from([(::G::identity(), 0)]), + &mut |_, _| { + let res = last_nonce; + last_nonce += 1; + Some(res) + }, validators.clone(), commit, unsigned_in_chain, diff --git a/coordinator/tributary/src/tests/blockchain.rs b/coordinator/tributary/src/tests/blockchain.rs index 79f304f0..a7ef1e87 100644 --- a/coordinator/tributary/src/tests/blockchain.rs +++ b/coordinator/tributary/src/tests/blockchain.rs @@ -156,7 +156,7 @@ fn signed_transaction() { let signer = tx.1.signer; let (_, mut blockchain) = new_blockchain::(genesis, &[signer]); - assert_eq!(blockchain.next_nonce(signer), Some(0)); + assert_eq!(blockchain.next_nonce(&signer, &[]), Some(0)); let test = |blockchain: &mut Blockchain, mempool: Vec>| { @@ -165,11 +165,11 @@ fn signed_transaction() { let Transaction::Application(tx) = tx else { panic!("tendermint tx found"); }; - let next_nonce = blockchain.next_nonce(signer).unwrap(); + let next_nonce = blockchain.next_nonce(&signer, &[]).unwrap(); blockchain .add_transaction::(true, Transaction::Application(tx), validators.clone()) .unwrap(); - assert_eq!(next_nonce + 1, blockchain.next_nonce(signer).unwrap()); + assert_eq!(next_nonce + 1, blockchain.next_nonce(&signer, &[]).unwrap()); } let block = blockchain.build_block::(validators.clone()); assert_eq!(block, Block::new(blockchain.tip(), vec![], mempool.clone())); @@ -192,7 +192,7 @@ fn signed_transaction() { // Test with a single nonce test(&mut blockchain, vec![Transaction::Application(tx)]); - assert_eq!(blockchain.next_nonce(signer), Some(1)); + assert_eq!(blockchain.next_nonce(&signer, &[]), Some(1)); // Test with a flood of nonces let mut mempool = vec![]; @@ -202,7 +202,7 @@ fn signed_transaction() { ))); } test(&mut blockchain, mempool); - assert_eq!(blockchain.next_nonce(signer), Some(64)); + assert_eq!(blockchain.next_nonce(&signer, &[]), Some(64)); } #[test] diff --git a/coordinator/tributary/src/tests/mempool.rs b/coordinator/tributary/src/tests/mempool.rs index 3d321507..9d8590c2 100644 --- a/coordinator/tributary/src/tests/mempool.rs +++ b/coordinator/tributary/src/tests/mempool.rs @@ -36,16 +36,15 @@ async fn mempool_addition() { let first_tx = signed_transaction(&mut OsRng, genesis, &key, 0); let signer = first_tx.1.signer; - assert_eq!(mempool.next_nonce(&signer), None); + assert_eq!(mempool.next_nonce_in_mempool(&signer, vec![]), None); // validators let validators = Arc::new(Validators::new(genesis, vec![(signer, 1)]).unwrap()); // Add TX 0 - let mut blockchain_next_nonces = HashMap::from([(signer, 0)]); assert!(mempool - .add::( - &blockchain_next_nonces, + .add::( + &|_, _| Some(0), true, Transaction::Application(first_tx.clone()), validators.clone(), @@ -53,15 +52,15 @@ async fn mempool_addition() { commit, ) .unwrap()); - assert_eq!(mempool.next_nonce(&signer), Some(1)); + assert_eq!(mempool.next_nonce_in_mempool(&signer, vec![]), Some(1)); // add a tendermint evidence tx let evidence_tx = random_evidence_tx::(Signer::new(genesis, key.clone()).into(), TendermintBlock(vec![])) .await; assert!(mempool - .add::( - &blockchain_next_nonces, + .add::( + &|_, _| None, true, Transaction::Tendermint(evidence_tx.clone()), validators.clone(), @@ -75,8 +74,8 @@ async fn mempool_addition() { // Adding them again should fail assert_eq!( - mempool.add::( - &blockchain_next_nonces, + mempool.add::( + &|_, _| Some(0), true, Transaction::Application(first_tx.clone()), validators.clone(), @@ -86,8 +85,8 @@ async fn mempool_addition() { Err(TransactionError::InvalidNonce) ); assert_eq!( - mempool.add::( - &blockchain_next_nonces, + mempool.add::( + &|_, _| None, true, Transaction::Tendermint(evidence_tx.clone()), validators.clone(), @@ -100,8 +99,8 @@ async fn mempool_addition() { // Do the same with the next nonce let second_tx = signed_transaction(&mut OsRng, genesis, &key, 1); assert_eq!( - mempool.add::( - &blockchain_next_nonces, + mempool.add::( + &|_, _| Some(0), true, Transaction::Application(second_tx.clone()), validators.clone(), @@ -110,10 +109,10 @@ async fn mempool_addition() { ), Ok(true) ); - assert_eq!(mempool.next_nonce(&signer), Some(2)); + assert_eq!(mempool.next_nonce_in_mempool(&signer, vec![]), Some(2)); assert_eq!( - mempool.add::( - &blockchain_next_nonces, + mempool.add::( + &|_, _| Some(0), true, Transaction::Application(second_tx.clone()), validators.clone(), @@ -128,11 +127,10 @@ async fn mempool_addition() { let second_key = Zeroizing::new(::F::random(&mut OsRng)); let tx = signed_transaction(&mut OsRng, genesis, &second_key, 2); let second_signer = tx.1.signer; - assert_eq!(mempool.next_nonce(&second_signer), None); - blockchain_next_nonces.insert(second_signer, 2); + assert_eq!(mempool.next_nonce_in_mempool(&second_signer, vec![]), None); assert!(mempool - .add::( - &blockchain_next_nonces, + .add::( + &|_, _| Some(2), true, Transaction::Application(tx.clone()), validators.clone(), @@ -140,24 +138,18 @@ async fn mempool_addition() { commit ) .unwrap()); - assert_eq!(mempool.next_nonce(&second_signer), Some(3)); + assert_eq!(mempool.next_nonce_in_mempool(&second_signer, vec![]), Some(3)); // Getting a block should work - assert_eq!(mempool.block(&blockchain_next_nonces, unsigned_in_chain).len(), 4); + assert_eq!(mempool.block().len(), 4); - // If the blockchain says an account had its nonce updated, it should cause a prune - blockchain_next_nonces.insert(signer, 1); - let mut block = mempool.block(&blockchain_next_nonces, unsigned_in_chain); - assert_eq!(block.len(), 3); - assert!(!block.iter().any(|tx| tx.hash() == first_tx.hash())); - assert_eq!(mempool.txs(), &block.drain(..).map(|tx| (tx.hash(), tx)).collect::>()); - - // Removing should also successfully prune + // Removing should successfully prune mempool.remove(&tx.hash()); assert_eq!( mempool.txs(), &HashMap::from([ + (first_tx.hash(), Transaction::Application(first_tx)), (second_tx.hash(), Transaction::Application(second_tx)), (evidence_tx.hash(), Transaction::Tendermint(evidence_tx)) ]) @@ -173,13 +165,12 @@ fn too_many_mempool() { }; let unsigned_in_chain = |_: [u8; 32]| false; let key = Zeroizing::new(::F::random(&mut OsRng)); - let signer = signed_transaction(&mut OsRng, genesis, &key, 0).1.signer; // We should be able to add transactions up to the limit for i in 0 .. ACCOUNT_MEMPOOL_LIMIT { assert!(mempool - .add::( - &HashMap::from([(signer, 0)]), + .add::( + &|_, _| Some(0), false, Transaction::Application(signed_transaction(&mut OsRng, genesis, &key, i)), validators.clone(), @@ -190,8 +181,8 @@ fn too_many_mempool() { } // Yet adding more should fail assert_eq!( - mempool.add::( - &HashMap::from([(signer, 0)]), + mempool.add::( + &|_, _| Some(0), false, Transaction::Application(signed_transaction( &mut OsRng, diff --git a/coordinator/tributary/src/tests/transaction/mod.rs b/coordinator/tributary/src/tests/transaction/mod.rs index e6389efc..806b6139 100644 --- a/coordinator/tributary/src/tests/transaction/mod.rs +++ b/coordinator/tributary/src/tests/transaction/mod.rs @@ -1,5 +1,5 @@ use core::ops::Deref; -use std::{sync::Arc, io, collections::HashMap}; +use std::{sync::Arc, io}; use zeroize::Zeroizing; use rand::{RngCore, CryptoRng, rngs::OsRng}; @@ -114,7 +114,7 @@ impl ReadWrite for SignedTransaction { impl Transaction for SignedTransaction { fn kind(&self) -> TransactionKind<'_> { - TransactionKind::Signed(&self.1) + TransactionKind::Signed(vec![], &self.1) } fn hash(&self) -> [u8; 32] { @@ -145,9 +145,7 @@ pub fn signed_transaction( tx.1.signature.R = Ristretto::generator() * sig_nonce.deref(); tx.1.signature = SchnorrSignature::sign(key, sig_nonce, tx.sig_hash(genesis)); - let mut nonces = HashMap::from([(signer, nonce)]); - verify_transaction(&tx, genesis, &mut nonces).unwrap(); - assert_eq!(nonces, HashMap::from([(tx.1.signer, tx.1.nonce.wrapping_add(1))])); + verify_transaction(&tx, genesis, &mut |_, _| Some(tx.1.nonce)).unwrap(); tx } diff --git a/coordinator/tributary/src/tests/transaction/signed.rs b/coordinator/tributary/src/tests/transaction/signed.rs index 637be5c0..fee290db 100644 --- a/coordinator/tributary/src/tests/transaction/signed.rs +++ b/coordinator/tributary/src/tests/transaction/signed.rs @@ -1,5 +1,3 @@ -use std::collections::HashMap; - use rand::rngs::OsRng; use blake2::{Digest, Blake2s256}; @@ -35,29 +33,23 @@ fn signed_transaction() { // Mutate various properties and verify it no longer works // Different genesis - assert!(verify_transaction( - &tx, - Blake2s256::digest(genesis).into(), - &mut HashMap::from([(tx.1.signer, tx.1.nonce)]), - ) + assert!(verify_transaction(&tx, Blake2s256::digest(genesis).into(), &mut |_, _| Some( + tx.1.nonce + )) .is_err()); // Different data { let mut tx = tx.clone(); tx.0 = Blake2s256::digest(tx.0).to_vec(); - assert!( - verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err() - ); + assert!(verify_transaction(&tx, genesis, &mut |_, _| Some(tx.1.nonce)).is_err()); } // Different signer { let mut tx = tx.clone(); tx.1.signer += Ristretto::generator(); - assert!( - verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err() - ); + assert!(verify_transaction(&tx, genesis, &mut |_, _| Some(tx.1.nonce)).is_err()); } // Different nonce @@ -65,41 +57,28 @@ fn signed_transaction() { #[allow(clippy::redundant_clone)] // False positive? let mut tx = tx.clone(); tx.1.nonce = tx.1.nonce.wrapping_add(1); - assert!( - verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err() - ); + assert!(verify_transaction(&tx, genesis, &mut |_, _| Some(tx.1.nonce)).is_err()); } // Different signature { let mut tx = tx.clone(); tx.1.signature.R += Ristretto::generator(); - assert!( - verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err() - ); + assert!(verify_transaction(&tx, genesis, &mut |_, _| Some(tx.1.nonce)).is_err()); } { let mut tx = tx.clone(); tx.1.signature.s += ::F::ONE; - assert!( - verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err() - ); + assert!(verify_transaction(&tx, genesis, &mut |_, _| Some(tx.1.nonce)).is_err()); } // Sanity check the original TX was never mutated and is valid - let mut nonces = HashMap::from([(tx.1.signer, tx.1.nonce)]); - verify_transaction(&tx, genesis, &mut nonces).unwrap(); - assert_eq!(nonces, HashMap::from([(tx.1.signer, tx.1.nonce.wrapping_add(1))])); + verify_transaction(&tx, genesis, &mut |_, _| Some(tx.1.nonce)).unwrap(); } #[test] fn invalid_nonce() { let (genesis, tx) = random_signed_transaction(&mut OsRng); - assert!(verify_transaction( - &tx, - genesis, - &mut HashMap::from([(tx.1.signer, tx.1.nonce.wrapping_add(1))]), - ) - .is_err()); + assert!(verify_transaction(&tx, genesis, &mut |_, _| Some(tx.1.nonce.wrapping_add(1)),).is_err()); } diff --git a/coordinator/tributary/src/transaction.rs b/coordinator/tributary/src/transaction.rs index fdee7f2b..6f238ef5 100644 --- a/coordinator/tributary/src/transaction.rs +++ b/coordinator/tributary/src/transaction.rs @@ -1,5 +1,5 @@ use core::fmt::Debug; -use std::{io, collections::HashMap}; +use std::io; use zeroize::Zeroize; use thiserror::Error; @@ -82,7 +82,7 @@ impl ReadWrite for Signed { } impl Signed { - fn read_without_nonce(reader: &mut R, nonce: u32) -> io::Result { + pub fn read_without_nonce(reader: &mut R, nonce: u32) -> io::Result { let signer = Ristretto::read_G(reader)?; let mut signature = SchnorrSignature::::read(reader)?; @@ -97,7 +97,7 @@ impl Signed { Ok(Signed { signer, nonce, signature }) } - fn write_without_nonce(&self, writer: &mut W) -> io::Result<()> { + pub fn write_without_nonce(&self, writer: &mut W) -> io::Result<()> { // This is either an invalid signature or a private key leak if self.signature.R.is_identity().into() { Err(io::Error::other("signature nonce was identity"))?; @@ -132,7 +132,7 @@ pub enum TransactionKind<'a> { Unsigned, /// A signed transaction. - Signed(&'a Signed), + Signed(Vec, &'a Signed), } // TODO: Should this be renamed TransactionTrait now that a literal Transaction exists? @@ -156,13 +156,14 @@ pub trait Transaction: 'static + Send + Sync + Clone + Eq + Debug + ReadWrite { /// Panics if called on non-signed transactions. fn sig_hash(&self, genesis: [u8; 32]) -> ::F { match self.kind() { - TransactionKind::Signed(Signed { signature, .. }) => { + TransactionKind::Signed(order, Signed { signature, .. }) => { ::F::from_bytes_mod_order_wide( &Blake2b512::digest( [ b"Tributary Signed Transaction", genesis.as_ref(), &self.hash(), + order.as_ref(), signature.R.to_bytes().as_ref(), ] .concat(), @@ -175,11 +176,14 @@ pub trait Transaction: 'static + Send + Sync + Clone + Eq + Debug + ReadWrite { } } +pub trait GAIN: FnMut(&::G, &[u8]) -> Option {} +impl::G, &[u8]) -> Option> GAIN for F {} + // This will only cause mutations when the transaction is valid -pub(crate) fn verify_transaction( +pub(crate) fn verify_transaction( tx: &T, genesis: [u8; 32], - next_nonces: &mut HashMap<::G, u32>, + get_and_increment_nonce: &mut F, ) -> Result<(), TransactionError> { if tx.serialize().len() > TRANSACTION_SIZE_LIMIT { Err(TransactionError::TooLargeTransaction)?; @@ -190,9 +194,9 @@ pub(crate) fn verify_transaction( match tx.kind() { TransactionKind::Provided(_) => {} TransactionKind::Unsigned => {} - TransactionKind::Signed(Signed { signer, nonce, signature }) => { - if let Some(next_nonce) = next_nonces.get(signer) { - if nonce != next_nonce { + TransactionKind::Signed(order, Signed { signer, nonce, signature }) => { + if let Some(next_nonce) = get_and_increment_nonce(signer, &order) { + if *nonce != next_nonce { Err(TransactionError::InvalidNonce)?; } } else { @@ -204,8 +208,6 @@ pub(crate) fn verify_transaction( if !signature.verify(*signer, tx.sig_hash(genesis)) { Err(TransactionError::InvalidSignature)?; } - - next_nonces.insert(*signer, nonce + 1); } }