Skip to content

Commit

Permalink
reverse state proof siblings
Browse files Browse the repository at this point in the history
Our implementation outputs the new order natually without the 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.
  • Loading branch information
msmouse committed May 15, 2024
1 parent 6cdd4c2 commit edd9d16
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 24 deletions.
14 changes: 2 additions & 12 deletions storage/jellyfish-merkle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
},
};
},
Expand All @@ -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 => {
Expand All @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions storage/scratchpad/src/sparse_merkle/sparse_merkle_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
});
Expand All @@ -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())
});
Expand Down Expand Up @@ -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::<StateValue>(old_root_hash, key4, None)
Expand Down Expand Up @@ -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());

Expand Down
13 changes: 11 additions & 2 deletions storage/scratchpad/src/sparse_merkle/test_utils/naive_smt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ impl<'a> NaiveSubTree<'a> {
&'a self,
key: &HashValue,
cache: &mut Cache,
) -> (Option<SparseMerkleLeafNode>, Vec<NodeInProof>) {
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<SparseMerkleLeafNode>, Vec<NodeInProof>) {
if self.is_empty() {
(None, Vec::new())
Expand All @@ -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)
}
Expand Down
3 changes: 1 addition & 2 deletions storage/scratchpad/src/sparse_merkle/updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
},
Expand Down
9 changes: 5 additions & 4 deletions types/src/proof/definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ pub struct SparseMerkleProof {
/// empty.
leaf: Option<SparseMerkleLeafNode>,

/// 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<HashValue>,
}

Expand Down Expand Up @@ -183,8 +183,8 @@ impl NodeInProof {
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
pub struct SparseMerkleProofExt {
leaf: Option<SparseMerkleLeafNode>,
/// 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<NodeInProof>,
}

Expand Down Expand Up @@ -349,6 +349,7 @@ impl SparseMerkleProof {
let actual_root_hash = self
.siblings
.iter()
.rev()
.zip(
element_key
.iter_bits()
Expand Down

0 comments on commit edd9d16

Please sign in to comment.