-
Notifications
You must be signed in to change notification settings - Fork 78
add SMT integration tests for History mechanism #754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Conversation
fbc7985 to
6f43466
Compare
huitseeker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither test actually updates an SMT. They create an empty SMT, get its root, but then manually construct CompactLeaf and LeafChanges / NodeChanges instead of using the SMT's mutation machinery.
|
Updated. |
| assert_eq!(view_v1.value(&key_2), Some(Some(&value_2))); | ||
|
|
||
| // Verify node changes were captured (mutations produce inner node updates) | ||
| assert!(!node_changes_v0.is_empty(), "SMT insertion should produce node changes"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test verifies node changes were captured, but doesn't compare the stored hashes against the actual SMT. Could we add something like:
for (index, hash) in node_changes_v0.iter() {
assert_eq!(*hash, smt.get_node_hash(*index));
}This would confirm the History node values match what the SMT actually computed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Added hash verification assertions after each mutation is applied.
| use crate::merkle::smt::{MutationSet, NodeMutation, SMT_DEPTH, Smt, SparseMerkleTree}; | ||
|
|
||
| /// Converts a MutationSet into the format expected by History. | ||
| fn mutation_set_to_history_changes( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this helper is duplicated in both tests. Could be extracted to a shared function at the top of the SMT integration tests section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Extracted the helper to module level right after the SMT integration tests header.
99fc7d1 to
4e3c8b3
Compare
huitseeker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
|
|
||
| // Verify version 0 has original value | ||
| let view_v0 = history.get_view_at(0)?; | ||
| assert_eq!(view_v0.value(&key), Some(Some(&value_v0))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: since we have the SMT available, could also verify view_v0.value(&key) == Some(smt.get_value(key).as_ref()) to complete the round-trip the issue described. Not blocking, the current assertion is equivalent for this test.
Add tests verifying History overlay works correctly with SMT data structures: - smt_leaf_history_tracking: Tests leaf data tracking across versions - smt_node_history_tracking: Tests inner node hash tracking at SMT-valid depths
Replace manually constructed test data with actual SMT mutations obtained via compute_mutations() and apply_mutations(). The new tests: - smt_history_with_real_mutations: Tests History tracking of node and leaf changes from real SMT insertions across multiple versions - smt_history_value_updates: Tests History tracking when updating existing values in the SMT This ensures the tests reflect real-world usage patterns where History receives data derived from actual SMT mutation operations.
- Moved mutation_set_to_history_changes to module level to eliminate duplication - Added assertions to verify stored node hashes match SMT computed values
6c91d56 to
7c51eb2
Compare
Adds integration tests for the History mechanism with SMT:
smt_history_integration: Tests creating history overlay from SMT mutations and querying historical statesmt_history_multiple_updates: Tests history with multiple sequential SMT updatesCloses #699