feat: extend `InitStorageData` and allow passing native structs #2230

igamigo · 2026-01-06T04:17:42Z

Closes #1860
Also simplifies/removes some code

mmagician

Just to make sure my understanding is correct, the title "allow passing native structs" refers to allowing native Miden value types (Word, Felt) to be passed directly into InitStorageData, instead of requiring parsed WordValues? (basically 1. from @partylikeits1983's proposal)

I haven't done a full review of the fine details, but on a high level there might be a chance of simplifying the structs a bit, see my comment below

crates/miden-protocol/src/account/component/storage/init_storage_data.rs

mmagician

I'm wondering when exactly do we utilize InitStorageData::new() in practice?
I imagine that typically the init storage data will be provided via a TOML file, i.e. using InitStorageData::from_toml constructor.

cc @partylikeits1983 as this PR addresses #1860

Other than this question, the changes LGTM ✅
I added a few unit tests to cover the missing error variants.

crates/miden-protocol/src/account/component/metadata/mod.rs

crates/miden-protocol/src/account/component/storage/toml/tests.rs

mmagician · 2026-01-13T11:20:15Z

crates/miden-protocol/src/account/component/storage/init_storage_data.rs

+    /// Inserts a single map entry.
+    ///
+    /// See [`Self::insert_value`] for examples of supported types for `key` and `value`.
+    pub fn insert_map_entry(
+        &mut self,
+        slot_name: StorageSlotName,
+        key: impl Into<WordValue>,
+        value: impl Into<WordValue>,
+    ) {
+        self.map_entries.entry(slot_name).or_default().push((key.into(), value.into()));


should this return a Result instead, and reject if duplicates are inserted?

mmagician · 2026-01-13T16:27:58Z

Regarding my previous question, I was just genuinely wondering about the different constructors. Because by removing the new() constructor, the value of converting from native structs becomes much less important AFAICS?

igamigo · 2026-01-13T16:49:51Z

Regarding my previous question, I was just genuinely wondering about the different constructors. Because by removing the new() constructor, the value of converting from native structs becomes much less important AFAICS?

I had thought of this as well. Before this PR, the main advantage of having something like InitStorageData was that it provided a way to pick up defaults from TOML and then validate those values against a schema. With #1860, the idea is that it also becomes usable with "native"/domain structs. This lends itself to one of two ways of using the struct:

InitStorageData::from_toml() -> maybe mutate it? -> use it to instantiate a component.
InitStorageData::default() -> insert enough values -> instantiate a component.

So I agree with the fact that it maybe decreases the value of converting from native structs, but also the mutators added in this PR are hopefully ergonomic enough that it's not a nuisance to do this pattern instead of preparing a list of values to pass to InitStorageData::new().

bobbinth

Looks good! Thank you! I left some comments inline - most of them can be addressed in a follow-up.

crates/miden-protocol/src/account/component/storage/init_storage_data.rs

bobbinth · 2026-01-16T01:02:30Z

crates/miden-protocol/src/account/component/storage/init_storage_data.rs

+    pub fn insert_value(
+        &mut self,
+        name: StorageValueName,
+        value: impl Into<WordValue>,
+    ) -> Result<(), InitStorageDataError> {


Maybe not for this PR, but would it make sense to also define the type for the name parameter as impl TryInto<StorageValueName>? This way, it may be possible to write something like:

data.insert_value("foo::bar.baz", Felt::from(42_u32));

Also, maybe we should implement more conversions for WordValue - e.g., From<u32>, From<[u32; 4]> etc. Or if there is a way somehow to do a blanket implementation so that anything that can be converted into word can be converted into WordValue, that would be even better - but not sure that's possible.

bobbinth · 2026-01-16T01:26:47Z

crates/miden-protocol/src/account/component/storage/init_storage_data.rs

+    pub fn insert_map_entry(
+        &mut self,
+        slot_name: StorageSlotName,
+        key: impl Into<WordValue>,
+        value: impl Into<WordValue>,
+    ) -> Result<(), InitStorageDataError> {


Same comment re potentially making slot_name be impl TryInto<StorageSlotName>.

bobbinth · 2026-01-16T01:27:32Z

crates/miden-protocol/src/account/component/storage/init_storage_data.rs

+    pub fn set_map_values(
+        &mut self,
+        slot_name: StorageSlotName,
+        entries: Vec<(WordValue, WordValue)>,
+    ) -> Result<(), InitStorageDataError> {


Same comment re slot_name as above.

crates/miden-protocol/src/account/component/storage/init_storage_data.rs

crates/miden-protocol/src/account/component/storage/toml/init_storage_data.rs

crates/miden-protocol/src/account/component/storage/schema.rs

bobbinth · 2026-01-16T01:41:09Z

crates/miden-protocol/src/account/component/storage/schema.rs

 // TESTS
 // ================================================================================================


Maybe not for this PR, but I'd move the tests into a separate file as this file is already quite large.

Or another option could be to split the file up into logical components (e.g., map slot schema, value slot schema) under src/account/component/storage/schema directory.

bobbinth

Looks good! Thank you! I'll merge this as soon as CI passes. Could you create an issue with the remaining follow-ups?

igamigo force-pushed the igamigo-init-refactor branch 2 times, most recently from 0fe55a6 to 537805b Compare January 7, 2026 22:10

feat: Extend InitStorageData and allow passing native structs

98c040f

igamigo force-pushed the igamigo-init-refactor branch from 537805b to 98c040f Compare January 7, 2026 22:14

igamigo marked this pull request as ready for review January 7, 2026 22:14

igamigo added 2 commits January 7, 2026 19:14

Merge branch 'next' into igamigo-init-refactor

636f727

chore: re-add refactored comments

c1c7f8f

igamigo changed the title ~~feat: Extend InitStorageData and allow passing native structs~~ feat: extend InitStorageData and allow passing native structs Jan 7, 2026

igamigo requested a review from PhilippGackstatter January 8, 2026 15:18

igamigo assigned partylikeits1983 Jan 8, 2026

igamigo requested a review from partylikeits1983 January 8, 2026 15:20

bobbinth requested review from bobbinth and mmagician and removed request for PhilippGackstatter January 8, 2026 16:20

mmagician reviewed Jan 9, 2026

View reviewed changes

crates/miden-protocol/src/account/component/storage/init_storage_data.rs Outdated Show resolved Hide resolved

igamigo and others added 6 commits January 9, 2026 17:16

review: refactor and simplify structs

a4250a7

chore: lints

935e521

chore: move serde code

012de90

chore: unit test for ExcessiveNesting

8de6882

unit test for InvalidMapEntryKey

b6180d2

chore: unit test for InvalidMapEntrySchema

5b1f9ed

mmagician approved these changes Jan 13, 2026

View reviewed changes

crates/miden-protocol/src/account/component/metadata/mod.rs Outdated Show resolved Hide resolved

crates/miden-protocol/src/account/component/storage/toml/tests.rs Show resolved Hide resolved

mmagician added 2 commits January 13, 2026 11:13

chore: unit test for EmptyTable & InvalidStorageValueName

e1a3fd3

Merge branch 'next' into igamigo-init-refactor

3438219

mmagician reviewed Jan 13, 2026

View reviewed changes

igamigo added 2 commits January 13, 2026 11:48

reviews: amend comment

a4844ab

reviews: remove initstoragedata::new

2a145c1

Merge branch 'next' into igamigo-init-refactor

ac6e3c0

bobbinth approved these changes Jan 16, 2026

View reviewed changes

igamigo and others added 3 commits January 16, 2026 11:44

reviews: comments, move code around, conflict on duplicates when parsing

fde5efd

chore: merge

8d47859

Merge branch 'next' into igamigo-init-refactor

743fcb2

bobbinth approved these changes Jan 16, 2026

View reviewed changes

bobbinth merged commit 60baeda into next Jan 16, 2026
17 checks passed

bobbinth deleted the igamigo-init-refactor branch January 16, 2026 17:46

bobbinth mentioned this pull request Jan 16, 2026

feat: StorageSchema::commitment #2244

Merged

		// TESTS
		// ================================================================================================

feat: extend InitStorageData and allow passing native structs #2230

feat: extend InitStorageData and allow passing native structs #2230

Uh oh!

Conversation

igamigo commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mmagician Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

mmagician commented Jan 13, 2026

Uh oh!

igamigo commented Jan 13, 2026

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bobbinth Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

bobbinth Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

bobbinth Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bobbinth Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

feat: extend `InitStorageData` and allow passing native structs #2230

feat: extend `InitStorageData` and allow passing native structs #2230

igamigo commented Jan 6, 2026 •

edited

Loading