-
Notifications
You must be signed in to change notification settings - Fork 144
feat(standards): add guardian component #3125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # The MASM code of the Guardian Account Component. | ||
| # | ||
| # See the `Guardian` Rust type's documentation for more details. | ||
|
|
||
| pub use ::miden::standards::access::guardian::get_guardian | ||
| pub use ::miden::standards::access::guardian::set_guardian |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| # miden::standards::access::guardian | ||
| # | ||
| # Provides a standalone "guardian" actor for account components: a single guardian account ID | ||
| # stored in one value slot, and account-ID equality authorization primitives. | ||
| # | ||
| # The guardian is a second privileged actor, distinct from the owner, intended as a safety brake: | ||
| # other components consult `assert_sender_is_guardian` / `is_sender_guardian` to authorize | ||
| # some actions (e.g. an emergency freeze, or cancelling a scheduled operation). The guardian | ||
| # can never grant roles, move assets, or perform owner-only actions. It is purely an authorization | ||
| # subject checked by the procedures that opt into it. | ||
| # | ||
| # Authorization is a plain account-ID equality check, so this component needs no RBAC | ||
| # infrastructure and behaves the same across all authority modes. If no guardian is assigned the | ||
| # stored ID is the zero address `(0, 0)`, and the equality check returns 0 for every real sender, | ||
| # yielding "no guardian => nobody passes the guardian check" for free. | ||
| # | ||
| # Storage layout (single slot): | ||
| # Word: [guardian_suffix, guardian_prefix, 0, 0] | ||
|
|
||
| use miden::protocol::active_account | ||
| use miden::protocol::account_id | ||
| use miden::protocol::active_note | ||
| use miden::protocol::native_account | ||
| use miden::standards::access::authority | ||
|
|
||
| # CONSTANTS | ||
| # ================================================================================================ | ||
|
|
||
| # The slot in this component's storage layout where the guardian account ID is stored. | ||
| const GUARDIAN_SLOT = word("miden::standards::access::guardian::guardian_id") | ||
|
|
||
| # ERRORS | ||
| # ================================================================================================ | ||
|
|
||
| const ERR_SENDER_NOT_GUARDIAN = "note sender is not the guardian" | ||
|
|
||
| # INTERNAL PROCEDURES | ||
| # ================================================================================================ | ||
|
|
||
| #! Returns the guardian account ID from storage. | ||
| #! | ||
| #! Inputs: [] | ||
| #! Outputs: [guardian_suffix, guardian_prefix] | ||
| #! | ||
| #! Where: | ||
| #! - guardian_{suffix, prefix} are the suffix and prefix felts of the guardian account ID. Both | ||
| #! are zero if no guardian is assigned. | ||
| proc get_guardian_internal | ||
| push.GUARDIAN_SLOT[0..2] exec.active_account::get_item | ||
| # => [guardian_suffix, guardian_prefix, 0, 0] | ||
|
|
||
| movup.2 drop movup.2 drop | ||
| # => [guardian_suffix, guardian_prefix] | ||
| end | ||
|
|
||
| #! Writes the guardian word to storage and drops the old value. | ||
| #! | ||
| #! Inputs: [guardian_suffix, guardian_prefix, 0, 0] | ||
| #! Outputs: [] | ||
| proc save_guardian_info | ||
| push.GUARDIAN_SLOT[0..2] | ||
| # => [slot_suffix, slot_prefix, guardian_suffix, guardian_prefix, 0, 0] | ||
|
|
||
| exec.native_account::set_item | ||
| # => [OLD_GUARDIAN_WORD] | ||
|
|
||
| dropw | ||
| # => [] | ||
| end | ||
|
|
||
| #! Checks if the given account ID is the guardian. | ||
| #! | ||
| #! Inputs: [account_id_suffix, account_id_prefix] | ||
| #! Outputs: [is_guardian] | ||
| #! | ||
| #! Where: | ||
| #! - account_id_{suffix, prefix} are the suffix and prefix felts of the account ID to check. | ||
| #! - is_guardian is 1 if the account is the guardian, 0 otherwise. | ||
| proc is_guardian_internal | ||
| exec.get_guardian_internal | ||
| # => [guardian_suffix, guardian_prefix, account_id_suffix, account_id_prefix] | ||
|
|
||
| exec.account_id::is_equal | ||
| # => [is_guardian] | ||
| end | ||
|
|
||
| #! Returns 1 if the note sender is the guardian, otherwise 0. | ||
| #! | ||
| #! Inputs: [] | ||
| #! Outputs: [is_sender_guardian] | ||
| #! | ||
| #! Where: | ||
| #! - is_sender_guardian is 1 if the note sender is the guardian, otherwise 0. | ||
| #! | ||
| #! Invocation: exec | ||
| pub proc is_sender_guardian | ||
| exec.active_note::get_sender | ||
| # => [sender_suffix, sender_prefix] | ||
|
|
||
| exec.is_guardian_internal | ||
| # => [is_sender_guardian] | ||
| end | ||
|
|
||
| #! Asserts that the note sender is the guardian. | ||
| #! | ||
| #! Inputs: [] | ||
| #! Outputs: [] | ||
| #! | ||
| #! Panics if: | ||
| #! - the note sender is not the guardian. | ||
| #! | ||
| #! Invocation: exec | ||
| pub proc assert_sender_is_guardian | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: This procedure is also in the internal section -> move to "PUBLIC INTERFACE" section. |
||
| exec.is_sender_guardian | ||
| # => [is_sender_guardian] | ||
|
|
||
| assert.err=ERR_SENDER_NOT_GUARDIAN | ||
| # => [] | ||
| end | ||
|
|
||
| # PUBLIC INTERFACE | ||
| # ================================================================================================ | ||
|
|
||
| #! Returns the guardian account ID. | ||
| #! | ||
| #! Inputs: [pad(16)] | ||
| #! Outputs: [guardian_suffix, guardian_prefix, pad(14)] | ||
| #! | ||
| #! Where: | ||
| #! - guardian_{suffix, prefix} are the suffix and prefix felts of the guardian account ID. Both | ||
| #! are zero if no guardian is assigned. | ||
| #! | ||
| #! Invocation: call | ||
| pub proc get_guardian | ||
| exec.get_guardian_internal | ||
| # => [guardian_suffix, guardian_prefix, pad(16)] | ||
|
|
||
| movup.2 drop movup.2 drop | ||
| # => [guardian_suffix, guardian_prefix, pad(14)] | ||
| end | ||
|
|
||
| #! Sets or clears the guardian account ID. | ||
| #! | ||
| #! Authorized through `authority::assert_authorized`, so the gate is mode-aware. The owner under | ||
| #! `OwnerControlled`, the configured role (or the owner fallback) under `RbacControlled`, and the | ||
| #! account's auth component under `AuthControlled`. Requires the [`Authority`] component to be | ||
| #! installed on the account. | ||
| #! | ||
| #! Clearing behaviour: | ||
| #! - If `new_guardian` is the zero address `(0, 0)`, the guardian is cleared. The zero address is | ||
| #! treated as a clear value and is not validated as an account ID. | ||
| #! - Otherwise, `new_guardian` is validated and stored as the guardian. | ||
| #! | ||
| #! Inputs: [new_guardian_suffix, new_guardian_prefix, pad(14)] | ||
| #! Outputs: [pad(16)] | ||
| #! | ||
| #! Panics if: | ||
| #! - the sender is not authorized. | ||
| #! - new_guardian is non-zero and the account ID is invalid. | ||
| #! | ||
| #! Invocation: call | ||
| pub proc set_guardian | ||
| exec.authority::assert_authorized | ||
| # => [new_guardian_suffix, new_guardian_prefix, pad(14)] | ||
|
|
||
| # Detect explicit clear via the zero address. The zero address is not a valid account ID, so | ||
| # we must check for it before validating. | ||
| dup.1 eq.0 dup.1 eq.0 and | ||
| # => [is_zero_address, new_guardian_suffix, new_guardian_prefix, pad(14)] | ||
|
Comment on lines
+169
to
+172
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should we add |
||
|
|
||
| if.false | ||
| # Non-zero new guardian: validate before storing. | ||
| dup.1 dup.1 exec.account_id::validate | ||
| end | ||
| # => [new_guardian_suffix, new_guardian_prefix, pad(14)] | ||
|
|
||
| push.0.0 | ||
| # => [0, 0, new_guardian_suffix, new_guardian_prefix, pad(14)] | ||
|
|
||
| movup.3 movup.3 | ||
| # => [new_guardian_suffix, new_guardian_prefix, 0, 0, pad(14)] | ||
|
|
||
| exec.save_guardian_info | ||
| # => [pad(16)] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Consider building the word in |
||
| end | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,192 @@ | ||||||||||||||||
| use miden_protocol::account::component::{ | ||||||||||||||||
| AccountComponentCode, | ||||||||||||||||
| AccountComponentMetadata, | ||||||||||||||||
| FeltSchema, | ||||||||||||||||
| StorageSchema, | ||||||||||||||||
| StorageSlotSchema, | ||||||||||||||||
| }; | ||||||||||||||||
| use miden_protocol::account::{ | ||||||||||||||||
| AccountComponent, | ||||||||||||||||
| AccountComponentName, | ||||||||||||||||
| AccountId, | ||||||||||||||||
| AccountStorage, | ||||||||||||||||
| StorageSlot, | ||||||||||||||||
| StorageSlotName, | ||||||||||||||||
| }; | ||||||||||||||||
| use miden_protocol::errors::AccountIdError; | ||||||||||||||||
| use miden_protocol::utils::sync::LazyLock; | ||||||||||||||||
| use miden_protocol::{Felt, Word}; | ||||||||||||||||
|
|
||||||||||||||||
| use crate::account::account_component_code; | ||||||||||||||||
|
|
||||||||||||||||
| account_component_code!(GUARDIAN_CODE, "access/guardian.masl"); | ||||||||||||||||
|
|
||||||||||||||||
| static GUARDIAN_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| { | ||||||||||||||||
| StorageSlotName::new("miden::standards::access::guardian::guardian_id") | ||||||||||||||||
| .expect("storage slot name should be valid") | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| /// A standalone guardian actor for account components. | ||||||||||||||||
| /// | ||||||||||||||||
| /// Stores a single guardian account ID. The guardian is a second privileged actor, distinct from | ||||||||||||||||
| /// the owner, intended as a safety brake: other components consult the MASM | ||||||||||||||||
| /// `assert_sender_is_guardian` / `is_sender_guardian` primitives to authorize stop-like actions. | ||||||||||||||||
| /// The guardian can never grant roles, move assets, or perform owner-only actions. | ||||||||||||||||
| /// | ||||||||||||||||
| /// Authorization is a plain account-ID equality check, so the component needs no RBAC | ||||||||||||||||
| /// infrastructure and behaves the same across all authority modes. When no guardian is assigned | ||||||||||||||||
| /// the stored ID is the zero address and the guardian check fails for every sender. | ||||||||||||||||
| /// | ||||||||||||||||
| /// ## Storage Layout | ||||||||||||||||
| /// | ||||||||||||||||
| /// The guardian data is stored in a single word: | ||||||||||||||||
| /// | ||||||||||||||||
| /// ```text | ||||||||||||||||
| /// Word: [guardian_suffix, guardian_prefix, 0, 0] | ||||||||||||||||
| /// word[0] word[1] word[2] word[3] | ||||||||||||||||
| /// ``` | ||||||||||||||||
|
Comment on lines
+44
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Nit: I think this matches the layout we use in most places. |
||||||||||||||||
| pub struct Guardian { | ||||||||||||||||
| /// The current guardian. `None` when no guardian is assigned. | ||||||||||||||||
| guardian: Option<AccountId>, | ||||||||||||||||
| } | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the name is fine in principle, but since it is technically distinct from the Miden Guardian, I wonder if we should choose a different name. Then again, this guardian will probably be the same entity as the Miden Guardian most of the time, so maybe that's okay?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my mind, Miden Guardian should use this component eventually for some use cases such as freezing procedures, but I'm not sure if it is planned to support this as a feature or not. I think @bobbinth was suggesting to change this name to something else. |
||||||||||||||||
|
|
||||||||||||||||
| impl Guardian { | ||||||||||||||||
| /// The name of the component. | ||||||||||||||||
| pub const NAME: &'static str = "miden::standards::access::guardian"; | ||||||||||||||||
|
|
||||||||||||||||
| /// Returns the canonical [`AccountComponentName`] of this component. | ||||||||||||||||
| pub const fn name() -> AccountComponentName { | ||||||||||||||||
| AccountComponentName::from_static_str(Self::NAME) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Returns the [`AccountComponentCode`] of this component. | ||||||||||||||||
| pub fn code() -> &'static AccountComponentCode { | ||||||||||||||||
| &GUARDIAN_CODE | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // CONSTRUCTORS | ||||||||||||||||
| // -------------------------------------------------------------------------------------------- | ||||||||||||||||
|
|
||||||||||||||||
| /// Creates a new [`Guardian`] with the given guardian account ID. | ||||||||||||||||
| pub fn new(guardian: AccountId) -> Self { | ||||||||||||||||
| Self { guardian: Some(guardian) } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Creates a new [`Guardian`] with no guardian assigned. | ||||||||||||||||
| pub fn unassigned() -> Self { | ||||||||||||||||
| Self { guardian: None } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Reads guardian data from account storage, validating any non-zero account ID. | ||||||||||||||||
| /// | ||||||||||||||||
| /// Returns an error if the guardian contains an invalid (but non-zero) account ID. | ||||||||||||||||
| pub fn try_from_storage(storage: &AccountStorage) -> Result<Self, GuardianError> { | ||||||||||||||||
| let word: Word = storage | ||||||||||||||||
| .get_item(Self::slot_name()) | ||||||||||||||||
| .map_err(GuardianError::StorageLookupFailed)?; | ||||||||||||||||
|
|
||||||||||||||||
| Self::try_from_word(word) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Reconstructs a [`Guardian`] from a raw storage word. | ||||||||||||||||
| /// | ||||||||||||||||
| /// Format: `[guardian_suffix, guardian_prefix, 0, 0]` | ||||||||||||||||
| pub fn try_from_word(word: Word) -> Result<Self, GuardianError> { | ||||||||||||||||
| let guardian = account_id_from_felt_pair(word[0], word[1]) | ||||||||||||||||
| .map_err(GuardianError::InvalidGuardianId)?; | ||||||||||||||||
|
|
||||||||||||||||
| Ok(Self { guardian }) | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+90
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should use the same layout as |
||||||||||||||||
|
|
||||||||||||||||
| // PUBLIC ACCESSORS | ||||||||||||||||
| // -------------------------------------------------------------------------------------------- | ||||||||||||||||
|
|
||||||||||||||||
| /// Returns the [`StorageSlotName`] where guardian data is stored. | ||||||||||||||||
| pub fn slot_name() -> &'static StorageSlotName { | ||||||||||||||||
| &GUARDIAN_SLOT_NAME | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Returns the storage slot schema for the guardian configuration slot. | ||||||||||||||||
| pub fn slot_schema() -> (StorageSlotName, StorageSlotSchema) { | ||||||||||||||||
| ( | ||||||||||||||||
| Self::slot_name().clone(), | ||||||||||||||||
| StorageSlotSchema::value( | ||||||||||||||||
| "Guardian account ID", | ||||||||||||||||
| [ | ||||||||||||||||
| FeltSchema::felt("guardian_suffix"), | ||||||||||||||||
| FeltSchema::felt("guardian_prefix"), | ||||||||||||||||
| FeltSchema::felt("unused_0"), | ||||||||||||||||
| FeltSchema::felt("unused_1"), | ||||||||||||||||
| ], | ||||||||||||||||
| ), | ||||||||||||||||
| ) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Returns the current guardian, or `None` if no guardian is assigned. | ||||||||||||||||
| pub fn guardian(&self) -> Option<AccountId> { | ||||||||||||||||
| self.guardian | ||||||||||||||||
| } | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: |
||||||||||||||||
|
|
||||||||||||||||
| /// Converts this guardian data into a [`StorageSlot`]. | ||||||||||||||||
| pub fn to_storage_slot(&self) -> StorageSlot { | ||||||||||||||||
| StorageSlot::with_value(Self::slot_name().clone(), self.to_word()) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Converts this guardian data into a raw [`Word`]. | ||||||||||||||||
| pub fn to_word(&self) -> Word { | ||||||||||||||||
| let (guardian_suffix, guardian_prefix) = match self.guardian { | ||||||||||||||||
| Some(id) => (id.suffix(), id.prefix().as_felt()), | ||||||||||||||||
| None => (Felt::ZERO, Felt::ZERO), | ||||||||||||||||
| }; | ||||||||||||||||
| [guardian_suffix, guardian_prefix, Felt::ZERO, Felt::ZERO].into() | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /// Returns the [`AccountComponentMetadata`] for this component. | ||||||||||||||||
| pub fn component_metadata() -> AccountComponentMetadata { | ||||||||||||||||
| let storage_schema = | ||||||||||||||||
| StorageSchema::new([Self::slot_schema()]).expect("storage schema should be valid"); | ||||||||||||||||
|
|
||||||||||||||||
| AccountComponentMetadata::new(Self::NAME) | ||||||||||||||||
| .with_description("Standalone guardian actor component") | ||||||||||||||||
| .with_storage_schema(storage_schema) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| impl From<Guardian> for AccountComponent { | ||||||||||||||||
| fn from(guardian: Guardian) -> Self { | ||||||||||||||||
| let storage_slot = guardian.to_storage_slot(); | ||||||||||||||||
| let metadata = Guardian::component_metadata(); | ||||||||||||||||
|
|
||||||||||||||||
| AccountComponent::new(Guardian::code().clone(), vec![storage_slot], metadata).expect( | ||||||||||||||||
| "Guardian component should satisfy the requirements of a valid account component", | ||||||||||||||||
| ) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // GUARDIAN ERROR | ||||||||||||||||
| // ================================================================================================ | ||||||||||||||||
|
|
||||||||||||||||
| /// Errors that can occur when reading [`Guardian`] data from storage. | ||||||||||||||||
| #[derive(Debug, thiserror::Error)] | ||||||||||||||||
| pub enum GuardianError { | ||||||||||||||||
| #[error("failed to read guardian slot from storage")] | ||||||||||||||||
| StorageLookupFailed(#[source] miden_protocol::errors::AccountError), | ||||||||||||||||
| #[error("invalid guardian account ID in storage")] | ||||||||||||||||
| InvalidGuardianId(#[source] AccountIdError), | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // HELPERS | ||||||||||||||||
| // ================================================================================================ | ||||||||||||||||
|
|
||||||||||||||||
| /// Constructs an `Option<AccountId>` from a suffix/prefix felt pair. | ||||||||||||||||
| /// Returns `Ok(None)` when both felts are zero (no guardian assigned). | ||||||||||||||||
| fn account_id_from_felt_pair( | ||||||||||||||||
| suffix: Felt, | ||||||||||||||||
| prefix: Felt, | ||||||||||||||||
| ) -> Result<Option<AccountId>, AccountIdError> { | ||||||||||||||||
| if suffix == Felt::ZERO && prefix == Felt::ZERO { | ||||||||||||||||
| Ok(None) | ||||||||||||||||
| } else { | ||||||||||||||||
| AccountId::try_from_elements(suffix, prefix).map(Some) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question, could this be an internal procedure?
Also since this procedure is in the "internal procedures" section, maybe it makes sense to make it an internal procedure or move it lower in this file. This same comment applies to
assert_sender_is_guardianprocedure.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the catch! I think this procedure shouldn't be in the internal procedures part, instead it should be in the public procedures part. Then, the procedures exposed to the account component should be in the external procedures section.