diff --git a/coordinator/tributary/src/blockchain.rs b/coordinator/tributary/src/blockchain.rs index 86b7f37b..f73c874b 100644 --- a/coordinator/tributary/src/blockchain.rs +++ b/coordinator/tributary/src/blockchain.rs @@ -54,16 +54,23 @@ impl Blockchain { block.verify(self.genesis, self.tip, locally_provided, self.next_nonces.clone()) } - /// Add a block, assuming it's valid. - /// - /// Do not call this without either verifying the block or having it confirmed under consensus. - /// Doing so will cause a panic or action an invalid transaction. - pub fn add_block(&mut self, block: &Block) { + /// Add a block. + #[must_use] + pub fn add_block(&mut self, block: &Block) -> bool { + // TODO: Handle desyncs re: provided transactions properly + if self.verify_block(block).is_err() { + return false; + } + + // None of the following assertions should be reachable since we verified the block self.tip = block.hash(); for tx in &block.transactions { match tx.kind() { TransactionKind::Provided => { - self.provided.withdraw(tx.hash()); + assert!( + self.provided.withdraw(tx.hash()), + "verified block had a provided transaction we didn't have" + ); } TransactionKind::Unsigned => {} TransactionKind::Signed(Signed { signer, nonce, .. }) => { @@ -72,10 +79,12 @@ impl Blockchain { .insert(*signer, nonce + 1) .expect("block had signed transaction from non-participant"); if prev != *nonce { - panic!("block had an invalid nonce"); + panic!("verified block had an invalid nonce"); } } } } + + true } } diff --git a/coordinator/tributary/src/tests/blockchain.rs b/coordinator/tributary/src/tests/blockchain.rs index 4ccd793a..c4c49f43 100644 --- a/coordinator/tributary/src/tests/blockchain.rs +++ b/coordinator/tributary/src/tests/blockchain.rs @@ -35,7 +35,7 @@ fn block_addition() { assert_eq!(block.header.parent, genesis); assert_eq!(block.header.transactions, [0; 32]); blockchain.verify_block(&block).unwrap(); - blockchain.add_block(&block); + assert!(blockchain.add_block(&block)); assert_eq!(blockchain.tip(), block.hash()); } @@ -155,7 +155,7 @@ fn signed_transaction() { // Verify and add the block blockchain.verify_block(&block).unwrap(); - blockchain.add_block(&block); + assert!(blockchain.add_block(&block)); assert_eq!(blockchain.tip(), block.hash()); }; @@ -194,11 +194,11 @@ fn provided_transaction() { blockchain.verify_block(&block).unwrap(); // add_block should work for verified blocks - blockchain.add_block(&block); + assert!(blockchain.add_block(&block)); let block = Block::new(blockchain.tip(), &txs, HashMap::new()); // The provided transaction should no longer considered provided, causing this error assert!(blockchain.verify_block(&block).is_err()); - // add_block should also work for unverified provided transactions if told to add them - blockchain.add_block(&block); + // add_block should fail for unverified provided transactions if told to add them + assert!(!blockchain.add_block(&block)); } diff --git a/coordinator/tributary/src/transaction.rs b/coordinator/tributary/src/transaction.rs index 290038d0..d4e9aa0e 100644 --- a/coordinator/tributary/src/transaction.rs +++ b/coordinator/tributary/src/transaction.rs @@ -58,8 +58,6 @@ impl ReadWrite for Signed { #[derive(Clone, PartialEq, Eq, Debug)] pub enum TransactionKind<'a> { /// This tranaction should be provided by every validator, solely ordered by the block producer. - /// - /// This transaction is only valid if a supermajority of validators provided it. Provided, /// An unsigned transaction, only able to be included by the block producer.