From fb462d74e60995b3793cd959f46c27995950995b Mon Sep 17 00:00:00 2001 From: aldenhu Date: Tue, 14 May 2024 17:32:53 +0000 Subject: [PATCH] reverse state proof siblings Our implementation outputs the new order natually without the need for reversing. Lucky that we didn't expose the state proof to outside of the node The range proof unfortunately is already exposed, so not gonna be changed. --- storage/jellyfish-merkle/src/lib.rs | 14 ++------------ .../src/sparse_merkle/sparse_merkle_test.rs | 9 +++++---- .../src/sparse_merkle/test_utils/naive_smt.rs | 13 +++++++++++-- storage/scratchpad/src/sparse_merkle/updater.rs | 3 +-- types/src/proof/definition.rs | 9 +++++---- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/storage/jellyfish-merkle/src/lib.rs b/storage/jellyfish-merkle/src/lib.rs index 6418e61a4127f7..1d10cb7e0db4b2 100644 --- a/storage/jellyfish-merkle/src/lib.rs +++ b/storage/jellyfish-merkle/src/lib.rs @@ -769,13 +769,7 @@ where next_node_key = match child_node_key { Some(node_key) => node_key, None => { - return Ok(( - None, - SparseMerkleProofExt::new(None, { - siblings.reverse(); - siblings - }), - )); + return Ok((None, SparseMerkleProofExt::new(None, siblings))); }, }; }, @@ -786,10 +780,7 @@ where } else { None }, - SparseMerkleProofExt::new(Some(leaf_node.into()), { - siblings.reverse(); - siblings - }), + SparseMerkleProofExt::new(Some(leaf_node.into()), siblings), )); }, Node::Null => { @@ -812,7 +803,6 @@ where let siblings = proof .siblings() .iter() - .rev() .zip(rightmost_key_to_prove.iter_bits()) .filter_map(|(sibling, bit)| { // We only need to keep the siblings on the right. diff --git a/storage/scratchpad/src/sparse_merkle/sparse_merkle_test.rs b/storage/scratchpad/src/sparse_merkle/sparse_merkle_test.rs index b1010b063a5bd3..bec1430c6c0f01 100644 --- a/storage/scratchpad/src/sparse_merkle/sparse_merkle_test.rs +++ b/storage/scratchpad/src/sparse_merkle/sparse_merkle_test.rs @@ -214,11 +214,11 @@ fn test_update_256_siblings_in_proof() { .take(255) .collect(); siblings.push(leaf2.into()); - siblings.reverse(); let proof_of_key1 = SparseMerkleProofExt::new(Some(leaf1), siblings.clone()); let old_root_hash = siblings .iter() + .rev() .fold(leaf1.hash(), |previous_hash, node_in_proof| { hash_internal(previous_hash, node_in_proof.hash()) }); @@ -237,6 +237,7 @@ fn test_update_256_siblings_in_proof() { let new_leaf1_hash = hash_leaf(key1, new_value1_hash); let new_root_hash = siblings .iter() + .rev() .fold(new_leaf1_hash, |previous_hash, node_in_proof| { hash_internal(previous_hash, node_in_proof.hash()) }); @@ -300,8 +301,8 @@ fn test_update() { let y_hash = hash_internal(x_hash, *SPARSE_MERKLE_PLACEHOLDER_HASH); let old_root_hash = hash_internal(y_hash, leaf3.hash()); let proof = SparseMerkleProofExt::new(None, vec![ - NodeInProof::Other(x_hash), NodeInProof::Leaf(leaf3), + NodeInProof::Other(x_hash), ]); assert!(proof .verify::(old_root_hash, key4, None) @@ -340,9 +341,9 @@ fn test_update() { // Next, we are going to delete key1. Create a proof for key1. let proof = SparseMerkleProofExt::new(Some(leaf1), vec![ - leaf2.into(), - (*SPARSE_MERKLE_PLACEHOLDER_HASH).into(), leaf3.into(), + (*SPARSE_MERKLE_PLACEHOLDER_HASH).into(), + leaf2.into(), ]); assert!(proof.verify(old_root_hash, key1, Some(&value1)).is_ok()); diff --git a/storage/scratchpad/src/sparse_merkle/test_utils/naive_smt.rs b/storage/scratchpad/src/sparse_merkle/test_utils/naive_smt.rs index 231c5cdb4ca9c8..dc86fe5b9ff5a9 100644 --- a/storage/scratchpad/src/sparse_merkle/test_utils/naive_smt.rs +++ b/storage/scratchpad/src/sparse_merkle/test_utils/naive_smt.rs @@ -25,6 +25,15 @@ impl<'a> NaiveSubTree<'a> { &'a self, key: &HashValue, cache: &mut Cache, + ) -> (Option, Vec) { + let (leaf, rev_proof) = self.get_proof_(key, cache); + (leaf, rev_proof.into_iter().rev().collect()) + } + + fn get_proof_( + &'a self, + key: &HashValue, + cache: &mut Cache, ) -> (Option, Vec) { if self.is_empty() { (None, Vec::new()) @@ -37,11 +46,11 @@ impl<'a> NaiveSubTree<'a> { } else { let (left, right) = self.children(); if key.bit(self.depth) { - let (ret_leaf, mut ret_siblings) = right.get_proof(key, cache); + let (ret_leaf, mut ret_siblings) = right.get_proof_(key, cache); ret_siblings.push(left.get_node_in_proof(cache)); (ret_leaf, ret_siblings) } else { - let (ret_leaf, mut ret_siblings) = left.get_proof(key, cache); + let (ret_leaf, mut ret_siblings) = left.get_proof_(key, cache); ret_siblings.push(right.get_node_in_proof(cache)); (ret_leaf, ret_siblings) } diff --git a/storage/scratchpad/src/sparse_merkle/updater.rs b/storage/scratchpad/src/sparse_merkle/updater.rs index 58a3a6690a0aea..f923687680eaac 100644 --- a/storage/scratchpad/src/sparse_merkle/updater.rs +++ b/storage/scratchpad/src/sparse_merkle/updater.rs @@ -246,8 +246,7 @@ impl<'a, V: Clone + CryptoHash + Send + Sync + 'static> SubTreeInfo<'a, V> { PersistedSubTreeInfo::ProofPathInternal { proof } => { let siblings = proof.siblings(); assert!(siblings.len() > depth); - let sibling_child = - SubTreeInfo::new_proof_sibling(&siblings[siblings.len() - depth - 1]); + let sibling_child = SubTreeInfo::new_proof_sibling(&siblings[depth]); let on_path_child = SubTreeInfo::new_on_proof_path(proof, depth + 1); swap_if(on_path_child, sibling_child, a_descendent_key.bit(depth)) }, diff --git a/types/src/proof/definition.rs b/types/src/proof/definition.rs index a2b2b9c096632b..1d88b56c52a66e 100644 --- a/types/src/proof/definition.rs +++ b/types/src/proof/definition.rs @@ -146,8 +146,8 @@ pub struct SparseMerkleProof { /// empty. leaf: Option, - /// All siblings in this proof, including the default ones. Siblings are ordered from the bottom - /// level to the root level. + /// All siblings in this proof, including the default ones. Siblings are ordered from the root + /// level to the bottom level. siblings: Vec, } @@ -183,8 +183,8 @@ impl NodeInProof { #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct SparseMerkleProofExt { leaf: Option, - /// All siblings in this proof, including the default ones. Siblings are ordered from the bottom - /// level to the root level. + /// All siblings in this proof, including the default ones. Siblings are ordered from the root + /// level to the bottom level. siblings: Vec, } @@ -349,6 +349,7 @@ impl SparseMerkleProof { let actual_root_hash = self .siblings .iter() + .rev() .zip( element_key .iter_bits()