Add DoS limits to tributary and require provided transactions be ordered

coordinator/tributary/src/block.rs

@@ -1,7 +1,4 @@
-use std::{
+use std::{io, collections::HashMap};
-  io,
-  collections::{HashSet, HashMap},
-};
 
 use thiserror::Error;
 
@@ -11,19 +8,31 @@ use ciphersuite::{Ciphersuite, Ristretto};
 
 #[derive(Clone, PartialEq, Eq, Debug, Error)]
 pub enum BlockError {
+  /// Block was too large.
+  #[error("block exceeded size limit")]
+  TooLargeBlock,
   /// Header specified a parent which wasn't the chain tip.
   #[error("header doesn't build off the chain tip")]
   InvalidParent,
   /// Header specified an invalid transactions merkle tree hash.
   #[error("header transactions hash is incorrect")]
   InvalidTransactions,
+  /// A provided transaction was placed after a non-provided transaction.
+  #[error("a provided transaction was included after a non-provided transaction")]
+  ProvidedAfterNonProvided,
+  /// The block had a provided transaction this validator has yet to be provided.
+  #[error("block had a provided transaction not yet locally provided: {0:?}")]
+  NonLocalProvided([u8; 32]),
+  /// The provided transaction was distinct from the locally provided transaction.
+  #[error("block had a distinct provided transaction")]
+  DistinctProvided,
   /// An included transaction was invalid.
   #[error("included transaction had an error")]
   TransactionError(TransactionError),
 }
 
 use crate::{
-  ReadWrite, TransactionError, Signed, TransactionKind, Transaction, ProvidedTransactions, merkle,
+  BLOCK_SIZE_LIMIT, ReadWrite, TransactionError, Signed, TransactionKind, Transaction, merkle,
   verify_transaction,
 };
 
