fix: rewind bug causing SMT to be broken

base_layer/core/src/base_node/sync/horizon_state_sync/synchronizer.rs

@@ -32,7 +32,7 @@ use log::*;
 use tari_common_types::types::{Commitment, FixedHash, RangeProofService};
 use tari_comms::{connectivity::ConnectivityRequester, peer_manager::NodeId, protocol::rpc::RpcClient, PeerConnection};
 use tari_crypto::commitment::HomomorphicCommitment;
-use tari_mmr::sparse_merkle_tree::{NodeKey, ValueHash};
+use tari_mmr::sparse_merkle_tree::{DeleteResult, NodeKey, ValueHash};
 use tari_utilities::{hex::Hex, ByteArray};
 use tokio::task;
 
@@ -663,7 +663,14 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> {
                     batch_verify_range_proofs(&self.prover, &[&output])?;
                     let smt_key = NodeKey::try_from(output.commitment.as_bytes())?;
                     let smt_node = ValueHash::try_from(output.smt_hash(current_header.height).as_slice())?;
-                    output_smt.insert(smt_key, smt_node)?;
+                    if let Err(e) = output_smt.insert(smt_key, smt_node) {
+                        error!(
+                            target: LOG_TARGET,
+                            "Output commitment({}) already in SMT",
+                            output.commitment.to_hex(),
+                        );
+                        return Err(e.into());
+                    }
                     txn.insert_output_via_horizon_sync(
                         output,
                         current_header.hash(),
@@ -691,7 +698,14 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> {
                                 stxo_counter,
                             );
                             let smt_key = NodeKey::try_from(commitment_bytes.as_slice())?;
-                            output_smt.delete(&smt_key)?;
+                            match output_smt.delete(&smt_key)? {
+                                DeleteResult::Deleted(_value_hash) => {},
+                                DeleteResult::KeyNotFound => {
+                                    return Err(HorizonSyncError::ChainStorageError(
+                                        ChainStorageError::UnspendableInput,
+                                    ))
+                                },
+                            };
                             // This will only be committed once the SMT has been verified due to rewind difficulties if
                             // we need to abort the sync
                             inputs_to_delete.push((output_hash, commitment));

base_layer/core/src/chain_storage/blockchain_database.rs

@@ -1334,15 +1334,20 @@ pub fn calculate_mmr_roots<T: BlockchainBackend>(
         if !output.is_burned() {
             let smt_key = NodeKey::try_from(output.commitment.as_bytes())?;
             let smt_node = ValueHash::try_from(output.smt_hash(header.height).as_slice())?;
-            output_smt.insert(smt_key, smt_node)?;
+            if let Err(e) = output_smt.insert(smt_key, smt_node) {
+                error!(
+                    target: LOG_TARGET,
+                    "Output commitment({}) already in SMT",
+                    output.commitment.to_hex(),
+                );
+                return Err(e.into());
+            }
         }
     }
 
     for input in body.inputs().iter() {
         input_mmr.push(input.canonical_hash().to_vec())?;
 
-        // 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 smt_key = NodeKey::try_from(input.commitment()?.as_bytes())?;
         match output_smt.delete(&smt_key)? {
             DeleteResult::Deleted(_value_hash) => {},

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

@@ -888,6 +888,7 @@ impl LMDBDatabase {
             .fetch_height_from_hash(write_txn, block_hash)
             .or_not_found("Block", "hash", hash_hex)?;
         let next_height = height.saturating_add(1);
+        let prev_height = height.saturating_sub(1);
         if self.fetch_block_accumulated_data(write_txn, next_height)?.is_some() {
             return Err(ChainStorageError::InvalidOperation(format!(
                 "Attempted to delete block at height {} while next block still exists",
@@ -904,6 +905,21 @@ impl LMDBDatabase {
         let mut smt = self.fetch_tip_smt()?;
 
         self.delete_block_inputs_outputs(write_txn, block_hash.as_slice(), &mut smt)?;
+
+        let new_tip_header = self.fetch_chain_header_by_height(prev_height)?;
+        let root = FixedHash::try_from(smt.hash().as_slice())?;
+        if root != new_tip_header.header().output_mr {
+            error!(
+                target: LOG_TARGET,
+                "Deleting block, new smt root(#{}) did not match expected (#{}) smt root",
+                    root.to_hex(),
+                    new_tip_header.header().output_mr.to_hex(),
+            );
+            return Err(ChainStorageError::InvalidOperation(
+                "Deleting block, new smt root did not match expected smt root".to_string(),
+            ));
+        }
+
         self.insert_tip_smt(write_txn, &smt)?;
         self.delete_block_kernels(write_txn, block_hash.as_slice())?;
 
@@ -941,7 +957,10 @@ impl LMDBDatabase {
                 continue;
             }
             let smt_key = NodeKey::try_from(utxo.output.commitment.as_bytes())?;
-            output_smt.delete(&smt_key)?;
+            match output_smt.delete(&smt_key)? {
+                DeleteResult::Deleted(_value_hash) => {},
+                DeleteResult::KeyNotFound => return Err(ChainStorageError::UnspendableInput),
+            };
             lmdb_delete(
                 txn,
                 &self.utxo_commitment_index,
@@ -993,7 +1012,14 @@ impl LMDBDatabase {
             );
             let smt_key = NodeKey::try_from(input.commitment()?.as_bytes())?;
             let smt_node = ValueHash::try_from(input.smt_hash(row.spent_height).as_slice())?;
-            output_smt.insert(smt_key, smt_node)?;
+            if let Err(e) = output_smt.insert(smt_key, smt_node) {
+                error!(
+                    target: LOG_TARGET,
+                    "Output commitment({}) already in SMT",
+                    input.commitment()?.to_hex(),
+                );
+                return Err(e.into());
+            }
 
             trace!(target: LOG_TARGET, "Input moved to UTXO set: {}", input);
             lmdb_insert(
@@ -1210,7 +1236,14 @@ impl LMDBDatabase {
             if !output.is_burned() {
                 let smt_key = NodeKey::try_from(output.commitment.as_bytes())?;
                 let smt_node = ValueHash::try_from(output.smt_hash(header.height).as_slice())?;
-                output_smt.insert(smt_key, smt_node)?;
+                if let Err(e) = output_smt.insert(smt_key, smt_node) {
+                    error!(
+                        target: LOG_TARGET,
+                        "Output commitment({}) already in SMT",
+                        output.commitment.to_hex(),
+                    );
+                    return Err(e.into());
+                }
             }
 
             let output_hash = output.hash();

base_layer/core/src/transactions/aggregated_body.rs

@@ -408,7 +408,7 @@ impl From<Transaction> for AggregateBody {
 impl Display for AggregateBody {
     fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), Error> {
         if !self.is_sorted() {
-            writeln!(fmt, "WARNING: Block body is not sorted.")?;
+            writeln!(fmt, "WARNING: Body is not sorted.")?;
         }
         writeln!(fmt, "--- Transaction Kernels ---")?;
         for (i, kernel) in self.kernels.iter().enumerate() {

base_layer/core/src/validation/aggregate_body/aggregate_body_internal_validator.rs

@@ -311,7 +311,7 @@ fn check_weight(
     if block_weight <= max_weight {
         trace!(
             target: LOG_TARGET,
-            "SV - Block contents for block #{} : {}; weight {}.",
+            "Aggregated body at height #{} : {}; weight {} is valid.",
             height,
             body.to_counts_string(),
             block_weight,