fix!: monero merkle tree params (#6336)

Description --- Changes how monero merge mined params are encoded and decoded Motivation and Context --- Current ones are not coded exactly correct How Has This Been Tested? --- Unit tests
tari-project · May 14, 2024 · 9920916 · 9920916
1 parent 808dcfb
commit 9920916
Show file tree

Hide file tree

Showing 4 changed files with 273 additions and 116 deletions.
diff --git a/applications/minotari_node/src/grpc/base_node_grpc_server.rs b/applications/minotari_node/src/grpc/base_node_grpc_server.rs
@@ -235,10 +235,12 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
         let (start_height, end_height) = get_heights(&request, handler.clone())
             .await
             .map_err(|e| obscure_error_if_true(report_error_flag, e))?;
-        let num_requested = end_height.checked_sub(start_height).ok_or(obscure_error_if_true(
-            report_error_flag,
-            Status::invalid_argument("Start height is more than end height"),
-        ))?;
+        let num_requested = end_height.checked_sub(start_height).ok_or_else(|| {
+            obscure_error_if_true(
+                report_error_flag,
+                Status::invalid_argument("Start height is more than end height"),
+            )
+        })?;
         if num_requested > GET_DIFFICULTY_MAX_HEIGHTS {
             return Err(obscure_error_if_true(
                 report_error_flag,
@@ -810,10 +812,9 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
             coinbase.value = u64::try_from(
                 (cur_share_sum.saturating_mul(reward))
                     .checked_div(total_shares)
-                    .ok_or(obscure_error_if_true(
-                        report_error_flag,
-                        Status::internal("total shares are zero".to_string()),
-                    ))? -
+                    .ok_or_else(|| {
+                        obscure_error_if_true(report_error_flag, Status::internal("total shares are zero".to_string()))
+                    })? -
                     prev_coinbase_value,
             )
             .map_err(|_| {
@@ -981,10 +982,12 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
         debug!(target: LOG_TARGET, "Incoming GRPC request for get new block with coinbases");
         let mut block_template: NewBlockTemplate = request
             .new_template
-            .ok_or(obscure_error_if_true(
-                report_error_flag,
-                Status::invalid_argument("Malformed block template provided".to_string()),
-            ))?
+            .ok_or_else(|| {
+                obscure_error_if_true(
+                    report_error_flag,
+                    Status::invalid_argument("Malformed block template provided".to_string()),
+                )
+            })?
             .try_into()
             .map_err(|s| {
                 obscure_error_if_true(

diff --git a/base_layer/core/src/proof_of_work/monero_rx/error.rs b/base_layer/core/src/proof_of_work/monero_rx/error.rs
@@ -24,7 +24,11 @@ use tari_utilities::hex::HexError;
 
 use crate::{
     common::{BanPeriod, BanReason},
-    proof_of_work::{randomx_factory::RandomXVMFactoryError, DifficultyError},
+    proof_of_work::{
+        monero_rx::merkle_tree_parameters::MerkleTreeParametersError,
+        randomx_factory::RandomXVMFactoryError,
+        DifficultyError,
+    },
 };
 
 /// Errors that can occur when merging Monero PoW data with Tari PoW data
@@ -50,6 +54,8 @@ pub enum MergeMineError {
     DifficultyError(#[from] DifficultyError),
     #[error("Cannot mine with 0 aux chains")]
     ZeroAuxChains,
+    #[error("Merkle Tree Parameters error: {0}")]
+    MerkleTreeParamsError(#[from] MerkleTreeParametersError),
 }
 
 impl MergeMineError {
@@ -66,7 +72,9 @@ impl MergeMineError {
                 reason: err.to_string(),
                 ban_duration: BanPeriod::Long,
             }),
-            MergeMineError::RandomXVMFactoryError(_) | MergeMineError::ZeroAuxChains => None,
+            MergeMineError::RandomXVMFactoryError(_) |
+            MergeMineError::ZeroAuxChains |
+            MergeMineError::MerkleTreeParamsError(_) => None,
         }
     }
 }

diff --git a/base_layer/core/src/proof_of_work/monero_rx/helpers.rs b/base_layer/core/src/proof_of_work/monero_rx/helpers.rs
@@ -162,18 +162,18 @@ fn check_aux_chains(
         }
     }
     let merkle_tree_params = MerkleTreeParameters::from_varint(merge_mining_params);
-    if merkle_tree_params.number_of_chains == 0 {
+    if merkle_tree_params.number_of_chains() == 0 {
         return false;
     }
     let hash_position = U256::from_little_endian(
         &Sha256::new()
             .chain_update(tari_genesis_block_hash)
-            .chain_update(merkle_tree_params.aux_nonce.to_le_bytes())
+            .chain_update(merkle_tree_params.aux_nonce().to_le_bytes())
             .chain_update((109_u8).to_le_bytes())
             .finalize(),
     )
     .low_u32() %
-        u32::from(merkle_tree_params.number_of_chains);
+        u32::from(merkle_tree_params.number_of_chains());
     let (merkle_root, pos) = monero_data.aux_chain_merkle_proof.calculate_root_with_pos(&t_hash);
     if hash_position != pos {
         return false;
@@ -342,10 +342,7 @@ pub fn insert_aux_chain_mr_and_info_into_block<T: AsRef<[u8]>>(
     let encoded = if aux_chain_count == 1 {
         VarInt(0)
     } else {
-        let mt_params = MerkleTreeParameters {
-            number_of_chains: aux_chain_count,
-            aux_nonce,
-        };
+        let mt_params = MerkleTreeParameters::new(aux_chain_count, aux_nonce)?;
         mt_params.to_varint()
     };
     extra_field.0.insert(0, SubField::MergeMining(encoded, hash));