fix: Canonicalize SmtLeaf::Multiple #663
Conversation
huitseeker
left a comment
huitseeker
left a comment
There was a problem hiding this comment.
SmtLeaf::new_multiple() previously accepted entries in arbitrary order and allowed duplicate keys, which violated the documented invariant that SmtLeaf::Multiple must be canonicalized by key order and contain unique keys.
I missed that documentation, where is it?
Is this a behavior change, or fixing an inconsistency?
| 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 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
Fixing an inconsistency. |
I missed that documentation, where is it? - Yes, i was wrong in my description, i should've said "which violated the implicit invariant that SmtLeaf::Multiple must be canonicalized by key order and contain unique keys" |
3 participants
Describe your changes
SmtLeaf::new_multiple() previously accepted entries in arbitrary order and allowed duplicate keys, which violated the documented invariant that SmtLeaf::Multiple must be canonicalized by key order and contain unique keys. This inconsistency could corrupt downstream behavior in several places: SmtLeaf::insert() and remove() rely on binary_search over a sorted vector; SmtLeaf::hash() depends on canonical order while the specification requires hashing entries ordered by key; and deserialization admitted unsorted or duplicate entries, which could then be inserted into a PartialSmt and later produce incorrect updates. To fix this, new_multiple() now sorts entries by cmp_keys() and rejects duplicate keys after enforcing that all keys map to the same leaf index and the count is within MAX_LEAF_ENTRIES. A new error DuplicateKeysInMultipleLeaf has been added for this case. Tests were added to validate canonical sorting and hash stability, duplicate-key rejection, and that deserialization canonicalizes order while rejecting duplicate keys.