feat(standards): add NetworkAccount auth component#2817
feat(standards): add NetworkAccount auth component#2817partylikeits1983 wants to merge 11 commits intoajl-kernel-tx-script-rootfrom
NetworkAccount auth component#2817Conversation
Adds a new auth component intended for network-owned accounts such as the AggLayer bridge and the AggLayer faucet. Replaces the NoAuth pattern which lets any transaction emit output notes authored by the account. The auth procedure enforces two invariants: 1. Rejects the transaction if a tx script was executed, using `tx::get_script_root` from the kernel. 2. Rejects the transaction if any consumed input note's script root is not present in the whitelist stored at a well-known storage slot. If both checks pass, the nonce is incremented when the account state changed or the account is new, matching NoAuth/SingleSig behavior. The whitelist is a StorageMap keyed by script root (with a sentinel value marking presence) and is fixed at account creation. A mutable whitelist is a possible future improvement. - new network_account.masm auth component - new NetworkAccount Rust struct with From<NetworkAccount> for AccountComponent - register library in components::mod and extend StandardAccountComponent - extend AccountComponentInterface and AccountInterfaceExt with AuthNetworkAccount variant (maps to AuthMethod::NoAuth for now) - unit tests for the Rust builder - integration tests in tests/auth/network_account.rs covering tx-script-reject, non-whitelisted-note-reject, and happy-path Part of #2814. Depends on #2813 (merged via #2816). Related to the #2797 fix.
NetworkAccount auth component
Drop the stale AggLayer-faucet reference in the doc comment (the component is used by network faucets more generally) and rewrite the WHITELIST_SENTINEL comment to explain why a non-empty marker value is needed (storage maps use the empty word as the "key absent" marker). Also shorten the fully qualified miden_protocol::Felt path to use a direct Felt import, matching the pattern used elsewhere in the crate.
…work_account.masm Document the post-stack state after the if.true/end block that conditionally increments the nonce, matching the commenting style used by the rest of the procedure.
…Account Update the auth call site and drop the stale CHANGELOG bullet left behind by the merge of `ajl-kernel-tx-script-root`.
… example accounts Reword the doc-comment example from "AggLayer bridge and a network faucet" to "network faucets and the AggLayer bridge" to foreground the general case over the specific deployment. Expand the WHITELIST_SENTINEL comment to define the term "sentinel value" and explain why we call the constant one: we only check that the stored value differs from the empty word, so the contents carry no information on their own.
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good! I took a high level look.
As mentioned in #2797 (comment), I would suggest renaming this from the more general NetworkAccount to something like NoteScriptAllowlistAuth.
Also, I suggest renaming whitelist to allowlist.
| /// Builds a minimal account that uses the [`NetworkAccount`] auth component with the provided | ||
| /// whitelist of input-note script roots. | ||
| fn build_network_account(allowed_script_roots: Vec<Word>) -> anyhow::Result<Account> { | ||
| Ok(AccountBuilder::new([0; 32]) | ||
| .with_auth_component(NetworkAccount::new(allowed_script_roots)) | ||
| .with_component(BasicWallet) | ||
| .account_type(AccountType::RegularAccountUpdatableCode) | ||
| .storage_mode(AccountStorageMode::Public) | ||
| .build_existing()?) | ||
| } |
There was a problem hiding this comment.
To me, this seems a bit misleading because the function name suggest we build a "network account", but, at least currently, network accounts are defined as accounts with AccountStorageMode::Network, but this one is public.
| static WHITELIST_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| { | ||
| StorageSlotName::new("miden::standards::auth::network_account::whitelist") | ||
| .expect("storage slot name should be valid") | ||
| }); |
There was a problem hiding this comment.
Nit: I'd call this "allow list", which is a bit more consistent with "block list" (used in other contexts #2819).
|
|
||
| /// Creates a new [`NetworkAccount`] component with the provided list of allowed input-note | ||
| /// script roots. | ||
| pub fn new(allowed_script_roots: Vec<Word>) -> Self { |
There was a problem hiding this comment.
This makes me wish we had a NoteScriptRoot newtype wrapper like AccountProcedureRoot for more type-safety. Maybe we should create an issue to introduce this?
| ( | ||
| Self::whitelist_slot().clone(), | ||
| StorageSlotSchema::map( | ||
| "Allowed input-note script roots", |
There was a problem hiding this comment.
| "Allowed input-note script roots", | |
| "Allowed input note script roots", |
| #! Inputs: [pad(16)] | ||
| #! Outputs: [pad(16)] | ||
| #! | ||
| #! Invocation: call | ||
| @auth_script | ||
| pub proc auth_tx_network_account |
There was a problem hiding this comment.
This takes auth args as inputs, right? If so, I'd explicitly drop them here for safety.
| exec.tx::get_tx_script_root | ||
| # => [TX_SCRIPT_ROOT, pad(16)] | ||
|
|
||
| padw exec.word::eq | ||
| # => [no_tx_script, pad(16)] |
There was a problem hiding this comment.
| exec.tx::get_tx_script_root | |
| # => [TX_SCRIPT_ROOT, pad(16)] | |
| padw exec.word::eq | |
| # => [no_tx_script, pad(16)] | |
| exec.tx::get_tx_script_root | |
| # => [TX_SCRIPT_ROOT, pad(16)] | |
| exec.word::eqz | |
| # => [has_no_tx_script, pad(16)] |
| # => [i, pad(16)] | ||
| sub.1 | ||
| # => [i-1, pad(16)] |
There was a problem hiding this comment.
| # => [i, pad(16)] | |
| sub.1 | |
| # => [i-1, pad(16)] | |
| # => [note_idx+1, pad(16)] | |
| sub.1 | |
| # => [note_idx, pad(16)] |
Nit: We could rename i to note_idx for more clarity.
| exec.active_account::get_map_item | ||
| # => [VALUE, i-1, pad(16)] | ||
|
|
||
| padw exec.word::eq not |
There was a problem hiding this comment.
| padw exec.word::eq not | |
| exec.word::eqz not |
Nit
Resolves: #2814
Summary
Adds a new auth component intended for network-owned accounts (AggLayer bridge, AggLayer faucet). Replaces the
NoAuthpattern which lets any transaction emit output notes authored by the account and thus forge bridge-authoredMINTnotes (see #2797).The auth procedure enforces two invariants:
tx::get_script_rootfrom the kernel (added in feat(kernel): expose tx script root via public kernel procedure #2816).If both checks pass, the nonce is incremented when the account state changed or the account is new, matching the
NoAuth/SingleSigbehavior.Changes
asm/account_components/auth/network_account.masm— new auth procedureauth_tx_network_account.src/account/auth/network_account.rs—NetworkAccountRust struct withFrom<NetworkAccount> for AccountComponentand unit tests.src/account/components/mod.rs— registersNETWORK_ACCOUNT_LIBRARY, addsnetwork_account_library(), extendsStandardAccountComponentwithAuthNetworkAccount.src/account/interface/component.rs,extension.rs— extendAccountComponentInterfacewithAuthNetworkAccount(maps toAuthMethod::NoAuthfor now; a distinctAuthMethodvariant can be added later if needed).tests/auth/network_account.rs— integration tests covering: tx-script rejection, non-whitelisted-note rejection, and the happy path.Storage layout
miden::standards::auth::network_account::whitelist.Word). Values: sentinel[1, 0, 0, 0]marking presence.Stacking
This PR is stacked on #2816 (PR 1 of the #2797 fix chain). It is the second PR of three; the AggLayer wire-up in #2815 will land on top.
Closes #2814. Related to #2797.