refactor: move initial-state getters to native_account#3218
Open
Fumuran wants to merge 4 commits into
Open
Conversation
Move get_initial_commitment, get_initial_storage_commitment, get_initial_vault_root, get_initial_item, get_initial_map_item, get_initial_asset and get_initial_balance from the active_account module to native_account. These procedures operate on the transaction's native account, so they now assert the active account is native (via memory::assert_native_account) and panic with ERR_ACCOUNT_IS_NOT_NATIVE when invoked from a foreign procedure invocation (FPI) context. Removes the is_native_account conditional logic from the kernel get_initial_* procedures and the now-unused branching pointer helpers get_account_initial_storage_slots_ptr and get_account_initial_vault_root_ptr. Updates all standards/auth callers, the mock account code, kernel and FPI tests, protocol library docs and the changelog.
Fumuran
commented
Jul 7, 2026
- Re-export get_initial_commitment/storage_commitment/vault_root as pub use aliases of the underlying memory procedures instead of one-line wrappers. - Remove the redundant 'assumed to be invoked after asserting native' doc comments in account.masm. - Restore the foreign-account read tests to verify correct values via the active_account current getters (get_item/get_map_item/get_balance), since foreign accounts are immutable. - Note in protocol_library.md that foreign accounts are immutable and their initial values should be read via active_account procedures. - Shorten the changelog entry to reference get_initial_*.
…gorize-initial-getters
Fumuran
changed the title
native_account
PhilippGackstatter
approved these changes
Jul 8, 2026
PhilippGackstatter
left a comment
PhilippGackstatter
left a comment
Contributor
There was a problem hiding this comment.
Looks good!
Did not re-review the procedures in crates/miden-protocol/asm/protocol/src/native_account.masm in detail, assuming they are copies.
Comment on lines
-870
to
+874
| async fn foreign_account_get_initial_balance() -> anyhow::Result<()> { | ||
| async fn foreign_account_get_balance() -> anyhow::Result<()> { |
Contributor
There was a problem hiding this comment.
Does this test anything that foreign_account_can_get_balance_and_presence_of_asset does not already test? If not, I would remove foreign_account_get_balance.
Comment on lines
-1889
to
+1897
| async fn test_get_initial_item_and_get_initial_map_item_with_foreign_account() -> anyhow::Result<()> | ||
| { | ||
| async fn test_get_item_and_get_map_item_with_foreign_account() -> anyhow::Result<()> { |
Contributor
There was a problem hiding this comment.
Seems like get_item and get_map_item are also covered by other tests (e.g. test_fpi_memory_single_account), so does this test add anything additional? If not, I'd remove it or repurpose it for some other interesting test case.
mmagician
approved these changes
Jul 8, 2026
mmagician
left a comment
mmagician
left a comment
Collaborator
There was a problem hiding this comment.
LGTM modulo the now-incorrect docs
One more general comment, unrelated to this PR: I wonder if executing account would be a better name than native account. I remember the naming had confused me quite a bit in the past: native to what?
Comment on lines
+76
to
+82
| | `get_initial_commitment` | Returns the native account commitment at the beginning of the transaction.<br/><br/>**Inputs:** `[]`<br/>**Outputs:** `[INIT_COMMITMENT]` | Any | | ||
| | `get_initial_storage_commitment` | Returns the storage commitment of the native account at the beginning of the transaction.<br/><br/>**Inputs:** `[]`<br/>**Outputs:** `[INIT_STORAGE_COMMITMENT]` | Any | | ||
| | `get_initial_item` | Gets the initial item from the native account storage slot as it was at the beginning of the transaction.<br/><br/>**Inputs:** `[slot_id_suffix, slot_id_prefix]`<br/>**Outputs:** `[VALUE]` | Account | | ||
| | `get_initial_map_item` | Gets the initial VALUE from the native account storage map as it was at the beginning of the transaction.<br/><br/>**Inputs:** `[slot_id_suffix, slot_id_prefix, KEY]`<br/>**Outputs:** `[VALUE]` | Account | | ||
| | `get_initial_asset` | Returns the asset associated with the provided asset ID in the native account's vault at the beginning of the transaction.<br/><br/>**Inputs:** `[ASSET_ID]`<br/>**Outputs:** `[ASSET_VALUE]` | Any | | ||
| | `get_initial_balance` | Returns the balance of the fungible asset associated with the provided faucet_id in the native account's vault at the beginning of the transaction.<br/><br/>**Inputs:** `[faucet_id_suffix, faucet_id_prefix]`<br/>**Outputs:** `[init_balance]` | Any | | ||
| | `get_initial_vault_root` | Returns the vault root of the native account at the beginning of the transaction.<br/><br/>**Inputs:** `[]`<br/>**Outputs:** `[INIT_VAULT_ROOT]` | Any | |
Collaborator
There was a problem hiding this comment.
the context should be updated - "Any" no longer holds
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2034.
Recategorizes the seven initial-state account getters from
miden::protocol::active_accounttomiden::protocol::native_account. Because the concept of an "initial" (beginning-of-transaction) state is only meaningful for the transaction's native account, these procedures now always operate on the native account and panic withERR_ACCOUNT_IS_NOT_NATIVEwhen invoked from a foreign procedure invocation (FPI) context, rather than silently branching on native vs. foreign.Moved procedures:
get_initial_commitmentget_initial_storage_commitmentget_initial_vault_rootget_initial_itemget_initial_map_itemget_initial_assetget_initial_balanceChanges
active_account.masmintonative_account.masm(with the required offset/asset imports).api.masm): addedexec.memory::assert_native_accountto the six initial-getter procs, matching the existing native-only guard pattern (e.g.account_incr_nonce,account_set_item).account.masm): removed theis_native_accountbranches from the commitment / storage-commitment / vault-root getters, and switched the item / map-item / asset getters (andget_item_patch) to native-only pointers.memory.masm: removed the now-unused branching helpersget_account_initial_storage_slots_ptrandget_account_initial_vault_root_ptr.singlesig,singlesig_acl,multisig,multisig_smart,signature,note_/tx_script_allowlist,no_auth,network_account) and the mock account code tonative_account::, fixinguseimports per file.test_account.rs; rewrote the two FPI tests so foreign-context calls now assertERR_ACCOUNT_IS_NOT_NATIVE.protocol_library.mdand added a[BREAKING]changelog entry.Design note
Per the discussion, FPI invocation of these getters is now a hard error (via
memory::assert_native_account). This technically adds an assert rather than purely removing conditional logic, but it is consistent with how other native-only procedures (set_item,incr_nonce,add_asset) are already guarded, and it removes the dual-behavior branches the issue called out.Testing
cargo build -p miden-protocol -p miden-standards(compiles the MASM kernel and libraries).cargo test -p miden-testing: 30test_accounttests, 12test_fpitests (including two new error tests), and 81 auth integration tests — all passing.🤖 Generated with Claude Code