feat(standards): extend `Authority` with role assignment map by onurinanc · Pull Request #3072 · 0xMiden/protocol

onurinanc · 2026-06-09T13:28:23Z

Closes #3070.

PhilippGackstatter

Looks good. The main comment is about removal of the default role.

PhilippGackstatter · 2026-06-10T13:24:18Z

+    # => [mapped_role, default_role, 0, 0]
+
+    dup eq.0
+    if.true


Suggested change

# => [mapped_role, default_role, 0, 0]

dup eq.0

if.true

# => [mapped_role, default_role, 0, 0]

dup eq.0

# => [has_no_mapped_role, default_role, 0, 0]

if.true

Nit: I would indicate what the comparison of a role to zero means.

PhilippGackstatter · 2026-06-10T13:25:32Z

+        # No per-procedure role; fall back to the default.
+        drop
+        # => [default_role, 0, 0]
+
+        dup eq.0
+        if.true
+            # Zero default role — defer to the Ownable2Step owner.
+            drop drop drop
+            # => []
+            exec.ownable2step::assert_sender_is_owner
+            # => []
+        else
+            # Non-zero default role — require it.
+            # => [default_role, 0, 0]
+            exec.rbac::assert_sender_has_role
+            # => [0, 0]
+            drop drop
+            # => []
+        end


In the interest of simplification, I would remove the default_role and only have a two step logic: If a procedure is configured, assert the sender has the role, if not, require the owner.

PhilippGackstatter · 2026-06-10T13:28:09Z

+static AUTHORITY_ROLE_MAP_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| {
+    StorageSlotName::new("miden::standards::access::authority::role_map")
+        .expect("storage slot name should be valid")
+});


Nit: Maybe we can capture the "procedure root to role" mapping in the slot name somehow? Something like procedure_roles?

partylikeits1983

Looks good! I agree with Philipp, I think the default_role should be removed. Essentially if a procedure is configured with some role, check if the sender has that role, if not require the owner.

Also left some nits regarding masm formatting, but these might be removed if / once default_role is removed.

partylikeits1983 · 2026-06-10T21:07:00Z

+    # Resolve the calling procedure's root and look up its assigned role in the map.
+    padw caller
+    # => [CALLER_ROOT, default_role, 0, 0]
+    push.AUTHORITY_ROLE_MAP_SLOT[0..2]
+    # => [slot_suffix, slot_prefix, CALLER_ROOT, default_role, 0, 0]
+    exec.active_account::get_map_item
+    # => [mapped_role, 0, 0, 0, default_role, 0, 0]


formatting:

Suggested change

# Resolve the calling procedure's root and look up its assigned role in the map.

padw caller

# => [CALLER_ROOT, default_role, 0, 0]

push.AUTHORITY_ROLE_MAP_SLOT[0..2]

# => [slot_suffix, slot_prefix, CALLER_ROOT, default_role, 0, 0]

exec.active_account::get_map_item

# => [mapped_role, 0, 0, 0, default_role, 0, 0]

# Resolve the calling procedure's root and look up its assigned role in the map.

padw caller

# => [CALLER_ROOT, default_role, 0, 0]

push.AUTHORITY_ROLE_MAP_SLOT[0..2]

# => [slot_suffix, slot_prefix, CALLER_ROOT, default_role, 0, 0]

exec.active_account::get_map_item

# => [mapped_role, 0, 0, 0, default_role, 0, 0]

partylikeits1983 · 2026-06-10T21:09:57Z

+        # Per-procedure role found — require it.
+        # => [mapped_role, default_role, 0, 0]
+        exec.rbac::assert_sender_has_role
+        # => [default_role, 0, 0]
+        drop drop drop
+        # => []


I think the wording of this comment is a bit weird, can we update it to something like this? Also line formatting is off.

Suggested change

# Per-procedure role found — require it.

# => [mapped_role, default_role, 0, 0]

exec.rbac::assert_sender_has_role

# => [default_role, 0, 0]

drop drop drop

# => []

# Assert sender has role

# => [mapped_role, default_role, 0, 0]

exec.rbac::assert_sender_has_role

# => [default_role, 0, 0]

drop drop drop

# => []

partylikeits1983 · 2026-06-10T21:12:09Z

+            # Zero default role — defer to the Ownable2Step owner.
+            drop drop drop
+            # => []
+            exec.ownable2step::assert_sender_is_owner
+            # => []


Suggested change

# Zero default role — defer to the Ownable2Step owner.

drop drop drop

# => []

exec.ownable2step::assert_sender_is_owner

# => []

# Zero default role, defer to the Ownable2Step owner.

drop drop drop

# => []

exec.ownable2step::assert_sender_is_owner

# => []

bobbinth

Looks good! Thank you! I agree with @PhilippGackstatter and @partylikeits1983 that we could probably get rid of the default role and fall back to the owner immediately.

