arnaucube
arnaucube/miden-cryptoPublic Self-hosted repository
bugfix: fix internal nodes of for empty leafs of a SMT

The path returned by `EmptySubtreeRoots` starts at the root, and goes to
the leaf. The MerkleStore constructor assumed the other direction, so
the parent/child hashes were reversed.

This fixes the bug and adds a test.
Augusto F. Hack hack.augusto@gmail.com
Fri, 24 Mar 2023 20:22:55 +0100
commit

9e6c8ff70030336940ac551988b36e3200d19941

parent

854892ba9d809477f7227d92bbb607626ada14e1

2 files changed, 80 insertions(+), 10 deletions(-)

jump to
M src/merkle/store/mod.rs → src/merkle/store/mod.rs

@@ -43,7 +43,7 @@ /// # let ROOT1 = T1.root();

/// let mut store = MerkleStore::new(); /// /// // the store is initialized with the SMT empty nodes -/// assert_eq!(store.num_internal_nodes(), 64); +/// assert_eq!(store.num_internal_nodes(), 255); /// /// // populates the store with two merkle trees, common nodes are shared /// store.add_merkle_tree([A, B, C, D, E, F, G, H0]);

@@ -65,7 +65,7 @@ /// }

/// /// // Common internal nodes are shared, the two added trees have a total of 30, but the store has /// // only 10 new entries, corresponding to the 10 unique internal nodes of these trees. -/// assert_eq!(store.num_internal_nodes() - 64, 10); +/// assert_eq!(store.num_internal_nodes() - 255, 10); /// ``` #[derive(Debug, Clone, Eq, PartialEq)] pub struct MerkleStore {

@@ -85,11 +85,12 @@

/// Creates an empty `MerkleStore` instance. pub fn new() -> MerkleStore { // pre-populate the store with the empty hashes - let subtrees = EmptySubtreeRoots::empty_hashes(64); + let subtrees = EmptySubtreeRoots::empty_hashes(255); let nodes = subtrees .iter() + .rev() .copied() - .zip(subtrees.iter().skip(1).copied()) + .zip(subtrees.iter().rev().skip(1).copied()) .map(|(child, parent)| { ( parent,
M src/merkle/store/tests.rs → src/merkle/store/tests.rs

@@ -2,7 +2,7 @@ use super::*;

use crate::{ hash::rpo::Rpo256, merkle::{int_to_node, MerklePathSet}, - Felt, Word, + Felt, Word, WORD_SIZE, ZERO, }; const KEYS4: [u64; 4] = [0, 1, 2, 3];

@@ -12,6 +12,7 @@ int_to_node(2),

int_to_node(3), int_to_node(4), ]; +const EMPTY: Word = [ZERO; WORD_SIZE]; #[test] fn test_root_not_in_store() -> Result<(), MerkleError> {

@@ -142,6 +143,51 @@ Ok(())

} #[test] +fn test_empty_roots() { + let store = MerkleStore::default(); + let mut root = RpoDigest::new(EMPTY); + + for depth in 0..255 { + root = Rpo256::merge(&[root; 2]); + assert!( + store.get_node(root.into(), NodeIndex::new(0, 0)).is_ok(), + "The root of the empty tree of depth {depth} must be registered" + ); + } +} + +#[test] +fn test_leaf_paths_for_empty_trees() -> Result<(), MerkleError> { + let store = MerkleStore::default(); + + // Starts at 1 because leafs are not included in the store. + // Ends at 64 because it is not possible to represent an index of a depth greater than 64, + // because a u64 is used to index the leaf. + for depth in 1..64 { + let smt = SimpleSmt::new(depth)?; + + let index = NodeIndex::new(depth, 0); + let store_path = store.get_path(smt.root(), index)?; + let smt_path = smt.get_path(index)?; + assert_eq!( + store_path.value, EMPTY, + "the leaf of an empty tree is always ZERO" + ); + assert_eq!( + store_path.path, smt_path, + "the returned merkle path does not match the computed values" + ); + assert_eq!( + store_path.path.compute_root(depth.into(), EMPTY), + smt.root(), + "computed root from the path must match the empty tree root" + ); + } + + Ok(()) +} + +#[test] fn test_get_invalid_node() { let mut store = MerkleStore::default(); let mtree = MerkleTree::new(LEAVES4.to_vec()).expect("creating a merkle tree must work");

@@ -211,6 +257,11 @@ store.get_node(smt.root(), NodeIndex::new(smt.depth(), 3)),

Ok(LEAVES4[3]), "node 3 must be in the tree" ); + assert_eq!( + store.get_node(smt.root(), NodeIndex::new(smt.depth(), 4)), + Ok(EMPTY), + "unmodified node 4 must be ZERO" + ); // STORE LEAVES MATCH TREE =============================================================== // sanity check the values returned by the store and the tree

@@ -234,6 +285,11 @@ smt.get_node(&NodeIndex::new(smt.depth(), 3)),

store.get_node(smt.root(), NodeIndex::new(smt.depth(), 3)), "node 3 must be the same for both SparseMerkleTree and MerkleStore" ); + assert_eq!( + smt.get_node(&NodeIndex::new(smt.depth(), 4)), + store.get_node(smt.root(), NodeIndex::new(smt.depth(), 4)), + "node 4 must be the same for both SparseMerkleTree and MerkleStore" + ); // STORE MERKLE PATH MATCHS ============================================================== // assert the merkle path returned by the store is the same as the one in the tree

@@ -255,7 +311,7 @@ .get_path(smt.root(), NodeIndex::new(smt.depth(), 1))

.unwrap(); assert_eq!( LEAVES4[1], result.value, - "Value for merkle path at index 0 must match leaf value" + "Value for merkle path at index 1 must match leaf value" ); assert_eq!( smt.get_path(NodeIndex::new(smt.depth(), 1)),

@@ -268,12 +324,12 @@ .get_path(smt.root(), NodeIndex::new(smt.depth(), 2))

.unwrap(); assert_eq!( LEAVES4[2], result.value, - "Value for merkle path at index 0 must match leaf value" + "Value for merkle path at index 2 must match leaf value" ); assert_eq!( smt.get_path(NodeIndex::new(smt.depth(), 2)), Ok(result.path), - "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" + "merkle path for index 2 must be the same for the MerkleTree and MerkleStore" ); let result = store

@@ -281,12 +337,25 @@ .get_path(smt.root(), NodeIndex::new(smt.depth(), 3))

.unwrap(); assert_eq!( LEAVES4[3], result.value, - "Value for merkle path at index 0 must match leaf value" + "Value for merkle path at index 3 must match leaf value" ); assert_eq!( smt.get_path(NodeIndex::new(smt.depth(), 3)), Ok(result.path), - "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" + "merkle path for index 3 must be the same for the MerkleTree and MerkleStore" + ); + + let result = store + .get_path(smt.root(), NodeIndex::new(smt.depth(), 4)) + .unwrap(); + assert_eq!( + EMPTY, result.value, + "Value for merkle path at index 4 must match leaf value" + ); + assert_eq!( + smt.get_path(NodeIndex::new(smt.depth(), 4)), + Ok(result.path), + "merkle path for index 4 must be the same for the MerkleTree and MerkleStore" ); Ok(())