feat: implement API changes for named storage slots

[PhilippGackstatter]

Implements the user-facing API changes for named storage slots, building on top of #1987.

This is a big PR, because it refactors a core part of accounts and as a result, the PR needs to touch many corners of the codebase. The only significant part that was deactivated instead of refactored in this PR are AccountComponentTemplates, which will be dealt with in a separate PR.

Also, the majority of changes in this PR are simple: They replace a slot index with a slot name. See the "PR Review" section for what a good focus for a review might be.

Choices

• I made the choice to use unique slot names for the public keys of the RPO Falcon 512 family of auth components. It does mean, that code can not abstract over those by using the same slot name, but if those components provide their slot name, it is still possible to abstract over them. This can be revisited of course, but it's probably best left for another PR.
• Slot names are generally lazy statics instead of consts. The rationale for this is essentially the outcome of the discussion here. So, in a follow-up PR, I think it would be best to remove SlotName::from_static_str and leave only the non-const new constructor. The const stuff is pretty sad to remove again, but I now think it's an overall better approach. There are quite a few places where we use the Ord implementation of SlotName, e.g. in every BTreeMap<SlotName, _> and currently this recomputes the SlotNameId many times.

PR Review

I think there are really only two bigger areas that need a detailed review:

• The find_storage_slot API is used in a lot of places now, and it returning a slot_ptr caused refactorings to storage-related APIs, e.g. set_map_item, set_item, get_item, etc. This also meant changes to the transaction host event handlers. These would be good to review in detail.
  • The changes to AccountStorageHeader, mostly as a way to support the transaction host to react to events.
• The changes to the account delta.
  • The delta still accesses slots by index, as it iterates over storage, and so it requires index-based storage APIs in $kernel::account.
  • The delta commitment computation has changed. It previously included the slot_idx but now includes the slot name ID.
    • In hindsight, this may not actually have been necessary, because slot indices still uniquely identify a given slot, due to sorting by slot name ID. However, once we support insertion of new slots, we'd have to carry along the slot name anyway, and therefore also want to commit to it, because we need to be able to insert that slot into the AccountStorage. So, I think this makes the delta architecture more consistent with account storage, but I still realized this too late, so this could maybe have been a separate PR - sorry about that.
  • A good focus here are the changes to AccountStorageDelta::append_delta_elements. I implemented a minimal way to deal with the necessary changes.
    • As a potential, but not necessary follow-up, we can consider refactoring AccountStorageDelta so it contains a single BTreeMap<SlotName, StorageSlotDelta>, but as always, this comes with trade-offs.

Notes on Changes

• Removes test_account_code_procedure_offset_out_of_bounds because this will be removed in a follow-up PR together with offset and storage size of account procedures and fixing the test would've been non-trivial.
  • In general, some storage offset related tests were already removed, even though the offset code still exists for now. As the code will be removed in a future PR anyway, this seems fine.
• At some point we discussed that it would be nice if the account delta's commitment format and serialization format would be identical. With named storage slots, this is no longer possible because the slot name must be serialized to be reconstructed during deserialization, but the commitment commitment must use the slot name ID, because the slot name itself cannot be accessed in MASM.
• impl TryFrom<Package> for AccountComponentTemplate was moved unchanged to a more appropriate place. That made deactivating the template code easier.

Follow-Ups to this PR

