feat: StorageSchema::commitment
#2244
Conversation
igamigo
force-pushed
the
igamigo-schema-commit
branch
from
d5da574 to
b5ea229
Compare
igamigo
force-pushed
the
igamigo-schema-commit
branch
from
abbe163 to
9aa118c
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a StorageSchema::commitment() method that computes a cryptographic commitment to the storage schema definition, excluding default values. The commitment enables schemas with different default values and slot orderings to produce identical commitments as long as their structural definitions match.
Changes:
- Added
StorageSchema::commitment()method that computes a hash of the serialized schema without default values - Implemented
write_into_with_defaults()helper methods across all schema types to support conditional serialization - Added ASCII validation for component and slot descriptions (both in TOML parsing and deserialization)
- Refactored
parse_storage_value_with_schema()for clearer error handling - Added comprehensive test coverage for commitment behavior and ASCII validation
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
crates/miden-protocol/src/account/component/storage/schema.rs |
Core implementation: added commitment() method, write_into_with_defaults() helpers, ASCII validation function, and refactored parse logic |
crates/miden-protocol/src/account/component/storage/toml/mod.rs |
Added ASCII validation for component descriptions during TOML parsing and updated references from AccountStorageSchema to StorageSchema |
crates/miden-protocol/src/account/component/storage/toml/tests.rs |
Added tests for ASCII validation and commitment behavior demonstrating that commitments ignore defaults and ordering |
crates/miden-protocol/src/account/component/storage/init_storage_data.rs |
Minor reorganization of From implementations and updated documentation |
crates/miden-protocol/src/account/component/mod.rs |
Updated test fixtures to use StorageSchema instead of AccountStorageSchema |
crates/miden-protocol/src/account/component/metadata/mod.rs |
Updated type references and added ASCII validation during deserialization |
CHANGELOG.md |
Added entry documenting the new StorageSchema::commitment() method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| use core::error::Error; | ||
|
|
||
| use miden_core::{Felt, FieldElement, Word}; | ||
| use miden_air::FieldElement; |
There was a problem hiding this comment.
The import FieldElement from miden_air is unused. Consider removing this import to reduce clutter.
| use miden_air::FieldElement; |
mmagician
left a comment
mmagician
left a comment
There was a problem hiding this comment.
Looks good - I would see whether the serialization without defaults can be simplified, as per my comment below (if so, this will propagate into other uses of write_into_*)
igamigo
changed the title
AccountStorageSchema::commitmentStorageSchema::commitment
User-provided names and descriptions are both committed to. Added a small test that verifies this. Or did you mean the component's metadata? |
Perfect, that's what I meant 👍🏼 I think I got lost in the nested |
mmagician
left a comment
mmagician
left a comment
There was a problem hiding this comment.
Overall LGTM, great work!
I would want to think through the consumers of StorageSchema::commitment to see how easy it would be for them to replicate this computation.
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! Not a super detailed review from my end, but I left one question inline.
Adds
StorageSchema::commitment(), which commits to mostStorageSchemafields. Namely, it commits to the type of each slot and the description of such slot (or element of a composite type slot). This commitment disregards ordering in the TOML.StorageSchemais explicitly associated to a single account component right now (since it's a part ofAccountComponentMetadata). A follow-up (#2253) was created to expose a hash of an arbitrary number of these commitments to express the type of a full account.