…ment

…authority-role-assignment

PhilippGackstatter

LGTM - left a few non-blocking nits.

PhilippGackstatter · 2026-06-11T12:18:14Z

+    fn config_word(&self) -> Word {
+        let discriminant = match self {
+            Authority::AuthControlled => AUTH_CONTROLLED,
+            Authority::OwnerControlled => OWNER_CONTROLLED,
+            Authority::RbacControlled { .. } => RBAC_CONTROLLED,
+        };
+        Word::new([Felt::from(discriminant), Felt::ZERO, Felt::ZERO, Felt::ZERO])


Nit: I'd maybe extract this logic into a separate fn as_u8(&self) -> u8 as it may become useful in other places.

PhilippGackstatter · 2026-06-11T12:20:57Z

+        let slot = storage
+            .slots()
+            .iter()
+            .find(|slot| slot.name().id() == AUTHORITY_PROCEDURE_ROLES_SLOT_NAME.id())
+            .ok_or(AuthorityError::InvalidProcedureRolesSlot)?;


Nit: Should we align the name of the error with MissingStorageSlot and call it MissingProcedureRolesSlot since it's missing in this case, not invalid? Alternatively, we reuse and adapt MissingStorageSlot for this use case.

PhilippGackstatter · 2026-06-11T12:26:05Z

+    /// Encodes the authority configuration value slot word: `[authority, 0, 0, 0]`.
+    fn config_word(&self) -> Word {


I think this could be just to_word, now that it encodes only the discriminant.

PhilippGackstatter · 2026-06-11T12:33:32Z

@@ -38,6 +47,9 @@ const ERR_PAUSABLE_IS_PAUSED: MasmError = MasmError::from_static_str("the contra

 const ERR_SENDER_NOT_OWNER: MasmError = MasmError::from_static_str("note sender is not the owner");

+const ERR_SENDER_LACKS_ROLE: MasmError =
+    MasmError::from_static_str("note sender does not hold the required role");
+


These can all be imported as miden_standards::errors::standards::{ERR_PAUSABLE_IS_PAUSED, ERR_SENDER_LACKS_ROLE, ERR_SENDER_NOT_OWNER}

PhilippGackstatter · 2026-06-11T12:56:06Z

+    let mut mock_chain = builder.build()?;
+    mock_chain.prove_next_block()?;
+
+    execute_note_on_faucet(&mut mock_chain, faucet.id(), &grant_pauser).await?;
+    execute_note_on_faucet(&mut mock_chain, faucet.id(), &grant_unpauser).await?;
+
+    // PAUSER pauses → the faucet records the paused state.
+    execute_note_on_faucet(&mut mock_chain, faucet.id(), &pause_note).await?;
+    assert!(is_faucet_paused(&mock_chain, faucet.id())?);
+
+    // UNPAUSER unpauses → the faucet records the unpaused state.
+    execute_note_on_faucet(&mut mock_chain, faucet.id(), &unpause_note).await?;
+    assert!(!is_faucet_paused(&mock_chain, faucet.id())?);


This is a super clean test 🙌

bobbinth

Looks good! Thank you!

onurinanc added 3 commits June 9, 2026 15:27

extend authority with role assignment map

2f63c43

changelog

7efcf41

Merge branch 'next' into authority-role-assignment

6ee936c

onurinanc requested review from PhilippGackstatter and bobbinth June 9, 2026 14:06

PhilippGackstatter reviewed Jun 10, 2026

View reviewed changes

partylikeits1983 reviewed Jun 10, 2026

View reviewed changes

bobbinth reviewed Jun 11, 2026

View reviewed changes

onurinanc added 3 commits June 11, 2026 11:46

remove default role

36b480e

Merge remote-tracking branch 'origin/next' into authority-role-assign…

348794e

…ment

Merge remote-tracking branch 'origin/authority-role-assignment' into …

920bc79

…authority-role-assignment

onurinanc requested review from PhilippGackstatter, bobbinth and partylikeits1983 June 11, 2026 09:58

PhilippGackstatter approved these changes Jun 11, 2026

View reviewed changes

onurinanc and others added 2 commits June 11, 2026 16:21

nits

71c8fd3

Merge branch 'next' into authority-role-assignment

bfce45e

bobbinth approved these changes Jun 12, 2026

View reviewed changes

Merge branch 'next' into authority-role-assignment

c562188

bobbinth enabled auto-merge June 12, 2026 21:21

bobbinth added this pull request to the merge queue Jun 12, 2026

Merged via the queue into next with commit cb11680 Jun 12, 2026
20 checks passed

bobbinth deleted the authority-role-assignment branch June 12, 2026 21:38

		/// Encodes the authority configuration value slot word: `[authority, 0, 0, 0]`.
		fn config_word(&self) -> Word {

Conversation

onurinanc commented Jun 9, 2026

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

partylikeits1983 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants