From 16475fb99afce4772145c1cafcfcae437b90a946 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 28 Oct 2024 10:46:39 +0100 Subject: [PATCH] feat(core): Error instead of panic in fingerprint computation --- assembly/src/assembler/mast_forest_builder.rs | 1 + core/src/mast/merger/mod.rs | 3 ++ core/src/mast/mod.rs | 2 + core/src/mast/node_fingerprint.rs | 46 +++++++++++-------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/assembly/src/assembler/mast_forest_builder.rs b/assembly/src/assembler/mast_forest_builder.rs index 7e899c212..56de6f939 100644 --- a/assembly/src/assembler/mast_forest_builder.rs +++ b/assembly/src/assembler/mast_forest_builder.rs @@ -450,6 +450,7 @@ impl MastForestBuilder { impl MastForestBuilder { fn fingerprint_for_node(&self, node: &MastNode) -> MastNodeFingerprint { MastNodeFingerprint::from_mast_node(&self.mast_forest, &self.hash_by_node_id, node) + .expect("hash_by_node_id should contain the fingerprints of all children of `node`") } } diff --git a/core/src/mast/merger/mod.rs b/core/src/mast/merger/mod.rs index ae5e8cacc..ae0273371 100644 --- a/core/src/mast/merger/mod.rs +++ b/core/src/mast/merger/mod.rs @@ -186,6 +186,9 @@ impl MastForestMerger { &self.mast_forest, &self.hash_by_node_id, &remapped_node, + ) + .expect( + "hash_by_node_id should contain the fingerprints of all children of `remapped_node`", ); match self.lookup_node_by_fingerprint(&node_fingerprint) { diff --git a/core/src/mast/mod.rs b/core/src/mast/mod.rs index 062898da0..5185bc3cd 100644 --- a/core/src/mast/mod.rs +++ b/core/src/mast/mod.rs @@ -663,4 +663,6 @@ pub enum MastForestError { DecoratorIdOverflow(DecoratorId, usize), #[error("basic block cannot be created from an empty list of operations")] EmptyBasicBlock, + #[error("decorator root of child with node id {0} is missing but required for fingerprint computation")] + ChildFingerprintMissing(MastNodeId), } diff --git a/core/src/mast/node_fingerprint.rs b/core/src/mast/node_fingerprint.rs index 00b2d617f..6bf41a2ce 100644 --- a/core/src/mast/node_fingerprint.rs +++ b/core/src/mast/node_fingerprint.rs @@ -7,7 +7,7 @@ use miden_crypto::hash::{ }; use crate::{ - mast::{DecoratorId, MastForest, MastNode, MastNodeId}, + mast::{DecoratorId, MastForest, MastForestError, MastNode, MastNodeId}, Operation, }; @@ -46,15 +46,14 @@ impl MastNodeFingerprint { /// Creates a [`MastNodeFingerprint`] from a [`MastNode`]. /// - /// # Panics - /// /// The `hash_by_node_id` map must contain all children of the node for efficient lookup of - /// their fingerprints. This function panics if a child of the given `node` is not in this map. + /// their fingerprints. This function returns an error if a child of the given `node` is not in + /// this map. pub fn from_mast_node( forest: &MastForest, hash_by_node_id: &BTreeMap, node: &MastNode, - ) -> MastNodeFingerprint { + ) -> Result { match node { MastNode::Block(node) => { let mut bytes_to_hash = Vec::new(); @@ -85,10 +84,10 @@ impl MastNodeFingerprint { } if bytes_to_hash.is_empty() { - MastNodeFingerprint::new(node.digest()) + Ok(MastNodeFingerprint::new(node.digest())) } else { let decorator_root = Blake3_256::hash(&bytes_to_hash); - MastNodeFingerprint::with_decorator_root(node.digest(), decorator_root) + Ok(MastNodeFingerprint::with_decorator_root(node.digest(), decorator_root)) } }, MastNode::Join(node) => fingerprint_from_parts( @@ -158,35 +157,42 @@ fn fingerprint_from_parts( after_exit_ids: &[DecoratorId], children_ids: &[MastNodeId], node_digest: RpoDigest, -) -> MastNodeFingerprint { +) -> Result { let pre_decorator_hash_bytes = before_enter_ids.iter().flat_map(|&id| forest[id].fingerprint().as_bytes()); let post_decorator_hash_bytes = after_exit_ids.iter().flat_map(|&id| forest[id].fingerprint().as_bytes()); + let children_decorator_roots = children_ids + .iter() + .filter_map(|child_id| { + hash_by_node_id + .get(child_id) + .ok_or(MastForestError::ChildFingerprintMissing(*child_id)) + .map(|child_fingerprint| child_fingerprint.decorator_root) + .transpose() + }) + .collect::, MastForestError>>()?; + // Reminder: the `MastNodeFingerprint`'s decorator root will be `None` if and only if there are // no decorators attached to the node, and all children have no decorator roots (meaning // that there are no decorators in all the descendants). if pre_decorator_hash_bytes.clone().next().is_none() && post_decorator_hash_bytes.clone().next().is_none() - && children_ids - .iter() - .filter_map(|child_id| hash_by_node_id[child_id].decorator_root) - .next() - .is_none() + && children_decorator_roots.is_empty() { - MastNodeFingerprint::new(node_digest) + Ok(MastNodeFingerprint::new(node_digest)) } else { - let children_decorator_roots = children_ids - .iter() - .filter_map(|child_id| hash_by_node_id[child_id].decorator_root) - .flat_map(|decorator_root| decorator_root.as_bytes()); let decorator_bytes_to_hash: Vec = pre_decorator_hash_bytes .chain(post_decorator_hash_bytes) - .chain(children_decorator_roots) + .chain( + children_decorator_roots + .into_iter() + .flat_map(|decorator_root| decorator_root.as_bytes()), + ) .collect(); let decorator_root = Blake3_256::hash(&decorator_bytes_to_hash); - MastNodeFingerprint::with_decorator_root(node_digest, decorator_root) + Ok(MastNodeFingerprint::with_decorator_root(node_digest, decorator_root)) } }