• In general, address remaining TODO(named_slots).
• Enforce slot name uniqueness in the tx kernel during account creation.
• Refactor AccountComponentMetadata (Implement Named Storage Slots #1724 (comment))
• Remove SlotName::from_static_str (priority, to avoid stabilizing an API we'll remove anyway).
  • Make SlotNameId part of SlotName to avoid recomputing it in various cases (feat: implement API changes for named storage slots #2025 (comment))
• Remove account procedure offset and storage_size. Account component storage is no longer virtualized like before, and so this can go away in another PR.
• Test for error ERR_ACCOUNT_UNKNOWN_STORAGE_SLOT_NAME.
• Renames
  • StorageSlot -> StorageSlotContent (feat: implement API changes for named storage slots #2025 (comment))
  • NamedStorageSlot -> StorageSlot (https://github.com/0xMiden/miden-base/pull/
    1987#discussion_r2575545109)
  • SlotNameId -> StorageSlotId (feat: Implement named storage slots in the transaction kernel #1987 (comment)).
  • name_id_prefix -> slot_id_prefix (same for suffix) (feat: implement API changes for named storage slots #2025 (comment))
  • Make NEW_VALUE and OLD_MAP_VALUE consistent in set_map_item feat: implement API changes for named storage slots #2025 (comment).
  • Potentially rename "current storage slot section" to "latest" (feat: implement API changes for named storage slots #2025 (comment)).
• Add back a test that checks that accessing the reserved faucet storage slots fails. The existing one was basically removed as part of removing the procedure offset tests.
  • On second thought, since we'll likely remove this privileged slot, we can probably skip that one.
• Update get_kernel_mem_element to use updated Memory::read_element feat: implement API changes for named storage slots #2025 (comment)
• Error out on duplicate slot names in account storage header (feat: implement API changes for named storage slots #2025 (comment))
• Drop the sorting requirement for named storage slots (feat: implement API changes for named storage slots #2025 (comment))
  • Use linear search instead; check duplicates via set/map; simplifies storage upgrades in future.
• Open issue for discussing "design patterns" for named storage slots, e.g.:
  • Should similar components like basic and network fungible faucet use a unified slot name for metadata?
  • Should it be allowed to merge components with duplicate slots (as long as content is identical)?
• Update the account storage docs to explain named slots.
  • Include design patterns / recommendations in docs, too.
  • As part of this, checking the codebase for storage slot index references and updating them.
• Do not mix "tracked" and "trigger" terminology in ACL auth components (feat: implement API changes for named storage slots #2025 (comment))

part of #1724

[mmagician]

Minor comments left, otherwise LGTM ✅
I think overall this is a much needed feature, and the PRs do a great job at implementing the necessary changes as well as showing, via examples, how to use it.

LFG 🔥

[mmagician]

for another PR: I don't think this example is meaningful anymore, since we're extremely unlikely to get name_id_suffix = 0, name_id_prefix = 0.
Which makes me wonder whether we can simplify a bit and remove the domain element completely, and just use the name_id_{prefix/suffix} - it effectively functions as a domain separator

[PhilippGackstatter]

Thanks for the comment! The kernel only deals with slot IDs, and technically users are free to use prefix = 0 and suffix = 0 as their slot ID. They wouldn't be able to use our Rust APIs, but from a protocol perspective, it's allowed. Because of that, we still need the domain separators afaict.

This made me realize a potential problem with slot names not being committed to by the kernel, though not sure it's a problem at this stage.

Main points:

• We have a SlotName(String) from which we derive a SlotId([Felt; 2]) using BLAKE3.
• The transaction kernel only deals with slot IDs; it doesn't know about slot names at all.
• All Rust APIs use a SlotName, e.g. the account storage or the account delta. They all assume the slot name is available.
• Relevant when decentralizing Miden, but, afaict, a state update to L1 (Ethereum) in our case would basically consist of:
  • The block proof.
  • The public inputs/outputs of the block kernel (needed for proof verification).
  • The chain data required to reconstruct the new Miden state (e.g., accumulated account deltas, new notes, etc.).
    • This is what other nodes would use to construct the new public state of the chain.
• The only way to guarantee that the preimage of a slot ID (= the slot name) is available, is by committing to it through the chain data. Since the delta commits only to the slot ID, we currently don't have that guarantee.
• Without this, Rust nodes assuming the slot name is available/provided would crash or go out of sync.
• Solutions
  • L1 verifier contract needs to ensure that the slot name for each slot ID is provided. This means we can deal with this later (as in after mainnet).
  • Commit to the slot name via the kernel somehow. This means we need to deal with it now (as in before mainnet).
• I think the same is relevant for the account code in the delta, where only the MAST root is committed to, but the actual code would not be guaranteed to be available without further validation, e.g. on L1.

I believe we can go the L1 verifier route, but would be good to get another perspective on this.

[Fumuran]

Looks great, thank you! I left just a few little comments.

[Fumuran]

We are using push.SLOT_NAME[0..2] quite often, so I'm wondering should we create a new syntax for the slot names, which will work similar to the word(...) or event(...), but will hold just two first elements of the resulting hash. That way it will be more clear what in fact this constant is holding.

Probably it's an overkill, this feature will require some changes in the VM, but could be nice to have something like that.

[PhilippGackstatter]

I kind of agree, yes. I think it would already be sufficient to be able to define a sliced constant:

const APPROVER_PUBLIC_KEYS_SLOT = word("miden::standards::auth::ecdsa_k256_keccak_multisig::approver_public_keys")[0..2]

And then you could simply do:

push.APPROVER_PUBLIC_KEYS_SLOT

But not sure it's worth adding.

[bobbinth]

Looks good! Thank you!

Is the plan to merge this and #1987 into next? Or do we want to do the remaining work in stacked PRs?

If the latter, I would probably merge this and #1987 into a tracking branch so that the PR stack doesn't get too deep.

[PhilippGackstatter]

Is the plan to merge this and #1987 into next? Or do we want to do the remaining work in stacked PRs?

I think merging them now makes sense since that's the bulk of the work and avoids the feature branch getting out of sync too much.