Skip to content

Commit

Permalink
feat(core): Revert MastNodeEq rename
Browse files Browse the repository at this point in the history
  • Loading branch information
PhilippGackstatter committed Oct 16, 2024
1 parent 5c8c7be commit 8cf803e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 27 deletions.
10 changes: 5 additions & 5 deletions assembly/src/assembler/mast_forest_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use core::ops::{Index, IndexMut};

use vm_core::{
crypto::hash::{Blake3Digest, RpoDigest},
mast::{DecoratorId, MastForest, MastNode, MastNodeEq, MastNodeId},
mast::{DecoratorId, EqHash, MastForest, MastNode, MastNodeId},
Decorator, DecoratorList, Operation,
};

Expand Down Expand Up @@ -47,10 +47,10 @@ pub struct MastForestBuilder {
/// root.
proc_gid_by_mast_root: BTreeMap<RpoDigest, GlobalProcedureIndex>,
/// A map of MAST node eq hashes to their corresponding positions in the MAST forest.
node_id_by_hash: BTreeMap<MastNodeEq, MastNodeId>,
node_id_by_hash: BTreeMap<EqHash, MastNodeId>,
/// The reverse mapping of `node_id_by_hash`. This map caches the eq hashes of all nodes (for
/// performance reasons).
hash_by_node_id: BTreeMap<MastNodeId, MastNodeEq>,
hash_by_node_id: BTreeMap<MastNodeId, EqHash>,
/// A map of decorator hashes to their corresponding positions in the MAST forest.
decorator_id_by_hash: BTreeMap<Blake3Digest<32>, DecoratorId>,
/// A set of IDs for basic blocks which have been merged into a bigger basic blocks. This is
Expand Down Expand Up @@ -446,8 +446,8 @@ impl MastForestBuilder {
}

impl MastForestBuilder {
fn eq_hash_for_node(&self, node: &MastNode) -> MastNodeEq {
MastNodeEq::from_mast_node(&self.mast_forest, &self.hash_by_node_id, node)
fn eq_hash_for_node(&self, node: &MastNode) -> EqHash {
EqHash::from_mast_node(&self.mast_forest, &self.hash_by_node_id, node)
}
}

Expand Down
16 changes: 8 additions & 8 deletions core/src/mast/merger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use alloc::{collections::BTreeMap, vec::Vec};
use miden_crypto::hash::blake::Blake3Digest;

use crate::mast::{
DecoratorId, MastForest, MastForestDfsIter, MastForestError, MastNode, MastNodeEq, MastNodeId,
DecoratorId, EqHash, MastForest, MastForestDfsIter, MastForestError, MastNode, MastNodeId,
};

/// A type that allows merging [`MastForest`]s.
Expand Down Expand Up @@ -39,8 +39,8 @@ use crate::mast::{
/// - [`MastForest::merge_multiple`]
pub(crate) struct MastForestMerger<'forest> {
forest: &'forest mut MastForest,
node_id_by_hash: BTreeMap<MastNodeEq, MastForestIndexEntry>,
hash_by_node_id: BTreeMap<MastNodeId, MastNodeEq>,
node_id_by_hash: BTreeMap<EqHash, MastForestIndexEntry>,
hash_by_node_id: BTreeMap<MastNodeId, EqHash>,
decorators_by_hash: BTreeMap<Blake3Digest<32>, DecoratorId>,
}

Expand Down Expand Up @@ -84,9 +84,9 @@ impl<'forest> MastForestMerger<'forest> {
}

for (merging_id, node) in MastForestDfsIter::new(other_forest) {
// We need to remap the node prior to computing the MastNodeEq.
// We need to remap the node prior to computing the EqHash.
//
// This is because the MastNodeEq computation looks up its descendants and decorators in
// This is because the EqHash computation looks up its descendants and decorators in
// the internal index, and if we were to pass the original node to that
// computation, it would look up the incorrect descendants and decorators.
//
Expand All @@ -98,7 +98,7 @@ impl<'forest> MastForestMerger<'forest> {
Self::remap_node(node, &decorator_id_remapping, &node_id_remapping, self.forest);

let node_eq =
MastNodeEq::from_mast_node(self.forest, &self.hash_by_node_id, &remapped_node);
EqHash::from_mast_node(self.forest, &self.hash_by_node_id, &remapped_node);

match self.node_id_by_hash.get_mut(&node_eq) {
Some(existing_entry) => {
Expand Down Expand Up @@ -127,7 +127,7 @@ impl<'forest> MastForestMerger<'forest> {
node_id_remapping.insert(merging_id, new_node_id);

// We need to update the indices with the newly inserted nodes
// since the MastNodeEq computation requires all descendants of a node
// since the EqHash computation requires all descendants of a node
// to be in this index. Hence when we encounter a node in the merging forest
// which has descendants (Call, Loop, Split, ...), then those need to be in the
// indices.
Expand Down Expand Up @@ -222,7 +222,7 @@ impl<'forest> MastForestMerger<'forest> {
/// Builds the index of nodes and decorators of the contained forest.
fn build_index(&mut self) {
for (id, node) in MastForestDfsIter::new(self.forest) {
let node_eq = MastNodeEq::from_mast_node(self.forest, &self.hash_by_node_id, node);
let node_eq = EqHash::from_mast_node(self.forest, &self.hash_by_node_id, node);
self.hash_by_node_id.insert(id, node_eq);
self.node_id_by_hash.insert(
node_eq,
Expand Down
24 changes: 10 additions & 14 deletions core/src/mast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,23 +628,19 @@ impl Serializable for DecoratorId {
// MAST NODE EQUALITY
// ================================================================================================

// TODO: We need to export this so we can use it in MastForestBuilder, but do we really want this to
// be public? Alternatively we can move move MastForestBuilder into core as well, but this might not
// be a good fit since it is only used during assembly.

/// Represents the hash used to test for equality between [`MastNode`]s.
///
/// The 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).
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct MastNodeEq {
pub struct EqHash {
mast_root: RpoDigest,
decorator_root: Option<Blake3Digest<32>>,
}

// TODO: Document public functions and assumptions about forest and the index map.
impl MastNodeEq {
impl EqHash {
pub fn new(mast_root: RpoDigest) -> Self {
Self { mast_root, decorator_root: None }
}
Expand All @@ -658,9 +654,9 @@ impl MastNodeEq {

pub fn from_mast_node(
forest: &MastForest,
hash_by_node_id: &BTreeMap<MastNodeId, MastNodeEq>,
hash_by_node_id: &BTreeMap<MastNodeId, EqHash>,
node: &MastNode,
) -> MastNodeEq {
) -> EqHash {
match node {
MastNode::Block(node) => {
let mut bytes_to_hash = Vec::new();
Expand Down Expand Up @@ -691,10 +687,10 @@ impl MastNodeEq {
}

if bytes_to_hash.is_empty() {
MastNodeEq::new(node.digest())
EqHash::new(node.digest())
} else {
let decorator_root = Blake3_256::hash(&bytes_to_hash);
MastNodeEq::with_decorator_root(node.digest(), decorator_root)
EqHash::with_decorator_root(node.digest(), decorator_root)
}
},
MastNode::Join(node) => eq_hash_from_parts(
Expand Down Expand Up @@ -751,12 +747,12 @@ impl MastNodeEq {

fn eq_hash_from_parts(
forest: &MastForest,
hash_by_node_id: &BTreeMap<MastNodeId, MastNodeEq>,
hash_by_node_id: &BTreeMap<MastNodeId, EqHash>,
before_enter_ids: &[DecoratorId],
after_exit_ids: &[DecoratorId],
children_ids: &[MastNodeId],
node_digest: RpoDigest,
) -> MastNodeEq {
) -> EqHash {
let pre_decorator_hash_bytes =
before_enter_ids.iter().flat_map(|&id| forest[id].eq_hash().as_bytes());
let post_decorator_hash_bytes =
Expand All @@ -773,7 +769,7 @@ fn eq_hash_from_parts(
.next()
.is_none()
{
MastNodeEq::new(node_digest)
EqHash::new(node_digest)
} else {
let children_decorator_roots = children_ids
.iter()
Expand All @@ -785,7 +781,7 @@ fn eq_hash_from_parts(
.collect();

let decorator_root = Blake3_256::hash(&decorator_bytes_to_hash);
MastNodeEq::with_decorator_root(node_digest, decorator_root)
EqHash::with_decorator_root(node_digest, decorator_root)
}
}

Expand Down

0 comments on commit 8cf803e

Please sign in to comment.