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

@@ -121,15 +121,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,37 @@ 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>> {
+        // 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 <= N::MAX_FUNCTIONS,
+            "The fee index ('{fee_index}') in the transaction tree must be less than {}",
+            N::MAX_FUNCTIONS
+        );
+        // 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
+        );
+
+        // If a fee is provided, append the fee leaf to the transaction tree.
+        if let Some(fee) = fee {
+            // 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>> {
         // Use the V1 or V2 deployment tree based on whether or not the program checksum exists.
@@ -137,29 +162,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.
@@ -178,20 +186,28 @@ impl<N: Network> Transaction<N> {
         let functions = program.functions();
         // Retrieve the verifying keys.
         let verifying_keys = deployment.verifying_keys();
+        // Retrieve the number of functions.
+        let num_functions = functions.len();
 
         // Ensure the number of functions and verifying keys match.
         ensure!(
-            functions.len() == verifying_keys.len(),
-            "Number of functions ('{}') and verifying keys ('{}') do not match",
-            functions.len(),
+            num_functions == verifying_keys.len(),
+            "Number of functions ('{num_functions}') and verifying keys ('{}') do not match",
             verifying_keys.len()
         );
+        // Ensure there are functions.
+        ensure!(num_functions > 0, "Deployment must contain at least one function");
+        // Ensure the number of functions is within the allowed range.
+        ensure!(
+            num_functions <= N::MAX_FUNCTIONS,
+            "Deployment must contain at most {} functions, found {num_functions}",
+            N::MAX_FUNCTIONS,
+        );
         // Ensure the number of functions is within the allowed range.
         ensure!(
-            functions.len() < Self::MAX_TRANSITIONS, // Note: Observe we hold back 1 for the fee.
-            "Deployment must contain less than {} functions, found {}",
+            num_functions < Self::MAX_TRANSITIONS, // Note: Observe we hold back 1 for the fee.
+            "Deployment must contain less than {} functions, found {num_functions}",
             Self::MAX_TRANSITIONS,
-            functions.len()
         );
         Ok(())
     }
@@ -218,22 +234,16 @@ impl<N: Network> Transaction<N> {
         // Prepare the header for the hash.
         let header = deployment.program().id().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 V2 deployment tree.
@@ -246,21 +256,36 @@ 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<_>>>()?)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    type CurrentNetwork = console::network::MainnetV0;
+
+    #[test]
+    fn test_transaction_depth_is_correct() {
+        // We ensure 2^TRANSACTION_DEPTH == MAX_FUNCTIONS + 1.
+        // The "1 extra" is for the fee transition.
+        assert_eq!(
+            2u32.checked_pow(TRANSACTION_DEPTH as u32).unwrap() as usize,
+            Transaction::<CurrentNetwork>::MAX_TRANSITIONS
+        );
+        assert_eq!(
+            CurrentNetwork::MAX_FUNCTIONS.checked_add(1).unwrap(),
+            Transaction::<CurrentNetwork>::MAX_TRANSITIONS
+        );
     }
 }

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())
     }
 }
 

ledger/store/src/helpers/memory/internal/map.rs

@@ -33,7 +33,7 @@ use std::{
 #[derive(Clone)]
 pub struct MemoryMap<
     K: Copy + Clone + PartialEq + Eq + Hash + Serialize + for<'de> Deserialize<'de> + Send + Sync,
-    V: Clone + PartialEq + Eq + Serialize + for<'de> Deserialize<'de> + Send + Sync,
+    V: Clone + Serialize + for<'de> Deserialize<'de> + Send + Sync,
 > {
     // The reason for using BTreeMap with binary keys is for the order of items to be the same as
     // the one in the RocksDB-backed DataMap; if not for that, it could be any map
@@ -46,7 +46,7 @@ pub struct MemoryMap<
 
 impl<
     K: Copy + Clone + PartialEq + Eq + Hash + Serialize + for<'de> Deserialize<'de> + Send + Sync,
-    V: Clone + PartialEq + Eq + Serialize + for<'de> Deserialize<'de> + Send + Sync,
+    V: Clone + Serialize + for<'de> Deserialize<'de> + Send + Sync,
 > Default for MemoryMap<K, V>
 {
     fn default() -> Self {
@@ -61,7 +61,7 @@ impl<
 
 impl<
     K: Copy + Clone + PartialEq + Eq + Hash + Serialize + for<'de> Deserialize<'de> + Send + Sync,
-    V: Clone + PartialEq + Eq + Serialize + for<'de> Deserialize<'de> + Send + Sync,
+    V: Clone + Serialize + for<'de> Deserialize<'de> + Send + Sync,
 > FromIterator<(K, V)> for MemoryMap<K, V>
 {
     /// Initializes a new `MemoryMap` from the given iterator.
@@ -82,7 +82,7 @@ impl<
 impl<
     'a,
     K: 'a + Copy + Clone + PartialEq + Eq + Hash + Serialize + for<'de> Deserialize<'de> + Send + Sync,
-    V: 'a + Clone + PartialEq + Eq + Serialize + for<'de> Deserialize<'de> + Send + Sync,
+    V: 'a + Clone + Serialize + for<'de> Deserialize<'de> + Send + Sync,
 > Map<'a, K, V> for MemoryMap<K, V>
 {
     ///
@@ -253,7 +253,7 @@ impl<
 impl<
     'a,
     K: 'a + Copy + Clone + PartialEq + Eq + Hash + Serialize + for<'de> Deserialize<'de> + Send + Sync,
-    V: 'a + Clone + PartialEq + Eq + Serialize + for<'de> Deserialize<'de> + Send + Sync,
+    V: 'a + Clone + Serialize + for<'de> Deserialize<'de> + Send + Sync,
 > MapRead<'a, K, V> for MemoryMap<K, V>
 {
     type Iterator = core::iter::Map<btree_map::IntoIter<Vec<u8>, V>, fn((Vec<u8>, V)) -> (Cow<'a, K>, Cow<'a, V>)>;
@@ -371,7 +371,7 @@ impl<
 
 impl<
     K: Copy + Clone + PartialEq + Eq + Hash + Serialize + for<'de> Deserialize<'de> + Send + Sync,
-    V: Clone + PartialEq + Eq + Serialize + for<'de> Deserialize<'de> + Send + Sync,
+    V: Clone + Serialize + for<'de> Deserialize<'de> + Send + Sync,
 > Deref for MemoryMap<K, V>
 {
     type Target = Arc<RwLock<BTreeMap<Vec<u8>, V>>>;