Improves safety & readability on transaction root derivation logic

console/program/src/state_path/configuration/mod.rs

@@ -57,15 +57,16 @@ pub type TransactionsTree<N> = BHPMerkleTree<N, TRANSACTIONS_DEPTH>;
 /// The Merkle path for a transaction in a block.
 pub type TransactionsPath<N> = MerklePath<N, TRANSACTIONS_DEPTH>;
 
-/// The Merkle tree for the execution.
-pub type ExecutionTree<N> = BHPMerkleTree<N, TRANSACTION_DEPTH>;
-/// The Merkle tree for the deployment.
-pub type DeploymentTree<N> = BHPMerkleTree<N, TRANSACTION_DEPTH>;
 /// The Merkle tree for the transaction.
 pub type TransactionTree<N> = BHPMerkleTree<N, TRANSACTION_DEPTH>;
 /// The Merkle path for a function or transition in the transaction.
 pub type TransactionPath<N> = MerklePath<N, TRANSACTION_DEPTH>;
 
+/// The Merkle tree for the execution.
+pub type ExecutionTree<N> = BHPMerkleTree<N, TRANSACTION_DEPTH>;
+/// The Merkle tree for the deployment.
+pub type DeploymentTree<N> = BHPMerkleTree<N, TRANSACTION_DEPTH>;
+
 /// The Merkle tree for the transition.
 pub type TransitionTree<N> = BHPMerkleTree<N, TRANSITION_DEPTH>;
 /// The Merkle path for an input or output ID in the transition.

ledger/block/src/transaction/bytes.rs