@@ -89,21 +98,14 @@ impl<T: Transaction> Block<T> {
   /// Create a new block.
   ///
   /// mempool is expected to only have valid, non-conflicting transactions.
-  pub(crate) fn new(
+  pub(crate) fn new(parent: [u8; 32], provided: Vec<T>, mempool: Vec<T>) -> Self {
-    parent: [u8; 32],
+    let mut txs = provided;
-    provided: &ProvidedTransactions<T>,
+    for tx in mempool {
-    mempool: HashMap<[u8; 32], T>,
-  ) -> Self {
-    let mut txs = vec![];
-    for tx in provided.transactions.values().cloned() {
-      txs.push(tx);
-    }
-    for tx in mempool.values().cloned() {
       assert!(tx.kind() != TransactionKind::Provided, "provided transaction entered mempool");
       txs.push(tx);
     }
 
-    // Sort txs by nonces.
+    // Check TXs are sorted by nonce.
     let nonce = |tx: &T| {
       if let TransactionKind::Signed(Signed { nonce, .. }) = tx.kind() {
         *nonce
@@ -111,9 +113,6 @@ impl<T: Transaction> Block<T> {
         0
       }
     };
-    txs.sort_by(|a, b| nonce(a).partial_cmp(&nonce(b)).unwrap());
-
-    // Check the sort.
     let mut last = 0;
     for tx in &txs {
       let nonce = nonce(tx);
@@ -123,33 +122,62 @@ impl<T: Transaction> Block<T> {
       last = nonce;
     }
 
-    let hashes = txs.iter().map(Transaction::hash).collect::<Vec<_>>();
+    let mut res =
-    Block { header: BlockHeader { parent, transactions: merkle(&hashes) }, transactions: txs }
+      Block { header: BlockHeader { parent, transactions: [0; 32] }, transactions: txs };
+    while res.serialize().len() > BLOCK_SIZE_LIMIT {
+      assert!(res.transactions.pop().is_some());
+    }
+    let hashes = res.transactions.iter().map(Transaction::hash).collect::<Vec<_>>();
+    res.header.transactions = merkle(&hashes);
+    res
   }
 
   pub fn hash(&self) -> [u8; 32] {
     self.header.hash()
   }
 
-  pub fn verify(
+  pub(crate) fn verify(
     &self,
     genesis: [u8; 32],
     last_block: [u8; 32],
-    mut locally_provided: HashSet<[u8; 32]>,
+    locally_provided: &[[u8; 32]],
     mut next_nonces: HashMap<<Ristretto as Ciphersuite>::G, u32>,
   ) -> Result<(), BlockError> {
+    if self.serialize().len() > BLOCK_SIZE_LIMIT {
+      Err(BlockError::TooLargeBlock)?;
+    }
+
     if self.header.parent != last_block {
       Err(BlockError::InvalidParent)?;
     }
 
+    let mut found_non_provided = false;
     let mut txs = Vec::with_capacity(self.transactions.len());
-    for tx in &self.transactions {
+    for (i, tx) in self.transactions.iter().enumerate() {
-      match verify_transaction(tx, genesis, &mut locally_provided, &mut next_nonces) {
+      txs.push(tx.hash());
+
+      if tx.kind() == TransactionKind::Provided {
+        if found_non_provided {
+          Err(BlockError::ProvidedAfterNonProvided)?;
+        }
+
+        let Some(local) = locally_provided.get(i) else {
+          Err(BlockError::NonLocalProvided(txs.pop().unwrap()))?
+        };
+        if txs.last().unwrap() != local {
+          Err(BlockError::DistinctProvided)?;
+        }
+
+        // We don't need to call verify_transaction since we did when we locally provided this
+        // transaction. Since it's identical, it must be valid
+        continue;
+      }
+
+      found_non_provided = true;
+      match verify_transaction(tx, genesis, &mut next_nonces) {
         Ok(()) => {}
         Err(e) => Err(BlockError::TransactionError(e))?,
       }
-
-      txs.push(tx.hash());
     }
 
     if merkle(&txs) != self.header.transactions {

coordinator/tributary/src/blockchain.rs

@@ -1,11 +1,14 @@
-use std::collections::{HashSet, HashMap};
+use std::collections::HashMap;
 
 use ciphersuite::{Ciphersuite, Ristretto};
 
-use crate::{Signed, TransactionKind, Transaction, ProvidedTransactions, BlockError, Block, Mempool};
+use crate::{
+  Signed, TransactionKind, Transaction, verify_transaction, ProvidedTransactions, BlockError,
+  Block, Mempool,
+};
 
 #[derive(Clone, PartialEq, Eq, Debug)]
-pub struct Blockchain<T: Transaction> {
+pub(crate) struct Blockchain<T: Transaction> {
   genesis: [u8; 32],
   // TODO: db
   block_number: u64,
@@ -17,7 +20,7 @@ pub struct Blockchain<T: Transaction> {
 }
 
 impl<T: Transaction> Blockchain<T> {
-  pub fn new(genesis: [u8; 32], participants: &[<Ristretto as Ciphersuite>::G]) -> Self {
+  pub(crate) fn new(genesis: [u8; 32], participants: &[<Ristretto as Ciphersuite>::G]) -> Self {
     // TODO: Reload block_number/tip/next_nonces/provided/mempool
 
     let mut next_nonces = HashMap::new();
@@ -37,44 +40,54 @@ impl<T: Transaction> Blockchain<T> {
     }
   }
 
-  pub fn tip(&self) -> [u8; 32] {
+  pub(crate) fn tip(&self) -> [u8; 32] {
     self.tip
   }
 
-  pub fn block_number(&self) -> u64 {
+  pub(crate) fn block_number(&self) -> u64 {
     self.block_number
   }
 
-  pub fn add_transaction(&mut self, tx: T) -> bool {
+  pub(crate) fn add_transaction(&mut self, internal: bool, tx: T) -> bool {
-    self.mempool.add(&self.next_nonces, tx)
+    self.mempool.add(&self.next_nonces, internal, tx)
   }
 
-  pub fn provide_transaction(&mut self, tx: T) {
+  pub(crate) fn provide_transaction(&mut self, tx: T) -> bool {
-    self.provided.provide(tx)
+    // TODO: Should this check be internal to ProvidedTransactions?
+    if verify_transaction(&tx, self.genesis, &mut HashMap::new()).is_err() {
+      return false;
+    }
+    self.provided.provide(tx);
+    true
   }
 
-  /// Returns the next nonce, or None if they aren't a participant.
+  /// Returns the next nonce for signing, or None if they aren't a participant.
-  pub fn next_nonce(&self, key: <Ristretto as Ciphersuite>::G) -> Option<u32> {
+  pub(crate) fn next_nonce(&self, key: <Ristretto as Ciphersuite>::G) -> Option<u32> {
-    self.next_nonces.get(&key).cloned()
+    Some(self.next_nonces.get(&key).cloned()?.max(self.mempool.next_nonce(&key).unwrap_or(0)))
   }
 
-  pub fn build_block(&mut self) -> Block<T> {
+  pub(crate) fn build_block(&mut self) -> Block<T> {
-    let block = Block::new(self.tip, &self.provided, self.mempool.block(&self.next_nonces));
+    let block = Block::new(
+      self.tip,
+      self.provided.transactions.iter().cloned().collect(),
+      self.mempool.block(&self.next_nonces),
+    );
     // build_block should not return invalid blocks
     self.verify_block(&block).unwrap();
     block
   }
 
-  pub fn verify_block(&self, block: &Block<T>) -> Result<(), BlockError> {
+  pub(crate) fn verify_block(&self, block: &Block<T>) -> Result<(), BlockError> {
-    let mut locally_provided = HashSet::new();
+    block.verify(
-    for provided in self.provided.transactions.keys() {
+      self.genesis,
-      locally_provided.insert(*provided);
+      self.tip,
-    }
+      &self.provided.transactions.iter().map(Transaction::hash).collect::<Vec<_>>(),
-    block.verify(self.genesis, self.tip, locally_provided, self.next_nonces.clone())
+      self.next_nonces.clone(),
+    )
   }
 
   /// Add a block.
-  pub fn add_block(&mut self, block: &Block<T>) -> Result<(), BlockError> {
+  pub(crate) fn add_block(&mut self, block: &Block<T>) -> Result<(), BlockError> {
     self.verify_block(block)?;
 
     // None of the following assertions should be reachable since we verified the block
@@ -83,10 +96,7 @@ impl<T: Transaction> Blockchain<T> {
     for tx in &block.transactions {
       match tx.kind() {
         TransactionKind::Provided => {
-          assert!(
+          self.provided.complete(tx.hash());
-            self.provided.withdraw(tx.hash()),
-            "verified block had a provided transaction we didn't have"
-          );
         }
         TransactionKind::Unsigned => {}
         TransactionKind::Signed(Signed { signer, nonce, .. }) => {
@@ -97,6 +107,8 @@ impl<T: Transaction> Blockchain<T> {
           if prev != *nonce {
             panic!("verified block had an invalid nonce");
           }
+
+          self.mempool.remove(&tx.hash());
         }
       }
     }

coordinator/tributary/src/lib.rs

@@ -32,10 +32,10 @@ mod block;
 pub use block::*;
 
 mod blockchain;
-pub use blockchain::*;
+pub(crate) use blockchain::*;
 
 mod mempool;
-pub use mempool::*;
+pub(crate) use mempool::*;
 
 mod tendermint;
 pub(crate) use crate::tendermint::*;
@@ -43,6 +43,15 @@ pub(crate) use crate::tendermint::*;
 #[cfg(any(test, feature = "tests"))]
 pub mod tests;
 
+/// Size limit for an individual transaction.
+pub const TRANSACTION_SIZE_LIMIT: usize = 50_000;
+/// Amount of transactions a single account may have in the mempool.
+pub const ACCOUNT_MEMPOOL_LIMIT: u32 = 50;
+/// Block size limit.
+// This targets a growth limit of roughly 5 GB a day, under load, in order to prevent a malicious
+// participant from flooding disks and causing out of space errors in order processes.
+pub const BLOCK_SIZE_LIMIT: usize = 350_000;
+
 pub(crate) const TRANSACTION_MESSAGE: u8 = 0;
 pub(crate) const TENDERMINT_MESSAGE: u8 = 1;
 
@@ -108,15 +117,19 @@ impl<T: Transaction, P: P2p> Tributary<T, P> {
     Self { network, synced_block, messages }
   }
 
-  pub fn provide_transaction(&self, tx: T) {
+  pub fn provide_transaction(&self, tx: T) -> bool {
     self.network.blockchain.write().unwrap().provide_transaction(tx)
   }
 
+  pub fn next_nonce(&self, signer: <Ristretto as Ciphersuite>::G) -> Option<u32> {
+    self.network.blockchain.read().unwrap().next_nonce(signer)
+  }
+
   // Returns if the transaction was valid.
-  pub async fn add_transaction(&self, tx: T) -> bool {
+  pub async fn add_transaction(&mut self, tx: T) -> bool {
     let mut to_broadcast = vec![TRANSACTION_MESSAGE];
     tx.write(&mut to_broadcast).unwrap();
-    let res = self.network.blockchain.write().unwrap().add_transaction(tx);
+    let res = self.network.blockchain.write().unwrap().add_transaction(true, tx);
     if res {
       self.network.p2p.broadcast(to_broadcast).await;
     }
@@ -158,7 +171,7 @@ impl<T: Transaction, P: P2p> Tributary<T, P> {
 
         // TODO: Sync mempools with fellow peers
         // Can we just rebroadcast transactions not included for at least two blocks?
-        self.network.blockchain.write().unwrap().add_transaction(tx)
+        self.network.blockchain.write().unwrap().add_transaction(false, tx)
       }
 
       TENDERMINT_MESSAGE => {

coordinator/tributary/src/mempool.rs

@@ -1,25 +1,26 @@
-use std::collections::{HashSet, HashMap};
+use std::collections::HashMap;
 
 use ciphersuite::{Ciphersuite, Ristretto};
 
-use crate::{Signed, TransactionKind, Transaction, verify_transaction};
+use crate::{ACCOUNT_MEMPOOL_LIMIT, Signed, TransactionKind, Transaction, verify_transaction};
 
 #[derive(Clone, PartialEq, Eq, Debug)]
-pub struct Mempool<T: Transaction> {
+pub(crate) struct Mempool<T: Transaction> {
   genesis: [u8; 32],
   txs: HashMap<[u8; 32], T>,
   next_nonces: HashMap<<Ristretto as Ciphersuite>::G, u32>,
 }
 
 impl<T: Transaction> Mempool<T> {
-  pub fn new(genesis: [u8; 32]) -> Self {
+  pub(crate) fn new(genesis: [u8; 32]) -> Self {
     Mempool { genesis, txs: HashMap::new(), next_nonces: HashMap::new() }
   }
 
   /// Returns true if this is a valid, new transaction.
-  pub fn add(
+  pub(crate) fn add(
     &mut self,
     blockchain_next_nonces: &HashMap<<Ristretto as Ciphersuite>::G, u32>,
+    internal: bool,
     tx: T,
   ) -> bool {
     match tx.kind() {
@@ -41,9 +42,13 @@ impl<T: Transaction> Mempool<T> {
           self.next_nonces.insert(*signer, blockchain_next_nonce);
         }
 
-        if verify_transaction(&tx, self.genesis, &mut HashSet::new(), &mut self.next_nonces)
+        // If we have too many transactions from this sender, don't add this yet UNLESS we are
-          .is_err()
+        // this sender
-        {
+        if !internal && (nonce >= &(blockchain_next_nonce + ACCOUNT_MEMPOOL_LIMIT)) {
+          return false;
+        }
+
+        if verify_transaction(&tx, self.genesis, &mut self.next_nonces).is_err() {
           return false;
         }
         assert_eq!(self.next_nonces[signer], nonce + 1);
@@ -56,18 +61,16 @@ impl<T: Transaction> Mempool<T> {
   }
 
   // Returns None if the mempool doesn't have a nonce tracked.
-  // The nonce to use when signing should be:
+  pub(crate) fn next_nonce(&self, signer: &<Ristretto as Ciphersuite>::G) -> Option<u32> {
-  // max(blockchain.next_nonce().unwrap(), mempool.next_nonce().unwrap_or(0))
-  pub fn next_nonce(&self, signer: &<Ristretto as Ciphersuite>::G) -> Option<u32> {
     self.next_nonces.get(signer).cloned()
   }
 
   /// Get transactions to include in a block.
-  pub fn block(
+  pub(crate) fn block(
     &mut self,
     blockchain_next_nonces: &HashMap<<Ristretto as Ciphersuite>::G, u32>,
-  ) -> HashMap<[u8; 32], T> {
+  ) -> Vec<T> {
-    let mut res = HashMap::new();
+    let mut res = vec![];
     for hash in self.txs.keys().cloned().collect::<Vec<_>>() {
       let tx = &self.txs[&hash];
       // Verify this hasn't gone stale
@@ -82,13 +85,24 @@ impl<T: Transaction> Mempool<T> {
       }
 
       // Since this TX isn't stale, include it
-      res.insert(hash, tx.clone());
+      res.push(tx.clone());
     }
+
+    // Sort res by nonce.
+    let nonce = |tx: &T| {
+      if let TransactionKind::Signed(Signed { nonce, .. }) = tx.kind() {
+        *nonce
+      } else {
+        0
+      }
+    };
+    res.sort_by(|a, b| nonce(a).partial_cmp(&nonce(b)).unwrap());
+
     res
   }
 
   /// Remove a transaction from the mempool.
-  pub fn remove(&mut self, tx: &[u8; 32]) {
+  pub(crate) fn remove(&mut self, tx: &[u8; 32]) {
     self.txs.remove(tx);
   }
 

coordinator/tributary/src/provided.rs

@@ -1,33 +1,32 @@
-use std::collections::HashMap;
+use std::collections::VecDeque;
 
 use crate::{TransactionKind, Transaction};
 
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct ProvidedTransactions<T: Transaction> {
-  pub(crate) transactions: HashMap<[u8; 32], T>,
+  pub(crate) transactions: VecDeque<T>,
 }
 
 impl<T: Transaction> Default for ProvidedTransactions<T> {
   fn default() -> Self {
-    ProvidedTransactions { transactions: HashMap::new() }
+    ProvidedTransactions { transactions: VecDeque::new() }
   }
 }
 
 impl<T: Transaction> ProvidedTransactions<T> {
-  pub fn new() -> Self {
+  pub(crate) fn new() -> Self {
     ProvidedTransactions::default()
   }
 
   /// Provide a transaction for inclusion in a block.
-  pub fn provide(&mut self, tx: T) {
+  pub(crate) fn provide(&mut self, tx: T) {
+    // TODO: Make an error out of this
     assert_eq!(tx.kind(), TransactionKind::Provided, "provided a non-provided transaction");
-    self.transactions.insert(tx.hash(), tx);
+    self.transactions.push_back(tx);
   }
 
-  /// Withdraw a transaction, no longer proposing it or voting for its validity.
+  /// Complete a provided transaction, no longer proposing it nor voting for its validity.
-  ///
+  pub(crate) fn complete(&mut self, tx: [u8; 32]) {
-  /// Returns true if the transaction was withdrawn and false otherwise.
+    assert_eq!(self.transactions.pop_front().unwrap().hash(), tx);
-  pub fn withdraw(&mut self, tx: [u8; 32]) -> bool {
-    self.transactions.remove(&tx).is_some()
   }
 }

coordinator/tributary/src/tendermint.rs

@@ -35,8 +35,7 @@ use tendermint::{
 use tokio::time::{Duration, sleep};
 
 use crate::{
-  TENDERMINT_MESSAGE, ReadWrite, Transaction, TransactionError, BlockHeader, Block, BlockError,
+  TENDERMINT_MESSAGE, ReadWrite, Transaction, BlockHeader, Block, BlockError, Blockchain, P2p,
-  Blockchain, P2p,
 };
 
 fn challenge(
@@ -266,9 +265,7 @@ impl<T: Transaction, P: P2p> NetworkTrait for Network<T, P> {
     let block =
       Block::read::<&[u8]>(&mut block.0.as_ref()).map_err(|_| TendermintBlockError::Fatal)?;
     self.blockchain.read().unwrap().verify_block(&block).map_err(|e| match e {
-      BlockError::TransactionError(TransactionError::MissingProvided(_)) => {
+      BlockError::NonLocalProvided(_) => TendermintBlockError::Temporal,
-        TendermintBlockError::Temporal
-      }
       _ => TendermintBlockError::Fatal,
     })
   }
@@ -297,7 +294,7 @@ impl<T: Transaction, P: P2p> NetworkTrait for Network<T, P> {
       let block_res = self.blockchain.write().unwrap().add_block(&block);
       match block_res {
         Ok(()) => break,
-        Err(BlockError::TransactionError(TransactionError::MissingProvided(hash))) => {
+        Err(BlockError::NonLocalProvided(hash)) => {
           log::error!(
             "missing provided transaction {} which other validators on tributary {} had",
             hex::encode(hash),

coordinator/tributary/src/tests/block.rs

@@ -1,9 +1,4 @@
-use std::{
+use std::{io, collections::HashMap};
-  io,
-  collections::{HashSet, HashMap},
-};
-
-use rand::{RngCore, rngs::OsRng};
 
 use blake2::{Digest, Blake2s256};
 
@@ -13,9 +8,8 @@ use ciphersuite::{
 };
 use schnorr::SchnorrSignature;
 
-use crate::{
+use crate::{ReadWrite, TransactionError, Signed, TransactionKind, Transaction, BlockError, Block};
-  ReadWrite, TransactionError, Signed, TransactionKind, Transaction, ProvidedTransactions, Block,
+
-};
 // A transaction solely defined by its nonce and a distinguisher (to allow creating distinct TXs
 // sharing a nonce).
 #[derive(Clone, PartialEq, Eq, Debug)]
@@ -74,8 +68,8 @@ impl Transaction for NonceTransaction {
 fn empty_block() {
   const GENESIS: [u8; 32] = [0xff; 32];
   const LAST: [u8; 32] = [0x01; 32];
-  Block::new(LAST, &ProvidedTransactions::<NonceTransaction>::new(), HashMap::new())
+  Block::<NonceTransaction>::new(LAST, vec![], vec![])
-    .verify(GENESIS, LAST, HashSet::new(), HashMap::new())
+    .verify(GENESIS, LAST, &[], HashMap::new())
     .unwrap();
 }
 
@@ -87,51 +81,21 @@ fn duplicate_nonces() {
   // Run once without duplicating a nonce, and once with, so that's confirmed to be the faulty
   // component
   for i in [1, 0] {
-    let mut mempool = HashMap::new();
+    let mut mempool = vec![];
-    let mut insert = |tx: NonceTransaction| mempool.insert(tx.hash(), tx);
+    let mut insert = |tx: NonceTransaction| mempool.push(tx);
     insert(NonceTransaction::new(0, 0));
     insert(NonceTransaction::new(i, 1));
 
-    let res = Block::new(LAST, &ProvidedTransactions::new(), mempool).verify(
+    let res = Block::new(LAST, vec![], mempool).verify(
       GENESIS,
       LAST,
-      HashSet::new(),
+      &[],
       HashMap::from([(<Ristretto as Ciphersuite>::G::identity(), 0)]),
     );
     if i == 1 {
       res.unwrap();
     } else {
-      assert!(res.is_err());
+      assert_eq!(res, Err(BlockError::TransactionError(TransactionError::InvalidNonce)));
     }
   }
 }
-
-#[test]
-fn unsorted_nonces() {
-  let mut mempool = HashMap::new();
-  // Create a large amount of nonces so the retrieval from the HashMapis effectively guaranteed to
-  // be out of order
-  let mut nonces = (0 .. 64).collect::<Vec<_>>();
-  // Insert in a random order
-  while !nonces.is_empty() {
-    let nonce = nonces.swap_remove(
-      usize::try_from(OsRng.next_u64() % u64::try_from(nonces.len()).unwrap()).unwrap(),
-    );
-    let tx = NonceTransaction::new(nonce, 0);
-    mempool.insert(tx.hash(), tx);
-  }
-
-  // Create and verify the block
-  const GENESIS: [u8; 32] = [0xff; 32];
-  const LAST: [u8; 32] = [0x01; 32];
-  let nonces = HashMap::from([(<Ristretto as Ciphersuite>::G::identity(), 0)]);
-  Block::new(LAST, &ProvidedTransactions::new(), mempool.clone())
-    .verify(GENESIS, LAST, HashSet::new(), nonces.clone())
-    .unwrap();
-
-  let skip = NonceTransaction::new(65, 0);
-  mempool.insert(skip.hash(), skip);
-  assert!(Block::new(LAST, &ProvidedTransactions::new(), mempool)
-    .verify(GENESIS, LAST, HashSet::new(), nonces)
-    .is_err());
-}

coordinator/tributary/src/tests/blockchain.rs

@@ -1,5 +1,3 @@
-use std::collections::{HashSet, HashMap};
-
 use zeroize::Zeroizing;
 use rand::{RngCore, rngs::OsRng};
 
@@ -8,7 +6,7 @@ use blake2::{Digest, Blake2s256};
 use ciphersuite::{group::ff::Field, Ciphersuite, Ristretto};
 
 use crate::{
-  merkle, Signed, TransactionKind, Transaction, ProvidedTransactions, Block, Blockchain,
+  merkle, Transaction, ProvidedTransactions, Block, Blockchain,
   tests::{ProvidedTransaction, SignedTransaction, random_provided_transaction},
 };
 
@@ -69,11 +67,7 @@ fn invalid_block() {
   // Not a participant
   {
     // Manually create the block to bypass build_block's checks
-    let block = Block::new(
+    let block = Block::new(blockchain.tip(), vec![], vec![tx.clone()]);
-      blockchain.tip(),
-      &ProvidedTransactions::new(),
-      HashMap::from([(tx.hash(), tx.clone())]),
-    );
     assert_eq!(block.header.transactions, merkle(&[tx.hash()]));
     assert!(blockchain.verify_block(&block).is_err());
   }
@@ -83,11 +77,7 @@ fn invalid_block() {
 
   // Re-run the not a participant block to make sure it now works
   {
-    let block = Block::new(
+    let block = Block::new(blockchain.tip(), vec![], vec![tx.clone()]);
-      blockchain.tip(),
-      &ProvidedTransactions::new(),
-      HashMap::from([(tx.hash(), tx.clone())]),
-    );
     assert_eq!(block.header.transactions, merkle(&[tx.hash()]));
     blockchain.verify_block(&block).unwrap();
   }
@@ -95,7 +85,7 @@ fn invalid_block() {
   {
     // Add a valid transaction
     let mut blockchain = blockchain.clone();
-    assert!(blockchain.add_transaction(tx.clone()));
+    assert!(blockchain.add_transaction(true, tx.clone()));
     let mut block = blockchain.build_block();
     assert_eq!(block.header.transactions, merkle(&[tx.hash()]));
     blockchain.verify_block(&block).unwrap();
@@ -109,15 +99,14 @@ fn invalid_block() {
     // Invalid nonce
     let tx = crate::tests::signed_transaction(&mut OsRng, genesis, &key, 5);
     // Manually create the block to bypass build_block's checks
-    let block =
+    let block = Block::new(blockchain.tip(), vec![], vec![tx]);
-      Block::new(blockchain.tip(), &ProvidedTransactions::new(), HashMap::from([(tx.hash(), tx)]));
     assert!(blockchain.verify_block(&block).is_err());
   }
 
   {
     // Invalid signature
     let mut blockchain = blockchain;
-    assert!(blockchain.add_transaction(tx));
+    assert!(blockchain.add_transaction(true, tx));
     let mut block = blockchain.build_block();
     blockchain.verify_block(&block).unwrap();
     block.transactions[0].1.signature.s += <Ristretto as Ciphersuite>::F::ONE;
@@ -140,49 +129,25 @@ fn signed_transaction() {
   let mut blockchain = new_blockchain::<SignedTransaction>(genesis, &[signer]);
   assert_eq!(blockchain.next_nonce(signer), Some(0));
 
-  let test = |blockchain: &mut Blockchain<SignedTransaction>,
+  let test = |blockchain: &mut Blockchain<SignedTransaction>, mempool: Vec<SignedTransaction>| {
-              mempool: HashMap<[u8; 32], SignedTransaction>| {
-    let mut hashes = mempool.keys().cloned().collect::<HashSet<_>>();
-
-    // These transactions do need to be added, in-order, to the mempool for the blockchain to
-    // build a block off them
-    {
-      let mut ordered = HashMap::new();
-      for (_, tx) in mempool.clone().drain() {
-        let nonce = if let TransactionKind::Signed(Signed { nonce, .. }) = tx.kind() {
-          *nonce
-        } else {
-          panic!("non-signed TX in test mempool");
-        };
-        ordered.insert(nonce, tx);
-      }
-
-      let mut i = 0;
-      while !ordered.contains_key(&i) {
-        i += 1;
-      }
-      for i in i .. (i + u32::try_from(ordered.len()).unwrap()) {
-        assert!(blockchain.add_transaction(ordered.remove(&i).unwrap()));
-      }
-    }
-
     let tip = blockchain.tip();
+    for tx in mempool.clone() {
+      let next_nonce = blockchain.next_nonce(signer).unwrap();
+      assert!(blockchain.add_transaction(true, tx));
+      assert_eq!(next_nonce + 1, blockchain.next_nonce(signer).unwrap());
+    }
     let block = blockchain.build_block();
-    // The Block constructor should sort these these, and build_block should've called Block::new
+    assert_eq!(block, Block::new(blockchain.tip(), vec![], mempool.clone()));
-    assert_eq!(block, Block::new(blockchain.tip(), &ProvidedTransactions::new(), mempool));
     assert_eq!(blockchain.tip(), tip);
     assert_eq!(block.header.parent, tip);
 
     // Make sure all transactions were included
-    let mut ordered_hashes = vec![];
+    assert_eq!(block.transactions, mempool);
-    assert_eq!(hashes.len(), block.transactions.len());
-    for transaction in &block.transactions {
-      let hash = transaction.hash();
-      assert!(hashes.remove(&hash));
-      ordered_hashes.push(hash);
-    }
     // Make sure the merkle was correct
-    assert_eq!(block.header.transactions, merkle(&ordered_hashes));
+    assert_eq!(
+      block.header.transactions,
+      merkle(&mempool.iter().map(Transaction::hash).collect::<Vec<_>>())
+    );
 
     // Verify and add the block
     blockchain.verify_block(&block).unwrap();
@@ -191,19 +156,13 @@ fn signed_transaction() {
   };
 
   // Test with a single nonce
-  test(&mut blockchain, HashMap::from([(tx.hash(), tx)]));
+  test(&mut blockchain, vec![tx]);
   assert_eq!(blockchain.next_nonce(signer), Some(1));
 
   // Test with a flood of nonces
-  let mut mempool = HashMap::new();
+  let mut mempool = vec![];
-  let mut nonces = (1 .. 64).collect::<Vec<_>>();
+  for nonce in 1 .. 64 {
-  // Randomize insertion order into HashMap, even though it should already have unordered iteration
+    mempool.push(crate::tests::signed_transaction(&mut OsRng, genesis, &key, nonce));
-  while !nonces.is_empty() {
-    let nonce = nonces.swap_remove(
-      usize::try_from(OsRng.next_u64() % u64::try_from(nonces.len()).unwrap()).unwrap(),
-    );
-    let tx = crate::tests::signed_transaction(&mut OsRng, genesis, &key, nonce);
-    mempool.insert(tx.hash(), tx);
   }
   test(&mut blockchain, mempool);
   assert_eq!(blockchain.next_nonce(signer), Some(64));
@@ -214,20 +173,24 @@ fn provided_transaction() {
   let mut blockchain = new_blockchain::<ProvidedTransaction>(new_genesis(), &[]);
 
   let tx = random_provided_transaction(&mut OsRng);
+
+  // This should be provideable
   let mut txs = ProvidedTransactions::new();
   txs.provide(tx.clone());
+  txs.complete(tx.hash());
+
   // Non-provided transactions should fail verification
-  let block = Block::new(blockchain.tip(), &txs, HashMap::new());
+  let block = Block::new(blockchain.tip(), vec![tx.clone()], vec![]);
   assert!(blockchain.verify_block(&block).is_err());
 
   // Provided transactions should pass verification
-  blockchain.provide_transaction(tx);
+  blockchain.provide_transaction(tx.clone());
   blockchain.verify_block(&block).unwrap();
 
   // add_block should work for verified blocks
   assert!(blockchain.add_block(&block).is_ok());
 
-  let block = Block::new(blockchain.tip(), &txs, HashMap::new());
+  let block = Block::new(blockchain.tip(), vec![tx], vec![]);
   // The provided transaction should no longer considered provided, causing this error
   assert!(blockchain.verify_block(&block).is_err());
   // add_block should fail for unverified provided transactions if told to add them

coordinator/tributary/src/tests/mempool.rs

@@ -6,7 +6,7 @@ use rand::{RngCore, rngs::OsRng};
 use ciphersuite::{group::ff::Field, Ciphersuite, Ristretto};
 
 use crate::{
-  Transaction, Mempool,
+  ACCOUNT_MEMPOOL_LIMIT, Transaction, Mempool,
   tests::{SignedTransaction, signed_transaction},
 };
 
@@ -28,17 +28,17 @@ fn mempool_addition() {
 
   // Add TX 0
   let mut blockchain_next_nonces = HashMap::from([(signer, 0)]);
-  assert!(mempool.add(&blockchain_next_nonces, first_tx.clone()));
+  assert!(mempool.add(&blockchain_next_nonces, true, first_tx.clone()));
   assert_eq!(mempool.next_nonce(&signer), Some(1));
 
   // Adding it again should fail
-  assert!(!mempool.add(&blockchain_next_nonces, first_tx.clone()));
+  assert!(!mempool.add(&blockchain_next_nonces, true, first_tx.clone()));
 
   // Do the same with the next nonce
   let second_tx = signed_transaction(&mut OsRng, genesis, &key, 1);
-  assert!(mempool.add(&blockchain_next_nonces, second_tx.clone()));
+  assert!(mempool.add(&blockchain_next_nonces, true, second_tx.clone()));
   assert_eq!(mempool.next_nonce(&signer), Some(2));
-  assert!(!mempool.add(&blockchain_next_nonces, second_tx.clone()));
+  assert!(!mempool.add(&blockchain_next_nonces, true, second_tx.clone()));
 
   // If the mempool doesn't have a nonce for an account, it should successfully use the
   // blockchain's
@@ -47,7 +47,7 @@ fn mempool_addition() {
   let second_signer = tx.1.signer;
   assert_eq!(mempool.next_nonce(&second_signer), None);
   blockchain_next_nonces.insert(second_signer, 2);
-  assert!(mempool.add(&blockchain_next_nonces, tx.clone()));
+  assert!(mempool.add(&blockchain_next_nonces, true, tx.clone()));
   assert_eq!(mempool.next_nonce(&second_signer), Some(3));
 
   // Getting a block should work
@@ -55,12 +55,35 @@ fn mempool_addition() {
 
   // If the blockchain says an account had its nonce updated, it should cause a prune
   blockchain_next_nonces.insert(signer, 1);
-  let block = mempool.block(&blockchain_next_nonces);
+  let mut block = mempool.block(&blockchain_next_nonces);
   assert_eq!(block.len(), 2);
-  assert!(!block.contains_key(&first_tx.hash()));
+  assert!(!block.iter().any(|tx| tx.hash() == first_tx.hash()));
-  assert_eq!(mempool.txs(), &block);
+  assert_eq!(mempool.txs(), &block.drain(..).map(|tx| (tx.hash(), tx)).collect::<HashMap<_, _>>());
 
   // Removing should also successfully prune
   mempool.remove(&tx.hash());
   assert_eq!(mempool.txs(), &HashMap::from([(second_tx.hash(), second_tx)]));
 }
+
+#[test]
+fn too_many_mempool() {
+  let (genesis, mut mempool) = new_mempool::<SignedTransaction>();
+
+  let key = Zeroizing::new(<Ristretto as Ciphersuite>::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)]),
+      false,
+      signed_transaction(&mut OsRng, genesis, &key, i)
+    ));
+  }
+  // Yet adding more should fail
+  assert!(!mempool.add(
+    &HashMap::from([(signer, 0)]),
+    false,
+    signed_transaction(&mut OsRng, genesis, &key, ACCOUNT_MEMPOOL_LIMIT)
+  ));
+}

coordinator/tributary/src/tests/transaction/mod.rs

@@ -1,7 +1,4 @@
-use std::{
+use std::{io, collections::HashMap};
-  io,
-  collections::{HashSet, HashMap},
-};
 
 use zeroize::Zeroizing;
 use rand::{RngCore, CryptoRng};
@@ -19,9 +16,6 @@ use crate::{ReadWrite, Signed, TransactionError, TransactionKind, Transaction, v
 #[cfg(test)]
 mod signed;
 
-#[cfg(test)]
-mod provided;
-
 pub fn random_signed<R: RngCore + CryptoRng>(rng: &mut R) -> Signed {
   Signed {
     signer: <Ristretto as Ciphersuite>::G::random(&mut *rng),
@@ -127,7 +121,7 @@ pub fn signed_transaction<R: RngCore + CryptoRng>(
   );
 
   let mut nonces = HashMap::from([(signer, nonce)]);
-  verify_transaction(&tx, genesis, &mut HashSet::new(), &mut nonces).unwrap();
+  verify_transaction(&tx, genesis, &mut nonces).unwrap();
   assert_eq!(nonces, HashMap::from([(tx.1.signer, tx.1.nonce.wrapping_add(1))]));
 
   tx

coordinator/tributary/src/tests/transaction/provided.rs

@@ -1,18 +0,0 @@
-use std::collections::{HashSet, HashMap};
-
-use rand::rngs::OsRng;
-
-use crate::{Transaction, verify_transaction, tests::random_provided_transaction};
-
-#[test]
-fn provided_transaction() {
-  let tx = random_provided_transaction(&mut OsRng);
-
-  // Make sure this works when provided
-  let mut provided = HashSet::from([tx.hash()]);
-  verify_transaction(&tx, [0x88; 32], &mut provided, &mut HashMap::new()).unwrap();
-  assert_eq!(provided.len(), 0);
-
-  // Make sure this fails when not provided
-  assert!(verify_transaction(&tx, [0x88; 32], &mut HashSet::new(), &mut HashMap::new()).is_err());
-}

coordinator/tributary/src/tests/transaction/signed.rs

@@ -1,4 +1,4 @@
-use std::collections::{HashSet, HashMap};
+use std::collections::HashMap;
 
 use rand::rngs::OsRng;
 
@@ -37,7 +37,6 @@ fn signed_transaction() {
   assert!(verify_transaction(
     &tx,
     Blake2s256::digest(genesis).into(),
-    &mut HashSet::new(),
     &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),
   )
   .is_err());
@@ -46,26 +45,18 @@ fn signed_transaction() {
   {
     let mut tx = tx.clone();
     tx.0 = Blake2s256::digest(tx.0).to_vec();
-    assert!(verify_transaction(
+    assert!(
-      &tx,
+      verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err()
-      genesis,
+    );
-      &mut HashSet::new(),
-      &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),
-    )
-    .is_err());
   }
 
   // Different signer
   {
     let mut tx = tx.clone();
     tx.1.signer += Ristretto::generator();
-    assert!(verify_transaction(
+    assert!(
-      &tx,
+      verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err()
-      genesis,
+    );
-      &mut HashSet::new(),
-      &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),
-    )
-    .is_err());
   }
 
   // Different nonce
@@ -73,42 +64,30 @@ 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(
+    assert!(
-      &tx,
+      verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err()
-      genesis,
+    );
-      &mut HashSet::new(),
-      &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),
-    )
-    .is_err());
   }
 
   // Different signature
   {
     let mut tx = tx.clone();
     tx.1.signature.R += Ristretto::generator();
-    assert!(verify_transaction(
+    assert!(
-      &tx,
+      verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err()
-      genesis,
+    );
-      &mut HashSet::new(),
-      &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),
-    )
-    .is_err());
   }
   {
     let mut tx = tx.clone();
     tx.1.signature.s += <Ristretto as Ciphersuite>::F::ONE;
-    assert!(verify_transaction(
+    assert!(
-      &tx,
+      verify_transaction(&tx, genesis, &mut HashMap::from([(tx.1.signer, tx.1.nonce)]),).is_err()
-      genesis,
+    );
-      &mut HashSet::new(),
-      &mut HashMap::from([(tx.1.signer, 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 HashSet::new(), &mut nonces).unwrap();
+  verify_transaction(&tx, genesis, &mut nonces).unwrap();
   assert_eq!(nonces, HashMap::from([(tx.1.signer, tx.1.nonce.wrapping_add(1))]));
 }
 
@@ -119,7 +98,6 @@ fn invalid_nonce() {
   assert!(verify_transaction(
     &tx,
     genesis,
-    &mut HashSet::new(),
     &mut HashMap::from([(tx.1.signer, tx.1.nonce.wrapping_add(1))]),
   )
   .is_err());

coordinator/tributary/src/transaction.rs

@@ -1,8 +1,5 @@
 use core::fmt::Debug;
-use std::{
+use std::{io, collections::HashMap};
-  io,
-  collections::{HashSet, HashMap},
-};
 
 use thiserror::Error;
 
@@ -11,13 +8,13 @@ use blake2::{Digest, Blake2b512};
 use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto};
 use schnorr::SchnorrSignature;
 
-use crate::ReadWrite;
+use crate::{TRANSACTION_SIZE_LIMIT, ReadWrite};
 
 #[derive(Clone, PartialEq, Eq, Debug, Error)]
 pub enum TransactionError {
-  /// A provided transaction wasn't locally provided.
+  /// Transaction exceeded the size limit.
-  #[error("provided transaction wasn't locally provided")]
+  #[error("transaction was too large")]
-  MissingProvided([u8; 32]),
+  TooLargeTransaction,
   /// This transaction's signer isn't a participant.
   #[error("invalid signer")]
   InvalidSigner,
@@ -63,7 +60,13 @@ impl ReadWrite for Signed {
 #[allow(clippy::large_enum_variant)]
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum TransactionKind<'a> {
-  /// This tranaction should be provided by every validator, solely ordered by the block producer.
+  /// This tranaction should be provided by every validator, in an exact order.
+  ///
+  /// The only malleability is in when this transaction appears on chain. The block producer will
+  /// include it when they have it. Block verification will fail for validators without it.
+  ///
+  /// If a supermajority of validators still produce a commit for a block with a provided
+  /// transaction which isn't locally held, the chain will sleep until it is locally provided.
   Provided,
 
   /// An unsigned transaction, only able to be included by the block producer.
@@ -99,18 +102,16 @@ pub trait Transaction: 'static + Send + Sync + Clone + Eq + Debug + ReadWrite {
 pub(crate) fn verify_transaction<T: Transaction>(
   tx: &T,
   genesis: [u8; 32],
-  locally_provided: &mut HashSet<[u8; 32]>,
   next_nonces: &mut HashMap<<Ristretto as Ciphersuite>::G, u32>,
 ) -> Result<(), TransactionError> {
+  if tx.serialize().len() > TRANSACTION_SIZE_LIMIT {
+    Err(TransactionError::TooLargeTransaction)?;
+  }
+
   tx.verify()?;
 
   match tx.kind() {
-    TransactionKind::Provided => {
+    TransactionKind::Provided => {}
-      let hash = tx.hash();
-      if !locally_provided.remove(&hash) {
-        Err(TransactionError::MissingProvided(hash))?;
-      }
-    }
     TransactionKind::Unsigned => {}
     TransactionKind::Signed(Signed { signer, nonce, signature }) => {
       if let Some(next_nonce) = next_nonces.get(signer) {