Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Features

- Added `Authority::read_procedure_roles`, which reads the per-procedure role map even when a role felt carries no `RoleSymbol` ([#3524](https://github.com/0xMiden/protocol/pull/3524)).

### Changes

- Documented the RBAC freeze-only actor pattern on `Authority` and added test coverage pinning that a `FREEZER` can trip the emergency switch but can never unfreeze the account ([#3520](https://github.com/0xMiden/protocol/pull/3520)).
Expand Down
125 changes: 119 additions & 6 deletions crates/miden-standards/src/account/access/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,41 @@ const OWNER_CONTROLLED: u8 = 1;
/// Authority value written to the storage slot for [`Authority::RbacControlled`].
const RBAC_CONTROLLED: u8 = 2;

// PROCEDURE ROLE
// ================================================================================================

/// The role required by an authority-gated procedure, as read from the procedure-roles map.
///
/// A role is identified on-chain by a field element: `rbac::assert_sender_has_role` matches that
/// felt against the RBAC membership map without ever decoding it. A [`RoleSymbol`] is the printable
/// name the felt decodes to, which exists only for canonically encoded values, so
/// [`to_symbol`][Self::to_symbol] returns `None` for a role that carries no name.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct ProcedureRole(Felt);
Comment on lines +83 to +84

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably go the other way here: instead of accepting invalid RoleSymbols, we can enforce that all roles are valid role symbols onchain. I believe this would be just an extra "less-than" check.


impl ProcedureRole {
/// Returns the role identified by `felt`.
pub fn new(felt: Felt) -> Self {
Self(felt)
}

/// Returns the felt identifying the role, the form on-chain authorization matches on.
pub fn as_felt(&self) -> Felt {
self.0
}

/// Returns the role's symbol, or `None` if its felt is not a valid [`RoleSymbol`] encoding.
pub fn to_symbol(&self) -> Option<RoleSymbol> {
RoleSymbol::try_from(self.0).ok()
}
}

impl From<&RoleSymbol> for ProcedureRole {
fn from(role: &RoleSymbol) -> Self {
Self(role.as_element())
}
}

// AUTHORITY
// ================================================================================================

Expand Down Expand Up @@ -253,6 +288,22 @@ impl Authority {
Ok(word[1] != Felt::ZERO)
}

/// Reads the per-procedure role map without requiring every role to carry a decodable symbol.
///
/// # Errors
///
/// Returns an error if:
/// - the procedure-roles storage slot is missing or is not a map.
/// - a role's value word carries data in its reserved felts.
pub fn read_procedure_roles(
storage: &AccountStorage,
) -> Result<BTreeMap<AccountProcedureRoot, ProcedureRole>, AuthorityError> {
Ok(Self::read_role_felts_from_storage(storage)?
.into_iter()
.map(|(proc_root, role_felt)| (proc_root, ProcedureRole::new(role_felt)))
.collect())
}

/// Returns the [`AccountComponentMetadata`] for this configuration.
pub fn component_metadata(&self) -> AccountComponentMetadata {
let mut slots = vec![(
Expand Down Expand Up @@ -323,10 +374,25 @@ impl Authority {
Ok(word)
}

/// Reconstructs the per-procedure role map from the procedure-roles storage slot.
/// Reconstructs the per-procedure role map from the procedure-roles storage slot, requiring
/// every role felt to decode into a [`RoleSymbol`].
fn read_roles_from_storage(
storage: &AccountStorage,
) -> Result<BTreeMap<AccountProcedureRoot, RoleSymbol>, AuthorityError> {
Self::read_role_felts_from_storage(storage)?
.into_iter()
.map(|(proc_root, role_felt)| {
let role =
RoleSymbol::try_from(role_felt).map_err(AuthorityError::InvalidRoleSymbol)?;
Ok((proc_root, role))
})
.collect()
}

/// Reads the procedure-roles storage slot into the raw role felt of every entry.
fn read_role_felts_from_storage(
storage: &AccountStorage,
) -> Result<BTreeMap<AccountProcedureRoot, Felt>, AuthorityError> {
let slot = storage
.slots()
.iter()
Expand All @@ -337,18 +403,16 @@ impl Authority {
return Err(AuthorityError::MissingProcedureRolesSlot);
};

let mut roles = BTreeMap::new();
let mut role_felts = BTreeMap::new();
for (key, value) in map.entries() {
// Enforce the canonical encoding on read: the reserved felts must be zero.
if value[1..4].iter().any(|v| *v != Felt::ZERO) {
return Err(AuthorityError::NonCanonicalConfig);
}
let proc_root = AccountProcedureRoot::from_raw(key.as_word());
let role = RoleSymbol::try_from(value[0]).map_err(AuthorityError::InvalidRoleSymbol)?;
roles.insert(proc_root, role);
role_felts.insert(AccountProcedureRoot::from_raw(key.as_word()), value[0]);
}

Ok(roles)
Ok(role_felts)
}
}

Expand Down Expand Up @@ -507,5 +571,54 @@ mod tests {
Authority::try_from_storage(&storage),
Err(AuthorityError::NonCanonicalConfig)
);
assert_matches!(
Authority::read_procedure_roles(&storage),
Err(AuthorityError::NonCanonicalConfig)
);
}

/// On-chain a role is just a felt, so an account built outside `RbacControlled` can hold one
/// that decodes to no symbol.
#[test]
fn role_felt_without_a_symbol_is_read_without_one() {
// 27 is the smallest non-zero felt whose base-27 length digit is 0, so it encodes no
// symbol.
let unnamed_role_felt = Felt::from(27u8);
let expected_root = AccountProcedureRoot::from_raw(Word::from(ROLE_KEY_WORD));
let storage = rbac_storage_with_role_value(Word::new([
unnamed_role_felt,
Felt::ZERO,
Felt::ZERO,
Felt::ZERO,
]));

// Reconstructing the typed value still fails: `RbacControlled` holds `RoleSymbol`s.
assert_matches!(
Authority::try_from_storage(&storage),
Err(AuthorityError::InvalidRoleSymbol(_))
);

// Reading the map succeeds and preserves the felt on-chain authorization matches on.
let roles = Authority::read_procedure_roles(&storage).unwrap();
let role = roles.get(&expected_root).expect("the entry should be read");
assert_eq!(role.as_felt(), unnamed_role_felt);
assert_eq!(role.to_symbol(), None);
}

/// A canonical role felt is read with its symbol, which is the `RoleSymbol` that wrote it.
#[test]
fn role_felt_with_a_symbol_is_read_with_it() {
let role = RoleSymbol::new("ADMIN").unwrap();
let expected_root = AccountProcedureRoot::from_raw(Word::from(ROLE_KEY_WORD));
let storage = rbac_storage_with_role_value(Word::new([
role.as_element(),
Felt::ZERO,
Felt::ZERO,
Felt::ZERO,
]));

let roles = Authority::read_procedure_roles(&storage).unwrap();
assert_eq!(roles.get(&expected_root), Some(&ProcedureRole::from(&role)));
assert_eq!(roles[&expected_root].to_symbol(), Some(role));
}
}
2 changes: 1 addition & 1 deletion crates/miden-standards/src/account/access/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl IntoIterator for AccessControl {
}
}

pub use authority::{Authority, AuthorityError};
pub use authority::{Authority, AuthorityError, ProcedureRole};
pub use ownable2step::{Ownable2Step, Ownable2StepError};
pub use pausable::{Pausable, PausableManager, PausableStorage};
pub use rbac::{RoleBasedAccessControl, RoleBasedAccessControlError, RoleConfig};
Expand Down
Loading