-
Notifications
You must be signed in to change notification settings - Fork 105
feat: SchemaCommitment component
#2253
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: igamigo-schema-commit
Are you sure you want to change the base?
Conversation
abbe163 to
9aa118c
Compare
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.
Pull request overview
This PR introduces a new AccountSchemaCommitment component that enables accounts to expose their storage schema commitments on-chain. The component stores a cryptographic commitment to the account's storage schemas in a designated storage slot and provides a procedure to retrieve this commitment.
Changes:
- Added new
AccountSchemaCommitmentcomponent to compute and store deterministic storage schema commitments - Implemented schema commitment computation with lexicographic ordering for order-independence
- Added MASM procedure
get_schema_commitmentto retrieve the schema commitment from account storage
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/miden-standards/src/account/mod.rs | Exports the new metadata module |
| crates/miden-standards/src/account/metadata/mod.rs | Implements AccountSchemaCommitment component with schema commitment computation |
| crates/miden-standards/src/account/components/mod.rs | Adds storage_schema_library() function to access the metadata library |
| crates/miden-standards/asm/account_components/metadata/storage_schema.masm | Implements get_schema_commitment MASM procedure |
| CHANGELOG.md | Documents the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,21 @@ | |||
| # TODO: docs | |||
Copilot
AI
Jan 13, 2026
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.
Missing documentation for this module. Add a header comment explaining the purpose of this module and the storage_schema component.
| # TODO: docs | |
| # METADATA STORAGE SCHEMA COMPONENT | |
| # ------------------------------------------------------------------------------------------------- | |
| # This module defines the metadata storage_schema component for accounts. It exposes the storage | |
| # slot at which an account stores a commitment to its storage schema, and provides a helper | |
| # procedure to load that commitment from the active account's storage. | |
| # | |
| # The SCHEMA_COMMITMENT_SLOT constant identifies the word in the account's storage layout that | |
| # contains the storage schema commitment. The get_schema_commitment procedure reads this slot | |
| # via active_account::get_item and returns the stored commitment on the stack. |
crates/miden-standards/asm/account_components/metadata/storage_schema.masm
Outdated
Show resolved
Hide resolved
crates/miden-standards/asm/account_components/metadata/storage_schema.masm
Show resolved
Hide resolved
crates/miden-standards/asm/account_components/metadata/storage_schema.masm
Outdated
Show resolved
Hide resolved
mmagician
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.
Whether for now or for another PR, TBD, but it would be useful to also have an option (perhaps default to true) to set AccountBuilder::with_schema(boolean) to indicate whether this component should be built automatically at the time of account creation.
As discussed here, it might be a little more tricky than originally envisioned because at the time when we'd call AccountBuilder::with_schema(boolean), I think we've lost the metadata already.
But, we should have some way (with account builder or otherwise) to conveniently create this component, otherwise I don't see much value in doing all the pre-requisite steps in #2104.
As I mentioned in the second paragraph of this comment, you are already working with components at that point, which do not have metadata associated directly (you might have gotten the component by building it yourself, without |
|
Created #2269 with a possible approach cc @mmagician, left a couple of caveats in the description |
crates/miden-standards/asm/account_components/metadata/schema_commitment.masm
Show resolved
Hide resolved
bobbinth
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.
Looks good! Thank you! I left a couple of comments inline - the main one is about the methodology for combining schemas.
| fn compute_schema_commitment(schemas: &[StorageSchema]) -> Word { | ||
| if schemas.is_empty() { | ||
| return Word::empty(); | ||
| } | ||
|
|
||
| let mut commitments: Vec<Word> = schemas.iter().map(StorageSchema::commitment).collect(); | ||
| commitments.sort_by(|a, b| LexicographicWord::new(*a).cmp(&LexicographicWord::new(*b))); | ||
|
|
||
| let mut bytes = Vec::with_capacity(commitments.len() * Word::SERIALIZED_SIZE); | ||
| for commitment in commitments.iter() { | ||
| commitment.write_into(&mut bytes); | ||
| } | ||
|
|
||
| let elements = bytes_to_elements_with_padding(&bytes); | ||
| Hasher::hash_elements(&elements) | ||
| } |
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.
I was imagining this working slightly differently:
- We'd first merge schemas together into a single schema.
- Then, we'd compute a commitment to this schema.
The above approach would be a bit "semantically robust". And I think it shouldn't be too difficult to implement because we already have the ability to merge schemas?
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.
I would move the code under asm/standards/metadata/storage_schema.masm and then this file would just re-export the procedure - e.g., as:
pub use ::miden::standards::metadata::storage_schema::get_schema_commitment
This is the approach we've been taking with account components recently.
Adds
AccountSchemaCommitmentcomponent, which exposes a single storage slot (namedmiden::standards::metadata::storage_schema) and contains a commitment to the schema of the account, given by the hash of ordered the list ofStorageSchemacommitments - one per component - introduced in #2244.A follow-up (#2269) was created for adding metadata to account component in a way that makes it easy to leverage on
AccountBuilder.