From 84086bdb9520da89b321fe6f95250f5d8f0d50bf Mon Sep 17 00:00:00 2001 From: Victor Lopez Date: Tue, 21 Mar 2023 10:40:48 +0100 Subject: [PATCH] feat: add merkle path containers and return them on tree update Returning tuples is often confusing as they don't convey meaning and it should be used only when there is no possible ambiguity. For `MerkleStore`, we had a couple of tuples being returned, and reading the implementation was required in order to distinguish if they were leaf values or computed roots. This commit introduces two containers that will self-document these returns: `RootPath` and `ValuePath`. It will also update `set_node` to return both the new root & the new path, so we can prevent duplicated traversals downstream when updating a node (one to update, the second to fetch the new path/root). --- src/merkle/mod.rs | 2 +- src/merkle/path.rs | 24 +++++++++++++++++ src/merkle/store/mod.rs | 42 +++++++++++++++++------------ src/merkle/store/tests.rs | 57 ++++++++++++++++++++------------------- 4 files changed, 80 insertions(+), 45 deletions(-) diff --git a/src/merkle/mod.rs b/src/merkle/mod.rs index 0afbce9..5b204ac 100644 --- a/src/merkle/mod.rs +++ b/src/merkle/mod.rs @@ -18,7 +18,7 @@ mod merkle_tree; pub use merkle_tree::MerkleTree; mod path; -pub use path::MerklePath; +pub use path::{MerklePath, RootPath, ValuePath}; mod path_set; pub use path_set::MerklePathSet; diff --git a/src/merkle/path.rs b/src/merkle/path.rs index d7edd5d..9a4e46b 100644 --- a/src/merkle/path.rs +++ b/src/merkle/path.rs @@ -82,3 +82,27 @@ impl IntoIterator for MerklePath { self.nodes.into_iter() } } + +// MERKLE PATH CONTAINERS +// ================================================================================================ + +/// A container for a [Word] value and its [MerklePath] opening. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct ValuePath { + /// The node value opening for `path`. + pub value: Word, + /// The path from `value` to `root` (exclusive). + pub path: MerklePath, +} + +/// A container for a [MerklePath] and its [Word] root. +/// +/// This structure does not provide any guarantees regarding the correctness of the path to the +/// root. For more information, check [MerklePath::verify]. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct RootPath { + /// The node value opening for `path`. + pub root: Word, + /// The path from `value` to `root` (exclusive). + pub path: MerklePath, +} diff --git a/src/merkle/store/mod.rs b/src/merkle/store/mod.rs index 5610429..13ca5f9 100644 --- a/src/merkle/store/mod.rs +++ b/src/merkle/store/mod.rs @@ -5,7 +5,7 @@ //! implementation of efficient persistent data structures use super::{ BTreeMap, BTreeSet, EmptySubtreeRoots, MerkleError, MerklePath, MerklePathSet, MerkleTree, - NodeIndex, Rpo256, RpoDigest, SimpleSmt, Vec, Word, + NodeIndex, RootPath, Rpo256, RpoDigest, SimpleSmt, ValuePath, Vec, Word, }; #[cfg(test)] @@ -122,11 +122,7 @@ impl MerkleStore { /// This method can return the following errors: /// - `RootNotInStore` if the `root` is not present in the store. /// - `NodeNotInStore` if a node needed to traverse from `root` to `index` is not present in the store. - pub fn get_path( - &self, - root: Word, - index: NodeIndex, - ) -> Result<(Word, MerklePath), MerkleError> { + pub fn get_path(&self, root: Word, index: NodeIndex) -> Result { let mut hash: RpoDigest = root.into(); let mut path = Vec::with_capacity(index.depth().into()); @@ -150,8 +146,13 @@ impl MerkleStore { } } + // the path is computed from root to leaf, so it must be reversed path.reverse(); - Ok((hash.into(), MerklePath::new(path))) + + Ok(ValuePath { + value: hash.into(), + path: MerklePath::new(path), + }) } // STATE MUTATORS @@ -233,17 +234,17 @@ impl MerkleStore { Ok(smt.root()) } - /// Adds all the nodes of a Merkle path represented by `path`. + /// Adds all the nodes of a Merkle path represented by `path`, opening to `node`. Returns the + /// new root. /// /// This will compute the sibling elements determined by the Merkle `path` and `node`, and /// include all the nodes into the store. pub fn add_merkle_path( &mut self, index_value: u64, - node: Word, + mut node: Word, path: MerklePath, ) -> Result { - let mut node = node; let mut index = NodeIndex::new(self.nodes.len() as u8, index_value); for sibling in path { @@ -272,6 +273,8 @@ impl MerkleStore { /// This will compute the sibling elements for each Merkle `path` and include all the nodes /// into the store. /// + /// For further reference, check [MerkleStore::add_merkle_path]. + /// /// # Errors /// /// Every path must resolve to the same root, otherwise this will return an `ConflictingRoots` @@ -301,6 +304,8 @@ impl MerkleStore { } /// Appends the provided [MerklePathSet] into the store. + /// + /// For further reference, check [MerkleStore::add_merkle_path]. pub fn add_merkle_path_set(&mut self, path_set: &MerklePathSet) -> Result { let root = path_set.root(); path_set.indexes().try_fold(root, |_, index| { @@ -319,16 +324,19 @@ impl MerkleStore { /// - `NodeNotInStore` if a node needed to traverse from `root` to `index` is not present in the store. pub fn set_node( &mut self, - root: Word, + mut root: Word, index: NodeIndex, value: Word, - ) -> Result { - let result = self.get_path(root, index)?; - if result.0 != value { - self.add_merkle_path(index.value(), value, result.1) - } else { - Ok(root) + ) -> Result { + let node = value; + let ValuePath { value, path } = self.get_path(root, index)?; + + // performs the update only if the node value differs from the opening + if node != value { + root = self.add_merkle_path(index.value(), node, path.clone())?; } + + Ok(RootPath { root, path }) } pub fn merge_roots(&mut self, root1: Word, root2: Word) -> Result { diff --git a/src/merkle/store/tests.rs b/src/merkle/store/tests.rs index c469553..97e38a7 100644 --- a/src/merkle/store/tests.rs +++ b/src/merkle/store/tests.rs @@ -90,12 +90,12 @@ fn test_merkle_tree() -> Result<(), MerkleError> { .get_path(mtree.root(), NodeIndex::new(mtree.depth(), 0)) .unwrap(); assert_eq!( - LEAVES4[0], result.0, + LEAVES4[0], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( mtree.get_path(NodeIndex::new(mtree.depth(), 0)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -103,12 +103,12 @@ fn test_merkle_tree() -> Result<(), MerkleError> { .get_path(mtree.root(), NodeIndex::new(mtree.depth(), 1)) .unwrap(); assert_eq!( - LEAVES4[1], result.0, + LEAVES4[1], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( mtree.get_path(NodeIndex::new(mtree.depth(), 1)), - Ok(result.1), + Ok(result.path), "merkle path for index 1 must be the same for the MerkleTree and MerkleStore" ); @@ -116,12 +116,12 @@ fn test_merkle_tree() -> Result<(), MerkleError> { .get_path(mtree.root(), NodeIndex::new(mtree.depth(), 2)) .unwrap(); assert_eq!( - LEAVES4[2], result.0, + LEAVES4[2], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( mtree.get_path(NodeIndex::new(mtree.depth(), 2)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -129,12 +129,12 @@ fn test_merkle_tree() -> Result<(), MerkleError> { .get_path(mtree.root(), NodeIndex::new(mtree.depth(), 3)) .unwrap(); assert_eq!( - LEAVES4[3], result.0, + LEAVES4[3], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( mtree.get_path(NodeIndex::new(mtree.depth(), 3)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -241,12 +241,12 @@ fn test_sparse_merkle_tree() -> Result<(), MerkleError> { .get_path(smt.root(), NodeIndex::new(smt.depth(), 0)) .unwrap(); assert_eq!( - LEAVES4[0], result.0, + LEAVES4[0], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( smt.get_path(NodeIndex::new(smt.depth(), 0)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -254,12 +254,12 @@ fn test_sparse_merkle_tree() -> Result<(), MerkleError> { .get_path(smt.root(), NodeIndex::new(smt.depth(), 1)) .unwrap(); assert_eq!( - LEAVES4[1], result.0, + LEAVES4[1], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( smt.get_path(NodeIndex::new(smt.depth(), 1)), - Ok(result.1), + Ok(result.path), "merkle path for index 1 must be the same for the MerkleTree and MerkleStore" ); @@ -267,12 +267,12 @@ fn test_sparse_merkle_tree() -> Result<(), MerkleError> { .get_path(smt.root(), NodeIndex::new(smt.depth(), 2)) .unwrap(); assert_eq!( - LEAVES4[2], result.0, + LEAVES4[2], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( smt.get_path(NodeIndex::new(smt.depth(), 2)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -280,12 +280,12 @@ fn test_sparse_merkle_tree() -> Result<(), MerkleError> { .get_path(smt.root(), NodeIndex::new(smt.depth(), 3)) .unwrap(); assert_eq!( - LEAVES4[3], result.0, + LEAVES4[3], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( smt.get_path(NodeIndex::new(smt.depth(), 3)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -375,12 +375,12 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { .get_path(set.root(), NodeIndex::new(set.depth() - 1, 0)) .unwrap(); assert_eq!( - LEAVES4[0], result.0, + LEAVES4[0], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( set.get_path(NodeIndex::new(set.depth(), 0)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -388,12 +388,12 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { .get_path(set.root(), NodeIndex::new(set.depth() - 1, 1)) .unwrap(); assert_eq!( - LEAVES4[1], result.0, + LEAVES4[1], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( set.get_path(NodeIndex::new(set.depth(), 1)), - Ok(result.1), + Ok(result.path), "merkle path for index 1 must be the same for the MerkleTree and MerkleStore" ); @@ -401,12 +401,12 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { .get_path(set.root(), NodeIndex::new(set.depth() - 1, 2)) .unwrap(); assert_eq!( - LEAVES4[2], result.0, + LEAVES4[2], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( set.get_path(NodeIndex::new(set.depth(), 2)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -414,12 +414,12 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { .get_path(set.root(), NodeIndex::new(set.depth() - 1, 3)) .unwrap(); assert_eq!( - LEAVES4[3], result.0, + LEAVES4[3], result.value, "Value for merkle path at index 0 must match leaf value" ); assert_eq!( set.get_path(NodeIndex::new(set.depth(), 3)), - Ok(result.1), + Ok(result.path), "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -474,10 +474,13 @@ fn store_path_opens_from_leaf() { let store = MerkleStore::default() .with_merkle_tree([a, b, c, d, e, f, g, h]) .unwrap(); - let path = store.get_path(root.into(), NodeIndex::new(3, 1)).unwrap(); + let path = store + .get_path(root.into(), NodeIndex::new(3, 1)) + .unwrap() + .path; let expected = MerklePath::new([a.into(), j.into(), n.into()].to_vec()); - assert_eq!(path.1, expected); + assert_eq!(path, expected); } #[test] @@ -486,7 +489,7 @@ fn test_set_node() -> Result<(), MerkleError> { let mut store = MerkleStore::default().with_merkle_tree(LEAVES4)?; let value = int_to_node(42); let index = NodeIndex::new(mtree.depth(), 0); - let new_root = store.set_node(mtree.root(), index, value)?; + let new_root = store.set_node(mtree.root(), index, value)?.root; assert_eq!( store.get_node(new_root, index), Ok(value),