Skip to content

refactor: move storage schema component into a separate file#2603

Open
bobbinth wants to merge 2 commits intonextfrom
bobbin-account-schema
Open

refactor: move storage schema component into a separate file#2603
bobbinth wants to merge 2 commits intonextfrom
bobbin-account-schema

Conversation

@bobbinth
Copy link
Contributor

This PR moves the AccountSchemaComponent into a separate module. The main intent here is to make the diff in #2439 smaller, but it also fixes a couple of things in the component (see inline comments).

Comment on lines -104 to -110
static STORAGE_SCHEMA_LIBRARY: LazyLock<Library> = LazyLock::new(|| {
let bytes = include_bytes!(concat!(
env!("OUT_DIR"),
"/assets/account_components/metadata/schema_commitment.masl"
));
Library::read_from_bytes(bytes).expect("Shipped Storage Schema library is well-formed")
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should start moving these into their corresponding module, and eventually, try to get rid of the StandardAccountComponent (I may be forgetting something, but it is not clear to me why we need it).

Comment on lines +61 to +63
/// Name of the component is set to match the path of the corresponding module in the standards
/// library.
const NAME: &str = "miden::standards::metadata::storage_schema";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the name of the component to be the same as its module path in the standards library.

Comment on lines +36 to +39
static SCHEMA_COMMITMENT_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| {
StorageSlotName::new("miden::standards::metadata::storage_schema::commitment")
.expect("storage slot name should be valid")
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, changed the name of the storage slot to make it consistent with the name of the component.

Comment on lines +92 to +103
let storage_schema = StorageSchema::new([(
Self::schema_commitment_slot().clone(),
StorageSlotSchema::value(
"Commitment to the storage schema of an account",
WordSchema::new_simple(SchemaType::native_word()),
),
)])
.expect("storage schema should be valid");

AccountComponentMetadata::new(Self::NAME, AccountType::all())
.with_description("Component exposing the account storage schema commitment")
.with_storage_schema(storage_schema)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, we were building AccountComponentMetadata without storage schema. I didn't remember if this was intentional, but couldn't come up with a good reason why this component shouldn't have storage schema. cc @igamigo in case I'm missing something.

Comment on lines +115 to +116
AccountComponent::new(STORAGE_SCHEMA_LIBRARY.clone(), storage, metadata)
.expect("AccountSchemaCommitment is a valid account component")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like AccountComponent::new() does not verify consistency between storage and metadata. @igamigo - is this intentional? If not, let's create an issue and fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant