diff --git a/miden-crypto/src/merkle/smt/full/concurrent/mod.rs b/miden-crypto/src/merkle/smt/full/concurrent/mod.rs index 6a6b65524..04d4a4710 100644 --- a/miden-crypto/src/merkle/smt/full/concurrent/mod.rs +++ b/miden-crypto/src/merkle/smt/full/concurrent/mod.rs @@ -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, @@ -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 { diff --git a/miden-crypto/src/merkle/smt/full/error.rs b/miden-crypto/src/merkle/smt/full/error.rs index 1e66ffd4a..2145deeb2 100644 --- a/miden-crypto/src/merkle/smt/full/error.rs +++ b/miden-crypto/src/merkle/smt/full/error.rs @@ -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 diff --git a/miden-crypto/src/merkle/smt/full/leaf.rs b/miden-crypto/src/merkle/smt/full/leaf.rs index 031ade9a6..cce5d44e9 100644 --- a/miden-crypto/src/merkle/smt/full/leaf.rs +++ b/miden-crypto/src/merkle/smt/full/leaf.rs @@ -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 }); + } + } + Ok(Self::Multiple(entries)) } diff --git a/miden-crypto/src/merkle/smt/full/tests.rs b/miden-crypto/src/merkle/smt/full/tests.rs index db8fdc426..f95617b28 100644 --- a/miden-crypto/src/merkle/smt/full/tests.rs +++ b/miden-crypto/src/merkle/smt/full/tests.rs @@ -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]