Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions miden-crypto/src/merkle/smt/full/concurrent/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rayon::prelude::*;

use super::{
EmptySubtreeRoots, InnerNode, InnerNodes, Leaves, MerkleError, MutationSet, NodeIndex,
SMT_DEPTH, Smt, SmtLeaf, SparseMerkleTree, Word, leaf,
SMT_DEPTH, Smt, SmtLeaf, SparseMerkleTree, Word,
};
use crate::merkle::{
SmtLeafError,
Expand Down Expand Up @@ -277,14 +277,16 @@ impl Smt {
assert!(!pairs.is_empty());

if pairs.len() > 1 {
pairs.sort_by(|(key_1, _), (key_2, _)| leaf::cmp_keys(*key_1, *key_2));
// Check for duplicates in a sorted list by comparing adjacent pairs
if let Some(window) = pairs.windows(2).find(|window| window[0].0 == window[1].0) {
// If we find a duplicate, return an error
let col = Self::key_to_leaf_index(&window[0].0).index.value();
return Err(MerkleError::DuplicateValuesForIndex(col));
}
Ok(Some(SmtLeaf::new_multiple(pairs).unwrap()))
SmtLeaf::new_multiple(pairs).map(Some).map_err(|e| match e {
SmtLeafError::DuplicateKeysInMultipleLeaf { key } => {
let col = Self::key_to_leaf_index(&key).index.value();
MerkleError::DuplicateValuesForIndex(col)
},
SmtLeafError::TooManyLeafEntries { actual } => {
MerkleError::TooManyLeafEntries { actual }
},
other => panic!("unexpected error in pairs_to_leaf: {:?}", other),
})
} else {
let (key, value) = pairs.pop().unwrap();
if value == Self::EMPTY_VALUE {
Expand Down
4 changes: 4 additions & 0 deletions miden-crypto/src/merkle/smt/full/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ pub enum SmtLeafError {
"multiple leaf contains {actual} entries but the maximum allowed is {MAX_LEAF_ENTRIES}"
)]
TooManyLeafEntries { actual: usize },

/// Multiple leaf contains duplicate keys, which violates the ordering/uniqueness invariant.
#[error("multiple leaf contains duplicate key {key}")]
DuplicateKeysInMultipleLeaf { key: Word },
}

// SMT PROOF ERROR
Expand Down
9 changes: 9 additions & 0 deletions miden-crypto/src/merkle/smt/full/leaf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,15 @@ impl SmtLeaf {
}
}

let mut entries = entries;
entries.sort_by(|(key_1, _), (key_2, _)| cmp_keys(*key_1, *key_2));

for pair in entries.windows(2) {
if pair[0].0 == pair[1].0 {
return Err(SmtLeafError::DuplicateKeysInMultipleLeaf { key: pair[0].0 });
}
}
Comment on lines +113 to +120
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks are currently done in pairs_to_leaf() function an din the insert() function. It may still make sense to move the checks here (to check these invariants during deserialization) - but then we should make sure we remove the no longer needed code these function.


Ok(Self::Multiple(entries))
}

Expand Down
64 changes: 64 additions & 0 deletions miden-crypto/src/merkle/smt/full/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,70 @@ fn test_smt_insert_at_same_key() {
}
}

#[test]
fn test_multiple_smt_leaf_new_multiple_sorts_and_hash() {
let msb = Felt::new(13);
let key_a = Word::new([Felt::new(0), Felt::new(0), Felt::new(5), msb]);
let key_b = Word::new([Felt::new(0), Felt::new(0), Felt::new(3), msb]);
let key_c = Word::new([Felt::new(0), Felt::new(0), Felt::new(7), msb]);

let value_a = Word::new([Felt::new(1), Felt::new(1), Felt::new(1), Felt::new(1)]);
let value_b = Word::new([Felt::new(2), Felt::new(2), Felt::new(2), Felt::new(2)]);
let value_c = Word::new([Felt::new(3), Felt::new(3), Felt::new(3), Felt::new(3)]);

let unsorted = vec![(key_a, value_a), (key_c, value_c), (key_b, value_b)];
let expected_sorted = vec![(key_b, value_b), (key_a, value_a), (key_c, value_c)];

let leaf = SmtLeaf::new_multiple(unsorted).unwrap();
assert_eq!(leaf.entries(), expected_sorted.as_slice());
let expected_hash = build_multiple_leaf_node(&expected_sorted);
assert_eq!(leaf.hash(), expected_hash);
}

#[test]
fn test_multiple_smt_leaf_new_multiple_rejects_duplicate_keys() {
let msb = Felt::new(21);
let key = Word::new([Felt::new(9), Felt::new(9), Felt::new(9), msb]);
let v1 = Word::new([Felt::new(1), Felt::new(0), Felt::new(0), Felt::new(0)]);
let v2 = Word::new([Felt::new(2), Felt::new(0), Felt::new(0), Felt::new(0)]);

let err = SmtLeaf::new_multiple(vec![(key, v1), (key, v2)]).unwrap_err();
assert_matches!(err, SmtLeafError::DuplicateKeysInMultipleLeaf { .. });
}

#[test]
fn test_multiple_smt_leaf_deserialization_sorts_canonical() {
let msb = Felt::new(33);

let k1 = Word::new([Felt::new(0), Felt::new(0), Felt::new(10), msb]);
let k2 = Word::new([Felt::new(0), Felt::new(0), Felt::new(5), msb]);

let v1 = Word::new([Felt::new(7), Felt::new(7), Felt::new(7), Felt::new(7)]);
let v2 = Word::new([Felt::new(8), Felt::new(8), Felt::new(8), Felt::new(8)]);

let unsorted_leaf = SmtLeaf::Multiple(vec![(k1, v1), (k2, v2)]);
let bytes = unsorted_leaf.to_bytes();

let decoded = SmtLeaf::read_from_bytes(&bytes).unwrap();
let expected = SmtLeaf::new_multiple(vec![(k1, v1), (k2, v2)]).unwrap();
assert_eq!(decoded, expected);
assert_eq!(decoded.hash(), expected.hash());
}

#[test]
fn test_multiple_smt_leaf_deserialization_rejects_duplicate_keys() {
let msb = Felt::new(44);
let key = Word::new([Felt::new(1), Felt::new(2), Felt::new(3), msb]);
let v1 = Word::new([Felt::new(10), Felt::new(0), Felt::new(0), Felt::new(0)]);
let v2 = Word::new([Felt::new(11), Felt::new(0), Felt::new(0), Felt::new(0)]);

let bad_leaf = SmtLeaf::Multiple(vec![(key, v1), (key, v2)]);
let bytes = bad_leaf.to_bytes();

let err = SmtLeaf::read_from_bytes(&bytes).unwrap_err();
let _ = err;
}

/// This test checks that inserting twice at the same key functions as expected. The test covers
/// only the case where the leaf type is `SmtLeaf::Multiple`
#[test]
Expand Down
Loading