fix: horizon sync (#5724)

Description
---
Fixes horizon sync
Run optimized needs to be forced with the correct mutability to ensure
it runs from the correct optimized bitmap

Motivation and Context
---
MMR root failure occurs during sync as the optimized run fails in Merkle
tree, this provides an already optimized bitmap to the merkle tree.

How Has This Been Tested?
---
Manual sync

base_layer/core/Cargo.toml

@@ -45,7 +45,7 @@ bytes = "0.5"
 chacha20poly1305 = "0.10.1"
 chrono = { version = "0.4.19", default-features = false, features = ["serde"] }
 criterion = { version = "0.4.0", optional = true  }
-croaring = { version = "0.5.2", optional = true }
+croaring = { version = "0.9", optional = true }
 decimal-rs = "0.1.42"
 derivative = "2.2.0"
 digest = "0.10"

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

@@ -639,7 +639,7 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> {
                     );
                     mmr_position += 1;
                 },
-                UtxoOrDeleted::DeletedDiff(diff_bitmap) => {
+                UtxoOrDeleted::DeletedDiff(diff_bitmap_buff) => {
                     if mmr_position != current_header.header().output_mmr_size {
                         return Err(HorizonSyncError::IncorrectResponse(format!(
                             "Peer unexpectedly sent a deleted bitmap. Expected at MMR index {} but it was sent at {}",
@@ -652,17 +652,17 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> {
                     // than isize::MAX, however isize::MAX is still an inordinate amount of data. An
                     // arbitrary 4 MiB limit is used.
                     const MAX_DIFF_BITMAP_BYTE_LEN: usize = 4 * 1024 * 1024;
-                    if diff_bitmap.len() > MAX_DIFF_BITMAP_BYTE_LEN {
+                    if diff_bitmap_buff.len() > MAX_DIFF_BITMAP_BYTE_LEN {
                         return Err(HorizonSyncError::IncorrectResponse(format!(
                             "Received difference bitmap (size = {}) that exceeded the maximum size limit of {} from \
                              peer {}",
-                            diff_bitmap.len(),
+                            diff_bitmap_buff.len(),
                             MAX_DIFF_BITMAP_BYTE_LEN,
                             sync_peer.node_id()
                         )));
                     }
 
-                    let diff_bitmap = Bitmap::try_deserialize(&diff_bitmap).ok_or_else(|| {
+                    let diff_bitmap = Bitmap::try_deserialize(&diff_bitmap_buff).ok_or_else(|| {
                         HorizonSyncError::IncorrectResponse(format!(
                             "Peer {} sent an invalid difference bitmap",
                             sync_peer.node_id()
@@ -673,11 +673,14 @@ impl<'a, B: BlockchainBackend + 'static> HorizonStateSynchronization<'a, B> {
                     // in the output MMR
                     let bitmap = self.full_bitmap_mut();
                     bitmap.or_inplace(&diff_bitmap);
+                    // let force optimize here as we need to ensure this runs as we compute the merkle root on the
+                    // optimized bitmap.
+                    bitmap.run_optimize();
 
                     let pruned_output_set = output_mmr.get_pruned_hash_set()?;
-                    let output_mmr = MutablePrunedOutputMmr::new(pruned_output_set.clone(), bitmap.clone())?;
+                    let total_output_mmr = MutablePrunedOutputMmr::new(pruned_output_set.clone(), bitmap.clone())?;
 
-                    let mmr_root = output_mmr.get_merkle_root()?;
+                    let mmr_root = total_output_mmr.get_merkle_root()?;
                     if mmr_root.as_slice() != current_header.header().output_mr.as_slice() {
                         return Err(HorizonSyncError::InvalidMmrRoot {
                             mmr_tree: MmrTree::Utxo,

base_layer/core/src/consensus/consensus_manager.rs

@@ -124,8 +124,8 @@ impl ConsensusManager {
         let constants = self.consensus_constants(height);
         let block_window = constants.difficulty_block_window();
 
-        let block_window_u = usize::try_from(block_window)
-            .map_err(|e| format!("difficulty block window exceeds usize::MAX: {}", e.to_string()))?;
+        let block_window_u =
+            usize::try_from(block_window).map_err(|e| format!("difficulty block window exceeds usize::MAX: {}", e))?;
 
         TargetDifficultyWindow::new(block_window_u, constants.pow_target_block_interval(pow_algo))
     }

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

@@ -304,9 +304,7 @@ fn check_weight(
 ) -> Result<(), ValidationError> {
     let block_weight = body
         .calculate_weight(consensus_constants.transaction_weight_params())
-        .map_err(|e| {
-            ValidationError::SerializationError(format!("Unable to calculate body weight: {}", e.to_string()))
-        })?;
+        .map_err(|e| ValidationError::SerializationError(format!("Unable to calculate body weight: {}", e)))?;
     let max_weight = consensus_constants.max_block_transaction_weight();
     if block_weight <= max_weight {
         trace!(

base_layer/core/src/validation/helpers.rs

@@ -198,9 +198,9 @@ pub fn check_input_is_utxo<B: BlockchainBackend>(db: &B, input: &TransactionInpu
 
 /// Checks the byte size of TariScript is less than or equal to the given size, otherwise returns an error.
 pub fn check_tari_script_byte_size(script: &TariScript, max_script_size: usize) -> Result<(), ValidationError> {
-    let script_size = script.get_serialized_size().map_err(|e| {
-        ValidationError::SerializationError(format!("Failed to get serialized script size: {}", e.to_string()))
-    })?;
+    let script_size = script
+        .get_serialized_size()
+        .map_err(|e| ValidationError::SerializationError(format!("Failed to get serialized script size: {}", e)))?;
     if script_size > max_script_size {
         return Err(ValidationError::TariScriptExceedsMaxSize {
             max_script_size,

base_layer/core/src/validation/transaction/transaction_chain_validator.rs

@@ -48,15 +48,12 @@ impl<B: BlockchainBackend> TransactionValidator for TransactionChainLinkedValida
         if tx
             .calculate_weight(consensus_constants.transaction_weight_params())
             .map_err(|e| {
-                ValidationError::SerializationError(format!(
-                    "Unable to calculate the transaction weight: {}",
-                    e.to_string()
-                ))
+                ValidationError::SerializationError(format!("Unable to calculate the transaction weight: {}", e))
             })? >
             consensus_constants.max_block_weight_excluding_coinbase().map_err(|e| {
                 ValidationError::ConsensusError(format!(
                     "Unable to get max block weight from consensus constants: {}",
-                    e.to_string()
+                    e
                 ))
             })?
         {

base_layer/mmr/Cargo.toml

@@ -20,7 +20,7 @@ borsh = "0.10"
 digest = "0.10"
 log = "0.4"
 serde = { version = "1.0", features = ["derive"] }
-croaring = { version = "0.5", optional = true }
+croaring = { version = "0.9", optional = true }
 
 [dev-dependencies]
 rand = "0.8"

base_layer/mmr/benches/smt_vs_mmr.rs

@@ -109,15 +109,15 @@ fn main() {
             insert_into_mmr(&keys, &mut mmr);
         });
         time_function("MMR: Calculating root hash", || {
-            let size = mmr.len();
+            let size = mmr.len().unwrap();
             let hash = mmr.get_merkle_root().unwrap();
             println!("Tree size: {size}. Root hash: {}", hash.to_hex());
         });
         time_function(&format!("MMR: Deleting {half_size} keys"), || {
             delete_from_mmr(0, u32::try_from(half_size).unwrap(), &mut mmr);
         });
         time_function("MMR: Calculating root hash", || {
-            let size = mmr.len();
+            let size = mmr.len().unwrap();
             let hash = mmr.get_merkle_root().unwrap();
             println!("Tree size: {size}. Root hash: {}", hash.to_hex());
         });
@@ -129,7 +129,7 @@ fn main() {
             );
         });
         time_function("MMR: Calculating root hash", || {
-            let size = mmr.len();
+            let size = mmr.len().unwrap();
             let hash = mmr.get_merkle_root().unwrap();
             println!("Tree size: {size}. Root hash: {}", hash.to_hex());
         });

base_layer/mmr/src/mutable_mmr.rs

@@ -62,17 +62,18 @@ where
     pub fn new(mmr_backend: B, deleted: Bitmap) -> Result<MutableMmr<D, B>, MerkleMountainRangeError> {
         let mmr = MerkleMountainRange::new(mmr_backend);
         Ok(MutableMmr {
-            size: u32::try_from(mmr.get_leaf_count()?).unwrap(),
+            size: u32::try_from(mmr.get_leaf_count()?).map_err(|_|MerkleMountainRangeError::InvalidMmrSize)?,
             mmr,
             deleted,
         })
     }
 
     /// Clear the MutableMmr and assign the MMR state from the set of leaf_hashes and deleted nodes given in `state`.
     pub fn assign(&mut self, state: MutableMmrLeafNodes) -> Result<(), MerkleMountainRangeError> {
+
         self.mmr.assign(state.leaf_hashes)?;
         self.deleted = state.deleted;
-        self.size = u32::try_from(self.mmr.get_leaf_count()?).unwrap();
+        self.size = u32::try_from(self.mmr.get_leaf_count()?).map_err(|_|MerkleMountainRangeError::InvalidMmrSize)?;
         Ok(())
     }
 
@@ -82,8 +83,10 @@ where
     /// nodes in the MMR, while this function returns the number of leaf nodes minus the number of nodes marked for
     /// deletion.
     #[allow(clippy::len_without_is_empty)]
-    pub fn len(&self) -> u32 {
-        self.size - u32::try_from(self.deleted.cardinality()).unwrap()
+    pub fn len(&self) -> Result<u32,MerkleMountainRangeError> {
+        let deleted_size = u32::try_from(self.deleted.cardinality()).map_err(|_|MerkleMountainRangeError::InvalidMmrSize)?;
+        let result = self.size.checked_sub(deleted_size).ok_or(MerkleMountainRangeError::InvalidMmrSize)?;
+        Ok(result )
     }
 
     /// Returns true if the the MMR contains no nodes, OR all nodes have been marked for deletion
@@ -209,13 +212,16 @@ where
     fn get_sub_bitmap(&self, leaf_index: LeafIndex, count: usize) -> Result<Bitmap, MerkleMountainRangeError> {
         let mut deleted = self.deleted.clone();
         if leaf_index.0 > 0 {
-            deleted.remove_range_closed(0..u32::try_from(leaf_index.0 - 1).unwrap())
+            let remove_range = leaf_index.0.checked_sub(1).ok_or(MerkleMountainRangeError::InvalidMmrSize)?;
+            deleted.remove_range(0..u32::try_from(remove_range).map_err(|_|MerkleMountainRangeError::InvalidMmrSize)?)
         }
         let leaf_count = self.mmr.get_leaf_count()?;
         if leaf_count > 1 {
-            let last_index = leaf_index.0 + count - 1;
+            let last_index = leaf_index.0.checked_add(count).ok_or(MerkleMountainRangeError::InvalidMmrSize)?.checked_sub(1).ok_or(MerkleMountainRangeError::InvalidMmrSize)?;
+            //Overflow not possible here as leaf_count will always be greater than 0, min > 1-1=0
             if last_index < leaf_count - 1 {
-                deleted.remove_range_closed(u32::try_from(last_index + 1).unwrap()..u32::try_from(leaf_count).unwrap());
+                let remove_range = last_index.checked_add(1).ok_or(MerkleMountainRangeError::InvalidMmrSize)?;
+                deleted.remove_range(u32::try_from(remove_range).map_err(|_|MerkleMountainRangeError::InvalidMmrSize)?..u32::try_from(leaf_count).map_err(|_|MerkleMountainRangeError::InvalidMmrSize)?);
             }
         }
         Ok(deleted)