Skip to content

Commit

Permalink
fix: remove statement from sparse Merkle tree proofs (#5768)
Browse files Browse the repository at this point in the history
Description
---
Removes the key and value hashes from sparse Merkle tree proofs. Adds a
path check to inclusion proof verification. Closes #5527.

Motivation and Context
---
Currently, sparse Merkle tree proofs
[include](https://github.com/tari-project/tari/blob/87c070305951152c62a0179e13fadc55065cc318/base_layer/mmr/src/sparse_merkle_tree/proofs.rs#L23-L24)
the key and value hashes for the proof. As these are effectively the
proof statement, it doesn't really make sense to include them in the
proof structure.

Including them means additional work for the verifier for both
[inclusion](https://github.com/tari-project/tari/blob/87c070305951152c62a0179e13fadc55065cc318/base_layer/mmr/src/sparse_merkle_tree/proofs.rs#L75-L76)
and
[exclusion](https://github.com/tari-project/tari/blob/87c070305951152c62a0179e13fadc55065cc318/base_layer/mmr/src/sparse_merkle_tree/proofs.rs#L83)
proofs that isn't needed. This design also seems like a footgun that
could lead an implementer to unintentionally trust the statement from a
malicious prover.

This PR removes the key and value hashes from the proof structure
altogether, and updates relevant functions accordingly. It also adds a
path check to the inclusion proof verifier for consistency with the
exclusion proof verifier, which may also help with faster detection of
invalid proofs.

How Has This Been Tested?
---
Existing tests pass.

What process can a PR reviewer use to test or verify this change?
---
Check that the new verification logic is correct.

Breaking Changes
---
None. While this changes the structure of sparse Merkle tree proofs,
they are not currently used and do not have serialization defined.

---------

Co-authored-by: Aaron Feickert <[email protected]>
Co-authored-by: CjS77 <[email protected]>
  • Loading branch information
3 people authored Sep 14, 2023
1 parent 19b3f21 commit d630d11
Show file tree
Hide file tree
Showing 7 changed files with 521 additions and 220 deletions.
84 changes: 10 additions & 74 deletions base_layer/mmr/src/sparse_merkle_tree/bit_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,9 @@ pub(crate) fn get_bit(data: &[u8], position: usize) -> usize {
0
}

/// Sets the n-th bit in a byte array to the given value. `position` 0 is the most significant bit.
///
/// # Panics
/// `set_bit` will panic if
/// * `position` is out of bounds
/// * `value` is not 0 or 1
#[inline]
pub(crate) fn set_bit(data: &mut [u8], position: usize, value: usize) {
match value {
0 => data[position / 8] &= !(1 << (7 - (position % 8))),
1 => data[position / 8] |= 1 << (7 - (position % 8)),
_ => panic!("Invalid bit value"),
}
}

/// Given two node keys, this function returns the number of bits that are common to both keys, starting from the most
/// significant bit. This function is used to tell you the height at which two node keys would diverge in the sparse
/// merkle tree. For example, key 0110 and 0101 would diverge at height 2, because the first two bits are the same.
/// Merkle& tree. For example, key 0110 and 0101 would diverge at height 2, because the first two bits are the same.
#[inline]
pub(crate) fn count_common_prefix(a: &NodeKey, b: &NodeKey) -> usize {
let mut offset = 0;
Expand All @@ -58,7 +43,7 @@ pub fn height_key(key: &NodeKey, height: usize) -> NodeKey {
let mut result = NodeKey::default();
// Keep the first `height` bits and ignore the rest
let key = key.as_slice();
let bytes = result.as_mut_slice();
let bytes = result.as_slice_mut();
// First height/8 bytes are the same, so just copy
bytes[0..height / 8].copy_from_slice(&key[0..height / 8]);
// The height/8th byte is only partially copied, so mask the byte & 11100000, where the number of 1s is
Expand All @@ -67,21 +52,12 @@ pub fn height_key(key: &NodeKey, height: usize) -> NodeKey {
result
}

pub fn path_matches_key(key: &NodeKey, path: &[TraverseDirection]) -> bool {
let height = path.len();

let prefix = path
.iter()
.enumerate()
.fold(NodeKey::default(), |mut prefix, (i, dir)| {
let bit = match dir {
TraverseDirection::Left => 0,
TraverseDirection::Right => 1,
};
set_bit(prefix.as_mut_slice(), i, bit);
prefix
});
count_common_prefix(key, &prefix) >= height
pub const fn bit_to_dir(bit: usize) -> TraverseDirection {
match bit {
0 => TraverseDirection::Left,
1 => TraverseDirection::Right,
_ => panic!("Invalid bit"),
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down Expand Up @@ -116,11 +92,8 @@ pub fn traverse_direction(
});
}

match get_bit(child_key.as_slice(), parent_height) {
0 => Ok(TraverseDirection::Left),
1 => Ok(TraverseDirection::Right),
_ => unreachable!(),
}
let dir = bit_to_dir(get_bit(child_key.as_slice(), parent_height));
Ok(dir)
}

#[cfg(test)]
Expand Down Expand Up @@ -234,43 +207,6 @@ mod test {
assert_eq!(hkey, expected);
}

#[test]
fn set_bits() {
let mut val = [0b10101010, 0b01010101];
set_bit(&mut val, 0, 1);
assert_eq!(val, [0b10101010, 0b01010101]);
set_bit(&mut val, 1, 1);
assert_eq!(val, [0b11101010, 0b01010101]);
set_bit(&mut val, 2, 1);
assert_eq!(val, [0b11101010, 0b01010101]);
set_bit(&mut val, 3, 1);
assert_eq!(val, [0b11111010, 0b01010101]);
set_bit(&mut val, 4, 1);
assert_eq!(val, [0b11111010, 0b01010101]);
set_bit(&mut val, 5, 1);
assert_eq!(val, [0b11111110, 0b01010101]);
set_bit(&mut val, 6, 1);
assert_eq!(val, [0b11111110, 0b01010101]);
set_bit(&mut val, 7, 1);
assert_eq!(val, [0b11111111, 0b01010101]);
set_bit(&mut val, 8, 0);
assert_eq!(val, [0b11111111, 0b01010101]);
set_bit(&mut val, 9, 0);
assert_eq!(val, [0b11111111, 0b00010101]);
set_bit(&mut val, 10, 0);
assert_eq!(val, [0b11111111, 0b00010101]);
set_bit(&mut val, 11, 0);
assert_eq!(val, [0b11111111, 0b00000101]);
set_bit(&mut val, 12, 0);
assert_eq!(val, [0b11111111, 0b00000101]);
set_bit(&mut val, 13, 0);
assert_eq!(val, [0b11111111, 0b00000001]);
set_bit(&mut val, 14, 0);
assert_eq!(val, [0b11111111, 0b00000001]);
set_bit(&mut val, 15, 0);
assert_eq!(val, [0b11111111, 0b00000000]);
}

#[test]
fn get_bits() {
let val = [0b10101010, 0b10101010, 0b00000000, 0b11111111];
Expand Down
4 changes: 4 additions & 0 deletions base_layer/mmr/src/sparse_merkle_tree/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,8 @@ pub enum SMTError {
IllegalKey(String),
#[error("The hash for the tree needs to be recalculated before calling this function")]
StaleHash,
#[error(
"Cannot construct a proof. Either the key exists for an exclusion proof, or it does not for an inclusion proof"
)]
NonViableProof,
}
4 changes: 2 additions & 2 deletions base_layer/mmr/src/sparse_merkle_tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
//! r###" │A│ │B│ │D│ │C│ "###
//! r###" └─┘ └─┘ └─┘ └─┘ "###
//!
//! The merkle root is calculated by hashing nodes in the familiar way.
//! The Merkle& root is calculated by hashing nodes in the familiar way.
//! ```rust
//! use blake2::Blake2b;
//! use digest::consts::U32;
Expand Down Expand Up @@ -84,5 +84,5 @@ mod tree;

pub use error::SMTError;
pub use node::{BranchNode, EmptyNode, LeafNode, Node, NodeHash, NodeKey, ValueHash, EMPTY_NODE_HASH};
pub use proofs::MerkleProof;
pub use proofs::{ExclusionProof, InclusionProof};
pub use tree::{SparseMerkleTree, UpdateResult};
Loading

0 comments on commit d630d11

Please sign in to comment.