fix: update transaction and block validator to use full deleted map (#…

…3137)

applications/tari_base_node/src/command_handler.rs

@@ -304,7 +304,7 @@ impl CommandHandler {
                 Ok(mut data) => match data.pop() {
                     Some(v) => println!("{}", v.block()),
                     _ => println!(
-                        "Pruned node: utxo found, but lock not found for utxo commitment {}",
+                        "Pruned node: utxo found, but block not found for utxo commitment {}",
                         commitment.to_hex()
                     ),
                 },

base_layer/core/src/base_node/comms_interface/inbound_handlers.rs

@@ -362,7 +362,13 @@ where T: BlockchainBackend + 'static
                     .await?
                     .into_iter()
                     .map(|tx| Arc::try_unwrap(tx).unwrap_or_else(|tx| (*tx).clone()))
-                    .collect();
+                    .collect::<Vec<_>>();
+
+                debug!(
+                    target: LOG_TARGET,
+                    "Adding {} transaction(s) to new block template",
+                    transactions.len()
+                );
 
                 let prev_hash = header.prev_hash.clone();
                 let height = header.height;

base_layer/core/src/chain_storage/async_db.rs

@@ -32,6 +32,7 @@ use crate::{
         ChainHeader,
         ChainStorageError,
         DbTransaction,
+        DeletedBitmap,
         HistoricalBlock,
         HorizonData,
         MmrTree,
@@ -215,6 +216,8 @@ impl<B: BlockchainBackend + 'static> AsyncBlockchainDb<B> {
     make_async_fn!(fetch_block_accumulated_data_by_height(height: u64) -> BlockAccumulatedData, "fetch_block_accumulated_data_by_height");
 
     //---------------------------------- Misc. --------------------------------------------//
+    make_async_fn!(fetch_deleted_bitmap() -> DeletedBitmap, "fetch_deleted_bitmap");
+
     make_async_fn!(fetch_block_timestamps(start_hash: HashOutput) -> RollingVec<EpochTime>, "fetch_block_timestamps");
 
     make_async_fn!(fetch_target_difficulty_for_next_block(pow_algo: PowAlgorithm, current_block_hash: HashOutput) -> TargetDifficultyWindow, "fetch_target_difficulty");

base_layer/core/src/chain_storage/blockchain_database.rs

@@ -35,6 +35,7 @@ use crate::{
         BlockchainBackend,
         ChainBlock,
         ChainHeader,
+        DeletedBitmap,
         HistoricalBlock,
         HorizonData,
         MmrTree,
@@ -58,6 +59,7 @@ use std::{
     cmp,
     cmp::Ordering,
     collections::VecDeque,
+    convert::TryFrom,
     mem,
     ops::Bound,
     sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard},
@@ -876,6 +878,12 @@ where B: BlockchainBackend
         let db = self.db_read_access()?;
         db.fetch_horizon_data()
     }
+
+    /// Returns the full deleted bitmap at the current blockchain tip
+    pub fn fetch_deleted_bitmap(&self) -> Result<DeletedBitmap, ChainStorageError> {
+        let db = self.db_read_access()?;
+        db.fetch_deleted_bitmap()
+    }
 }
 
 fn unexpected_result<T>(req: DbKey, res: DbValue) -> Result<T, ChainStorageError> {
@@ -942,26 +950,45 @@ pub fn calculate_mmr_roots<T: BlockchainBackend>(db: &T, block: &Block) -> Resul
     }
 
     for input in body.inputs().iter() {
+        input_mmr.push(input.hash())?;
+
         // Search the DB for the output leaf index so that it can be marked as spent/deleted.
         // If the output hash is not found, check the current output_mmr. This allows zero-conf transactions
-        let index =
-            match db.fetch_mmr_leaf_index(MmrTree::Utxo, &input.output_hash())? {
-                Some(index) => index,
-                None => output_mmr.find_leaf_index(&input.output_hash())?.ok_or_else(|| {
-                    ChainStorageError::ValueNotFound {
-                        entity: "UTXO".to_string(),
-                        field: "hash".to_string(),
-                        value: input.output_hash().to_hex(),
-                    }
-                })?,
-            };
-        input_mmr.push(input.hash())?;
+        let output_hash = input.output_hash();
+        let index = match db.fetch_mmr_leaf_index(MmrTree::Utxo, &output_hash)? {
+            Some(index) => index,
+            None => {
+                let index =
+                    output_mmr
+                        .find_leaf_index(&output_hash)?
+                        .ok_or_else(|| ChainStorageError::ValueNotFound {
+                            entity: "UTXO".to_string(),
+                            field: "hash".to_string(),
+                            value: output_hash.to_hex(),
+                        })?;
+                debug!(
+                    target: LOG_TARGET,
+                    "0-conf spend detected when calculating MMR roots for UTXO index {} ({})",
+                    index,
+                    output_hash.to_hex()
+                );
+                index
+            },
+        };
 
         if !output_mmr.delete(index) {
-            let len = output_mmr.len();
+            let num_leaves = u32::try_from(output_mmr.get_leaf_count())
+                .map_err(|_| ChainStorageError::CriticalError("UTXO MMR leaf count overflows u32".to_string()))?;
+            if index < num_leaves && output_mmr.deleted().contains(index) {
+                return Err(ChainStorageError::InvalidOperation(format!(
+                    "UTXO {} was already marked as deleted.",
+                    output_hash.to_hex()
+                )));
+            }
+
             return Err(ChainStorageError::InvalidOperation(format!(
-                "Could not delete index {} from the output MMR (length is {})",
-                index, len
+                "Could not delete index {} from the output MMR ({} leaves)",
+                index, num_leaves
             )));
         }
     }
@@ -1566,7 +1593,11 @@ fn reorganize_chain<T: BlockchainBackend>(
         let mut txn = DbTransaction::new();
         let block_hash_hex = block.accumulated_data().hash.to_hex();
         txn.delete_orphan(block.accumulated_data().hash.clone());
-        if let Err(e) = block_validator.validate_body_for_valid_orphan(&block, backend) {
+        let chain_metadata = backend.fetch_chain_metadata()?;
+        let deleted_bitmap = backend.fetch_deleted_bitmap()?;
+        if let Err(e) =
+            block_validator.validate_body_for_valid_orphan(&block, backend, &chain_metadata, &deleted_bitmap)
+        {
             warn!(
                 target: LOG_TARGET,
                 "Orphan block {} ({}) failed validation during chain reorg: {:?}",
@@ -2064,7 +2095,7 @@ mod test {
         fn it_inserts_true_orphan_chain() {
             let db = create_new_blockchain();
             let validator = MockValidator::new(true);
-            let (_, main_chain) = create_main_chain(&db, &[("A->GB", 1, 120), ("B->GB", 1, 120)]);
+            let (_, main_chain) = create_main_chain(&db, &[("A->GB", 1, 120), ("B->A", 1, 120)]);
 
             let block_b = main_chain.get("B").unwrap().clone();
             let (_, orphan_chain) =

base_layer/core/src/chain_storage/error.rs

@@ -85,8 +85,8 @@ pub enum ChainStorageError {
     },
     #[error("The MMR root for {0} in the provided block header did not match the MMR root in the database")]
     MismatchedMmrRoot(MmrTree),
-    #[error("An invalid block was submitted to the database")]
-    InvalidBlock,
+    #[error("An invalid block was submitted to the database: {0}")]
+    InvalidBlock(String),
     #[error("Blocking task spawn error: {0}")]
     BlockingTaskSpawnError(String),
     #[error("A request was out of range")]

base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs

@@ -1755,7 +1755,6 @@ impl BlockchainBackend for LMDBDatabase {
 
     fn kernel_count(&self) -> Result<usize, ChainStorageError> {
         let txn = self.read_transaction()?;
-
         lmdb_len(&txn, &self.kernels_db)
     }
 

base_layer/core/src/chain_storage/pruned_output.rs

@@ -31,6 +31,7 @@ pub enum PrunedOutput {
         output: TransactionOutput,
     },
 }
+
 impl PrunedOutput {
     pub fn is_pruned(&self) -> bool {
         matches!(self, PrunedOutput::Pruned { .. })

base_layer/core/src/validation/block_validators.rs

@@ -22,7 +22,7 @@
 use crate::{
     blocks::{Block, BlockValidationError},
     chain_storage,
-    chain_storage::{BlockchainBackend, ChainBlock, MmrTree},
+    chain_storage::{BlockchainBackend, ChainBlock, DeletedBitmap, MmrTree},
     consensus::ConsensusManager,
     transactions::{
         aggregated_body::AggregateBody,
@@ -39,6 +39,7 @@ use crate::{
 };
 use log::*;
 use std::marker::PhantomData;
+use tari_common_types::chain_metadata::ChainMetadata;
 use tari_crypto::{
     commitment::HomomorphicCommitmentFactory,
     tari_utilities::{hash::Hashable, hex::Hex},
@@ -124,10 +125,29 @@ impl<B: BlockchainBackend> PostOrphanBodyValidation<B> for BodyOnlyValidator {
     /// 1. Are all inputs currently in the UTXO set?
     /// 1. Are all inputs and outputs not in the STXO set?
     /// 1. Are the block header MMR roots valid?
-    fn validate_body_for_valid_orphan(&self, block: &ChainBlock, backend: &B) -> Result<(), ValidationError> {
+    fn validate_body_for_valid_orphan(
+        &self,
+        block: &ChainBlock,
+        backend: &B,
+        metadata: &ChainMetadata,
+        deleted_bitmap: &DeletedBitmap,
+    ) -> Result<(), ValidationError> {
+        if block.header().height != metadata.height_of_longest_chain() + 1 {
+            return Err(ValidationError::IncorrectNextTipHeight {
+                expected: metadata.height_of_longest_chain() + 1,
+                block_height: block.height(),
+            });
+        }
+        if block.header().prev_hash != *metadata.best_block() {
+            return Err(ValidationError::IncorrectPreviousHash {
+                expected: metadata.best_block().to_hex(),
+                block_hash: block.hash().to_hex(),
+            });
+        }
+
         let block_id = format!("block #{} ({})", block.header().height, block.hash().to_hex());
-        check_inputs_are_utxos(&block.block(), backend)?;
-        check_not_duplicate_txos(&block.block(), backend)?;
+        check_inputs_are_utxos(block.block(), backend, deleted_bitmap)?;
+        check_not_duplicate_txos(block.block(), backend)?;
         trace!(
             target: LOG_TARGET,
             "Block validation: All inputs and outputs are valid for {}",
@@ -157,14 +177,14 @@ fn check_sorting_and_duplicates(body: &AggregateBody) -> Result<(), ValidationEr
 }
 
 /// This function checks that all inputs in the blocks are valid UTXO's to be spend
-fn check_inputs_are_utxos<B: BlockchainBackend>(block: &Block, db: &B) -> Result<(), ValidationError> {
-    let data = db
-        .fetch_block_accumulated_data(&block.header.prev_hash)?
-        .ok_or(ValidationError::PreviousHashNotFound)?;
-
+fn check_inputs_are_utxos<B: BlockchainBackend>(
+    block: &Block,
+    db: &B,
+    deleted: &DeletedBitmap,
+) -> Result<(), ValidationError> {
     for input in block.body.inputs() {
         if let Some((_, index, _height)) = db.fetch_output(&input.output_hash())? {
-            if data.deleted().contains(index) {
+            if deleted.bitmap().contains(index) {
                 warn!(
                     target: LOG_TARGET,
                     "Block validation failed due to already spent input: {}", input

base_layer/core/src/validation/error.rs

@@ -81,6 +81,10 @@ pub enum ValidationError {
     MaxTransactionWeightExceeded,
     #[error("End of time: {0}")]
     EndOfTimeError(String),
+    #[error("Expected block height to be {expected}, but was {block_height}")]
+    IncorrectNextTipHeight { expected: u64, block_height: u64 },
+    #[error("Expected block previous hash to be {expected}, but was {block_hash}")]
+    IncorrectPreviousHash { expected: String, block_hash: String },
 }
 
 // ChainStorageError has a ValidationError variant, so to prevent a cyclic dependency we use a string representation in

base_layer/core/src/validation/mocks.rs

@@ -22,7 +22,7 @@
 
 use crate::{
     blocks::{Block, BlockHeader},
-    chain_storage::{BlockchainBackend, ChainBlock},
+    chain_storage::{BlockchainBackend, ChainBlock, DeletedBitmap},
     proof_of_work::{sha3_difficulty, AchievedTargetDifficulty, Difficulty, PowAlgorithm},
     transactions::{transaction::Transaction, types::Commitment},
     validation::{
@@ -40,6 +40,7 @@ use std::sync::{
     atomic::{AtomicBool, Ordering},
     Arc,
 };
+use tari_common_types::chain_metadata::ChainMetadata;
 
 #[derive(Clone)]
 pub struct MockValidator {
@@ -79,7 +80,13 @@ impl<B: BlockchainBackend> CandidateBlockBodyValidation<B> for MockValidator {
 }
 
 impl<B: BlockchainBackend> PostOrphanBodyValidation<B> for MockValidator {
-    fn validate_body_for_valid_orphan(&self, _item: &ChainBlock, _db: &B) -> Result<(), ValidationError> {
+    fn validate_body_for_valid_orphan(
+        &self,
+        _: &ChainBlock,
+        _: &B,
+        _: &ChainMetadata,
+        _: &DeletedBitmap,
+    ) -> Result<(), ValidationError> {
         if self.is_valid.load(Ordering::SeqCst) {
             Ok(())
         } else {

base_layer/core/src/validation/traits.rs

@@ -22,11 +22,12 @@
 
 use crate::{
     blocks::{Block, BlockHeader},
-    chain_storage::{BlockchainBackend, ChainBlock},
+    chain_storage::{BlockchainBackend, ChainBlock, DeletedBitmap},
     proof_of_work::AchievedTargetDifficulty,
     transactions::{transaction::Transaction, types::Commitment},
     validation::{error::ValidationError, DifficultyCalculator},
 };
+use tari_common_types::chain_metadata::ChainMetadata;
 
 /// A validator that determines if a block body is valid, assuming that the header has already been
 /// validated
@@ -36,7 +37,13 @@ pub trait CandidateBlockBodyValidation<B: BlockchainBackend>: Send + Sync {
 
 /// A validator that validates a body after it has been determined to be a valid orphan
 pub trait PostOrphanBodyValidation<B>: Send + Sync {
-    fn validate_body_for_valid_orphan(&self, block: &ChainBlock, backend: &B) -> Result<(), ValidationError>;
+    fn validate_body_for_valid_orphan(
+        &self,
+        block: &ChainBlock,
+        backend: &B,
+        metadata: &ChainMetadata,
+        deleted_bitmap: &DeletedBitmap,
+    ) -> Result<(), ValidationError>;
 }
 
 pub trait MempoolTransactionValidation: Send + Sync {

base_layer/core/src/validation/transaction_validators.rs

@@ -23,7 +23,6 @@
 use crate::{
     chain_storage::{BlockchainBackend, BlockchainDatabase, MmrTree},
     crypto::tari_utilities::Hashable,
-    tari_utilities::hex::Hex,
     transactions::{transaction::Transaction, types::CryptoFactories},
     validation::{MempoolTransactionValidation, ValidationError},
 };
@@ -119,23 +118,11 @@ fn verify_timelocks(tx: &Transaction, current_height: u64) -> Result<(), Validat
 
 // This function checks that the inputs exists in the UTXO set but do not exist in the STXO set.
 fn verify_not_stxos<B: BlockchainBackend>(tx: &Transaction, db: &B) -> Result<(), ValidationError> {
-    // `ChainMetadata::best_block` must always have the hash of the tip block.
-    // NOTE: the backend makes no guarantee that the tip header has a corresponding full body (interrupted header sync,
-    // pruned node) however the chain metadata best height MUST always correspond to the highest full block
-    // this node can provide
-    let metadata = db.fetch_chain_metadata()?;
-    let data = db
-        .fetch_block_accumulated_data(metadata.best_block())?
-        .unwrap_or_else(|| {
-            panic!(
-                "Expected best block `{}` to have corresponding accumulated block data, but none was found",
-                metadata.best_block().to_hex()
-            )
-        });
+    let deleted = db.fetch_deleted_bitmap()?;
     let mut not_found_input = Vec::new();
     for input in tx.body.inputs() {
         if let Some((_, index, _height)) = db.fetch_output(&input.output_hash())? {
-            if data.deleted().contains(index) {
+            if deleted.bitmap().contains(index) {
                 warn!(
                     target: LOG_TARGET,
                     "Transaction validation failed due to already spent input: {}", input

base_layer/core/tests/helpers/block_builders.rs

@@ -289,6 +289,7 @@ pub fn chain_block_with_new_coinbase(
         height + consensus_manager.consensus_constants(0).coinbase_lock_height(),
     );
     let mut header = BlockHeader::from_previous(&prev_block.header());
+    header.height = height;
     header.version = consensus_manager
         .consensus_constants(header.height)
         .blockchain_version();