@@ -96,7 +96,8 @@ impl<N: Network> ToBytes for Transaction<N> {
         1u8.write_le(&mut writer)?;
 
         // Write the transaction.
-        // We don't write the deployment or execution id, which are recomputed when creating the transaction.
+        // Note: We purposefully do not write out the deployment or execution ID,
+        // and instead recompute it when reconstructing the transaction, to ensure there was no malleability.
         match self {
             Self::Deploy(id, _, owner, deployment, fee) => {
                 // Write the variant.

ledger/block/src/transaction/deployment/mod.rs

@@ -113,15 +113,10 @@ impl<N: Network> Deployment<N> {
     }
 
     /// Returns the number of program functions in the deployment.
-    pub fn len(&self) -> usize {
+    pub fn num_functions(&self) -> usize {
         self.program.functions().len()
     }
 
-    /// Returns `true` if the deployment is empty.
-    pub fn is_empty(&self) -> bool {
-        self.program.functions().is_empty()
-    }
-
     /// Returns the edition.
     pub const fn edition(&self) -> u16 {
         self.edition

ledger/block/src/transaction/merkle.rs

@@ -93,17 +93,11 @@ impl<N: Network> Transaction<N> {
         match self {
             // Compute the deployment tree.
             Transaction::Deploy(_, _, _, deployment, fee) => {
-                let deployment_tree = Self::deployment_tree(deployment)?;
-                Self::transaction_tree(deployment_tree, deployment.len(), fee)
+                Self::transaction_tree(Self::deployment_tree(deployment)?, Some(fee))
             }
             // Compute the execution tree.
             Transaction::Execute(_, _, execution, fee) => {
-                let execution_tree = Self::execution_tree(execution)?;
-                if let Some(fee) = fee {
-                    Ok(Transaction::transaction_tree(execution_tree, execution.len(), fee)?)
-                } else {
-                    Ok(execution_tree)
-                }
+                Self::transaction_tree(Self::execution_tree(execution)?, fee.as_ref())
             }
             // Compute the fee tree.
             Transaction::Fee(_, fee) => Self::fee_tree(fee),
@@ -112,6 +106,30 @@ impl<N: Network> Transaction<N> {
 }
 
 impl<N: Network> Transaction<N> {
+    /// Returns the Merkle tree for the given transaction tree, fee index, and fee.
+    pub fn transaction_tree(
+        mut deployment_or_execution_tree: TransactionTree<N>,
+        fee: Option<&Fee<N>>,
+    ) -> Result<TransactionTree<N>> {
+        // If a fee is provided, append the fee leaf to the transaction tree.
+        if let Some(fee) = fee {
+            // Retrieve the fee index, defined as the last index in the transaction tree.
+            let fee_index = deployment_or_execution_tree.number_of_leaves();
+            // Ensure the fee index is within the Merkle tree size.
+            ensure!(
+                fee_index < Self::MAX_TRANSITIONS,
+                "The fee index ('{fee_index}') in the transaction tree must be less than {}",
+                Self::MAX_TRANSITIONS
+            );
+            // Construct the transaction leaf.
+            let leaf = TransactionLeaf::new_fee(u16::try_from(fee_index)?, **fee.transition_id()).to_bits_le();
+            // Append the fee leaf to the transaction tree.
+            deployment_or_execution_tree.append(&[leaf])?;
+        }
+        // Return the transaction tree.
+        Ok(deployment_or_execution_tree)
+    }
+
     /// Returns the Merkle tree for the given deployment.
     pub fn deployment_tree(deployment: &Deployment<N>) -> Result<DeploymentTree<N>> {
         // Ensure the number of leaves is within the Merkle tree size.
@@ -123,22 +141,16 @@ impl<N: Network> Transaction<N> {
             Some(program_checksum) => program_checksum.to_bits_le(),
         };
         // Prepare the leaves.
-        let leaves = deployment
-            .program()
-            .functions()
-            .values()
-            .enumerate()
-            .map(|(index, function)| {
-                // Construct the transaction leaf.
-                Ok(TransactionLeaf::new_deployment(
-                    u16::try_from(index)?,
-                    N::hash_bhp1024(&to_bits_le![header, function.to_bytes_le()?])?,
-                )
-                .to_bits_le())
-            })
-            .collect::<Result<Vec<_>>>()?;
+        let leaves = deployment.program().functions().values().enumerate().map(|(index, function)| {
+            // Construct the transaction leaf.
+            Ok(TransactionLeaf::new_deployment(
+                u16::try_from(index)?,
+                N::hash_bhp1024(&to_bits_le![header, function.to_bytes_le()?])?,
+            )
+            .to_bits_le())
+        });
         // Compute the deployment tree.
-        N::merkle_tree_bhp::<TRANSACTION_DEPTH>(&leaves)
+        N::merkle_tree_bhp::<TRANSACTION_DEPTH>(&leaves.collect::<Result<Vec<_>>>()?)
     }
 
     /// Returns the Merkle tree for the given execution.
@@ -155,29 +167,12 @@ impl<N: Network> Transaction<N> {
         // Ensure the number of leaves is within the Merkle tree size.
         Self::check_execution_size(num_transitions)?;
         // Prepare the leaves.
-        let leaves = transitions
-            .enumerate()
-            .map(|(index, transition)| {
-                // Construct the transaction leaf.
-                Ok::<_, Error>(TransactionLeaf::new_execution(u16::try_from(index)?, **transition.id()).to_bits_le())
-            })
-            .collect::<Result<Vec<_>, _>>()?;
+        let leaves = transitions.enumerate().map(|(index, transition)| {
+            // Construct the transaction leaf.
+            Ok::<_, Error>(TransactionLeaf::new_execution(u16::try_from(index)?, **transition.id()).to_bits_le())
+        });
         // Compute the execution tree.
-        N::merkle_tree_bhp::<TRANSACTION_DEPTH>(&leaves)
-    }
-
-    /// Returns the Merkle tree for the given 1. transaction or deployment tree and 2. fee.
-    pub fn transaction_tree(
-        mut deployment_or_execution_tree: TransactionTree<N>,
-        fee_index: usize,
-        fee: &Fee<N>,
-    ) -> Result<TransactionTree<N>> {
-        // Construct the transaction leaf.
-        let leaf = TransactionLeaf::new_fee(u16::try_from(fee_index)?, **fee.transition_id()).to_bits_le();
-        // Compute the updated transaction tree.
-        deployment_or_execution_tree.append(&[leaf])?;
-
-        Ok(deployment_or_execution_tree)
+        N::merkle_tree_bhp::<TRANSACTION_DEPTH>(&leaves.collect::<Result<Vec<_>, _>>()?)
     }
 
     /// Returns the Merkle tree for the given fee.

ledger/block/src/transaction/mod.rs

@@ -67,7 +67,7 @@ impl<N: Network> Transaction<N> {
         // Compute the deployment ID.
         let deployment_id = *deployment_tree.root();
         // Compute the transaction ID
-        let transaction_id = *Self::transaction_tree(deployment_tree, deployment.len(), &fee)?.root();
+        let transaction_id = *Self::transaction_tree(deployment_tree, Some(&fee))?.root();
         // Ensure the owner signed the correct transaction ID.
         ensure!(owner.verify(deployment_id), "Attempted to create a deployment transaction with an invalid owner");
         // Construct the deployment transaction.
@@ -82,14 +82,8 @@ impl<N: Network> Transaction<N> {
         let execution_tree = Self::execution_tree(&execution)?;
         // Compute the execution ID.
         let execution_id = *execution_tree.root();
-        // Compute the transaction ID
-        let transaction_id = match &fee {
-            Some(fee) => {
-                // Compute the root of the transaction tree.
-                *Self::transaction_tree(execution_tree, execution.len(), fee)?.root()
-            }
-            None => execution_id,
-        };
+        // Compute the transaction ID.
+        let transaction_id = *Self::transaction_tree(execution_tree, fee.as_ref())?.root();
         // Construct the execution transaction.
         Ok(Self::Execute(transaction_id.into(), execution_id, Box::new(execution), fee))
     }

ledger/block/src/transaction/serialize.rs

@@ -18,8 +18,9 @@ use super::*;
 impl<N: Network> Serialize for Transaction<N> {
     /// Serializes the transaction to a JSON-string or buffer.
     fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        // Note: We purposefully do not write out the deployment or execution ID,
+        // and instead recompute it when reconstructing the transaction, to ensure there was no malleability.
         match serializer.is_human_readable() {
-            // We don't write the deployment or execution id, which are recomputed when creating the Transaction.
             true => match self {
                 Self::Deploy(id, _, owner, deployment, fee) => {
                     let mut transaction = serializer.serialize_struct("Transaction", 5)?;

ledger/block/src/transactions/rejected/mod.rs

@@ -85,15 +85,13 @@ impl<N: Network> Rejected<N> {
     /// When a transaction is rejected, its fee transition is used to construct the confirmed transaction ID,
     /// changing the original transaction ID.
     pub fn to_unconfirmed_id(&self, fee: &Option<Fee<N>>) -> Result<Field<N>> {
-        let (tree, fee_index) = match self {
-            Self::Deployment(_, deployment) => (Transaction::deployment_tree(deployment)?, deployment.len()),
-            Self::Execution(execution) => (Transaction::execution_tree(execution)?, execution.len()),
+        // Compute the deployment or execution tree.
+        let tree = match self {
+            Self::Deployment(_, deployment) => Transaction::deployment_tree(deployment)?,
+            Self::Execution(execution) => Transaction::execution_tree(execution)?,
         };
-        if let Some(fee) = fee {
-            Ok(*Transaction::transaction_tree(tree, fee_index, fee)?.root())
-        } else {
-            Ok(*tree.root())
-        }
+        // Construct the transaction tree and return the unconfirmed transaction ID.
+        Ok(*Transaction::transaction_tree(tree, fee.as_ref())?.root())
     }
 }