Skip to content

Commit

Permalink
feat(core): Error instead of panic in fingerprint computation
Browse files Browse the repository at this point in the history
  • Loading branch information
PhilippGackstatter committed Oct 28, 2024
1 parent 19ae6bf commit 16475fb
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 20 deletions.
1 change: 1 addition & 0 deletions assembly/src/assembler/mast_forest_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`")
}
}

Expand Down
3 changes: 3 additions & 0 deletions core/src/mast/merger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions core/src/mast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
46 changes: 26 additions & 20 deletions core/src/mast/node_fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use miden_crypto::hash::{
};

use crate::{
mast::{DecoratorId, MastForest, MastNode, MastNodeId},
mast::{DecoratorId, MastForest, MastForestError, MastNode, MastNodeId},
Operation,
};

Expand Down Expand Up @@ -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<MastNodeId, MastNodeFingerprint>,
node: &MastNode,
) -> MastNodeFingerprint {
) -> Result<MastNodeFingerprint, MastForestError> {
match node {
MastNode::Block(node) => {
let mut bytes_to_hash = Vec::new();
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -158,35 +157,42 @@ fn fingerprint_from_parts(
after_exit_ids: &[DecoratorId],
children_ids: &[MastNodeId],
node_digest: RpoDigest,
) -> MastNodeFingerprint {
) -> Result<MastNodeFingerprint, MastForestError> {
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::<Result<Vec<DecoratorFingerprint>, 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<u8> = 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))
}
}

0 comments on commit 16475fb

Please sign in to comment.