From 70fc91a60dec49dae73b1c4d9ab323ea3b49748c Mon Sep 17 00:00:00 2001 From: Arthur Abeilice Date: Wed, 4 Mar 2026 12:01:24 -0300 Subject: [PATCH 1/6] feat: implement two-step ownership management for account components - Add `ownable2step.masm` to provide two-step ownership functionality, requiring explicit acceptance of ownership transfers. - Create Rust module for `Ownable2Step` struct to manage ownership state and storage layout. - Introduce error handling for ownership operations in `standards.rs`. - Develop comprehensive tests for ownership transfer, acceptance, and renouncement scenarios in `ownable2step.rs`. --- .../asm/standards/access/ownable2step.masm | 372 +++++++++++++ .../miden-standards/src/account/access/mod.rs | 3 + .../src/account/access/ownable2step.rs | 149 +++++ .../tests/scripts/ownable2step.rs | 515 ++++++++++++++++++ 4 files changed, 1039 insertions(+) create mode 100644 crates/miden-standards/asm/standards/access/ownable2step.masm create mode 100644 crates/miden-standards/src/account/access/mod.rs create mode 100644 crates/miden-standards/src/account/access/ownable2step.rs create mode 100644 crates/miden-testing/tests/scripts/ownable2step.rs diff --git a/crates/miden-standards/asm/standards/access/ownable2step.masm b/crates/miden-standards/asm/standards/access/ownable2step.masm new file mode 100644 index 0000000000..2de11ef0e5 --- /dev/null +++ b/crates/miden-standards/asm/standards/access/ownable2step.masm @@ -0,0 +1,372 @@ +# miden::standards::access::ownable2step +# +# Provides two-step ownership management functionality for account components. +# This module can be imported and used by any component that needs owner controls. +# +# Unlike a single-step ownership transfer, this module requires the new owner to explicitly +# accept the transfer before it takes effect. This prevents accidental transfers to incorrect +# addresses, which would permanently lock the component. +# +# The transfer flow is: +# 1. The current owner calls `transfer_ownership` to nominate a new owner. +# 2. The nominated account calls `accept_ownership` to complete the transfer. +# 3. Optionally, the current owner can call `transfer_ownership` with their own address +# to cancel the nominated transfer. +# +# Storage layout (single slot): +# Rust Word: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] +# word[0] word[1] word[2] word[3] +# +# After get_item (big-endian load), the stack is: +# [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +# (word[3]) (word[2]) (word[1]) (word[0]) + +use miden::protocol::active_account +use miden::protocol::account_id +use miden::protocol::active_note +use miden::protocol::native_account + +# CONSTANTS +# ================================================================================================ + +# Ownership config value representing renounced ownership (all zeros). +const RENOUNCED_OWNERSHIP_CONFIG = [0, 0, 0, 0] + +# The slot in this component's storage layout where the owner configuration is stored. +# Contains both the current owner and the nominated owner in a single word. +const OWNER_CONFIG_SLOT = word("miden::standards::access::ownable2step::owner_config") + +# ERRORS +# ================================================================================================ + +const ERR_SENDER_NOT_OWNER = "note sender is not the owner" +const ERR_SENDER_NOT_NOMINATED_OWNER = "note sender is not the nominated owner" +const ERR_NO_NOMINATED_OWNER = "no nominated ownership transfer exists" + +# LOCAL MEMORY ADDRESSES +# ================================================================================================ + +# transfer_ownership locals +const NEW_OWNER_PREFIX_LOC = 0 +const NEW_OWNER_SUFFIX_LOC = 1 +const OWNER_PREFIX_LOC = 2 +const OWNER_SUFFIX_LOC = 3 + +# INTERNAL PROCEDURES +# ================================================================================================ + +#! Returns the full ownership word from storage. +#! +#! Inputs: [] +#! Outputs: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +#! +#! Where: +#! - owner_prefix and owner_suffix are the prefix and suffix felts of the +#! current owner account ID. +#! - nominated_owner_prefix and nominated_owner_suffix are the prefix and suffix +#! felts of the nominated owner account ID. +proc load_ownership_info + push.OWNER_CONFIG_SLOT[0..2] exec.active_account::get_item + # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +end + +#! Writes the ownership word to storage and drops the old value. +#! +#! Inputs: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +#! Outputs: [] +proc save_ownership_info + push.OWNER_CONFIG_SLOT[0..2] + # => [slot_prefix, slot_suffix, owner_prefix, owner_suffix, + # nominated_owner_prefix, nominated_owner_suffix] + + exec.native_account::set_item + # => [OLD_OWNERSHIP_WORD] + + dropw + # => [] +end + +#! Returns the owner account ID from storage. +#! +#! Inputs: [] +#! Outputs: [owner_prefix, owner_suffix] +#! +#! Where: +#! - owner_prefix and owner_suffix are the prefix and suffix felts of the owner account ID. +proc get_owner_internal + exec.load_ownership_info + # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] + + movup.2 drop + # => [owner_prefix, owner_suffix, nominated_owner_suffix] + + movup.2 drop + # => [owner_prefix, owner_suffix] +end + +#! Returns the nominated owner account ID from storage. +#! +#! Inputs: [] +#! Outputs: [nominated_owner_prefix, nominated_owner_suffix] +#! +#! Where: +#! - nominated_owner_prefix and nominated_owner_suffix are the prefix and suffix +#! felts of the nominated owner account ID. +proc get_nominated_owner_internal + exec.load_ownership_info + # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] + + drop drop + # => [nominated_owner_prefix, nominated_owner_suffix] +end + +#! Checks if the given account ID is the owner of this component. +#! +#! Inputs: [account_id_prefix, account_id_suffix] +#! Outputs: [is_owner] +#! +#! Where: +#! - is_owner is 1 if the account is the owner, 0 otherwise. +proc is_owner_internal + exec.get_owner_internal + # => [owner_prefix, owner_suffix, account_id_prefix, account_id_suffix] + + exec.account_id::is_equal + # => [is_owner] +end + +#! Checks if the given account ID is the nominated owner of this component. +#! +#! Inputs: [account_id_prefix, account_id_suffix] +#! Outputs: [is_nominated_owner] +#! +#! Where: +#! - account_id_prefix and account_id_suffix are the prefix and suffix felts +#! of the account ID to check. +#! - is_nominated_owner is 1 if the account is the nominated owner, 0 otherwise. +proc is_nominated_owner_internal + exec.get_nominated_owner_internal + # => [nominated_owner_prefix, nominated_owner_suffix, account_id_prefix, account_id_suffix] + + exec.account_id::is_equal + # => [is_nominated_owner] +end + +#! Checks if the note sender is the owner and panics if not. +#! +#! Inputs: [] +#! Outputs: [] +#! +#! Panics if: +#! - the note sender is not the owner. +#! +#! Invocation: exec +proc assert_sender_is_owner_internal + exec.active_note::get_sender + # => [sender_prefix, sender_suffix] + + exec.is_owner_internal + # => [is_owner] + + assert.err=ERR_SENDER_NOT_OWNER + # => [] +end + +# PUBLIC INTERFACE +# ================================================================================================ + +#! Checks if the note sender is the owner and panics if not. +#! +#! Inputs: [pad(16)] +#! Outputs: [pad(16)] +#! +#! Panics if: +#! - the note sender is not the owner. +#! +#! Invocation: call +pub proc assert_sender_is_owner + exec.assert_sender_is_owner_internal + # => [pad(16)] +end + +#! Returns the owner account ID. +#! +#! Inputs: [pad(16)] +#! Outputs: [owner_prefix, owner_suffix, pad(14)] +#! +#! Where: +#! - owner_prefix and owner_suffix are the prefix and suffix felts of the owner account ID. +#! +#! Invocation: call +pub proc get_owner + exec.get_owner_internal + # => [owner_prefix, owner_suffix, pad(16)] + + movup.2 drop movup.2 drop + # => [owner_prefix, owner_suffix, pad(14)] +end + +#! Returns the nominated owner account ID. +#! +#! Inputs: [pad(16)] +#! Outputs: [nominated_owner_prefix, nominated_owner_suffix, pad(14)] +#! +#! Where: +#! - nominated_owner_prefix and nominated_owner_suffix are the prefix and suffix +#! felts of the nominated owner account ID. Both are zero if no nominated transfer +#! exists. +#! +#! Invocation: call +pub proc get_nominated_owner + exec.get_nominated_owner_internal + # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + + movup.2 drop movup.2 drop + # => [nominated_owner_prefix, nominated_owner_suffix, pad(14)] +end + +#! Initiates a two-step ownership transfer by setting the nominated owner. +#! +#! The current owner remains in control until the nominated owner calls `accept_ownership`. +#! Can only be called by the current owner. +#! +#! If the new owner is the current owner, any nominated transfer is cancelled and the +#! nominated owner field is cleared. +#! +#! Inputs: [new_owner_prefix, new_owner_suffix, pad(14)] +#! Outputs: [pad(16)] +#! +#! Panics if: +#! - the note sender is not the owner. +#! +#! Locals: +#! 0: new_owner_prefix +#! 1: new_owner_suffix +#! 2: owner_prefix +#! 3: owner_suffix +#! +#! Invocation: call +@locals(4) +pub proc transfer_ownership + exec.assert_sender_is_owner_internal + # => [new_owner_prefix, new_owner_suffix, pad(14)] + + dup.1 dup.1 exec.account_id::validate + # => [new_owner_prefix, new_owner_suffix, pad(14)] + + loc_store.NEW_OWNER_PREFIX_LOC + # => [new_owner_suffix, pad(15)] + + loc_store.NEW_OWNER_SUFFIX_LOC + # => [pad(16)] + + exec.get_owner_internal + # => [owner_prefix, owner_suffix, pad(16)] + + loc_store.OWNER_PREFIX_LOC + # => [owner_suffix, pad(15)] + + loc_store.OWNER_SUFFIX_LOC + # => [pad(16)] + + # Check if new_owner == owner (cancel case). + loc_load.NEW_OWNER_SUFFIX_LOC loc_load.NEW_OWNER_PREFIX_LOC + # => [new_owner_prefix, new_owner_suffix, pad(16)] + + loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC + # => [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix, pad(16)] + + exec.account_id::is_equal + # => [is_self_transfer, pad(16)] + + if.true + # Cancel ownership transfer and clear nominated owner. + loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC + # => [owner_prefix, owner_suffix, pad(16)] + + push.0.0 + # => [0, 0, owner_prefix, owner_suffix, pad(16)] + + movup.3 movup.3 + # => [owner_prefix, owner_suffix, 0, 0, pad(16)] + else + # Transfer ownership by setting nominated = new_owner. + loc_load.NEW_OWNER_SUFFIX_LOC loc_load.NEW_OWNER_PREFIX_LOC + # => [new_owner_prefix, new_owner_suffix, pad(16)] + + loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC + # => [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix, pad(16)] + end + + exec.save_ownership_info + # => [pad(16)] +end + +#! Accepts a nominated ownership transfer. The nominated owner becomes the new owner +#! and the nominated owner field is cleared. +#! +#! Can only be called by the nominated owner. +#! +#! Inputs: [pad(16)] +#! Outputs: [pad(16)] +#! +#! Panics if: +#! - there is no nominated ownership transfer (nominated owner is zero). +#! - the note sender is not the nominated owner. +#! +#! Invocation: call +pub proc accept_ownership + exec.get_nominated_owner_internal + # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + + # Check that a nominated transfer exists (nominated owner is not zero). + dup.1 eq.0 dup.1 eq.0 and + # => [is_zero, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + + assertz.err=ERR_NO_NOMINATED_OWNER + # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + + exec.active_note::get_sender + # => [sender_prefix, sender_suffix, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + + dup.3 dup.3 + exec.account_id::is_equal + # => [is_sender_nominated_owner, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + + assert.err=ERR_SENDER_NOT_NOMINATED_OWNER + # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + + # Build new ownership word: nominated becomes owner, clear nominated. + push.0 movdn.2 push.0 movdn.2 + # => [nominated_owner_prefix, nominated_owner_suffix, 0, 0, pad(16)] + + exec.save_ownership_info + # => [pad(16)] +end + +#! Renounces ownership, leaving the component without an owner. +#! +#! Can only be called by the current owner. Clears both the owner and any nominated owner. +#! +#! Important Note! +#! This feature allows the owner to relinquish administrative privileges, a common pattern +#! after an initial stage with centralized administration is over. Once ownership is renounced, +#! the component becomes permanently ownerless and cannot be managed by any account. +#! +#! Inputs: [pad(16)] +#! Outputs: [pad(16)] +#! +#! Panics if: +#! - the note sender is not the owner. +#! +#! Invocation: call +pub proc renounce_ownership + exec.assert_sender_is_owner_internal + # => [pad(16)] + + push.RENOUNCED_OWNERSHIP_CONFIG + # => [0, 0, 0, 0, pad(16)] + + exec.save_ownership_info + # => [pad(16)] +end diff --git a/crates/miden-standards/src/account/access/mod.rs b/crates/miden-standards/src/account/access/mod.rs new file mode 100644 index 0000000000..6e14ad58bf --- /dev/null +++ b/crates/miden-standards/src/account/access/mod.rs @@ -0,0 +1,3 @@ +pub mod ownable2step; + +pub use ownable2step::{Ownable2Step, Ownable2StepError}; diff --git a/crates/miden-standards/src/account/access/ownable2step.rs b/crates/miden-standards/src/account/access/ownable2step.rs new file mode 100644 index 0000000000..70b03283b2 --- /dev/null +++ b/crates/miden-standards/src/account/access/ownable2step.rs @@ -0,0 +1,149 @@ +use miden_protocol::account::component::{FeltSchema, StorageSlotSchema}; +use miden_protocol::account::{AccountId, AccountStorage, StorageSlot, StorageSlotName}; +use miden_protocol::errors::AccountIdError; +use miden_protocol::utils::sync::LazyLock; +use miden_protocol::{Felt, FieldElement, Word}; + +static OWNER_CONFIG_SLOT_NAME: LazyLock = LazyLock::new(|| { + StorageSlotName::new("miden::standards::access::ownable2step::owner_config") + .expect("storage slot name should be valid") +}); + +/// Two-step ownership management for account components. +/// +/// This struct holds the current owner and any nominated (pending) owner. A nominated owner +/// must explicitly accept the transfer before it takes effect, preventing accidental transfers +/// to incorrect addresses. +/// +/// ## Storage Layout +/// +/// The ownership data is stored in a single word: +/// +/// ```text +/// Rust Word: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] +/// word[0] word[1] word[2] word[3] +/// ``` +pub struct Ownable2Step { + owner: Option, + nominated_owner: Option, +} + +impl Ownable2Step { + // CONSTRUCTORS + // -------------------------------------------------------------------------------------------- + + /// Creates a new [`Ownable2Step`] with the given owner and no nominated owner. + pub fn new(owner: AccountId) -> Self { + Self { + owner: Some(owner), + nominated_owner: None, + } + } + + /// Reads ownership data from account storage, validating any non-zero account IDs. + /// + /// Returns an error if either owner or nominated owner contains an invalid (but non-zero) + /// account ID. + pub fn try_from_storage(storage: &AccountStorage) -> Result { + let word: Word = storage + .get_item(Self::slot_name()) + .map_err(Ownable2StepError::StorageLookupFailed)?; + + Self::try_from_word(word) + } + + /// Reconstructs an [`Ownable2Step`] from a raw storage word. + /// + /// Format: `[nominated_suffix, nominated_prefix, owner_suffix, owner_prefix]` + pub fn try_from_word(word: Word) -> Result { + let owner = account_id_from_felt_pair(word[3], word[2]) + .map_err(Ownable2StepError::InvalidOwnerId)?; + + let nominated_owner = account_id_from_felt_pair(word[1], word[0]) + .map_err(Ownable2StepError::InvalidNominatedOwnerId)?; + + Ok(Self { owner, nominated_owner }) + } + + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// Returns the [`StorageSlotName`] where ownership data is stored. + pub fn slot_name() -> &'static StorageSlotName { + &OWNER_CONFIG_SLOT_NAME + } + + /// Returns the storage slot schema for the ownership configuration slot. + pub fn slot_schema() -> (StorageSlotName, StorageSlotSchema) { + ( + Self::slot_name().clone(), + StorageSlotSchema::value( + "Ownership data (owner and nominated owner)", + [ + FeltSchema::felt("nominated_suffix"), + FeltSchema::felt("nominated_prefix"), + FeltSchema::felt("owner_suffix"), + FeltSchema::felt("owner_prefix"), + ], + ), + ) + } + + /// Returns the current owner, or `None` if ownership has been renounced. + pub fn owner(&self) -> Option { + self.owner + } + + /// Returns the nominated owner, or `None` if no transfer is in progress. + pub fn nominated_owner(&self) -> Option { + self.nominated_owner + } + + /// Converts this ownership data into a [`StorageSlot`]. + pub fn to_storage_slot(&self) -> StorageSlot { + StorageSlot::with_value(Self::slot_name().clone(), self.to_word()) + } + + /// Converts this ownership data into a raw [`Word`]. + pub fn to_word(&self) -> Word { + let (owner_prefix, owner_suffix) = match self.owner { + Some(id) => (id.prefix().as_felt(), id.suffix()), + None => (Felt::ZERO, Felt::ZERO), + }; + let (nominated_prefix, nominated_suffix) = match self.nominated_owner { + Some(id) => (id.prefix().as_felt(), id.suffix()), + None => (Felt::ZERO, Felt::ZERO), + }; + [nominated_suffix, nominated_prefix, owner_suffix, owner_prefix].into() + } +} + +// OWNABLE2STEP ERROR +// ================================================================================================ + +/// Errors that can occur when reading [`Ownable2Step`] data from storage. +#[derive(Debug, thiserror::Error)] +pub enum Ownable2StepError { + #[error("failed to read ownership slot from storage")] + StorageLookupFailed(#[source] miden_protocol::errors::AccountError), + #[error("invalid owner account ID in storage")] + InvalidOwnerId(#[source] AccountIdError), + #[error("invalid nominated owner account ID in storage")] + InvalidNominatedOwnerId(#[source] AccountIdError), +} + +// HELPERS +// ================================================================================================ + +/// Constructs an `Option` from a prefix/suffix felt pair. +/// Returns `Ok(None)` when both felts are zero (renounced / no nomination). +fn account_id_from_felt_pair( + prefix: Felt, + suffix: Felt, +) -> Result, AccountIdError> { + if prefix == Felt::ZERO && suffix == Felt::ZERO { + Ok(None) + } else { + AccountId::try_from([prefix, suffix]).map(Some) + } +} diff --git a/crates/miden-testing/tests/scripts/ownable2step.rs b/crates/miden-testing/tests/scripts/ownable2step.rs new file mode 100644 index 0000000000..d5d6a1147c --- /dev/null +++ b/crates/miden-testing/tests/scripts/ownable2step.rs @@ -0,0 +1,515 @@ +extern crate alloc; + +use alloc::sync::Arc; + +use miden_processor::crypto::RpoRandomCoin; +use miden_protocol::account::component::AccountComponentMetadata; +use miden_protocol::account::{ + Account, + AccountBuilder, + AccountComponent, + AccountId, + AccountStorageMode, + StorageSlot, + StorageSlotName, +}; +use miden_protocol::assembly::DefaultSourceManager; +use miden_protocol::assembly::debuginfo::SourceManagerSync; +use miden_protocol::note::Note; +use miden_protocol::testing::account_id::AccountIdBuilder; +use miden_protocol::transaction::OutputNote; +use miden_protocol::utils::sync::LazyLock; +use miden_protocol::{Felt, FieldElement, Word}; +use miden_standards::code_builder::CodeBuilder; +use miden_standards::errors::standards::{ + ERR_NO_NOMINATED_OWNER, + ERR_SENDER_NOT_NOMINATED_OWNER, + ERR_SENDER_NOT_OWNER, +}; +use miden_standards::testing::note::NoteBuilder; +use miden_testing::{Auth, MockChain, assert_transaction_executor_error}; + +static OWNER_CONFIG_SLOT_NAME: LazyLock = LazyLock::new(|| { + StorageSlotName::new("miden::standards::access::ownable2step::owner_config") + .expect("storage slot name should be valid") +}); + +// HELPERS +// ================================================================================================ + +fn create_ownable_account( + owner: AccountId, + initial_storage: Vec, +) -> anyhow::Result { + let component_code = r#" + use miden::standards::access::ownable2step + pub use ownable2step::get_owner + pub use ownable2step::get_nominated_owner + pub use ownable2step::transfer_ownership + pub use ownable2step::accept_ownership + pub use ownable2step::renounce_ownership + "#; + let component_code_obj = + CodeBuilder::default().compile_component_code("test::ownable", component_code)?; + + let ownership_word: Word = [ + Felt::ZERO, // word[0] → stack[3] = nominated_suffix + Felt::ZERO, // word[1] → stack[2] = nominated_prefix + owner.suffix(), // word[2] → stack[1] = owner_suffix + owner.prefix().as_felt(), // word[3] → stack[0] = owner_prefix + ] + .into(); + + let mut storage_slots = initial_storage; + storage_slots.push(StorageSlot::with_value(OWNER_CONFIG_SLOT_NAME.clone(), ownership_word)); + + let account = AccountBuilder::new([1; 32]) + .storage_mode(AccountStorageMode::Public) + .with_auth_component(Auth::IncrNonce) + .with_component({ + let metadata = AccountComponentMetadata::new("test::ownable").with_supports_all_types(); + AccountComponent::new(component_code_obj, storage_slots, metadata)? + }) + .build_existing()?; + Ok(account) +} + +fn get_owner_from_storage(account: &Account) -> anyhow::Result> { + let word = account.storage().get_item(&OWNER_CONFIG_SLOT_NAME)?; + let prefix = word[3]; + let suffix = word[2]; + if prefix == Felt::ZERO && suffix == Felt::ZERO { + Ok(None) + } else { + Ok(Some(AccountId::try_from([prefix, suffix])?)) + } +} + +fn get_nominated_owner_from_storage(account: &Account) -> anyhow::Result> { + let word = account.storage().get_item(&OWNER_CONFIG_SLOT_NAME)?; + let prefix = word[1]; + let suffix = word[0]; + if prefix == Felt::ZERO && suffix == Felt::ZERO { + Ok(None) + } else { + Ok(Some(AccountId::try_from([prefix, suffix])?)) + } +} + +fn create_transfer_note( + sender: AccountId, + new_owner: AccountId, + rng: &mut RpoRandomCoin, + source_manager: Arc, +) -> anyhow::Result { + let script = format!( + r#" + use miden::standards::access::ownable2step->test_account + begin + repeat.14 push.0 end + push.{new_owner_suffix} push.{new_owner_prefix} + call.test_account::transfer_ownership + dropw dropw dropw dropw + end + "#, + new_owner_suffix = new_owner.suffix(), + new_owner_prefix = new_owner.prefix().as_felt(), + ); + + let note = NoteBuilder::new(sender, rng) + .source_manager(source_manager) + .code(script) + .build()?; + + Ok(note) +} + +fn create_accept_note( + sender: AccountId, + rng: &mut RpoRandomCoin, + source_manager: Arc, +) -> anyhow::Result { + let script = r#" + use miden::standards::access::ownable2step->test_account + begin + repeat.16 push.0 end + call.test_account::accept_ownership + dropw dropw dropw dropw + end + "#; + + let note = NoteBuilder::new(sender, rng) + .source_manager(source_manager) + .code(script) + .build()?; + + Ok(note) +} + +fn create_renounce_note( + sender: AccountId, + rng: &mut RpoRandomCoin, + source_manager: Arc, +) -> anyhow::Result { + let script = r#" + use miden::standards::access::ownable2step->test_account + begin + repeat.16 push.0 end + call.test_account::renounce_ownership + dropw dropw dropw dropw + end + "#; + + let note = NoteBuilder::new(sender, rng) + .source_manager(source_manager) + .code(script) + .build()?; + + Ok(note) +} + +// TESTS +// ================================================================================================ + +#[tokio::test] +async fn test_transfer_ownership_only_owner() -> anyhow::Result<()> { + let owner = AccountIdBuilder::new().build_with_seed([1; 32]); + let non_owner = AccountIdBuilder::new().build_with_seed([2; 32]); + let new_owner = AccountIdBuilder::new().build_with_seed([3; 32]); + + let account = create_ownable_account(owner, vec![])?; + let mut builder = MockChain::builder(); + builder.add_account(account.clone())?; + + let source_manager: Arc = Arc::new(DefaultSourceManager::default()); + let mut rng = RpoRandomCoin::new([Felt::from(100u32); 4].into()); + let note = create_transfer_note(non_owner, new_owner, &mut rng, Arc::clone(&source_manager))?; + + builder.add_output_note(OutputNote::Full(note.clone())); + let mut mock_chain = builder.build()?; + mock_chain.prove_next_block()?; + + let tx = mock_chain + .build_tx_context(account.id(), &[note.id()], &[])? + .with_source_manager(source_manager) + .build()?; + let result = tx.execute().await; + + assert_transaction_executor_error!(result, ERR_SENDER_NOT_OWNER); + Ok(()) +} + +#[tokio::test] +async fn test_complete_ownership_transfer() -> anyhow::Result<()> { + let owner = AccountIdBuilder::new().build_with_seed([1; 32]); + let new_owner = AccountIdBuilder::new().build_with_seed([2; 32]); + + let account = create_ownable_account(owner, vec![])?; + + // Step 1: transfer ownership + let mut builder = MockChain::builder(); + builder.add_account(account.clone())?; + + let source_manager: Arc = Arc::new(DefaultSourceManager::default()); + let mut rng = RpoRandomCoin::new([Felt::from(100u32); 4].into()); + let transfer_note = + create_transfer_note(owner, new_owner, &mut rng, Arc::clone(&source_manager))?; + + builder.add_output_note(OutputNote::Full(transfer_note.clone())); + let mut mock_chain = builder.build()?; + mock_chain.prove_next_block()?; + + let tx = mock_chain + .build_tx_context(account.id(), &[transfer_note.id()], &[])? + .with_source_manager(Arc::clone(&source_manager)) + .build()?; + let executed = tx.execute().await?; + + let mut updated = account.clone(); + updated.apply_delta(executed.account_delta())?; + + // Verify intermediate state: owner unchanged, nominated set + assert_eq!(get_owner_from_storage(&updated)?, Some(owner)); + assert_eq!(get_nominated_owner_from_storage(&updated)?, Some(new_owner)); + + // Commit step 1 to the chain + mock_chain.add_pending_executed_transaction(&executed)?; + mock_chain.prove_next_block()?; + + // Step 2: accept ownership + let mut rng2 = RpoRandomCoin::new([Felt::from(200u32); 4].into()); + let accept_note = create_accept_note(new_owner, &mut rng2, Arc::clone(&source_manager))?; + + let tx2 = mock_chain + .build_tx_context(updated.clone(), &[], std::slice::from_ref(&accept_note))? + .with_source_manager(source_manager) + .build()?; + let executed2 = tx2.execute().await?; + + let mut final_account = updated.clone(); + final_account.apply_delta(executed2.account_delta())?; + + assert_eq!(get_owner_from_storage(&final_account)?, Some(new_owner)); + assert_eq!(get_nominated_owner_from_storage(&final_account)?, None); + Ok(()) +} + +#[tokio::test] +async fn test_accept_ownership_only_nominated_owner() -> anyhow::Result<()> { + let owner = AccountIdBuilder::new().build_with_seed([1; 32]); + let new_owner = AccountIdBuilder::new().build_with_seed([2; 32]); + let wrong = AccountIdBuilder::new().build_with_seed([3; 32]); + + let account = create_ownable_account(owner, vec![])?; + + // Step 1: transfer + let mut builder = MockChain::builder(); + builder.add_account(account.clone())?; + + let source_manager: Arc = Arc::new(DefaultSourceManager::default()); + let mut rng = RpoRandomCoin::new([Felt::from(100u32); 4].into()); + let transfer_note = + create_transfer_note(owner, new_owner, &mut rng, Arc::clone(&source_manager))?; + + builder.add_output_note(OutputNote::Full(transfer_note.clone())); + let mut mock_chain = builder.build()?; + mock_chain.prove_next_block()?; + + let tx = mock_chain + .build_tx_context(account.id(), &[transfer_note.id()], &[])? + .with_source_manager(Arc::clone(&source_manager)) + .build()?; + let executed = tx.execute().await?; + + let mut updated = account.clone(); + updated.apply_delta(executed.account_delta())?; + + // Commit step 1 to the chain + mock_chain.add_pending_executed_transaction(&executed)?; + mock_chain.prove_next_block()?; + + // Step 2: wrong account tries accept + let mut rng2 = RpoRandomCoin::new([Felt::from(200u32); 4].into()); + let accept_note = create_accept_note(wrong, &mut rng2, Arc::clone(&source_manager))?; + + let tx2 = mock_chain + .build_tx_context(updated.clone(), &[], std::slice::from_ref(&accept_note))? + .with_source_manager(source_manager) + .build()?; + let result = tx2.execute().await; + + assert_transaction_executor_error!(result, ERR_SENDER_NOT_NOMINATED_OWNER); + Ok(()) +} + +#[tokio::test] +async fn test_accept_ownership_no_nominated() -> anyhow::Result<()> { + let owner = AccountIdBuilder::new().build_with_seed([1; 32]); + + let account = create_ownable_account(owner, vec![])?; + let mut builder = MockChain::builder(); + builder.add_account(account.clone())?; + + let source_manager: Arc = Arc::new(DefaultSourceManager::default()); + let mut rng = RpoRandomCoin::new([Felt::from(200u32); 4].into()); + let accept_note = create_accept_note(owner, &mut rng, Arc::clone(&source_manager))?; + + builder.add_output_note(OutputNote::Full(accept_note.clone())); + let mut mock_chain = builder.build()?; + mock_chain.prove_next_block()?; + + let tx = mock_chain + .build_tx_context(account.id(), &[accept_note.id()], &[])? + .with_source_manager(source_manager) + .build()?; + let result = tx.execute().await; + + assert_transaction_executor_error!(result, ERR_NO_NOMINATED_OWNER); + Ok(()) +} + +#[tokio::test] +async fn test_cancel_transfer() -> anyhow::Result<()> { + let owner = AccountIdBuilder::new().build_with_seed([1; 32]); + let new_owner = AccountIdBuilder::new().build_with_seed([2; 32]); + + let account = create_ownable_account(owner, vec![])?; + + // Step 1: transfer + let mut builder = MockChain::builder(); + builder.add_account(account.clone())?; + + let source_manager: Arc = Arc::new(DefaultSourceManager::default()); + let mut rng = RpoRandomCoin::new([Felt::from(100u32); 4].into()); + let transfer_note = + create_transfer_note(owner, new_owner, &mut rng, Arc::clone(&source_manager))?; + + builder.add_output_note(OutputNote::Full(transfer_note.clone())); + let mut mock_chain = builder.build()?; + mock_chain.prove_next_block()?; + + let tx = mock_chain + .build_tx_context(account.id(), &[transfer_note.id()], &[])? + .with_source_manager(Arc::clone(&source_manager)) + .build()?; + let executed = tx.execute().await?; + + let mut updated = account.clone(); + updated.apply_delta(executed.account_delta())?; + + // Commit step 1 to the chain + mock_chain.add_pending_executed_transaction(&executed)?; + mock_chain.prove_next_block()?; + + // Step 2: cancel by transferring to self (owner) + let mut rng2 = RpoRandomCoin::new([Felt::from(200u32); 4].into()); + let cancel_note = create_transfer_note(owner, owner, &mut rng2, Arc::clone(&source_manager))?; + + let tx2 = mock_chain + .build_tx_context(updated.clone(), &[], std::slice::from_ref(&cancel_note))? + .with_source_manager(source_manager) + .build()?; + let executed2 = tx2.execute().await?; + + let mut final_account = updated.clone(); + final_account.apply_delta(executed2.account_delta())?; + + assert_eq!(get_nominated_owner_from_storage(&final_account)?, None); + assert_eq!(get_owner_from_storage(&final_account)?, Some(owner)); + Ok(()) +} + +/// Tests that an owner can transfer to themselves when no nominated transfer exists. +/// This is a no-op but should succeed without errors. +#[tokio::test] +async fn test_transfer_to_self_no_nominated() -> anyhow::Result<()> { + let owner = AccountIdBuilder::new().build_with_seed([1; 32]); + + let account = create_ownable_account(owner, vec![])?; + let mut builder = MockChain::builder(); + builder.add_account(account.clone())?; + + let source_manager: Arc = Arc::new(DefaultSourceManager::default()); + let mut rng = RpoRandomCoin::new([Felt::from(100u32); 4].into()); + let note = create_transfer_note(owner, owner, &mut rng, Arc::clone(&source_manager))?; + + builder.add_output_note(OutputNote::Full(note.clone())); + let mut mock_chain = builder.build()?; + mock_chain.prove_next_block()?; + + let tx = mock_chain + .build_tx_context(account.id(), &[note.id()], &[])? + .with_source_manager(source_manager) + .build()?; + let executed = tx.execute().await?; + + let mut updated = account.clone(); + updated.apply_delta(executed.account_delta())?; + + assert_eq!(get_owner_from_storage(&updated)?, Some(owner)); + assert_eq!(get_nominated_owner_from_storage(&updated)?, None); + Ok(()) +} + +#[tokio::test] +async fn test_renounce_ownership() -> anyhow::Result<()> { + let owner = AccountIdBuilder::new().build_with_seed([1; 32]); + let new_owner = AccountIdBuilder::new().build_with_seed([2; 32]); + + let account = create_ownable_account(owner, vec![])?; + + // Step 1: transfer (to have nominated) + let mut builder = MockChain::builder(); + builder.add_account(account.clone())?; + + let source_manager: Arc = Arc::new(DefaultSourceManager::default()); + let mut rng = RpoRandomCoin::new([Felt::from(100u32); 4].into()); + let transfer_note = + create_transfer_note(owner, new_owner, &mut rng, Arc::clone(&source_manager))?; + + builder.add_output_note(OutputNote::Full(transfer_note.clone())); + let mut mock_chain = builder.build()?; + mock_chain.prove_next_block()?; + + let tx = mock_chain + .build_tx_context(account.id(), &[transfer_note.id()], &[])? + .with_source_manager(Arc::clone(&source_manager)) + .build()?; + let executed = tx.execute().await?; + + let mut updated = account.clone(); + updated.apply_delta(executed.account_delta())?; + + // Commit step 1 to the chain + mock_chain.add_pending_executed_transaction(&executed)?; + mock_chain.prove_next_block()?; + + // Step 2: renounce + let mut rng2 = RpoRandomCoin::new([Felt::from(200u32); 4].into()); + let renounce_note = create_renounce_note(owner, &mut rng2, Arc::clone(&source_manager))?; + + let tx2 = mock_chain + .build_tx_context(updated.clone(), &[], std::slice::from_ref(&renounce_note))? + .with_source_manager(source_manager) + .build()?; + let executed2 = tx2.execute().await?; + + let mut final_account = updated.clone(); + final_account.apply_delta(executed2.account_delta())?; + + assert_eq!(get_owner_from_storage(&final_account)?, None); + assert_eq!(get_nominated_owner_from_storage(&final_account)?, None); + Ok(()) +} + +/// Tests that transfer_ownership fails when the new owner account ID is invalid. +/// An invalid account ID has its suffix's lower 8 bits set to a non-zero value. +#[tokio::test] +async fn test_transfer_ownership_fails_with_invalid_account_id() -> anyhow::Result<()> { + use miden_protocol::errors::protocol::ERR_ACCOUNT_ID_SUFFIX_LEAST_SIGNIFICANT_BYTE_MUST_BE_ZERO; + + let owner = AccountIdBuilder::new().build_with_seed([1; 32]); + + let account = create_ownable_account(owner, vec![])?; + let mut builder = MockChain::builder(); + builder.add_account(account.clone())?; + + let invalid_prefix = owner.prefix().as_felt(); + let invalid_suffix = Felt::new(1); + + let script = format!( + r#" + use miden::standards::access::ownable2step->test_account + begin + repeat.14 push.0 end + push.{invalid_suffix} + push.{invalid_prefix} + call.test_account::transfer_ownership + dropw dropw dropw dropw + end + "#, + ); + + let source_manager: Arc = Arc::new(DefaultSourceManager::default()); + let mut rng = RpoRandomCoin::new([Felt::from(100u32); 4].into()); + let note = NoteBuilder::new(owner, &mut rng) + .source_manager(Arc::clone(&source_manager)) + .code(script) + .build()?; + + builder.add_output_note(OutputNote::Full(note.clone())); + let mut mock_chain = builder.build()?; + mock_chain.prove_next_block()?; + + let tx = mock_chain + .build_tx_context(account.id(), &[note.id()], &[])? + .with_source_manager(source_manager) + .build()?; + let result = tx.execute().await; + + assert_transaction_executor_error!( + result, + ERR_ACCOUNT_ID_SUFFIX_LEAST_SIGNIFICANT_BYTE_MUST_BE_ZERO + ); + Ok(()) +} From fed9fefd4f8e2228f1f48a6b50e08a866b35d28a Mon Sep 17 00:00:00 2001 From: Arthur Abeilice Date: Wed, 4 Mar 2026 12:19:47 -0300 Subject: [PATCH 2/6] feat: add Ownable2Step account component for two-step ownership transfer --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaa00fa25f..520c0ed9d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ - Introduced `TokenMetadata` type to encapsulate fungible faucet metadata ([#2344](https://github.com/0xMiden/miden-base/issues/2344)). - Added `StandardNote::from_script_root()` and `StandardNote::name()` methods, and exposed `NoteType` `PUBLIC`/`PRIVATE` masks as public constants ([#2411](https://github.com/0xMiden/miden-base/pull/2411)). - Resolve standard note scripts directly in `TransactionExecutorHost` instead of querying the data store ([#2417](https://github.com/0xMiden/miden-base/pull/2417)). +- Added `DEFAULT_TAG` constant to `miden::standards::note_tag` MASM module ([#2482](https://github.com/0xMiden/miden-base/pull/2482)). +- Added `NoteExecutionHint` variant constants (`NONE`, `ALWAYS`, `AFTER_BLOCK`, `ON_BLOCK_SLOT`) to `miden::standards::note::execution_hint` MASM module ([#2493](https://github.com/0xMiden/miden-base/pull/2493)). +- Added `Ownable2Step` account component with two-step ownership transfer (`transfer_ownership`, `accept_ownership`) and `get_owner`, `get_pending_owner` procedures ([#2292](https://github.com/0xMiden/miden-base/pull/2292)). ### Changes From aa384095f4154ff014d9659d696a30aa645d61eb Mon Sep 17 00:00:00 2001 From: Arthur Abeilice Date: Wed, 4 Mar 2026 13:53:59 -0300 Subject: [PATCH 3/6] fix: correct ownership word layout and update transfer ownership logic in ownable2step --- .../asm/standards/access/ownable2step.masm | 173 +++++++++--------- crates/miden-testing/tests/scripts/mod.rs | 1 + .../tests/scripts/ownable2step.rs | 22 +-- 3 files changed, 97 insertions(+), 99 deletions(-) diff --git a/crates/miden-standards/asm/standards/access/ownable2step.masm b/crates/miden-standards/asm/standards/access/ownable2step.masm index 2de11ef0e5..917188cd62 100644 --- a/crates/miden-standards/asm/standards/access/ownable2step.masm +++ b/crates/miden-standards/asm/standards/access/ownable2step.masm @@ -15,11 +15,11 @@ # # Storage layout (single slot): # Rust Word: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] -# word[0] word[1] word[2] word[3] +# word[0] word[1] word[2] word[3] # -# After get_item (big-endian load), the stack is: -# [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] -# (word[3]) (word[2]) (word[1]) (word[0]) +# After get_item (little-endian load), the stack is: +# [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] +# (word[0]) (word[1]) (word[2]) (word[3]) use miden::protocol::active_account use miden::protocol::account_id @@ -47,10 +47,10 @@ const ERR_NO_NOMINATED_OWNER = "no nominated ownership transfer exists" # ================================================================================================ # transfer_ownership locals -const NEW_OWNER_PREFIX_LOC = 0 -const NEW_OWNER_SUFFIX_LOC = 1 -const OWNER_PREFIX_LOC = 2 -const OWNER_SUFFIX_LOC = 3 +const NEW_OWNER_SUFFIX_LOC = 0 +const NEW_OWNER_PREFIX_LOC = 1 +const OWNER_SUFFIX_LOC = 2 +const OWNER_PREFIX_LOC = 3 # INTERNAL PROCEDURES # ================================================================================================ @@ -58,26 +58,25 @@ const OWNER_SUFFIX_LOC = 3 #! Returns the full ownership word from storage. #! #! Inputs: [] -#! Outputs: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +#! Outputs: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] #! #! Where: -#! - owner_prefix and owner_suffix are the prefix and suffix felts of the -#! current owner account ID. -#! - nominated_owner_prefix and nominated_owner_suffix are the prefix and suffix -#! felts of the nominated owner account ID. +#! - owner_{suffix, prefix} are the suffix and prefix felts of the current owner account ID. +#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated +#! owner account ID. proc load_ownership_info push.OWNER_CONFIG_SLOT[0..2] exec.active_account::get_item - # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] + # => [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] end #! Writes the ownership word to storage and drops the old value. #! -#! Inputs: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +#! Inputs: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] #! Outputs: [] proc save_ownership_info push.OWNER_CONFIG_SLOT[0..2] - # => [slot_prefix, slot_suffix, owner_prefix, owner_suffix, - # nominated_owner_prefix, nominated_owner_suffix] + # => [slot_suffix, slot_prefix, nominated_owner_suffix, nominated_owner_prefix, + # owner_suffix, owner_prefix] exec.native_account::set_item # => [OLD_OWNERSHIP_WORD] @@ -89,47 +88,47 @@ end #! Returns the owner account ID from storage. #! #! Inputs: [] -#! Outputs: [owner_prefix, owner_suffix] +#! Outputs: [owner_suffix, owner_prefix] #! #! Where: -#! - owner_prefix and owner_suffix are the prefix and suffix felts of the owner account ID. +#! - owner_{suffix, prefix} are the suffix and prefix felts of the owner account ID. proc get_owner_internal exec.load_ownership_info - # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] + # => [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] - movup.2 drop - # => [owner_prefix, owner_suffix, nominated_owner_suffix] - - movup.2 drop - # => [owner_prefix, owner_suffix] + drop drop + # => [owner_suffix, owner_prefix] end #! Returns the nominated owner account ID from storage. #! #! Inputs: [] -#! Outputs: [nominated_owner_prefix, nominated_owner_suffix] +#! Outputs: [nominated_owner_suffix, nominated_owner_prefix] #! #! Where: -#! - nominated_owner_prefix and nominated_owner_suffix are the prefix and suffix -#! felts of the nominated owner account ID. +#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated +#! owner account ID. proc get_nominated_owner_internal exec.load_ownership_info - # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] + # => [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] - drop drop - # => [nominated_owner_prefix, nominated_owner_suffix] + movup.2 drop + # => [nominated_owner_suffix, nominated_owner_prefix, owner_prefix] + + movup.2 drop + # => [nominated_owner_suffix, nominated_owner_prefix] end #! Checks if the given account ID is the owner of this component. #! -#! Inputs: [account_id_prefix, account_id_suffix] +#! Inputs: [account_id_suffix, account_id_prefix] #! Outputs: [is_owner] #! #! Where: #! - is_owner is 1 if the account is the owner, 0 otherwise. proc is_owner_internal exec.get_owner_internal - # => [owner_prefix, owner_suffix, account_id_prefix, account_id_suffix] + # => [owner_suffix, owner_prefix, account_id_suffix, account_id_prefix] exec.account_id::is_equal # => [is_owner] @@ -137,16 +136,15 @@ end #! Checks if the given account ID is the nominated owner of this component. #! -#! Inputs: [account_id_prefix, account_id_suffix] +#! Inputs: [account_id_suffix, account_id_prefix] #! Outputs: [is_nominated_owner] #! #! Where: -#! - account_id_prefix and account_id_suffix are the prefix and suffix felts -#! of the account ID to check. +#! - account_id_{suffix, prefix} are the suffix and prefix felts of the account ID to check. #! - is_nominated_owner is 1 if the account is the nominated owner, 0 otherwise. proc is_nominated_owner_internal exec.get_nominated_owner_internal - # => [nominated_owner_prefix, nominated_owner_suffix, account_id_prefix, account_id_suffix] + # => [nominated_owner_suffix, nominated_owner_prefix, account_id_suffix, account_id_prefix] exec.account_id::is_equal # => [is_nominated_owner] @@ -163,7 +161,7 @@ end #! Invocation: exec proc assert_sender_is_owner_internal exec.active_note::get_sender - # => [sender_prefix, sender_suffix] + # => [sender_suffix, sender_prefix] exec.is_owner_internal # => [is_owner] @@ -192,37 +190,36 @@ end #! Returns the owner account ID. #! #! Inputs: [pad(16)] -#! Outputs: [owner_prefix, owner_suffix, pad(14)] +#! Outputs: [owner_suffix, owner_prefix, pad(14)] #! #! Where: -#! - owner_prefix and owner_suffix are the prefix and suffix felts of the owner account ID. +#! - owner_{suffix, prefix} are the suffix and prefix felts of the owner account ID. #! #! Invocation: call pub proc get_owner exec.get_owner_internal - # => [owner_prefix, owner_suffix, pad(16)] + # => [owner_suffix, owner_prefix, pad(16)] movup.2 drop movup.2 drop - # => [owner_prefix, owner_suffix, pad(14)] + # => [owner_suffix, owner_prefix, pad(14)] end #! Returns the nominated owner account ID. #! #! Inputs: [pad(16)] -#! Outputs: [nominated_owner_prefix, nominated_owner_suffix, pad(14)] +#! Outputs: [nominated_owner_suffix, nominated_owner_prefix, pad(14)] #! #! Where: -#! - nominated_owner_prefix and nominated_owner_suffix are the prefix and suffix -#! felts of the nominated owner account ID. Both are zero if no nominated transfer -#! exists. +#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated +#! owner account ID. Both are zero if no nominated transfer exists. #! #! Invocation: call pub proc get_nominated_owner exec.get_nominated_owner_internal - # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] movup.2 drop movup.2 drop - # => [nominated_owner_prefix, nominated_owner_suffix, pad(14)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(14)] end #! Initiates a two-step ownership transfer by setting the nominated owner. @@ -233,73 +230,72 @@ end #! If the new owner is the current owner, any nominated transfer is cancelled and the #! nominated owner field is cleared. #! -#! Inputs: [new_owner_prefix, new_owner_suffix, pad(14)] +#! Inputs: [new_owner_suffix, new_owner_prefix, pad(14)] #! Outputs: [pad(16)] #! #! Panics if: #! - the note sender is not the owner. #! #! Locals: -#! 0: new_owner_prefix -#! 1: new_owner_suffix -#! 2: owner_prefix -#! 3: owner_suffix +#! 0: new_owner_suffix +#! 1: new_owner_prefix +#! 2: owner_suffix +#! 3: owner_prefix #! #! Invocation: call @locals(4) pub proc transfer_ownership exec.assert_sender_is_owner_internal - # => [new_owner_prefix, new_owner_suffix, pad(14)] + # => [new_owner_suffix, new_owner_prefix, pad(14)] dup.1 dup.1 exec.account_id::validate - # => [new_owner_prefix, new_owner_suffix, pad(14)] - - loc_store.NEW_OWNER_PREFIX_LOC - # => [new_owner_suffix, pad(15)] + # => [new_owner_suffix, new_owner_prefix, pad(14)] loc_store.NEW_OWNER_SUFFIX_LOC - # => [pad(16)] + # => [new_owner_prefix, pad(14)] - exec.get_owner_internal - # => [owner_prefix, owner_suffix, pad(16)] + loc_store.NEW_OWNER_PREFIX_LOC + # => [pad(14)] - loc_store.OWNER_PREFIX_LOC - # => [owner_suffix, pad(15)] + exec.get_owner_internal + # => [owner_suffix, owner_prefix, pad(14)] loc_store.OWNER_SUFFIX_LOC - # => [pad(16)] + # => [owner_prefix, pad(13)] + + loc_store.OWNER_PREFIX_LOC + # => [pad(12)] # Check if new_owner == owner (cancel case). - loc_load.NEW_OWNER_SUFFIX_LOC loc_load.NEW_OWNER_PREFIX_LOC - # => [new_owner_prefix, new_owner_suffix, pad(16)] + loc_load.NEW_OWNER_PREFIX_LOC loc_load.NEW_OWNER_SUFFIX_LOC + # => [new_owner_suffix, new_owner_prefix, pad(12)] - loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC - # => [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix, pad(16)] + loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC + # => [owner_suffix, owner_prefix, new_owner_suffix, new_owner_prefix, pad(12)] exec.account_id::is_equal - # => [is_self_transfer, pad(16)] + # => [is_self_transfer, pad(12)] if.true # Cancel ownership transfer and clear nominated owner. - loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC - # => [owner_prefix, owner_suffix, pad(16)] + # Build word: [nominated_suffix=0, nominated_prefix=0, owner_suffix, owner_prefix] + loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC + # => [owner_suffix, owner_prefix, pad(12)] push.0.0 - # => [0, 0, owner_prefix, owner_suffix, pad(16)] - - movup.3 movup.3 - # => [owner_prefix, owner_suffix, 0, 0, pad(16)] + # => [0, 0, owner_suffix, owner_prefix, pad(12)] else # Transfer ownership by setting nominated = new_owner. - loc_load.NEW_OWNER_SUFFIX_LOC loc_load.NEW_OWNER_PREFIX_LOC - # => [new_owner_prefix, new_owner_suffix, pad(16)] + # Build word: [new_owner_suffix, new_owner_prefix, owner_suffix, owner_prefix] + loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC + # => [owner_suffix, owner_prefix, pad(12)] - loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC - # => [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix, pad(16)] + loc_load.NEW_OWNER_PREFIX_LOC loc_load.NEW_OWNER_SUFFIX_LOC + # => [new_owner_suffix, new_owner_prefix, owner_suffix, owner_prefix, pad(12)] end exec.save_ownership_info - # => [pad(16)] + # => [pad(12)] end #! Accepts a nominated ownership transfer. The nominated owner becomes the new owner @@ -317,28 +313,29 @@ end #! Invocation: call pub proc accept_ownership exec.get_nominated_owner_internal - # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] # Check that a nominated transfer exists (nominated owner is not zero). dup.1 eq.0 dup.1 eq.0 and - # => [is_zero, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [is_zero, nominated_owner_suffix, nominated_owner_prefix, pad(16)] assertz.err=ERR_NO_NOMINATED_OWNER - # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] exec.active_note::get_sender - # => [sender_prefix, sender_suffix, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [sender_suffix, sender_prefix, nominated_owner_suffix, nominated_owner_prefix, pad(16)] dup.3 dup.3 exec.account_id::is_equal - # => [is_sender_nominated_owner, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [is_sender_nominated_owner, nominated_owner_suffix, nominated_owner_prefix, pad(16)] assert.err=ERR_SENDER_NOT_NOMINATED_OWNER - # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] # Build new ownership word: nominated becomes owner, clear nominated. - push.0 movdn.2 push.0 movdn.2 - # => [nominated_owner_prefix, nominated_owner_suffix, 0, 0, pad(16)] + # Word: [new_nominated_suffix=0, new_nominated_prefix=0, new_owner_suffix, new_owner_prefix] + push.0.0 + # => [0, 0, nominated_owner_suffix, nominated_owner_prefix, pad(16)] exec.save_ownership_info # => [pad(16)] diff --git a/crates/miden-testing/tests/scripts/mod.rs b/crates/miden-testing/tests/scripts/mod.rs index 58bf4152ad..8d15402744 100644 --- a/crates/miden-testing/tests/scripts/mod.rs +++ b/crates/miden-testing/tests/scripts/mod.rs @@ -1,5 +1,6 @@ mod faucet; mod fee; +mod ownable2step; mod p2id; mod p2ide; mod send_note; diff --git a/crates/miden-testing/tests/scripts/ownable2step.rs b/crates/miden-testing/tests/scripts/ownable2step.rs index d5d6a1147c..df1b1857dc 100644 --- a/crates/miden-testing/tests/scripts/ownable2step.rs +++ b/crates/miden-testing/tests/scripts/ownable2step.rs @@ -2,7 +2,7 @@ extern crate alloc; use alloc::sync::Arc; -use miden_processor::crypto::RpoRandomCoin; +use miden_processor::crypto::random::RpoRandomCoin; use miden_protocol::account::component::AccountComponentMetadata; use miden_protocol::account::{ Account, @@ -19,7 +19,7 @@ use miden_protocol::note::Note; use miden_protocol::testing::account_id::AccountIdBuilder; use miden_protocol::transaction::OutputNote; use miden_protocol::utils::sync::LazyLock; -use miden_protocol::{Felt, FieldElement, Word}; +use miden_protocol::{Felt, Word}; use miden_standards::code_builder::CodeBuilder; use miden_standards::errors::standards::{ ERR_NO_NOMINATED_OWNER, @@ -53,10 +53,10 @@ fn create_ownable_account( CodeBuilder::default().compile_component_code("test::ownable", component_code)?; let ownership_word: Word = [ - Felt::ZERO, // word[0] → stack[3] = nominated_suffix - Felt::ZERO, // word[1] → stack[2] = nominated_prefix - owner.suffix(), // word[2] → stack[1] = owner_suffix - owner.prefix().as_felt(), // word[3] → stack[0] = owner_prefix + Felt::ZERO, // word[0] = nominated_suffix + Felt::ZERO, // word[1] = nominated_prefix + owner.suffix(), // word[2] = owner_suffix + owner.prefix().as_felt(), // word[3] = owner_prefix ] .into(); @@ -81,7 +81,7 @@ fn get_owner_from_storage(account: &Account) -> anyhow::Result if prefix == Felt::ZERO && suffix == Felt::ZERO { Ok(None) } else { - Ok(Some(AccountId::try_from([prefix, suffix])?)) + Ok(Some(AccountId::try_from_elements(suffix, prefix)?)) } } @@ -92,7 +92,7 @@ fn get_nominated_owner_from_storage(account: &Account) -> anyhow::Resulttest_account begin repeat.14 push.0 end - push.{new_owner_suffix} push.{new_owner_prefix} + push.{new_owner_prefix} push.{new_owner_suffix} call.test_account::transfer_ownership dropw dropw dropw dropw end "#, - new_owner_suffix = new_owner.suffix(), new_owner_prefix = new_owner.prefix().as_felt(), + new_owner_suffix = new_owner.suffix(), ); let note = NoteBuilder::new(sender, rng) @@ -482,8 +482,8 @@ async fn test_transfer_ownership_fails_with_invalid_account_id() -> anyhow::Resu use miden::standards::access::ownable2step->test_account begin repeat.14 push.0 end - push.{invalid_suffix} push.{invalid_prefix} + push.{invalid_suffix} call.test_account::transfer_ownership dropw dropw dropw dropw end From 30ff64280f747e1d266ebb3759a224aaef9add5b Mon Sep 17 00:00:00 2001 From: Arthur Abeilice Date: Fri, 6 Mar 2026 10:12:52 -0300 Subject: [PATCH 4/6] Refactor ownership management to use Ownable2Step pattern - Updated the access control mechanism to implement a two-step ownership transfer pattern in the ownable2step module. - Modified the storage layout to accommodate the new ownership structure, reversing the order of owner and nominated owner fields. - Refactored the network fungible faucet to utilize the new Ownable2Step for ownership management. - Adjusted related tests to validate the new ownership transfer logic and ensure correct behavior when transferring and accepting ownership. - Updated error handling to reflect changes in ownership management, including new error messages for ownership transfer scenarios. --- CHANGELOG.md | 2 +- .../faucets/network_fungible_faucet.masm | 3 + .../asm/standards/access/ownable2step.masm | 167 ++++++++++-------- .../standards/faucets/network_fungible.masm | 47 +---- .../src/account/access/ownable2step.rs | 9 +- .../src/account/faucets/mod.rs | 4 + .../src/account/faucets/network_fungible.rs | 100 ++++------- crates/miden-standards/src/account/mod.rs | 1 + .../miden-standards/src/errors/standards.rs | 5 + crates/miden-testing/tests/scripts/faucet.rs | 121 ++++++------- .../tests/scripts/ownable2step.rs | 22 +-- 11 files changed, 215 insertions(+), 266 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 520c0ed9d2..56874e67bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ - Resolve standard note scripts directly in `TransactionExecutorHost` instead of querying the data store ([#2417](https://github.com/0xMiden/miden-base/pull/2417)). - Added `DEFAULT_TAG` constant to `miden::standards::note_tag` MASM module ([#2482](https://github.com/0xMiden/miden-base/pull/2482)). - Added `NoteExecutionHint` variant constants (`NONE`, `ALWAYS`, `AFTER_BLOCK`, `ON_BLOCK_SLOT`) to `miden::standards::note::execution_hint` MASM module ([#2493](https://github.com/0xMiden/miden-base/pull/2493)). -- Added `Ownable2Step` account component with two-step ownership transfer (`transfer_ownership`, `accept_ownership`) and `get_owner`, `get_pending_owner` procedures ([#2292](https://github.com/0xMiden/miden-base/pull/2292)). +- Added `Ownable2Step` account component with two-step ownership transfer (`transfer_ownership`, `accept_ownership`, `renounce_ownership`) and `owner`, `nominated_owner` procedures ([#2292](https://github.com/0xMiden/miden-base/pull/2292)). ### Changes diff --git a/crates/miden-standards/asm/account_components/faucets/network_fungible_faucet.masm b/crates/miden-standards/asm/account_components/faucets/network_fungible_faucet.masm index 7d350a4224..45520f9090 100644 --- a/crates/miden-standards/asm/account_components/faucets/network_fungible_faucet.masm +++ b/crates/miden-standards/asm/account_components/faucets/network_fungible_faucet.masm @@ -4,5 +4,8 @@ pub use ::miden::standards::faucets::network_fungible::distribute pub use ::miden::standards::faucets::network_fungible::burn +pub use ::miden::standards::faucets::network_fungible::owner +pub use ::miden::standards::faucets::network_fungible::nominated_owner pub use ::miden::standards::faucets::network_fungible::transfer_ownership +pub use ::miden::standards::faucets::network_fungible::accept_ownership pub use ::miden::standards::faucets::network_fungible::renounce_ownership diff --git a/crates/miden-standards/asm/standards/access/ownable2step.masm b/crates/miden-standards/asm/standards/access/ownable2step.masm index 917188cd62..756efb9cbd 100644 --- a/crates/miden-standards/asm/standards/access/ownable2step.masm +++ b/crates/miden-standards/asm/standards/access/ownable2step.masm @@ -17,9 +17,9 @@ # Rust Word: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] # word[0] word[1] word[2] word[3] # -# After get_item (little-endian load), the stack is: -# [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] -# (word[0]) (word[1]) (word[2]) (word[3]) +# After get_item (reversed onto stack), the stack is: +# [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +# (word[3]) (word[2]) (word[1]) (word[0]) use miden::protocol::active_account use miden::protocol::account_id @@ -47,10 +47,10 @@ const ERR_NO_NOMINATED_OWNER = "no nominated ownership transfer exists" # ================================================================================================ # transfer_ownership locals -const NEW_OWNER_SUFFIX_LOC = 0 -const NEW_OWNER_PREFIX_LOC = 1 -const OWNER_SUFFIX_LOC = 2 -const OWNER_PREFIX_LOC = 3 +const NEW_OWNER_PREFIX_LOC = 0 +const NEW_OWNER_SUFFIX_LOC = 1 +const OWNER_PREFIX_LOC = 2 +const OWNER_SUFFIX_LOC = 3 # INTERNAL PROCEDURES # ================================================================================================ @@ -58,25 +58,25 @@ const OWNER_PREFIX_LOC = 3 #! Returns the full ownership word from storage. #! #! Inputs: [] -#! Outputs: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] +#! Outputs: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] #! #! Where: -#! - owner_{suffix, prefix} are the suffix and prefix felts of the current owner account ID. -#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated +#! - owner_{prefix, suffix} are the prefix and suffix felts of the current owner account ID. +#! - nominated_owner_{prefix, suffix} are the prefix and suffix felts of the nominated #! owner account ID. proc load_ownership_info push.OWNER_CONFIG_SLOT[0..2] exec.active_account::get_item - # => [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] + # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] end #! Writes the ownership word to storage and drops the old value. #! -#! Inputs: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] +#! Inputs: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] #! Outputs: [] proc save_ownership_info push.OWNER_CONFIG_SLOT[0..2] - # => [slot_suffix, slot_prefix, nominated_owner_suffix, nominated_owner_prefix, - # owner_suffix, owner_prefix] + # => [slot_suffix, slot_prefix, owner_prefix, owner_suffix, + # nominated_owner_prefix, nominated_owner_suffix] exec.native_account::set_item # => [OLD_OWNERSHIP_WORD] @@ -88,47 +88,47 @@ end #! Returns the owner account ID from storage. #! #! Inputs: [] -#! Outputs: [owner_suffix, owner_prefix] +#! Outputs: [owner_prefix, owner_suffix] #! #! Where: -#! - owner_{suffix, prefix} are the suffix and prefix felts of the owner account ID. +#! - owner_{prefix, suffix} are the prefix and suffix felts of the owner account ID. proc get_owner_internal exec.load_ownership_info - # => [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] + # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] - drop drop - # => [owner_suffix, owner_prefix] + movup.2 drop + # => [owner_prefix, owner_suffix, nominated_owner_suffix] + + movup.2 drop + # => [owner_prefix, owner_suffix] end #! Returns the nominated owner account ID from storage. #! #! Inputs: [] -#! Outputs: [nominated_owner_suffix, nominated_owner_prefix] +#! Outputs: [nominated_owner_prefix, nominated_owner_suffix] #! #! Where: -#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated +#! - nominated_owner_{prefix, suffix} are the prefix and suffix felts of the nominated #! owner account ID. proc get_nominated_owner_internal exec.load_ownership_info - # => [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] - - movup.2 drop - # => [nominated_owner_suffix, nominated_owner_prefix, owner_prefix] + # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] - movup.2 drop - # => [nominated_owner_suffix, nominated_owner_prefix] + drop drop + # => [nominated_owner_prefix, nominated_owner_suffix] end #! Checks if the given account ID is the owner of this component. #! -#! Inputs: [account_id_suffix, account_id_prefix] +#! Inputs: [account_id_prefix, account_id_suffix] #! Outputs: [is_owner] #! #! Where: #! - is_owner is 1 if the account is the owner, 0 otherwise. proc is_owner_internal exec.get_owner_internal - # => [owner_suffix, owner_prefix, account_id_suffix, account_id_prefix] + # => [owner_prefix, owner_suffix, account_id_prefix, account_id_suffix] exec.account_id::is_equal # => [is_owner] @@ -136,15 +136,15 @@ end #! Checks if the given account ID is the nominated owner of this component. #! -#! Inputs: [account_id_suffix, account_id_prefix] +#! Inputs: [account_id_prefix, account_id_suffix] #! Outputs: [is_nominated_owner] #! #! Where: -#! - account_id_{suffix, prefix} are the suffix and prefix felts of the account ID to check. +#! - account_id_{prefix, suffix} are the prefix and suffix felts of the account ID to check. #! - is_nominated_owner is 1 if the account is the nominated owner, 0 otherwise. proc is_nominated_owner_internal exec.get_nominated_owner_internal - # => [nominated_owner_suffix, nominated_owner_prefix, account_id_suffix, account_id_prefix] + # => [nominated_owner_prefix, nominated_owner_suffix, account_id_prefix, account_id_suffix] exec.account_id::is_equal # => [is_nominated_owner] @@ -161,7 +161,7 @@ end #! Invocation: exec proc assert_sender_is_owner_internal exec.active_note::get_sender - # => [sender_suffix, sender_prefix] + # => [sender_prefix, sender_suffix] exec.is_owner_internal # => [is_owner] @@ -190,36 +190,36 @@ end #! Returns the owner account ID. #! #! Inputs: [pad(16)] -#! Outputs: [owner_suffix, owner_prefix, pad(14)] +#! Outputs: [owner_prefix, owner_suffix, pad(14)] #! #! Where: -#! - owner_{suffix, prefix} are the suffix and prefix felts of the owner account ID. +#! - owner_{prefix, suffix} are the prefix and suffix felts of the owner account ID. #! #! Invocation: call -pub proc get_owner +pub proc owner exec.get_owner_internal - # => [owner_suffix, owner_prefix, pad(16)] + # => [owner_prefix, owner_suffix, pad(16)] movup.2 drop movup.2 drop - # => [owner_suffix, owner_prefix, pad(14)] + # => [owner_prefix, owner_suffix, pad(14)] end #! Returns the nominated owner account ID. #! #! Inputs: [pad(16)] -#! Outputs: [nominated_owner_suffix, nominated_owner_prefix, pad(14)] +#! Outputs: [nominated_owner_prefix, nominated_owner_suffix, pad(14)] #! #! Where: -#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated +#! - nominated_owner_{prefix, suffix} are the prefix and suffix felts of the nominated #! owner account ID. Both are zero if no nominated transfer exists. #! #! Invocation: call -pub proc get_nominated_owner +pub proc nominated_owner exec.get_nominated_owner_internal - # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] + # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] movup.2 drop movup.2 drop - # => [nominated_owner_suffix, nominated_owner_prefix, pad(14)] + # => [nominated_owner_prefix, nominated_owner_suffix, pad(14)] end #! Initiates a two-step ownership transfer by setting the nominated owner. @@ -230,68 +230,81 @@ end #! If the new owner is the current owner, any nominated transfer is cancelled and the #! nominated owner field is cleared. #! -#! Inputs: [new_owner_suffix, new_owner_prefix, pad(14)] +#! Inputs: [new_owner_prefix, new_owner_suffix, pad(14)] #! Outputs: [pad(16)] #! #! Panics if: #! - the note sender is not the owner. #! #! Locals: -#! 0: new_owner_suffix -#! 1: new_owner_prefix -#! 2: owner_suffix -#! 3: owner_prefix +#! 0: new_owner_prefix +#! 1: new_owner_suffix +#! 2: owner_prefix +#! 3: owner_suffix #! #! Invocation: call @locals(4) pub proc transfer_ownership exec.assert_sender_is_owner_internal - # => [new_owner_suffix, new_owner_prefix, pad(14)] + # => [new_owner_prefix, new_owner_suffix, pad(14)] dup.1 dup.1 exec.account_id::validate - # => [new_owner_suffix, new_owner_prefix, pad(14)] - - loc_store.NEW_OWNER_SUFFIX_LOC - # => [new_owner_prefix, pad(14)] + # => [new_owner_prefix, new_owner_suffix, pad(14)] loc_store.NEW_OWNER_PREFIX_LOC + # => [new_owner_suffix, pad(14)] + + loc_store.NEW_OWNER_SUFFIX_LOC # => [pad(14)] exec.get_owner_internal - # => [owner_suffix, owner_prefix, pad(14)] - - loc_store.OWNER_SUFFIX_LOC - # => [owner_prefix, pad(13)] + # => [owner_prefix, owner_suffix, pad(14)] loc_store.OWNER_PREFIX_LOC + # => [owner_suffix, pad(13)] + + loc_store.OWNER_SUFFIX_LOC # => [pad(12)] # Check if new_owner == owner (cancel case). loc_load.NEW_OWNER_PREFIX_LOC loc_load.NEW_OWNER_SUFFIX_LOC # => [new_owner_suffix, new_owner_prefix, pad(12)] + swap + # => [new_owner_prefix, new_owner_suffix, pad(12)] loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC - # => [owner_suffix, owner_prefix, new_owner_suffix, new_owner_prefix, pad(12)] + # => [owner_suffix, owner_prefix, new_owner_prefix, new_owner_suffix, pad(12)] + swap + # => [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix, pad(12)] exec.account_id::is_equal # => [is_self_transfer, pad(12)] if.true # Cancel ownership transfer and clear nominated owner. - # Build word: [nominated_suffix=0, nominated_prefix=0, owner_suffix, owner_prefix] + # Stack for save: [owner_prefix, owner_suffix, nominated_prefix=0, nominated_suffix=0] loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC # => [owner_suffix, owner_prefix, pad(12)] - - push.0.0 - # => [0, 0, owner_suffix, owner_prefix, pad(12)] + swap + # => [owner_prefix, owner_suffix, pad(12)] + + # Pad with zeros to form: [owner_prefix, owner_suffix, 0, 0, pad(12)] + # We need zeros AFTER the owner, but push puts them on top. + # Move owner on top of the zeros: + push.0.0 movup.3 movup.3 + # => [owner_prefix, owner_suffix, 0, 0, pad(12)] else # Transfer ownership by setting nominated = new_owner. - # Build word: [new_owner_suffix, new_owner_prefix, owner_suffix, owner_prefix] - loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC - # => [owner_suffix, owner_prefix, pad(12)] - + # Stack for save: [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix] loc_load.NEW_OWNER_PREFIX_LOC loc_load.NEW_OWNER_SUFFIX_LOC - # => [new_owner_suffix, new_owner_prefix, owner_suffix, owner_prefix, pad(12)] + # => [new_owner_suffix, new_owner_prefix, pad(12)] + swap + # => [new_owner_prefix, new_owner_suffix, pad(12)] + + loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC + # => [owner_suffix, owner_prefix, new_owner_prefix, new_owner_suffix, pad(12)] + swap + # => [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix, pad(12)] end exec.save_ownership_info @@ -313,29 +326,33 @@ end #! Invocation: call pub proc accept_ownership exec.get_nominated_owner_internal - # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] + # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] # Check that a nominated transfer exists (nominated owner is not zero). dup.1 eq.0 dup.1 eq.0 and - # => [is_zero, nominated_owner_suffix, nominated_owner_prefix, pad(16)] + # => [is_zero, nominated_owner_prefix, nominated_owner_suffix, pad(16)] assertz.err=ERR_NO_NOMINATED_OWNER - # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] + # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] exec.active_note::get_sender - # => [sender_suffix, sender_prefix, nominated_owner_suffix, nominated_owner_prefix, pad(16)] + # => [sender_prefix, sender_suffix, nominated_owner_prefix, nominated_owner_suffix, pad(16)] dup.3 dup.3 exec.account_id::is_equal - # => [is_sender_nominated_owner, nominated_owner_suffix, nominated_owner_prefix, pad(16)] + # => [is_sender_nominated_owner, nominated_owner_prefix, nominated_owner_suffix, pad(16)] assert.err=ERR_SENDER_NOT_NOMINATED_OWNER - # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] + # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] # Build new ownership word: nominated becomes owner, clear nominated. - # Word: [new_nominated_suffix=0, new_nominated_prefix=0, new_owner_suffix, new_owner_prefix] + # Stack for save: [owner_prefix, owner_suffix, nominated_prefix=0, nominated_suffix=0] push.0.0 - # => [0, 0, nominated_owner_suffix, nominated_owner_prefix, pad(16)] + # => [0, 0, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + + # Reorder: move nominated (now new owner) to owner position + movup.3 movup.3 + # => [nominated_owner_prefix, nominated_owner_suffix, 0, 0, pad(16)] exec.save_ownership_info # => [pad(16)] diff --git a/crates/miden-standards/asm/standards/faucets/network_fungible.masm b/crates/miden-standards/asm/standards/faucets/network_fungible.masm index 5f405db8fe..57f223bcae 100644 --- a/crates/miden-standards/asm/standards/faucets/network_fungible.masm +++ b/crates/miden-standards/asm/standards/faucets/network_fungible.masm @@ -1,49 +1,18 @@ use miden::protocol::active_note use miden::standards::faucets -use miden::standards::access::ownable +use miden::standards::access::ownable2step # PUBLIC INTERFACE # ================================================================================================ -# OWNER MANAGEMENT +# OWNER MANAGEMENT (re-exported from ownable2step) # ------------------------------------------------------------------------------------------------ -#! Returns the owner AccountId. -#! -#! Inputs: [] -#! Outputs: [owner_prefix, owner_suffix, pad(14)] -#! -#! Invocation: call -pub use ownable::get_owner - -#! Transfers ownership to a new account. -#! -#! Can only be called by the current owner. -#! -#! Inputs: [new_owner_prefix, new_owner_suffix, pad(14)] -#! Outputs: [pad(16)] -#! -#! Where: -#! - new_owner_{prefix, suffix} are the prefix and suffix felts of the new owner AccountId. -#! -#! Panics if: -#! - the note sender is not the owner. -#! -#! Invocation: call -pub use ownable::transfer_ownership - -#! Renounces ownership, leaving the component without an owner. -#! -#! Can only be called by the current owner. -#! -#! Inputs: [pad(16)] -#! Outputs: [pad(16)] -#! -#! Panics if: -#! - the note sender is not the owner. -#! -#! Invocation: call -pub use ownable::renounce_ownership +pub use ownable2step::owner +pub use ownable2step::nominated_owner +pub use ownable2step::transfer_ownership +pub use ownable2step::accept_ownership +pub use ownable2step::renounce_ownership # ASSET DISTRIBUTION # ------------------------------------------------------------------------------------------------ @@ -69,7 +38,7 @@ pub use ownable::renounce_ownership #! #! Invocation: call pub proc distribute - exec.ownable::verify_owner + exec.ownable2step::assert_sender_is_owner # => [amount, tag, aux, note_type, execution_hint, RECIPIENT, pad(7)] exec.faucets::distribute diff --git a/crates/miden-standards/src/account/access/ownable2step.rs b/crates/miden-standards/src/account/access/ownable2step.rs index 70b03283b2..266a7ec3d4 100644 --- a/crates/miden-standards/src/account/access/ownable2step.rs +++ b/crates/miden-standards/src/account/access/ownable2step.rs @@ -21,7 +21,14 @@ static OWNER_CONFIG_SLOT_NAME: LazyLock = LazyLock::new(|| { /// /// ```text /// Rust Word: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] -/// word[0] word[1] word[2] word[3] +/// word[0] word[1] word[2] word[3] +/// ``` +/// +/// After `get_item` (which reverses the word onto the MASM stack), the stack is: +/// +/// ```text +/// Stack: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +/// (word[3]) (word[2]) (word[1]) (word[0]) /// ``` pub struct Ownable2Step { owner: Option, diff --git a/crates/miden-standards/src/account/faucets/mod.rs b/crates/miden-standards/src/account/faucets/mod.rs index 2b7eab4fef..834f1eb18c 100644 --- a/crates/miden-standards/src/account/faucets/mod.rs +++ b/crates/miden-standards/src/account/faucets/mod.rs @@ -4,6 +4,8 @@ use miden_protocol::account::StorageSlotName; use miden_protocol::errors::{AccountError, TokenSymbolError}; use thiserror::Error; +use crate::account::access::Ownable2StepError; + mod basic_fungible; mod network_fungible; mod token_metadata; @@ -50,4 +52,6 @@ pub enum FungibleFaucetError { AccountError(#[source] AccountError), #[error("account is not a fungible faucet account")] NotAFungibleFaucetAccount, + #[error("failed to read ownership data from storage")] + OwnershipError(#[source] Ownable2StepError), } diff --git a/crates/miden-standards/src/account/faucets/network_fungible.rs b/crates/miden-standards/src/account/faucets/network_fungible.rs index db53a10dff..14d5927f15 100644 --- a/crates/miden-standards/src/account/faucets/network_fungible.rs +++ b/crates/miden-standards/src/account/faucets/network_fungible.rs @@ -13,19 +13,15 @@ use miden_protocol::account::{ AccountStorage, AccountStorageMode, AccountType, - StorageSlot, StorageSlotName, }; use miden_protocol::asset::TokenSymbol; -use miden_protocol::utils::sync::LazyLock; use miden_protocol::{Felt, Word}; use super::{FungibleFaucetError, TokenMetadata}; +use crate::account::access::Ownable2Step; use crate::account::auth::NoAuth; use crate::account::components::network_fungible_faucet_library; - -/// The schema type ID for token symbols. -const TOKEN_SYMBOL_TYPE_ID: &str = "miden::standards::fungible_faucets::metadata::token_symbol"; use crate::account::interface::{AccountComponentInterface, AccountInterface, AccountInterfaceExt}; use crate::procedure_digest; @@ -46,10 +42,8 @@ procedure_digest!( network_fungible_faucet_library ); -static OWNER_CONFIG_SLOT_NAME: LazyLock = LazyLock::new(|| { - StorageSlotName::new("miden::standards::access::ownable::owner_config") - .expect("storage slot name should be valid") -}); +/// The schema type ID for token symbols. +const TOKEN_SYMBOL_TYPE_ID: &str = "miden::standards::fungible_faucets::metadata::token_symbol"; /// An [`AccountComponent`] implementing a network fungible faucet. /// @@ -64,15 +58,18 @@ static OWNER_CONFIG_SLOT_NAME: LazyLock = LazyLock::new(|| { /// authentication while `burn` does not require authentication and can be called by anyone. /// Thus, this component must be combined with a component providing authentication. /// +/// Ownership is managed via a two-step transfer pattern ([`Ownable2Step`]). The current owner +/// must first nominate a new owner, who then accepts the transfer. +/// /// ## Storage Layout /// /// - [`Self::metadata_slot`]: Fungible faucet metadata. -/// - [`Self::owner_config_slot`]: The owner account of this network faucet. +/// - [`Ownable2Step::slot_name`]: The owner and nominated owner of this network faucet. /// /// [builder]: crate::code_builder::CodeBuilder pub struct NetworkFungibleFaucet { metadata: TokenMetadata, - owner_account_id: AccountId, + ownership: Ownable2Step, } impl NetworkFungibleFaucet { @@ -105,7 +102,8 @@ impl NetworkFungibleFaucet { owner_account_id: AccountId, ) -> Result { let metadata = TokenMetadata::new(symbol, decimals, max_supply)?; - Ok(Self { metadata, owner_account_id }) + let ownership = Ownable2Step::new(owner_account_id); + Ok(Self { metadata, ownership }) } /// Creates a new [`NetworkFungibleFaucet`] component from the given [`TokenMetadata`]. @@ -113,7 +111,8 @@ impl NetworkFungibleFaucet { /// This is a convenience constructor that allows creating a faucet from pre-validated /// metadata. pub fn from_metadata(metadata: TokenMetadata, owner_account_id: AccountId) -> Self { - Self { metadata, owner_account_id } + let ownership = Ownable2Step::new(owner_account_id); + Self { metadata, ownership } } /// Attempts to create a new [`NetworkFungibleFaucet`] component from the associated account @@ -144,21 +143,11 @@ impl NetworkFungibleFaucet { // Read token metadata from storage let metadata = TokenMetadata::try_from(storage)?; - // obtain owner account ID from the next storage slot - let owner_account_id_word: Word = storage - .get_item(NetworkFungibleFaucet::owner_config_slot()) - .map_err(|err| FungibleFaucetError::StorageLookupFailed { - slot_name: NetworkFungibleFaucet::owner_config_slot().clone(), - source: err, - })?; - - // Convert Word back to AccountId - // Storage format: [0, 0, suffix, prefix] - let prefix = owner_account_id_word[3]; - let suffix = owner_account_id_word[2]; - let owner_account_id = AccountId::new_unchecked([prefix, suffix]); - - Ok(Self { metadata, owner_account_id }) + // Read ownership data from storage + let ownership = Ownable2Step::try_from_storage(storage) + .map_err(|err| FungibleFaucetError::OwnershipError(err))?; + + Ok(Self { metadata, ownership }) } // PUBLIC ACCESSORS @@ -169,12 +158,6 @@ impl NetworkFungibleFaucet { TokenMetadata::metadata_slot() } - /// Returns the [`StorageSlotName`] where the [`NetworkFungibleFaucet`]'s owner configuration is - /// stored. - pub fn owner_config_slot() -> &'static StorageSlotName { - &OWNER_CONFIG_SLOT_NAME - } - /// Returns the storage slot schema for the metadata slot. pub fn metadata_slot_schema() -> (StorageSlotName, StorageSlotSchema) { let token_symbol_type = SchemaTypeId::new(TOKEN_SYMBOL_TYPE_ID).expect("valid type id"); @@ -192,22 +175,6 @@ impl NetworkFungibleFaucet { ) } - /// Returns the storage slot schema for the owner configuration slot. - pub fn owner_config_slot_schema() -> (StorageSlotName, StorageSlotSchema) { - ( - Self::owner_config_slot().clone(), - StorageSlotSchema::value( - "Owner account configuration", - [ - FeltSchema::new_void(), - FeltSchema::new_void(), - FeltSchema::felt("owner_suffix"), - FeltSchema::felt("owner_prefix"), - ], - ), - ) - } - /// Returns the token metadata. pub fn metadata(&self) -> &TokenMetadata { &self.metadata @@ -238,9 +205,19 @@ impl NetworkFungibleFaucet { self.metadata.token_supply() } - /// Returns the owner account ID of the faucet. - pub fn owner_account_id(&self) -> AccountId { - self.owner_account_id + /// Returns the owner account ID of the faucet, or `None` if ownership has been renounced. + pub fn owner_account_id(&self) -> Option { + self.ownership.owner() + } + + /// Returns the nominated owner account ID, or `None` if no transfer is in progress. + pub fn nominated_owner(&self) -> Option { + self.ownership.nominated_owner() + } + + /// Returns the ownership data of the faucet. + pub fn ownership(&self) -> &Ownable2Step { + &self.ownership } /// Returns the digest of the `distribute` account procedure. @@ -271,24 +248,11 @@ impl NetworkFungibleFaucet { impl From for AccountComponent { fn from(network_faucet: NetworkFungibleFaucet) -> Self { let metadata_slot = network_faucet.metadata.into(); - - // Convert AccountId into its Word encoding for storage. - let owner_account_id_word: Word = [ - Felt::new(0), - Felt::new(0), - network_faucet.owner_account_id.suffix(), - network_faucet.owner_account_id.prefix().as_felt(), - ] - .into(); - - let owner_slot = StorageSlot::with_value( - NetworkFungibleFaucet::owner_config_slot().clone(), - owner_account_id_word, - ); + let owner_slot = network_faucet.ownership.to_storage_slot(); let storage_schema = StorageSchema::new([ NetworkFungibleFaucet::metadata_slot_schema(), - NetworkFungibleFaucet::owner_config_slot_schema(), + Ownable2Step::slot_schema(), ]) .expect("storage schema should be valid"); diff --git a/crates/miden-standards/src/account/mod.rs b/crates/miden-standards/src/account/mod.rs index af3d4ff69b..767be8f692 100644 --- a/crates/miden-standards/src/account/mod.rs +++ b/crates/miden-standards/src/account/mod.rs @@ -1,5 +1,6 @@ use super::auth_scheme::AuthScheme; +pub mod access; pub mod auth; pub mod components; pub mod faucets; diff --git a/crates/miden-standards/src/errors/standards.rs b/crates/miden-standards/src/errors/standards.rs index f16cc23ed4..b4302d655b 100644 --- a/crates/miden-standards/src/errors/standards.rs +++ b/crates/miden-standards/src/errors/standards.rs @@ -34,6 +34,9 @@ pub const ERR_NOTE_TAG_MAX_ACCOUNT_TARGET_LENGTH_EXCEEDED: MasmError = MasmError /// Error Message: "attachment is not a valid network account target" pub const ERR_NOT_NETWORK_ACCOUNT_TARGET: MasmError = MasmError::from_static_str("attachment is not a valid network account target"); +/// Error Message: "no nominated ownership transfer exists" +pub const ERR_NO_NOMINATED_OWNER: MasmError = MasmError::from_static_str("no nominated ownership transfer exists"); + /// Error Message: "failed to reclaim P2IDE note because the reclaiming account is not the sender" pub const ERR_P2IDE_RECLAIM_ACCT_IS_NOT_SENDER: MasmError = MasmError::from_static_str("failed to reclaim P2IDE note because the reclaiming account is not the sender"); /// Error Message: "P2IDE reclaim is disabled" @@ -50,6 +53,8 @@ pub const ERR_P2ID_TARGET_ACCT_MISMATCH: MasmError = MasmError::from_static_str( /// Error Message: "P2ID note expects exactly 2 note storage items" pub const ERR_P2ID_UNEXPECTED_NUMBER_OF_STORAGE_ITEMS: MasmError = MasmError::from_static_str("P2ID note expects exactly 2 note storage items"); +/// Error Message: "note sender is not the nominated owner" +pub const ERR_SENDER_NOT_NOMINATED_OWNER: MasmError = MasmError::from_static_str("note sender is not the nominated owner"); /// Error Message: "note sender is not the owner" pub const ERR_SENDER_NOT_OWNER: MasmError = MasmError::from_static_str("note sender is not the owner"); diff --git a/crates/miden-testing/tests/scripts/faucet.rs b/crates/miden-testing/tests/scripts/faucet.rs index 351648ea5e..c8e3acc690 100644 --- a/crates/miden-testing/tests/scripts/faucet.rs +++ b/crates/miden-testing/tests/scripts/faucet.rs @@ -27,6 +27,7 @@ use miden_protocol::note::{ use miden_protocol::testing::account_id::ACCOUNT_ID_PRIVATE_SENDER; use miden_protocol::transaction::{ExecutedTransaction, OutputNote}; use miden_protocol::{Felt, Word}; +use miden_standards::account::access::Ownable2Step; use miden_standards::account::faucets::{ BasicFungibleFaucet, NetworkFungibleFaucet, @@ -535,12 +536,14 @@ async fn network_faucet_mint() -> anyhow::Result<()> { let actual_max_supply = TokenMetadata::try_from(faucet.storage())?.max_supply(); assert_eq!(actual_max_supply.as_int(), max_supply); - // Check that the creator account ID is stored in slot 2 (second storage slot of the component) - // The owner_account_id is stored as Word [0, 0, suffix, prefix] + // Check that the creator account ID is stored in the ownership slot. + // Rust Word: [nominated_suffix, nominated_prefix, owner_suffix, owner_prefix] let stored_owner_id = - faucet.storage().get_item(NetworkFungibleFaucet::owner_config_slot()).unwrap(); + faucet.storage().get_item(Ownable2Step::slot_name()).unwrap(); assert_eq!(stored_owner_id[3], faucet_owner_account_id.prefix().as_felt()); assert_eq!(stored_owner_id[2], Felt::new(faucet_owner_account_id.suffix().as_int())); + assert_eq!(stored_owner_id[1], Felt::new(0)); // no nominated owner + assert_eq!(stored_owner_id[0], Felt::new(0)); // Check that the faucet's token supply has been correctly initialized. // The already issued amount should be 50. @@ -745,18 +748,20 @@ async fn test_network_faucet_owner_storage() -> anyhow::Result<()> { let _mock_chain = builder.build()?; // Verify owner is stored correctly - let stored_owner = faucet.storage().get_item(NetworkFungibleFaucet::owner_config_slot())?; + let stored_owner = faucet.storage().get_item(Ownable2Step::slot_name())?; - // Storage format: [0, 0, suffix, prefix] + // Rust Word: [nominated_suffix, nominated_prefix, owner_suffix, owner_prefix] assert_eq!(stored_owner[3], owner_account_id.prefix().as_felt()); assert_eq!(stored_owner[2], Felt::new(owner_account_id.suffix().as_int())); - assert_eq!(stored_owner[1], Felt::new(0)); + assert_eq!(stored_owner[1], Felt::new(0)); // no nominated owner assert_eq!(stored_owner[0], Felt::new(0)); Ok(()) } -/// Tests that transfer_ownership updates the owner correctly. +/// Tests that two-step transfer_ownership updates the owner correctly. +/// Step 1: Owner nominates a new owner via transfer_ownership. +/// Step 2: Nominated owner accepts via accept_ownership. #[tokio::test] async fn test_network_faucet_transfer_ownership() -> anyhow::Result<()> { let mut builder = MockChain::builder(); @@ -805,26 +810,23 @@ async fn test_network_faucet_transfer_ownership() -> anyhow::Result<()> { &mut rng, )?; - // Action: Create transfer_ownership note script + // Step 1: Create transfer_ownership note script to nominate new owner let transfer_note_script_code = format!( r#" use miden::standards::faucets::network_fungible->network_faucet begin repeat.14 push.0 end - push.{new_owner_suffix} - push.{new_owner_prefix} + push.{new_owner_suffix} push.{new_owner_prefix} call.network_faucet::transfer_ownership dropw dropw dropw dropw end "#, new_owner_prefix = new_owner_account_id.prefix().as_felt(), - new_owner_suffix = Felt::new(new_owner_account_id.suffix().as_int()), + new_owner_suffix = new_owner_account_id.suffix(), ); let source_manager = Arc::new(DefaultSourceManager::default()); - let transfer_note_script = CodeBuilder::with_source_manager(source_manager.clone()) - .compile_note_script(transfer_note_script_code.clone())?; // Create the transfer note and add it to the builder so it exists on-chain let mut rng = RpoRandomCoin::new([Felt::from(200u32); 4].into()); @@ -847,10 +849,9 @@ async fn test_network_faucet_transfer_ownership() -> anyhow::Result<()> { let executed_transaction = tx_context.execute().await?; assert_eq!(executed_transaction.output_notes().num_notes(), 1); - // Action: Execute transfer_ownership via note script + // Execute transfer_ownership via note script (nominates new owner) let tx_context = mock_chain .build_tx_context(faucet.id(), &[transfer_note.id()], &[])? - .add_note_script(transfer_note_script.clone()) .with_source_manager(source_manager.clone()) .build()?; let executed_transaction = tx_context.execute().await?; @@ -859,48 +860,43 @@ async fn test_network_faucet_transfer_ownership() -> anyhow::Result<()> { mock_chain.add_pending_executed_transaction(&executed_transaction)?; mock_chain.prove_next_block()?; - // Apply the delta to the faucet account to reflect the ownership change let mut updated_faucet = faucet.clone(); updated_faucet.apply_delta(executed_transaction.account_delta())?; - // Validation 1: Try to mint using the old owner - should fail - let mut rng = RpoRandomCoin::new([Felt::from(300u32); 4].into()); - let mint_note_old_owner = MintNote::create( - updated_faucet.id(), - initial_owner_account_id, - mint_inputs.clone(), - NoteAttachment::default(), - &mut rng, - )?; - - // Use the note as an unauthenticated note (full note object) - it will be created in this - // transaction - let tx_context = mock_chain - .build_tx_context(updated_faucet.id(), &[], &[mint_note_old_owner])? - .build()?; - let result = tx_context.execute().await; + // Step 2: Accept ownership as the nominated owner + let accept_note_script_code = r#" + use miden::standards::faucets::network_fungible->network_faucet - // The distribute function uses ERR_ONLY_OWNER, which is "note sender is not the owner" - let expected_error = ERR_SENDER_NOT_OWNER; - assert_transaction_executor_error!(result, expected_error); + begin + repeat.16 push.0 end + call.network_faucet::accept_ownership + dropw dropw dropw dropw + end + "#; - // Validation 2: Try to mint using the new owner - should succeed let mut rng = RpoRandomCoin::new([Felt::from(400u32); 4].into()); - let mint_note_new_owner = MintNote::create( - updated_faucet.id(), - new_owner_account_id, - mint_inputs, - NoteAttachment::default(), - &mut rng, - )?; + let accept_note = NoteBuilder::new(new_owner_account_id, &mut rng) + .note_type(NoteType::Private) + .tag(NoteTag::default().into()) + .serial_number(Word::from([55, 66, 77, 88u32])) + .code(accept_note_script_code) + .build()?; let tx_context = mock_chain - .build_tx_context(updated_faucet.id(), &[], &[mint_note_new_owner])? + .build_tx_context(updated_faucet.clone(), &[], slice::from_ref(&accept_note))? + .with_source_manager(source_manager.clone()) .build()?; let executed_transaction = tx_context.execute().await?; - // Verify that minting succeeded - assert_eq!(executed_transaction.output_notes().num_notes(), 1); + let mut final_faucet = updated_faucet.clone(); + final_faucet.apply_delta(executed_transaction.account_delta())?; + + // Verify that owner changed to new_owner and nominated was cleared + let stored_owner = final_faucet.storage().get_item(Ownable2Step::slot_name())?; + assert_eq!(stored_owner[3], new_owner_account_id.prefix().as_felt()); + assert_eq!(stored_owner[2], Felt::new(new_owner_account_id.suffix().as_int())); + assert_eq!(stored_owner[1], Felt::new(0)); // nominated cleared + assert_eq!(stored_owner[0], Felt::new(0)); Ok(()) } @@ -941,19 +937,16 @@ async fn test_network_faucet_only_owner_can_transfer() -> anyhow::Result<()> { begin repeat.14 push.0 end - push.{new_owner_suffix} - push.{new_owner_prefix} + push.{new_owner_suffix} push.{new_owner_prefix} call.network_faucet::transfer_ownership dropw dropw dropw dropw end "#, new_owner_prefix = new_owner_account_id.prefix().as_felt(), - new_owner_suffix = Felt::new(new_owner_account_id.suffix().as_int()), + new_owner_suffix = new_owner_account_id.suffix(), ); let source_manager = Arc::new(DefaultSourceManager::default()); - let transfer_note_script = CodeBuilder::with_source_manager(source_manager.clone()) - .compile_note_script(transfer_note_script_code.clone())?; // Create a note from NON-OWNER that tries to transfer ownership let mut rng = RpoRandomCoin::new([Felt::from(100u32); 4].into()); @@ -966,14 +959,11 @@ async fn test_network_faucet_only_owner_can_transfer() -> anyhow::Result<()> { let tx_context = mock_chain .build_tx_context(faucet.id(), &[], &[transfer_note])? - .add_note_script(transfer_note_script.clone()) .with_source_manager(source_manager.clone()) .build()?; let result = tx_context.execute().await; - // Verify that the transaction failed with ERR_ONLY_OWNER - let expected_error = ERR_SENDER_NOT_OWNER; - assert_transaction_executor_error!(result, expected_error); + assert_transaction_executor_error!(result, ERR_SENDER_NOT_OWNER); Ok(()) } @@ -1001,7 +991,7 @@ async fn test_network_faucet_renounce_ownership() -> anyhow::Result<()> { // Check stored value before renouncing let stored_owner_before = - faucet.storage().get_item(NetworkFungibleFaucet::owner_config_slot())?; + faucet.storage().get_item(Ownable2Step::slot_name())?; assert_eq!(stored_owner_before[3], owner_account_id.prefix().as_felt()); assert_eq!(stored_owner_before[2], Felt::new(owner_account_id.suffix().as_int())); @@ -1017,8 +1007,6 @@ async fn test_network_faucet_renounce_ownership() -> anyhow::Result<()> { "#; let source_manager = Arc::new(DefaultSourceManager::default()); - let renounce_note_script = CodeBuilder::with_source_manager(source_manager.clone()) - .compile_note_script(renounce_note_script_code)?; // Create transfer note script (will be used after renounce) let transfer_note_script_code = format!( @@ -1027,19 +1015,15 @@ async fn test_network_faucet_renounce_ownership() -> anyhow::Result<()> { begin repeat.14 push.0 end - push.{new_owner_suffix} - push.{new_owner_prefix} + push.{new_owner_suffix} push.{new_owner_prefix} call.network_faucet::transfer_ownership dropw dropw dropw dropw end "#, new_owner_prefix = new_owner_account_id.prefix().as_felt(), - new_owner_suffix = Felt::new(new_owner_account_id.suffix().as_int()), + new_owner_suffix = new_owner_account_id.suffix(), ); - let transfer_note_script = CodeBuilder::with_source_manager(source_manager.clone()) - .compile_note_script(transfer_note_script_code.clone())?; - let mut rng = RpoRandomCoin::new([Felt::from(200u32); 4].into()); let renounce_note = NoteBuilder::new(owner_account_id, &mut rng) .note_type(NoteType::Private) @@ -1064,7 +1048,6 @@ async fn test_network_faucet_renounce_ownership() -> anyhow::Result<()> { // Execute renounce_ownership let tx_context = mock_chain .build_tx_context(faucet.id(), &[renounce_note.id()], &[])? - .add_note_script(renounce_note_script.clone()) .with_source_manager(source_manager.clone()) .build()?; let executed_transaction = tx_context.execute().await?; @@ -1077,26 +1060,22 @@ async fn test_network_faucet_renounce_ownership() -> anyhow::Result<()> { // Check stored value after renouncing - should be zero let stored_owner_after = - updated_faucet.storage().get_item(NetworkFungibleFaucet::owner_config_slot())?; + updated_faucet.storage().get_item(Ownable2Step::slot_name())?; assert_eq!(stored_owner_after[0], Felt::new(0)); assert_eq!(stored_owner_after[1], Felt::new(0)); assert_eq!(stored_owner_after[2], Felt::new(0)); assert_eq!(stored_owner_after[3], Felt::new(0)); // Try to transfer ownership - should fail because there's no owner - // The transfer note was already added to the builder, so we need to prove another block - // to make it available on-chain after the renounce transaction mock_chain.prove_next_block()?; let tx_context = mock_chain .build_tx_context(updated_faucet.id(), &[transfer_note.id()], &[])? - .add_note_script(transfer_note_script.clone()) .with_source_manager(source_manager.clone()) .build()?; let result = tx_context.execute().await; - let expected_error = ERR_SENDER_NOT_OWNER; - assert_transaction_executor_error!(result, expected_error); + assert_transaction_executor_error!(result, ERR_SENDER_NOT_OWNER); Ok(()) } diff --git a/crates/miden-testing/tests/scripts/ownable2step.rs b/crates/miden-testing/tests/scripts/ownable2step.rs index df1b1857dc..654fd08926 100644 --- a/crates/miden-testing/tests/scripts/ownable2step.rs +++ b/crates/miden-testing/tests/scripts/ownable2step.rs @@ -2,7 +2,7 @@ extern crate alloc; use alloc::sync::Arc; -use miden_processor::crypto::random::RpoRandomCoin; +use miden_processor::crypto::RpoRandomCoin; use miden_protocol::account::component::AccountComponentMetadata; use miden_protocol::account::{ Account, @@ -43,8 +43,8 @@ fn create_ownable_account( ) -> anyhow::Result { let component_code = r#" use miden::standards::access::ownable2step - pub use ownable2step::get_owner - pub use ownable2step::get_nominated_owner + pub use ownable2step::owner + pub use ownable2step::nominated_owner pub use ownable2step::transfer_ownership pub use ownable2step::accept_ownership pub use ownable2step::renounce_ownership @@ -53,8 +53,8 @@ fn create_ownable_account( CodeBuilder::default().compile_component_code("test::ownable", component_code)?; let ownership_word: Word = [ - Felt::ZERO, // word[0] = nominated_suffix - Felt::ZERO, // word[1] = nominated_prefix + Felt::new(0), // word[0] = nominated_suffix + Felt::new(0), // word[1] = nominated_prefix owner.suffix(), // word[2] = owner_suffix owner.prefix().as_felt(), // word[3] = owner_prefix ] @@ -78,10 +78,10 @@ fn get_owner_from_storage(account: &Account) -> anyhow::Result let word = account.storage().get_item(&OWNER_CONFIG_SLOT_NAME)?; let prefix = word[3]; let suffix = word[2]; - if prefix == Felt::ZERO && suffix == Felt::ZERO { + if prefix == Felt::new(0) && suffix == Felt::new(0) { Ok(None) } else { - Ok(Some(AccountId::try_from_elements(suffix, prefix)?)) + Ok(Some(AccountId::try_from([prefix, suffix])?)) } } @@ -89,10 +89,10 @@ fn get_nominated_owner_from_storage(account: &Account) -> anyhow::Resulttest_account begin repeat.14 push.0 end - push.{new_owner_prefix} push.{new_owner_suffix} + push.{new_owner_suffix} push.{new_owner_prefix} call.test_account::transfer_ownership dropw dropw dropw dropw end @@ -482,8 +482,8 @@ async fn test_transfer_ownership_fails_with_invalid_account_id() -> anyhow::Resu use miden::standards::access::ownable2step->test_account begin repeat.14 push.0 end - push.{invalid_prefix} push.{invalid_suffix} + push.{invalid_prefix} call.test_account::transfer_ownership dropw dropw dropw dropw end From d6d33f8b888030ba310c844bd894982d8f204176 Mon Sep 17 00:00:00 2001 From: Arthur Abeilice Date: Fri, 6 Mar 2026 11:55:43 -0300 Subject: [PATCH 5/6] refactor: update ownership word layout and adjust related logic in Ownable2Step --- .../asm/standards/access/ownable2step.masm | 161 +++++++++--------- .../src/account/access/ownable2step.rs | 41 ++--- .../src/account/faucets/network_fungible.rs | 4 +- crates/miden-testing/tests/scripts/faucet.rs | 42 +++-- .../tests/scripts/ownable2step.rs | 34 ++-- 5 files changed, 134 insertions(+), 148 deletions(-) diff --git a/crates/miden-standards/asm/standards/access/ownable2step.masm b/crates/miden-standards/asm/standards/access/ownable2step.masm index 756efb9cbd..7732420c5e 100644 --- a/crates/miden-standards/asm/standards/access/ownable2step.masm +++ b/crates/miden-standards/asm/standards/access/ownable2step.masm @@ -14,12 +14,8 @@ # to cancel the nominated transfer. # # Storage layout (single slot): -# Rust Word: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] -# word[0] word[1] word[2] word[3] -# -# After get_item (reversed onto stack), the stack is: -# [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] -# (word[3]) (word[2]) (word[1]) (word[0]) +# Word: [owner_suffix, owner_prefix, nominated_owner_suffix, nominated_owner_prefix] +# word[0] word[1] word[2] word[3] use miden::protocol::active_account use miden::protocol::account_id @@ -47,10 +43,10 @@ const ERR_NO_NOMINATED_OWNER = "no nominated ownership transfer exists" # ================================================================================================ # transfer_ownership locals -const NEW_OWNER_PREFIX_LOC = 0 -const NEW_OWNER_SUFFIX_LOC = 1 -const OWNER_PREFIX_LOC = 2 -const OWNER_SUFFIX_LOC = 3 +const NEW_OWNER_SUFFIX_LOC = 0 +const NEW_OWNER_PREFIX_LOC = 1 +const OWNER_SUFFIX_LOC = 2 +const OWNER_PREFIX_LOC = 3 # INTERNAL PROCEDURES # ================================================================================================ @@ -58,25 +54,25 @@ const OWNER_SUFFIX_LOC = 3 #! Returns the full ownership word from storage. #! #! Inputs: [] -#! Outputs: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +#! Outputs: [owner_suffix, owner_prefix, nominated_owner_suffix, nominated_owner_prefix] #! #! Where: -#! - owner_{prefix, suffix} are the prefix and suffix felts of the current owner account ID. -#! - nominated_owner_{prefix, suffix} are the prefix and suffix felts of the nominated +#! - owner_{suffix, prefix} are the suffix and prefix felts of the current owner account ID. +#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated #! owner account ID. proc load_ownership_info push.OWNER_CONFIG_SLOT[0..2] exec.active_account::get_item - # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] + # => [owner_suffix, owner_prefix, nominated_owner_suffix, nominated_owner_prefix] end #! Writes the ownership word to storage and drops the old value. #! -#! Inputs: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] +#! Inputs: [owner_suffix, owner_prefix, nominated_owner_suffix, nominated_owner_prefix] #! Outputs: [] proc save_ownership_info push.OWNER_CONFIG_SLOT[0..2] - # => [slot_suffix, slot_prefix, owner_prefix, owner_suffix, - # nominated_owner_prefix, nominated_owner_suffix] + # => [slot_suffix, slot_prefix, owner_suffix, owner_prefix, + # nominated_owner_suffix, nominated_owner_prefix] exec.native_account::set_item # => [OLD_OWNERSHIP_WORD] @@ -88,47 +84,47 @@ end #! Returns the owner account ID from storage. #! #! Inputs: [] -#! Outputs: [owner_prefix, owner_suffix] +#! Outputs: [owner_suffix, owner_prefix] #! #! Where: -#! - owner_{prefix, suffix} are the prefix and suffix felts of the owner account ID. +#! - owner_{suffix, prefix} are the suffix and prefix felts of the owner account ID. proc get_owner_internal exec.load_ownership_info - # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] + # => [owner_suffix, owner_prefix, nominated_owner_suffix, nominated_owner_prefix] movup.2 drop - # => [owner_prefix, owner_suffix, nominated_owner_suffix] + # => [owner_suffix, owner_prefix, nominated_owner_prefix] movup.2 drop - # => [owner_prefix, owner_suffix] + # => [owner_suffix, owner_prefix] end #! Returns the nominated owner account ID from storage. #! #! Inputs: [] -#! Outputs: [nominated_owner_prefix, nominated_owner_suffix] +#! Outputs: [nominated_owner_suffix, nominated_owner_prefix] #! #! Where: -#! - nominated_owner_{prefix, suffix} are the prefix and suffix felts of the nominated +#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated #! owner account ID. proc get_nominated_owner_internal exec.load_ownership_info - # => [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] + # => [owner_suffix, owner_prefix, nominated_owner_suffix, nominated_owner_prefix] drop drop - # => [nominated_owner_prefix, nominated_owner_suffix] + # => [nominated_owner_suffix, nominated_owner_prefix] end #! Checks if the given account ID is the owner of this component. #! -#! Inputs: [account_id_prefix, account_id_suffix] +#! Inputs: [account_id_suffix, account_id_prefix] #! Outputs: [is_owner] #! #! Where: #! - is_owner is 1 if the account is the owner, 0 otherwise. proc is_owner_internal exec.get_owner_internal - # => [owner_prefix, owner_suffix, account_id_prefix, account_id_suffix] + # => [owner_suffix, owner_prefix, account_id_suffix, account_id_prefix] exec.account_id::is_equal # => [is_owner] @@ -136,15 +132,15 @@ end #! Checks if the given account ID is the nominated owner of this component. #! -#! Inputs: [account_id_prefix, account_id_suffix] +#! Inputs: [account_id_suffix, account_id_prefix] #! Outputs: [is_nominated_owner] #! #! Where: -#! - account_id_{prefix, suffix} are the prefix and suffix felts of the account ID to check. +#! - account_id_{suffix, prefix} are the suffix and prefix felts of the account ID to check. #! - is_nominated_owner is 1 if the account is the nominated owner, 0 otherwise. proc is_nominated_owner_internal exec.get_nominated_owner_internal - # => [nominated_owner_prefix, nominated_owner_suffix, account_id_prefix, account_id_suffix] + # => [nominated_owner_suffix, nominated_owner_prefix, account_id_suffix, account_id_prefix] exec.account_id::is_equal # => [is_nominated_owner] @@ -161,7 +157,7 @@ end #! Invocation: exec proc assert_sender_is_owner_internal exec.active_note::get_sender - # => [sender_prefix, sender_suffix] + # => [sender_suffix, sender_prefix] exec.is_owner_internal # => [is_owner] @@ -190,36 +186,36 @@ end #! Returns the owner account ID. #! #! Inputs: [pad(16)] -#! Outputs: [owner_prefix, owner_suffix, pad(14)] +#! Outputs: [owner_suffix, owner_prefix, pad(14)] #! #! Where: -#! - owner_{prefix, suffix} are the prefix and suffix felts of the owner account ID. +#! - owner_{suffix, prefix} are the suffix and prefix felts of the owner account ID. #! #! Invocation: call pub proc owner exec.get_owner_internal - # => [owner_prefix, owner_suffix, pad(16)] + # => [owner_suffix, owner_prefix, pad(16)] movup.2 drop movup.2 drop - # => [owner_prefix, owner_suffix, pad(14)] + # => [owner_suffix, owner_prefix, pad(14)] end #! Returns the nominated owner account ID. #! #! Inputs: [pad(16)] -#! Outputs: [nominated_owner_prefix, nominated_owner_suffix, pad(14)] +#! Outputs: [nominated_owner_suffix, nominated_owner_prefix, pad(14)] #! #! Where: -#! - nominated_owner_{prefix, suffix} are the prefix and suffix felts of the nominated +#! - nominated_owner_{suffix, prefix} are the suffix and prefix felts of the nominated #! owner account ID. Both are zero if no nominated transfer exists. #! #! Invocation: call pub proc nominated_owner exec.get_nominated_owner_internal - # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] movup.2 drop movup.2 drop - # => [nominated_owner_prefix, nominated_owner_suffix, pad(14)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(14)] end #! Initiates a two-step ownership transfer by setting the nominated owner. @@ -230,81 +226,78 @@ end #! If the new owner is the current owner, any nominated transfer is cancelled and the #! nominated owner field is cleared. #! -#! Inputs: [new_owner_prefix, new_owner_suffix, pad(14)] +#! Inputs: [new_owner_suffix, new_owner_prefix, pad(14)] #! Outputs: [pad(16)] #! #! Panics if: #! - the note sender is not the owner. #! #! Locals: -#! 0: new_owner_prefix -#! 1: new_owner_suffix -#! 2: owner_prefix -#! 3: owner_suffix +#! 0: new_owner_suffix +#! 1: new_owner_prefix +#! 2: owner_suffix +#! 3: owner_prefix #! #! Invocation: call @locals(4) pub proc transfer_ownership exec.assert_sender_is_owner_internal - # => [new_owner_prefix, new_owner_suffix, pad(14)] + # => [new_owner_suffix, new_owner_prefix, pad(14)] dup.1 dup.1 exec.account_id::validate - # => [new_owner_prefix, new_owner_suffix, pad(14)] - - loc_store.NEW_OWNER_PREFIX_LOC - # => [new_owner_suffix, pad(14)] + # => [new_owner_suffix, new_owner_prefix, pad(14)] loc_store.NEW_OWNER_SUFFIX_LOC + # => [new_owner_prefix, pad(14)] + + loc_store.NEW_OWNER_PREFIX_LOC # => [pad(14)] exec.get_owner_internal - # => [owner_prefix, owner_suffix, pad(14)] - - loc_store.OWNER_PREFIX_LOC - # => [owner_suffix, pad(13)] + # => [owner_suffix, owner_prefix, pad(14)] loc_store.OWNER_SUFFIX_LOC + # => [owner_prefix, pad(13)] + + loc_store.OWNER_PREFIX_LOC # => [pad(12)] # Check if new_owner == owner (cancel case). - loc_load.NEW_OWNER_PREFIX_LOC loc_load.NEW_OWNER_SUFFIX_LOC - # => [new_owner_suffix, new_owner_prefix, pad(12)] - swap + loc_load.NEW_OWNER_SUFFIX_LOC loc_load.NEW_OWNER_PREFIX_LOC # => [new_owner_prefix, new_owner_suffix, pad(12)] + swap + # => [new_owner_suffix, new_owner_prefix, pad(12)] - loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC - # => [owner_suffix, owner_prefix, new_owner_prefix, new_owner_suffix, pad(12)] + loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC + # => [owner_prefix, owner_suffix, new_owner_suffix, new_owner_prefix, pad(12)] swap - # => [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix, pad(12)] + # => [owner_suffix, owner_prefix, new_owner_suffix, new_owner_prefix, pad(12)] exec.account_id::is_equal # => [is_self_transfer, pad(12)] if.true # Cancel ownership transfer and clear nominated owner. - # Stack for save: [owner_prefix, owner_suffix, nominated_prefix=0, nominated_suffix=0] - loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC - # => [owner_suffix, owner_prefix, pad(12)] - swap + # Stack for save: [owner_suffix, owner_prefix, nominated_suffix=0, nominated_prefix=0] + loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC # => [owner_prefix, owner_suffix, pad(12)] + swap + # => [owner_suffix, owner_prefix, pad(12)] - # Pad with zeros to form: [owner_prefix, owner_suffix, 0, 0, pad(12)] - # We need zeros AFTER the owner, but push puts them on top. - # Move owner on top of the zeros: push.0.0 movup.3 movup.3 - # => [owner_prefix, owner_suffix, 0, 0, pad(12)] + # => [owner_suffix, owner_prefix, 0, 0, pad(12)] else # Transfer ownership by setting nominated = new_owner. - # Stack for save: [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix] - loc_load.NEW_OWNER_PREFIX_LOC loc_load.NEW_OWNER_SUFFIX_LOC - # => [new_owner_suffix, new_owner_prefix, pad(12)] - swap + # Stack for save: [owner_suffix, owner_prefix, new_owner_suffix, new_owner_prefix] + loc_load.NEW_OWNER_SUFFIX_LOC loc_load.NEW_OWNER_PREFIX_LOC # => [new_owner_prefix, new_owner_suffix, pad(12)] + swap + # => [new_owner_suffix, new_owner_prefix, pad(12)] - loc_load.OWNER_PREFIX_LOC loc_load.OWNER_SUFFIX_LOC - # => [owner_suffix, owner_prefix, new_owner_prefix, new_owner_suffix, pad(12)] + loc_load.OWNER_SUFFIX_LOC loc_load.OWNER_PREFIX_LOC + # => [owner_prefix, owner_suffix, new_owner_suffix, new_owner_prefix, pad(12)] swap - # => [owner_prefix, owner_suffix, new_owner_prefix, new_owner_suffix, pad(12)] + # => [owner_suffix, owner_prefix, new_owner_suffix, new_owner_prefix, pad(12)] end exec.save_ownership_info @@ -326,33 +319,33 @@ end #! Invocation: call pub proc accept_ownership exec.get_nominated_owner_internal - # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] # Check that a nominated transfer exists (nominated owner is not zero). dup.1 eq.0 dup.1 eq.0 and - # => [is_zero, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [is_zero, nominated_owner_suffix, nominated_owner_prefix, pad(16)] assertz.err=ERR_NO_NOMINATED_OWNER - # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] exec.active_note::get_sender - # => [sender_prefix, sender_suffix, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [sender_suffix, sender_prefix, nominated_owner_suffix, nominated_owner_prefix, pad(16)] dup.3 dup.3 exec.account_id::is_equal - # => [is_sender_nominated_owner, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [is_sender_nominated_owner, nominated_owner_suffix, nominated_owner_prefix, pad(16)] assert.err=ERR_SENDER_NOT_NOMINATED_OWNER - # => [nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] # Build new ownership word: nominated becomes owner, clear nominated. - # Stack for save: [owner_prefix, owner_suffix, nominated_prefix=0, nominated_suffix=0] + # Stack for save: [owner_suffix, owner_prefix, nominated_suffix=0, nominated_prefix=0] push.0.0 - # => [0, 0, nominated_owner_prefix, nominated_owner_suffix, pad(16)] + # => [0, 0, nominated_owner_suffix, nominated_owner_prefix, pad(16)] # Reorder: move nominated (now new owner) to owner position movup.3 movup.3 - # => [nominated_owner_prefix, nominated_owner_suffix, 0, 0, pad(16)] + # => [nominated_owner_suffix, nominated_owner_prefix, 0, 0, pad(16)] exec.save_ownership_info # => [pad(16)] diff --git a/crates/miden-standards/src/account/access/ownable2step.rs b/crates/miden-standards/src/account/access/ownable2step.rs index 266a7ec3d4..cbcd4eece4 100644 --- a/crates/miden-standards/src/account/access/ownable2step.rs +++ b/crates/miden-standards/src/account/access/ownable2step.rs @@ -2,7 +2,7 @@ use miden_protocol::account::component::{FeltSchema, StorageSlotSchema}; use miden_protocol::account::{AccountId, AccountStorage, StorageSlot, StorageSlotName}; use miden_protocol::errors::AccountIdError; use miden_protocol::utils::sync::LazyLock; -use miden_protocol::{Felt, FieldElement, Word}; +use miden_protocol::{Felt, Word}; static OWNER_CONFIG_SLOT_NAME: LazyLock = LazyLock::new(|| { StorageSlotName::new("miden::standards::access::ownable2step::owner_config") @@ -20,15 +20,8 @@ static OWNER_CONFIG_SLOT_NAME: LazyLock = LazyLock::new(|| { /// The ownership data is stored in a single word: /// /// ```text -/// Rust Word: [nominated_owner_suffix, nominated_owner_prefix, owner_suffix, owner_prefix] -/// word[0] word[1] word[2] word[3] -/// ``` -/// -/// After `get_item` (which reverses the word onto the MASM stack), the stack is: -/// -/// ```text -/// Stack: [owner_prefix, owner_suffix, nominated_owner_prefix, nominated_owner_suffix] -/// (word[3]) (word[2]) (word[1]) (word[0]) +/// Word: [owner_suffix, owner_prefix, nominated_owner_suffix, nominated_owner_prefix] +/// word[0] word[1] word[2] word[3] /// ``` pub struct Ownable2Step { owner: Option, @@ -61,12 +54,12 @@ impl Ownable2Step { /// Reconstructs an [`Ownable2Step`] from a raw storage word. /// - /// Format: `[nominated_suffix, nominated_prefix, owner_suffix, owner_prefix]` + /// Format: `[owner_suffix, owner_prefix, nominated_suffix, nominated_prefix]` pub fn try_from_word(word: Word) -> Result { - let owner = account_id_from_felt_pair(word[3], word[2]) + let owner = account_id_from_felt_pair(word[0], word[1]) .map_err(Ownable2StepError::InvalidOwnerId)?; - let nominated_owner = account_id_from_felt_pair(word[1], word[0]) + let nominated_owner = account_id_from_felt_pair(word[2], word[3]) .map_err(Ownable2StepError::InvalidNominatedOwnerId)?; Ok(Self { owner, nominated_owner }) @@ -87,10 +80,10 @@ impl Ownable2Step { StorageSlotSchema::value( "Ownership data (owner and nominated owner)", [ - FeltSchema::felt("nominated_suffix"), - FeltSchema::felt("nominated_prefix"), FeltSchema::felt("owner_suffix"), FeltSchema::felt("owner_prefix"), + FeltSchema::felt("nominated_suffix"), + FeltSchema::felt("nominated_prefix"), ], ), ) @@ -113,15 +106,15 @@ impl Ownable2Step { /// Converts this ownership data into a raw [`Word`]. pub fn to_word(&self) -> Word { - let (owner_prefix, owner_suffix) = match self.owner { - Some(id) => (id.prefix().as_felt(), id.suffix()), + let (owner_suffix, owner_prefix) = match self.owner { + Some(id) => (id.suffix(), id.prefix().as_felt()), None => (Felt::ZERO, Felt::ZERO), }; - let (nominated_prefix, nominated_suffix) = match self.nominated_owner { - Some(id) => (id.prefix().as_felt(), id.suffix()), + let (nominated_suffix, nominated_prefix) = match self.nominated_owner { + Some(id) => (id.suffix(), id.prefix().as_felt()), None => (Felt::ZERO, Felt::ZERO), }; - [nominated_suffix, nominated_prefix, owner_suffix, owner_prefix].into() + [owner_suffix, owner_prefix, nominated_suffix, nominated_prefix].into() } } @@ -142,15 +135,15 @@ pub enum Ownable2StepError { // HELPERS // ================================================================================================ -/// Constructs an `Option` from a prefix/suffix felt pair. +/// Constructs an `Option` from a suffix/prefix felt pair. /// Returns `Ok(None)` when both felts are zero (renounced / no nomination). fn account_id_from_felt_pair( - prefix: Felt, suffix: Felt, + prefix: Felt, ) -> Result, AccountIdError> { - if prefix == Felt::ZERO && suffix == Felt::ZERO { + if suffix == Felt::ZERO && prefix == Felt::ZERO { Ok(None) } else { - AccountId::try_from([prefix, suffix]).map(Some) + AccountId::try_from_elements(suffix, prefix).map(Some) } } diff --git a/crates/miden-standards/src/account/faucets/network_fungible.rs b/crates/miden-standards/src/account/faucets/network_fungible.rs index 92e8622ac7..92e2c3c0f8 100644 --- a/crates/miden-standards/src/account/faucets/network_fungible.rs +++ b/crates/miden-standards/src/account/faucets/network_fungible.rs @@ -144,8 +144,8 @@ impl NetworkFungibleFaucet { let metadata = TokenMetadata::try_from(storage)?; // Read ownership data from storage - let ownership = Ownable2Step::try_from_storage(storage) - .map_err(|err| FungibleFaucetError::OwnershipError(err))?; + let ownership = + Ownable2Step::try_from_storage(storage).map_err(FungibleFaucetError::OwnershipError)?; Ok(Self { metadata, ownership }) } diff --git a/crates/miden-testing/tests/scripts/faucet.rs b/crates/miden-testing/tests/scripts/faucet.rs index 3ed68b061b..96ee6d1a5e 100644 --- a/crates/miden-testing/tests/scripts/faucet.rs +++ b/crates/miden-testing/tests/scripts/faucet.rs @@ -570,16 +570,15 @@ async fn network_faucet_mint() -> anyhow::Result<()> { assert_eq!(actual_max_supply.as_canonical_u64(), max_supply); // Check that the creator account ID is stored in the ownership slot. - // Rust Word: [nominated_suffix, nominated_prefix, owner_suffix, owner_prefix] - let stored_owner_id = - faucet.storage().get_item(Ownable2Step::slot_name()).unwrap(); - assert_eq!(stored_owner_id[3], faucet_owner_account_id.prefix().as_felt()); + // Word: [owner_suffix, owner_prefix, nominated_suffix, nominated_prefix] + let stored_owner_id = faucet.storage().get_item(Ownable2Step::slot_name()).unwrap(); assert_eq!( - stored_owner_id[2], + stored_owner_id[0], Felt::new(faucet_owner_account_id.suffix().as_canonical_u64()) ); - assert_eq!(stored_owner_id[1], Felt::new(0)); // no nominated owner - assert_eq!(stored_owner_id[0], Felt::new(0)); + assert_eq!(stored_owner_id[1], faucet_owner_account_id.prefix().as_felt()); + assert_eq!(stored_owner_id[2], Felt::new(0)); // no nominated owner + assert_eq!(stored_owner_id[3], Felt::new(0)); // Check that the faucet's token supply has been correctly initialized. // The already issued amount should be 50. @@ -787,11 +786,11 @@ async fn test_network_faucet_owner_storage() -> anyhow::Result<()> { // Verify owner is stored correctly let stored_owner = faucet.storage().get_item(Ownable2Step::slot_name())?; - // Rust Word: [nominated_suffix, nominated_prefix, owner_suffix, owner_prefix] - assert_eq!(stored_owner[3], owner_account_id.prefix().as_felt()); - assert_eq!(stored_owner[2], Felt::new(owner_account_id.suffix().as_canonical_u64())); - assert_eq!(stored_owner[1], Felt::new(0)); // no nominated owner - assert_eq!(stored_owner[0], Felt::new(0)); + // Word: [owner_suffix, owner_prefix, nominated_suffix, nominated_prefix] + assert_eq!(stored_owner[0], Felt::new(owner_account_id.suffix().as_canonical_u64())); + assert_eq!(stored_owner[1], owner_account_id.prefix().as_felt()); + assert_eq!(stored_owner[2], Felt::new(0)); // no nominated owner + assert_eq!(stored_owner[3], Felt::new(0)); Ok(()) } @@ -930,11 +929,12 @@ async fn test_network_faucet_transfer_ownership() -> anyhow::Result<()> { final_faucet.apply_delta(executed_transaction.account_delta())?; // Verify that owner changed to new_owner and nominated was cleared + // Word: [owner_suffix, owner_prefix, nominated_suffix, nominated_prefix] let stored_owner = final_faucet.storage().get_item(Ownable2Step::slot_name())?; - assert_eq!(stored_owner[3], new_owner_account_id.prefix().as_felt()); - assert_eq!(stored_owner[2], Felt::new(new_owner_account_id.suffix().as_int())); - assert_eq!(stored_owner[1], Felt::new(0)); // nominated cleared - assert_eq!(stored_owner[0], Felt::new(0)); + assert_eq!(stored_owner[0], Felt::new(new_owner_account_id.suffix().as_canonical_u64())); + assert_eq!(stored_owner[1], new_owner_account_id.prefix().as_felt()); + assert_eq!(stored_owner[2], Felt::new(0)); // nominated cleared + assert_eq!(stored_owner[3], Felt::new(0)); Ok(()) } @@ -1029,10 +1029,9 @@ async fn test_network_faucet_renounce_ownership() -> anyhow::Result<()> { let faucet = builder.add_existing_network_faucet("NET", 1000, owner_account_id, Some(50))?; // Check stored value before renouncing - let stored_owner_before = - faucet.storage().get_item(Ownable2Step::slot_name())?; - assert_eq!(stored_owner_before[3], owner_account_id.prefix().as_felt()); - assert_eq!(stored_owner_before[2], Felt::new(owner_account_id.suffix().as_canonical_u64())); + let stored_owner_before = faucet.storage().get_item(Ownable2Step::slot_name())?; + assert_eq!(stored_owner_before[0], Felt::new(owner_account_id.suffix().as_canonical_u64())); + assert_eq!(stored_owner_before[1], owner_account_id.prefix().as_felt()); // Create renounce_ownership note script let renounce_note_script_code = r#" @@ -1099,8 +1098,7 @@ async fn test_network_faucet_renounce_ownership() -> anyhow::Result<()> { updated_faucet.apply_delta(executed_transaction.account_delta())?; // Check stored value after renouncing - should be zero - let stored_owner_after = - updated_faucet.storage().get_item(Ownable2Step::slot_name())?; + let stored_owner_after = updated_faucet.storage().get_item(Ownable2Step::slot_name())?; assert_eq!(stored_owner_after[0], Felt::new(0)); assert_eq!(stored_owner_after[1], Felt::new(0)); assert_eq!(stored_owner_after[2], Felt::new(0)); diff --git a/crates/miden-testing/tests/scripts/ownable2step.rs b/crates/miden-testing/tests/scripts/ownable2step.rs index 654fd08926..1b4d6ae01f 100644 --- a/crates/miden-testing/tests/scripts/ownable2step.rs +++ b/crates/miden-testing/tests/scripts/ownable2step.rs @@ -2,7 +2,7 @@ extern crate alloc; use alloc::sync::Arc; -use miden_processor::crypto::RpoRandomCoin; +use miden_processor::crypto::random::RpoRandomCoin; use miden_protocol::account::component::AccountComponentMetadata; use miden_protocol::account::{ Account, @@ -10,6 +10,7 @@ use miden_protocol::account::{ AccountComponent, AccountId, AccountStorageMode, + AccountType, StorageSlot, StorageSlotName, }; @@ -53,10 +54,10 @@ fn create_ownable_account( CodeBuilder::default().compile_component_code("test::ownable", component_code)?; let ownership_word: Word = [ - Felt::new(0), // word[0] = nominated_suffix - Felt::new(0), // word[1] = nominated_prefix - owner.suffix(), // word[2] = owner_suffix - owner.prefix().as_felt(), // word[3] = owner_prefix + owner.suffix(), // word[0] = owner_suffix + owner.prefix().as_felt(), // word[1] = owner_prefix + Felt::new(0), // word[2] = nominated_suffix + Felt::new(0), // word[3] = nominated_prefix ] .into(); @@ -67,7 +68,7 @@ fn create_ownable_account( .storage_mode(AccountStorageMode::Public) .with_auth_component(Auth::IncrNonce) .with_component({ - let metadata = AccountComponentMetadata::new("test::ownable").with_supports_all_types(); + let metadata = AccountComponentMetadata::new("test::ownable", AccountType::all()); AccountComponent::new(component_code_obj, storage_slots, metadata)? }) .build_existing()?; @@ -76,23 +77,23 @@ fn create_ownable_account( fn get_owner_from_storage(account: &Account) -> anyhow::Result> { let word = account.storage().get_item(&OWNER_CONFIG_SLOT_NAME)?; - let prefix = word[3]; - let suffix = word[2]; - if prefix == Felt::new(0) && suffix == Felt::new(0) { + let suffix = word[0]; + let prefix = word[1]; + if suffix == Felt::new(0) && prefix == Felt::new(0) { Ok(None) } else { - Ok(Some(AccountId::try_from([prefix, suffix])?)) + Ok(Some(AccountId::try_from_elements(suffix, prefix)?)) } } fn get_nominated_owner_from_storage(account: &Account) -> anyhow::Result> { let word = account.storage().get_item(&OWNER_CONFIG_SLOT_NAME)?; - let prefix = word[1]; - let suffix = word[0]; - if prefix == Felt::new(0) && suffix == Felt::new(0) { + let suffix = word[2]; + let prefix = word[3]; + if suffix == Felt::new(0) && prefix == Felt::new(0) { Ok(None) } else { - Ok(Some(AccountId::try_from([prefix, suffix])?)) + Ok(Some(AccountId::try_from_elements(suffix, prefix)?)) } } @@ -107,13 +108,14 @@ fn create_transfer_note( use miden::standards::access::ownable2step->test_account begin repeat.14 push.0 end - push.{new_owner_suffix} push.{new_owner_prefix} + push.{new_owner_prefix} + push.{new_owner_suffix} call.test_account::transfer_ownership dropw dropw dropw dropw end "#, new_owner_prefix = new_owner.prefix().as_felt(), - new_owner_suffix = new_owner.suffix(), + new_owner_suffix = Felt::new(new_owner.suffix().as_canonical_u64()), ); let note = NoteBuilder::new(sender, rng) From 3ddc88e6acd37082f439e045afc44367ddc1affa Mon Sep 17 00:00:00 2001 From: Arthur Abeilice Date: Fri, 6 Mar 2026 20:16:55 -0300 Subject: [PATCH 6/6] refactor: rename owner and nominated_owner to get_owner and get_nominated_owner for consistency --- .../account_components/faucets/network_fungible_faucet.masm | 4 ++-- crates/miden-standards/asm/standards/access/ownable2step.masm | 4 ++-- .../asm/standards/faucets/network_fungible.masm | 4 ++-- crates/miden-standards/src/account/access/ownable2step.rs | 1 + crates/miden-testing/tests/scripts/ownable2step.rs | 4 ++-- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/miden-standards/asm/account_components/faucets/network_fungible_faucet.masm b/crates/miden-standards/asm/account_components/faucets/network_fungible_faucet.masm index 45520f9090..7cded18fb0 100644 --- a/crates/miden-standards/asm/account_components/faucets/network_fungible_faucet.masm +++ b/crates/miden-standards/asm/account_components/faucets/network_fungible_faucet.masm @@ -4,8 +4,8 @@ pub use ::miden::standards::faucets::network_fungible::distribute pub use ::miden::standards::faucets::network_fungible::burn -pub use ::miden::standards::faucets::network_fungible::owner -pub use ::miden::standards::faucets::network_fungible::nominated_owner +pub use ::miden::standards::faucets::network_fungible::get_owner +pub use ::miden::standards::faucets::network_fungible::get_nominated_owner pub use ::miden::standards::faucets::network_fungible::transfer_ownership pub use ::miden::standards::faucets::network_fungible::accept_ownership pub use ::miden::standards::faucets::network_fungible::renounce_ownership diff --git a/crates/miden-standards/asm/standards/access/ownable2step.masm b/crates/miden-standards/asm/standards/access/ownable2step.masm index 7732420c5e..d066181252 100644 --- a/crates/miden-standards/asm/standards/access/ownable2step.masm +++ b/crates/miden-standards/asm/standards/access/ownable2step.masm @@ -192,7 +192,7 @@ end #! - owner_{suffix, prefix} are the suffix and prefix felts of the owner account ID. #! #! Invocation: call -pub proc owner +pub proc get_owner exec.get_owner_internal # => [owner_suffix, owner_prefix, pad(16)] @@ -210,7 +210,7 @@ end #! owner account ID. Both are zero if no nominated transfer exists. #! #! Invocation: call -pub proc nominated_owner +pub proc get_nominated_owner exec.get_nominated_owner_internal # => [nominated_owner_suffix, nominated_owner_prefix, pad(16)] diff --git a/crates/miden-standards/asm/standards/faucets/network_fungible.masm b/crates/miden-standards/asm/standards/faucets/network_fungible.masm index 52123e6ee7..74aed661e2 100644 --- a/crates/miden-standards/asm/standards/faucets/network_fungible.masm +++ b/crates/miden-standards/asm/standards/faucets/network_fungible.masm @@ -7,8 +7,8 @@ use miden::standards::access::ownable2step # OWNER MANAGEMENT (re-exported from ownable2step) # ------------------------------------------------------------------------------------------------ -pub use ownable2step::owner -pub use ownable2step::nominated_owner +pub use ownable2step::get_owner +pub use ownable2step::get_nominated_owner pub use ownable2step::transfer_ownership pub use ownable2step::accept_ownership pub use ownable2step::renounce_ownership diff --git a/crates/miden-standards/src/account/access/ownable2step.rs b/crates/miden-standards/src/account/access/ownable2step.rs index cbcd4eece4..ec3668a7a4 100644 --- a/crates/miden-standards/src/account/access/ownable2step.rs +++ b/crates/miden-standards/src/account/access/ownable2step.rs @@ -24,6 +24,7 @@ static OWNER_CONFIG_SLOT_NAME: LazyLock = LazyLock::new(|| { /// word[0] word[1] word[2] word[3] /// ``` pub struct Ownable2Step { + /// The current owner of the component. `None` when ownership has been renounced. owner: Option, nominated_owner: Option, } diff --git a/crates/miden-testing/tests/scripts/ownable2step.rs b/crates/miden-testing/tests/scripts/ownable2step.rs index 1b4d6ae01f..bad70ee1b7 100644 --- a/crates/miden-testing/tests/scripts/ownable2step.rs +++ b/crates/miden-testing/tests/scripts/ownable2step.rs @@ -44,8 +44,8 @@ fn create_ownable_account( ) -> anyhow::Result { let component_code = r#" use miden::standards::access::ownable2step - pub use ownable2step::owner - pub use ownable2step::nominated_owner + pub use ownable2step::get_owner + pub use ownable2step::get_nominated_owner pub use ownable2step::transfer_ownership pub use ownable2step::accept_ownership pub use ownable2step::renounce_ownership