diff --git a/packages/utils/src/components.cairo b/packages/utils/src/components.cairo index 6581ec90..5d68863e 100644 --- a/packages/utils/src/components.cairo +++ b/packages/utils/src/components.cairo @@ -1,4 +1,5 @@ pub mod blocklist; +pub mod common_roles; pub mod deposit; pub mod nonce; pub mod pausable; diff --git a/packages/utils/src/components/blocklist/blocklist.cairo b/packages/utils/src/components/blocklist/blocklist.cairo index bc56b75c..d7f4a097 100644 --- a/packages/utils/src/components/blocklist/blocklist.cairo +++ b/packages/utils/src/components/blocklist/blocklist.cairo @@ -1,13 +1,13 @@ #[starknet::component] pub mod blocklist { - use RolesComponent::InternalTrait as RolesInternalTrait; + use CommonRolesComponent::InternalTrait as CommonRolesInternalTrait; use openzeppelin::access::accesscontrol::AccessControlComponent; use openzeppelin::introspection::src5::SRC5Component; use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess}; use starknet::{ContractAddress, get_caller_address}; use starkware_utils::components::blocklist::events::{Blocklisted, Unblocklisted}; use starkware_utils::components::blocklist::interface::IBlocklist; - use starkware_utils::components::roles::RolesComponent; + use starkware_utils::components::common_roles::CommonRolesComponent; #[storage] pub struct Storage { @@ -30,7 +30,7 @@ pub mod blocklist { TContractState, +HasComponent, +Drop, - impl Roles: RolesComponent::HasComponent, + impl CommonRoles: CommonRolesComponent::HasComponent, +AccessControlComponent::HasComponent, +SRC5Component::HasComponent, > of IBlocklist> { @@ -43,8 +43,8 @@ pub mod blocklist { /// /// Can be called only by security admin. fn add_to_blocklist(ref self: ComponentState, account: ContractAddress) { - let roles = get_dep_component!(@self, Roles); - roles.only_security_admin(); + let common_roles = get_dep_component!(@self, CommonRoles); + common_roles.only_security_admin(); self.blocklist.write(account, true); self.emit(Blocklisted { account, caller: get_caller_address() }); } @@ -55,8 +55,8 @@ pub mod blocklist { fn remove_from_blocklist( ref self: ComponentState, account: ContractAddress, ) { - let roles = get_dep_component!(@self, Roles); - roles.only_security_admin(); + let common_roles = get_dep_component!(@self, CommonRoles); + common_roles.only_security_admin(); self.blocklist.write(account, false); self.emit(Unblocklisted { account, caller: get_caller_address() }); } diff --git a/packages/utils/src/components/blocklist/mock_contract.cairo b/packages/utils/src/components/blocklist/mock_contract.cairo index 44fb591e..0ff77a4f 100644 --- a/packages/utils/src/components/blocklist/mock_contract.cairo +++ b/packages/utils/src/components/blocklist/mock_contract.cairo @@ -6,6 +6,7 @@ pub mod blocklist_mock_contract { use starknet::ContractAddress; use starkware_utils::components::blocklist::blocklist::blocklist as BlocklistComponent; use starkware_utils::components::blocklist::blocklist::blocklist::InternalTrait as BlocklistInternalTrait; + use starkware_utils::components::common_roles::CommonRolesComponent; use starkware_utils::components::roles::RolesComponent; use starkware_utils::components::roles::RolesComponent::InternalTrait as RolesInternalTrait; use starkware_utils::interfaces::mintable_token::IMintableToken; @@ -13,6 +14,7 @@ pub mod blocklist_mock_contract { component!(path: ERC20Component, storage: erc20, event: ERC20Event); component!(path: BlocklistComponent, storage: blocklist, event: BlocklistEvent); component!(path: RolesComponent, storage: roles, event: RolesEvent); + component!(path: CommonRolesComponent, storage: common_roles, event: CommonRolesEvent); component!(path: AccessControlComponent, storage: access_control, event: AccessControlEvent); component!(path: SRC5Component, storage: src5, event: SRC5Event); @@ -21,7 +23,6 @@ pub mod blocklist_mock_contract { #[abi(embed_v0)] impl ERC20MetadataImpl = ERC20Component::ERC20MetadataImpl; - #[abi(embed_v0)] impl BlocklistImpl = BlocklistComponent::BlocklistImpl; #[abi(embed_v0)] @@ -39,6 +40,8 @@ pub mod blocklist_mock_contract { #[substorage(v0)] roles: RolesComponent::Storage, #[substorage(v0)] + common_roles: CommonRolesComponent::Storage, + #[substorage(v0)] access_control: AccessControlComponent::Storage, #[substorage(v0)] src5: SRC5Component::Storage, @@ -54,6 +57,8 @@ pub mod blocklist_mock_contract { #[flat] RolesEvent: RolesComponent::Event, #[flat] + CommonRolesEvent: CommonRolesComponent::Event, + #[flat] AccessControlEvent: AccessControlComponent::Event, #[flat] SRC5Event: SRC5Component::Event, diff --git a/packages/utils/src/components/common_roles.cairo b/packages/utils/src/components/common_roles.cairo new file mode 100644 index 00000000..661efb15 --- /dev/null +++ b/packages/utils/src/components/common_roles.cairo @@ -0,0 +1,9 @@ +pub(crate) mod common_roles; + +pub use common_roles::CommonRolesComponent; + +#[cfg(test)] +pub(crate) mod mock_contract; + +#[cfg(test)] +mod test; diff --git a/packages/utils/src/components/common_roles/common_roles.cairo b/packages/utils/src/components/common_roles/common_roles.cairo new file mode 100644 index 00000000..27e2df63 --- /dev/null +++ b/packages/utils/src/components/common_roles/common_roles.cairo @@ -0,0 +1,335 @@ +#[starknet::component] +pub(crate) mod CommonRolesComponent { + use core::num::traits::Zero; + use openzeppelin::access::accesscontrol::AccessControlComponent; + use openzeppelin::access::accesscontrol::AccessControlComponent::{ + AccessControlImpl, InternalTrait as AccessInternalTrait, + }; + use openzeppelin::introspection::src5::SRC5Component; + use starknet::storage::{ + StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess, + StoragePointerWriteAccess, + }; + use starknet::{ContractAddress, get_caller_address}; + use starkware_utils::components::roles::errors::AccessErrors; + use starkware_utils::components::roles::interface::{ + APP_GOVERNOR, APP_ROLE_ADMIN, GOVERNANCE_ADMIN, ICommonRoles, OPERATOR, Role, RoleId, + SECURITY_ADMIN, SECURITY_AGENT, SECURITY_GOVERNOR, TOKEN_ADMIN, UPGRADE_AGENT, + UPGRADE_GOVERNOR, is_renounceable, + }; + + // Maps each role to its admin role — used both at initialization (to configure OZ + // AccessControl) + // and during legacy role reclaim (to ensure admin pairs are set on upgraded contracts). + // `GOVERNANCE_ADMIN` and `SECURITY_ADMIN` are self-administered (admin == role). + pub const ROLE_ADMIN_PAIRS: [(RoleId, RoleId); 10] = [ + (APP_GOVERNOR, APP_ROLE_ADMIN), (APP_ROLE_ADMIN, GOVERNANCE_ADMIN), + (GOVERNANCE_ADMIN, GOVERNANCE_ADMIN), (OPERATOR, APP_ROLE_ADMIN), + (TOKEN_ADMIN, APP_ROLE_ADMIN), (UPGRADE_AGENT, APP_ROLE_ADMIN), + (UPGRADE_GOVERNOR, GOVERNANCE_ADMIN), (SECURITY_ADMIN, SECURITY_ADMIN), + (SECURITY_AGENT, SECURITY_ADMIN), (SECURITY_GOVERNOR, SECURITY_ADMIN), + ]; + + #[storage] + pub struct Storage { + // LEGACY: Role-membership map from before OZ AccessControl integration. + // Written only by the legacy reclaim path (_reclaim_role erases each entry as it migrates + // it to OZ storage). Private — test mocks access it via the #[rename] storage-collision + // trick rather than requiring pub visibility on production code. + role_members: starknet::storage::Map<(RoleId, ContractAddress), bool>, + // Guards the one-time reclaim window. Fresh contracts set this to `true` in `initialize`; + // legacy-upgraded contracts leave it `false` until the upgrade is complete and + // `disable_legacy_role_reclaim` is called by an upgrade governor. + legacy_role_reclaim_disabled: bool, + } + + #[event] + #[derive(Drop, starknet::Event)] + pub enum Event {} + + // ─── Public ABI (ICommonRoles) + // ──────────────────────────────────────────── + // These 7 entry points are the minimal role-management surface. They are the + // only role-related entry points on Tier-A contracts (CommonRolesComponent + // only), and they complement the named-role entry points on Tier-B/C contracts + // (RolesComponent adds IRoles / category interfaces on top). + #[embeddable_as(CommonRolesImpl)] + pub impl CommonRoles< + TContractState, + +HasComponent, + +Drop, + impl Access: AccessControlComponent::HasComponent, + +SRC5Component::HasComponent, + > of ICommonRoles> { + fn grant_role( + ref self: ComponentState, role: Role, account: ContractAddress, + ) { + InternalTrait::grant_role(ref self, :role, :account); + } + + fn revoke_role( + ref self: ComponentState, role: Role, account: ContractAddress, + ) { + InternalTrait::revoke_role(ref self, :role, :account); + } + + fn has_role( + self: @ComponentState, role: Role, account: ContractAddress, + ) -> bool { + InternalTrait::has_role(self, role, account) + } + + fn renounce(ref self: ComponentState, role: Role) { + // GOVERNANCE_ADMIN and SECURITY_ADMIN are non-renounceable — self-removal of those + // roles would leave the contract permanently ungovernable / un-paused. + assert!(is_renounceable(role), "{}", AccessErrors::ROLE_CANNOT_BE_RENOUNCED); + let role_id: RoleId = role.into(); + let mut access_comp = get_dep_component_mut!(ref self, Access); + access_comp.renounce_role(role: role_id, account: get_caller_address()); + } + + // Allows the caller to migrate their own roles from legacy storage to OZ AccessControl. + // Also calls ensure_role_admins in case this is the first account to reclaim on a + // contract that was upgraded without going through initialize(). + fn reclaim_legacy_roles(ref self: ComponentState) { + InternalTrait::assert_role_reclaim_enabled(@self); + InternalTrait::_reclaim_legacy_roles_for_account(ref self, get_caller_address()); + InternalTrait::ensure_role_admins(ref self); + } + + // Batch version gated behind SECURITY_GOVERNOR — intended for the upgrade governor to + // migrate multiple accounts in one tx. ensure_role_admins is not called here because the + // upgrade constructor always calls it before any account can reach this path. + fn reclaim_legacy_roles_for_accounts( + ref self: ComponentState, accounts: Span, + ) { + InternalTrait::only_security_governor(@self); + InternalTrait::assert_role_reclaim_enabled(@self); + for account in accounts { + InternalTrait::_reclaim_legacy_roles_for_account(ref self, *account); + }; + } + + // Permanently closes the reclaim window. Idempotent. Called by an upgrade governor + // once all legacy roles have been migrated. + fn disable_legacy_role_reclaim(ref self: ComponentState) { + InternalTrait::only_upgrade_governor(@self); + self.legacy_role_reclaim_disabled.write(true); + } + } + + // ─── Internal helpers + // ───────────────────────────────────────────────────── + #[generate_trait] + pub impl InternalImpl< + TContractState, + +HasComponent, + +Drop, + impl Access: AccessControlComponent::HasComponent, + +SRC5Component::HasComponent, + > of InternalTrait { + // WARNING + // Unprotected — call only from a constructor (or test setup). + // Sets up OZ AccessControl (initializer + role admin pairs), grants GOVERNANCE_ADMIN and + // SECURITY_ADMIN to `governance_admin`, then immediately disables legacy role reclaim + // (fresh contracts have no legacy storage to migrate). + fn initialize(ref self: ComponentState, governance_admin: ContractAddress) { + let mut access_comp = get_dep_component_mut!(ref self, Access); + // Detect double-initialization: if GOVERNANCE_ADMIN already has an admin it means + // initialize() was already called on this deployment. + let uninitialized = access_comp.get_role_admin(role: GOVERNANCE_ADMIN).is_zero(); + assert!(uninitialized, "{}", AccessErrors::ALREADY_INITIALIZED); + access_comp.initializer(); + assert!(governance_admin.is_non_zero(), "{}", AccessErrors::ZERO_ADDRESS_GOV_ADMIN); + access_comp._grant_role(role: GOVERNANCE_ADMIN, account: governance_admin); + access_comp._grant_role(role: SECURITY_ADMIN, account: governance_admin); + Self::_set_role_admins(ref access_comp); + // Fresh contracts have no legacy storage — disable reclaim immediately. + self.legacy_role_reclaim_disabled.write(true); + } + + // Idempotent: only sets admin pairs whose current admin is zero. This lets it be called + // on both fresh deployments and upgrades without overwriting intentional customization. + fn _set_role_admins( + ref access_comp: AccessControlComponent::ComponentState, + ) { + for (role, admin_role) in ROLE_ADMIN_PAIRS.span() { + if access_comp.get_role_admin(role: *role).is_zero() { + access_comp.set_role_admin(role: *role, admin_role: *admin_role); + } + } + } + + // WARNING + // Unprotected — intended for use by contracts performing an upgrade migration + // that adds new roles to an already-initialized deployment. Idempotent: only sets + // role-admin pairs that are currently zero, so safe to call on a fresh contract too. + fn ensure_role_admins(ref self: ComponentState) { + let mut access_comp = get_dep_component_mut!(ref self, Access); + Self::_set_role_admins(ref access_comp); + } + + fn has_role( + self: @ComponentState, role: Role, account: ContractAddress, + ) -> bool { + let role_id: RoleId = role.into(); + let access_comp = get_dep_component!(self, Access); + access_comp.has_role(role: role_id, :account) + } + + /// Grants `role` to `account`. Enforces caller is the role's admin (via OZ grant_role). + /// Rejects the zero address — granting a role to zero is always a mistake and can make + /// the role permanently inaccessible if it is self-administered (e.g., GOVERNANCE_ADMIN). + fn grant_role( + ref self: ComponentState, role: Role, account: ContractAddress, + ) { + assert!(account.is_non_zero(), "{}", AccessErrors::ZERO_ADDRESS); + let role: RoleId = role.into(); + let mut access_comp = get_dep_component_mut!(ref self, Access); + access_comp.grant_role(:role, :account); + } + + /// Revokes `role` from `account`. Blocks self-revoke of non-renounceable roles. + /// Enforces caller is the role's admin (via OZ revoke_role). + fn revoke_role( + ref self: ComponentState, role: Role, account: ContractAddress, + ) { + assert!( + get_caller_address() != account || is_renounceable(role), + "{}", + AccessErrors::ROLE_CANNOT_BE_RENOUNCED, + ); + let role: RoleId = role.into(); + let mut access_comp = get_dep_component_mut!(ref self, Access); + access_comp.revoke_role(:role, :account); + } + + // ─── Role guards + // ───────────────────────────────────────────────────── + // These are the canonical authorization checks. Both `CommonRolesComponent` and + // `RolesComponent` expose them as internal methods so sibling components can call + // them without going through the public ABI. + fn only_role(self: @ComponentState, role: Role) { + let role_id: RoleId = role.into(); + let access_comp = get_dep_component!(self, Access); + assert!( + access_comp.has_role(role: role_id, account: get_caller_address()), + "{}", + AccessErrors::CALLER_MISSING_ROLE, + ); + } + + fn only_app_governor(self: @ComponentState) { + let access_comp = get_dep_component!(self, Access); + assert!( + access_comp.has_role(role: APP_GOVERNOR, account: get_caller_address()), + "{}", + AccessErrors::ONLY_APP_GOVERNOR, + ); + } + + fn only_operator(self: @ComponentState) { + let access_comp = get_dep_component!(self, Access); + assert!( + access_comp.has_role(role: OPERATOR, account: get_caller_address()), + "{}", + AccessErrors::ONLY_OPERATOR, + ); + } + + fn only_token_admin(self: @ComponentState) { + let access_comp = get_dep_component!(self, Access); + assert!( + access_comp.has_role(role: TOKEN_ADMIN, account: get_caller_address()), + "{}", + AccessErrors::ONLY_TOKEN_ADMIN, + ); + } + + fn only_upgrade_governor(self: @ComponentState) { + let access_comp = get_dep_component!(self, Access); + assert!( + access_comp.has_role(role: UPGRADE_GOVERNOR, account: get_caller_address()), + "{}", + AccessErrors::ONLY_UPGRADE_GOVERNOR, + ); + } + + fn only_upgrader(self: @ComponentState) { + let access_comp = get_dep_component!(self, Access); + let caller = get_caller_address(); + assert!( + access_comp.has_role(role: UPGRADE_AGENT, account: caller) + || access_comp.has_role(role: UPGRADE_GOVERNOR, account: caller), + "{}", + AccessErrors::ONLY_UPGRADER, + ); + } + + fn only_security_admin(self: @ComponentState) { + let access_comp = get_dep_component!(self, Access); + assert!( + access_comp.has_role(role: SECURITY_ADMIN, account: get_caller_address()), + "{}", + AccessErrors::ONLY_SECURITY_ADMIN, + ); + } + + fn only_security_agent(self: @ComponentState) { + let access_comp = get_dep_component!(self, Access); + assert!( + access_comp.has_role(role: SECURITY_AGENT, account: get_caller_address()), + "{}", + AccessErrors::ONLY_SECURITY_AGENT, + ); + } + + fn only_security_governor(self: @ComponentState) { + let access_comp = get_dep_component!(self, Access); + assert!( + access_comp.has_role(role: SECURITY_GOVERNOR, account: get_caller_address()), + "{}", + AccessErrors::ONLY_SECURITY_GOVERNOR, + ); + } + + fn assert_role_reclaim_enabled(self: @ComponentState) { + assert!( + !self.legacy_role_reclaim_disabled.read(), + "{}", + AccessErrors::LEGACY_ROLE_RECLAIM_DISABLED, + ); + } + + // NOTE: parameter order is (account, role) but the storage key is (role, account) — + // intentional, but keep in mind when reading call sites. + fn _has_legacy_role( + self: @ComponentState, account: ContractAddress, role: RoleId, + ) -> bool { + self.role_members.read((role, account)) + } + + // Migrates a single (role, account) entry: clears the legacy map entry and grants + // the role in OZ AccessControl. No-op if the account doesn't hold the role in legacy + // storage — safe to call unconditionally for all roles. + fn _reclaim_role( + ref self: ComponentState, role: RoleId, account: ContractAddress, + ) { + if self._has_legacy_role(account, role) { + self.role_members.write((role, account), false); + let mut access_comp = get_dep_component_mut!(ref self, Access); + access_comp._grant_role(:role, :account); + } + } + + // Iterates all 10 known roles for one account. O(10) regardless of which roles the + // account actually holds — acceptable for a one-time migration path. + fn _reclaim_legacy_roles_for_account( + ref self: ComponentState, account: ContractAddress, + ) { + for (role, _) in ROLE_ADMIN_PAIRS.span() { + self._reclaim_role(role: *role, :account); + } + } + } +} diff --git a/packages/utils/src/components/common_roles/mock_contract.cairo b/packages/utils/src/components/common_roles/mock_contract.cairo new file mode 100644 index 00000000..626a0936 --- /dev/null +++ b/packages/utils/src/components/common_roles/mock_contract.cairo @@ -0,0 +1,153 @@ +/// Backdoor for setting legacy role membership in tests. +#[starknet::interface] +pub(crate) trait ILegacySetup { + fn set_legacy_role(ref self: TState, role_id: felt252, account: starknet::ContractAddress); +} + +/// Simulates a contract that was deployed with the old role storage and then upgraded to +/// CommonRolesComponent without calling `initialize` (so `legacy_role_reclaim_disabled` +/// remains false, and legacy role data can be reclaimed). +#[starknet::contract] +pub(crate) mod LegacyCommonRolesMock { + use openzeppelin::access::accesscontrol::AccessControlComponent; + use openzeppelin::access::accesscontrol::AccessControlComponent::InternalTrait as AccessInternalTrait; + use openzeppelin::introspection::src5::SRC5Component; + use starknet::ContractAddress; + use starknet::storage::StorageMapWriteAccess; + use starkware_utils::components::common_roles::CommonRolesComponent; + use starkware_utils::components::common_roles::CommonRolesComponent::InternalTrait as CommonRolesInternalTrait; + use starkware_utils::components::roles::interface::{GOVERNANCE_ADMIN, SECURITY_ADMIN}; + use super::ILegacySetup; + + component!(path: CommonRolesComponent, storage: common_roles, event: CommonRolesEvent); + component!(path: AccessControlComponent, storage: accesscontrol, event: AccessControlEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + #[storage] + #[allow(starknet::colliding_storage_paths)] + struct Storage { + #[substorage(v0)] + common_roles: CommonRolesComponent::Storage, + #[substorage(v0)] + accesscontrol: AccessControlComponent::Storage, + #[substorage(v0)] + src5: SRC5Component::Storage, + // Collides with CommonRolesComponent::role_members via substorage(v0) flat layout. + // Used only for test setup — allows writing legacy role entries without pub visibility. + #[rename("role_members")] + legacy_role_members: starknet::storage::Map<(felt252, ContractAddress), bool>, + } + + #[event] + #[derive(Drop, starknet::Event)] + pub(crate) enum Event { + CommonRolesEvent: CommonRolesComponent::Event, + AccessControlEvent: AccessControlComponent::Event, + SRC5Event: SRC5Component::Event, + } + + #[constructor] + fn constructor(ref self: ContractState, governance_admin: ContractAddress) { + // Migrate path: wire roles without initialize(), leaving + // legacy_role_reclaim_disabled=false. + self.accesscontrol.initializer(); + self.accesscontrol._grant_role(role: GOVERNANCE_ADMIN, account: governance_admin); + self.accesscontrol._grant_role(role: SECURITY_ADMIN, account: governance_admin); + self.common_roles.ensure_role_admins(); + } + + #[abi(embed_v0)] + impl CommonRolesImpl = CommonRolesComponent::CommonRolesImpl; + + #[abi(embed_v0)] + impl LegacySetupImpl of ILegacySetup { + fn set_legacy_role(ref self: ContractState, role_id: felt252, account: ContractAddress) { + self.legacy_role_members.write((role_id, account), true); + } + } +} + +#[starknet::interface] +pub(crate) trait IGuardTest { + fn assert_only_role(self: @TState, role: starkware_utils::components::roles::interface::Role); + fn assert_only_app_governor(self: @TState); + fn assert_only_operator(self: @TState); + fn assert_only_token_admin(self: @TState); + fn assert_only_upgrade_governor(self: @TState); + fn assert_only_upgrader(self: @TState); + fn assert_only_security_admin(self: @TState); + fn assert_only_security_agent(self: @TState); + fn assert_only_security_governor(self: @TState); +} + +#[starknet::contract] +pub(crate) mod CommonRolesMock { + use openzeppelin::access::accesscontrol::AccessControlComponent; + use openzeppelin::introspection::src5::SRC5Component; + use starknet::ContractAddress; + use starkware_utils::components::common_roles::CommonRolesComponent; + use starkware_utils::components::common_roles::CommonRolesComponent::InternalTrait as CommonRolesInternalTrait; + use starkware_utils::components::roles::interface::Role; + use super::IGuardTest; + + component!(path: CommonRolesComponent, storage: common_roles, event: CommonRolesEvent); + component!(path: AccessControlComponent, storage: accesscontrol, event: AccessControlEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + #[storage] + struct Storage { + #[substorage(v0)] + common_roles: CommonRolesComponent::Storage, + #[substorage(v0)] + accesscontrol: AccessControlComponent::Storage, + #[substorage(v0)] + src5: SRC5Component::Storage, + } + + #[event] + #[derive(Drop, starknet::Event)] + pub(crate) enum Event { + CommonRolesEvent: CommonRolesComponent::Event, + AccessControlEvent: AccessControlComponent::Event, + SRC5Event: SRC5Component::Event, + } + + #[constructor] + fn constructor(ref self: ContractState, governance_admin: ContractAddress) { + self.common_roles.initialize(:governance_admin); + } + + #[abi(embed_v0)] + impl CommonRolesImpl = CommonRolesComponent::CommonRolesImpl; + + #[abi(embed_v0)] + impl GuardTestImpl of IGuardTest { + fn assert_only_role(self: @ContractState, role: Role) { + self.common_roles.only_role(:role); + } + fn assert_only_app_governor(self: @ContractState) { + self.common_roles.only_app_governor(); + } + fn assert_only_operator(self: @ContractState) { + self.common_roles.only_operator(); + } + fn assert_only_token_admin(self: @ContractState) { + self.common_roles.only_token_admin(); + } + fn assert_only_upgrade_governor(self: @ContractState) { + self.common_roles.only_upgrade_governor(); + } + fn assert_only_upgrader(self: @ContractState) { + self.common_roles.only_upgrader(); + } + fn assert_only_security_admin(self: @ContractState) { + self.common_roles.only_security_admin(); + } + fn assert_only_security_agent(self: @ContractState) { + self.common_roles.only_security_agent(); + } + fn assert_only_security_governor(self: @ContractState) { + self.common_roles.only_security_governor(); + } + } +} diff --git a/packages/utils/src/components/common_roles/spec.md b/packages/utils/src/components/common_roles/spec.md new file mode 100644 index 00000000..9e3c8b1d --- /dev/null +++ b/packages/utils/src/components/common_roles/spec.md @@ -0,0 +1,120 @@ +# CommonRolesComponent — Specification + +## Overview + +The role system is split into two layers: + +- **`CommonRolesComponent`** — lean infrastructure. Owns the role constants, the `Role` enum, role admin configuration, role guards (`only_X`), a 7-method ABI (`ICommonRoles`: `grant_role`, `revoke_role`, `has_role`, `renounce`, and the three legacy reclaim entry points), and the legacy reclaim storage (`role_members` map + `legacy_role_reclaim_disabled` flag). +- **`RolesComponent`** — full-featured layer. Delegates all role state to `CommonRolesComponent` internally. Adds named role events (20 variants) and category-scoped embeddable implementations (`IGovernanceRoles`, `ISecurityRoles`, `IAppRoles`), and the fat `IRoles` interface (~30 EPs). + +### Role wire format + +`Role` serializes as its corresponding `RoleId` felt (the keccak selector constant, e.g. `GOVERNANCE_ADMIN = 0x371…`), **not** as a zero-based variant index. This makes calldata self-documenting, stable across enum reordering, and backward-compatible with the felt values already in use by operators and indexers. + +The design lets contracts pick exactly the role surface they need — from zero ABI overhead up to the full named interface — without duplicating any role logic. + +--- + +## Role Hierarchy + +| Role | Admin Role | Category Interface | +|---|---|---| +| `GovernanceAdmin` | `GovernanceAdmin` (self) | `IGovernanceRoles` | +| `AppRoleAdmin` | `GovernanceAdmin` | `IGovernanceRoles` | +| `UpgradeGovernor` | `GovernanceAdmin` | `IGovernanceRoles` | +| `UpgradeAgent` | `AppRoleAdmin` | `IGovernanceRoles` | +| `SecurityAdmin` | `SecurityAdmin` (self) | `ISecurityRoles` | +| `SecurityAgent` | `SecurityAdmin` | `ISecurityRoles` | +| `SecurityGovernor` | `SecurityAdmin` | `ISecurityRoles` | +| `AppGovernor` | `AppRoleAdmin` | `IAppRoles` | +| `Operator` | `AppRoleAdmin` | `IAppRoles` | +| `TokenAdmin` | `AppRoleAdmin` | `IAppRoles` | + +`GovernanceAdmin` and `SecurityAdmin` are self-administered and cannot be renounced. All other roles are renounceable. + +--- + +## Category Sub-Interfaces + +Three `#[starknet::interface]` traits group related roles into a narrower ABI slice: + +- **`IGovernanceRoles`** — governance admin, app role admin, upgrade governor, upgrade agent. These are coupled: governance admin appoints upgrade governors; upgrade governors control contract upgrades; app role admin is the gateway to the app-level roles. +- **`ISecurityRoles`** — security admin, security agent, security governor. +- **`IAppRoles`** — app governor, operator, token admin. + +These interfaces are defined in `roles/interface.cairo` and **implemented only by `RolesComponent`** (`GovernanceRolesImpl`, `SecurityRolesImpl`, `AppRolesImpl`). They exist so contracts can expose a scoped ABI without pulling in all 30 `IRoles` entry points. + +--- + +## Usage Patterns + +### Tier A — Infrastructure only + +For components that only need role *checks*, with no named role management ABI of their own (e.g., replaceability, pausable, blocklist). + +**Wire:** `CommonRolesComponent` + `AccessControlComponent` + `SRC5Component`. No `RolesComponent`. + +**Embed:** `CommonRolesImpl` → 7 entry points: `grant_role`, `revoke_role`, `has_role`, `renounce`, `reclaim_legacy_roles`, `reclaim_legacy_roles_for_accounts`, `disable_legacy_role_reclaim`. + +**Role management:** callers use `ICommonRoles::grant_role(Role::UpgradeGovernor, account)`. + +**Constructor:** +```cairo +fn constructor(ref self: ContractState, governance_admin: ContractAddress) { + self.common_roles.initialize(:governance_admin); +} +``` + +--- + +### Tier B — Selective named roles + +For contracts that need a subset of named role management EPs and their named events, but not the full `IRoles` bloat (e.g., an ERC20 with replaceability that only needs governance + upgrade management). + +**Wire:** `RolesComponent` + `CommonRolesComponent` + `AccessControlComponent` + `SRC5Component`. + +**Embed:** only the category impl(s) you need — do NOT embed `RolesImpl`. + +```cairo +#[abi(embed_v0)] +impl GovernanceRolesImpl = RolesComponent::GovernanceRolesImpl; +#[abi(embed_v0)] +impl CommonRolesImpl = CommonRolesComponent::CommonRolesImpl; +// NO RolesImpl +``` + +**ABI result:** `IGovernanceRoles` (12 EPs) + `ICommonRoles` (7 EPs). Named events (`UpgradeGovernorAdded`, etc.) are emitted by the category impl methods. Unused event variants are dead-code eliminated from Sierra. + +**Constructor:** +```cairo +fn constructor(ref self: ContractState, governance_admin: ContractAddress) { + self.roles.initialize(:governance_admin); +} +``` + +--- + +### Tier C — Full roles + +For contracts that expose the complete named role management API (existing pattern). + +**Wire:** `RolesComponent` + `CommonRolesComponent` + `AccessControlComponent` + `SRC5Component`. + +**Embed:** `RolesImpl` → all ~30 entry points + all 20 named events. + +**Constructor:** +```cairo +fn constructor(ref self: ContractState, governance_admin: ContractAddress) { + self.roles.initialize(:governance_admin); +} +``` + +--- + +## Key Invariants + +- `CommonRolesComponent` owns all role state — `AccessControlComponent` holds the active role memberships; `CommonRolesComponent` holds the legacy reclaim storage (`role_members` map + `legacy_role_reclaim_disabled` flag). +- `RolesComponent` never duplicates role state — it reads and writes through `CommonRolesComponent`'s internal methods. +- Named events exist only in `RolesComponent::Event`. `CommonRolesComponent::Event` is always empty. +- Category impls exist only in `RolesComponent`. There is no duplication between layers. +- `IGovernanceRolesDispatcher` dispatches correctly against any contract that embeds `RolesImpl`, because `IRoles` exposes the same selectors as `IGovernanceRoles`. diff --git a/packages/utils/src/components/common_roles/test.cairo b/packages/utils/src/components/common_roles/test.cairo new file mode 100644 index 00000000..46e0eb4b --- /dev/null +++ b/packages/utils/src/components/common_roles/test.cairo @@ -0,0 +1,594 @@ +use openzeppelin::access::accesscontrol::AccessControlComponent::Errors as OZAccessErrors; +use snforge_std::{ContractClassTrait, DeclareResultTrait, declare}; +use starknet::ContractAddress; +use starkware_utils::components::common_roles::mock_contract::{ + IGuardTestDispatcher, IGuardTestDispatcherTrait, IGuardTestSafeDispatcher, + IGuardTestSafeDispatcherTrait, ILegacySetupDispatcher, ILegacySetupDispatcherTrait, +}; +use starkware_utils::components::roles::errors::AccessErrors; +use starkware_utils::components::roles::interface::{ + ICommonRolesDispatcher, ICommonRolesDispatcherTrait, ICommonRolesSafeDispatcher, + ICommonRolesSafeDispatcherTrait, Role, +}; +use starkware_utils::errors::Describable; +use starkware_utils_testing::constants; +use starkware_utils_testing::test_utils::{ + assert_panic_with_error, assert_panic_with_felt_error, cheat_caller_address_once, +}; + +fn deploy() -> (ContractAddress, ICommonRolesDispatcher) { + let contract = *declare("CommonRolesMock").unwrap().contract_class(); + let (address, _) = contract.deploy(@array![constants::INITIAL_ROOT_ADMIN.into()]).unwrap(); + (address, ICommonRolesDispatcher { contract_address: address }) +} + +fn deploy_safe() -> (ContractAddress, ICommonRolesSafeDispatcher) { + let (address, _) = deploy(); + (address, ICommonRolesSafeDispatcher { contract_address: address }) +} + +// ─── initialize +// ─────────────────────────────────────────────── + +#[test] +fn test_initialize_grants_governance_admin() { + let (_, dispatcher) = deploy(); + assert!(dispatcher.has_role(Role::GovernanceAdmin, constants::INITIAL_ROOT_ADMIN)); +} + +#[test] +fn test_initialize_grants_security_admin() { + let (_, dispatcher) = deploy(); + assert!(dispatcher.has_role(Role::SecurityAdmin, constants::INITIAL_ROOT_ADMIN)); +} + +// ─── grant_role / has_role +// ────────────────────────── + +#[test] +fn test_grant_role_authorized() { + let (address, dispatcher) = deploy(); + let account = constants::APP_ROLE_ADMIN; + let gov_admin = constants::INITIAL_ROOT_ADMIN; + + assert!(!dispatcher.has_role(Role::AppRoleAdmin, account)); + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.grant_role(Role::AppRoleAdmin, account); + assert!(dispatcher.has_role(Role::AppRoleAdmin, account)); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_grant_role_unauthorized() { + let (address, safe) = deploy_safe(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = safe.grant_role(Role::AppRoleAdmin, constants::APP_ROLE_ADMIN); + assert_panic_with_felt_error(:result, expected_error: OZAccessErrors::MISSING_ROLE); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_grant_role_zero_address() { + let (address, safe) = deploy_safe(); + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + let result = safe.grant_role(Role::GovernanceAdmin, 0.try_into().unwrap()); + assert_panic_with_error(:result, expected_error: AccessErrors::ZERO_ADDRESS.describe()); +} + +// ─── revoke_role +// ───────────────────────────────────────────── + +#[test] +fn test_revoke_role() { + let (address, dispatcher) = deploy(); + let account = constants::APP_ROLE_ADMIN; + let gov_admin = constants::INITIAL_ROOT_ADMIN; + + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.grant_role(Role::AppRoleAdmin, account); + assert!(dispatcher.has_role(Role::AppRoleAdmin, account)); + + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.revoke_role(Role::AppRoleAdmin, account); + assert!(!dispatcher.has_role(Role::AppRoleAdmin, account)); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_revoke_role_self_governance_admin_panics() { + let (address, safe) = deploy_safe(); + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + let result = safe.revoke_role(Role::GovernanceAdmin, constants::INITIAL_ROOT_ADMIN); + assert_panic_with_error( + :result, expected_error: AccessErrors::ROLE_CANNOT_BE_RENOUNCED.describe(), + ); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_revoke_role_self_security_admin_panics() { + let (address, safe) = deploy_safe(); + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + let result = safe.revoke_role(Role::SecurityAdmin, constants::INITIAL_ROOT_ADMIN); + assert_panic_with_error( + :result, expected_error: AccessErrors::ROLE_CANNOT_BE_RENOUNCED.describe(), + ); +} + +#[test] +fn test_revoke_role_other_account_governance_admin_succeeds() { + let (address, dispatcher) = deploy(); + let gov_admin = constants::INITIAL_ROOT_ADMIN; + let other_gov_admin = constants::GOVERNANCE_ADMIN; + + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.grant_role(Role::GovernanceAdmin, other_gov_admin); + assert!(dispatcher.has_role(Role::GovernanceAdmin, other_gov_admin)); + + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.revoke_role(Role::GovernanceAdmin, other_gov_admin); + assert!(!dispatcher.has_role(Role::GovernanceAdmin, other_gov_admin)); +} + +// ─── renounce +// ─────────────────────────────────────────────────── + +#[test] +fn test_renounce_renounceable_role() { + let (address, dispatcher) = deploy(); + let account = constants::APP_ROLE_ADMIN; + let gov_admin = constants::INITIAL_ROOT_ADMIN; + + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.grant_role(Role::AppRoleAdmin, account); + + cheat_caller_address_once(contract_address: address, caller_address: account); + dispatcher.renounce(Role::AppRoleAdmin); + assert!(!dispatcher.has_role(Role::AppRoleAdmin, account)); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_renounce_governance_admin_panics() { + let (address, safe) = deploy_safe(); + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + let result = safe.renounce(Role::GovernanceAdmin); + assert_panic_with_error( + :result, expected_error: AccessErrors::ROLE_CANNOT_BE_RENOUNCED.describe(), + ); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_renounce_security_admin_panics() { + let (address, safe) = deploy_safe(); + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + let result = safe.renounce(Role::SecurityAdmin); + assert_panic_with_error( + :result, expected_error: AccessErrors::ROLE_CANNOT_BE_RENOUNCED.describe(), + ); +} + +// ─── only_X guards +// ────────────────────────────────────────── + +fn setup_guard_test() -> (ContractAddress, ICommonRolesDispatcher, IGuardTestSafeDispatcher) { + let (address, dispatcher) = deploy(); + let guard = IGuardTestSafeDispatcher { contract_address: address }; + (address, dispatcher, guard) +} + +fn grant( + address: ContractAddress, dispatcher: ICommonRolesDispatcher, role: Role, to: ContractAddress, +) { + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.grant_role(:role, account: to); +} + +// ── only_role ── + +#[test] +fn test_only_role_authorized() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + grant(address, dispatcher, Role::AppRoleAdmin, constants::APP_ROLE_ADMIN); + cheat_caller_address_once(contract_address: address, caller_address: constants::APP_ROLE_ADMIN); + guard.assert_only_role(Role::AppRoleAdmin); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_role_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_role(Role::AppRoleAdmin); + assert_panic_with_error(:result, expected_error: AccessErrors::CALLER_MISSING_ROLE.describe()); +} + +// ── only_app_governor ── + +#[test] +fn test_only_app_governor_authorized() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.grant_role(Role::AppRoleAdmin, constants::APP_ROLE_ADMIN); + cheat_caller_address_once(contract_address: address, caller_address: constants::APP_ROLE_ADMIN); + dispatcher.grant_role(Role::AppGovernor, constants::APP_GOVERNOR); + cheat_caller_address_once(contract_address: address, caller_address: constants::APP_GOVERNOR); + guard.assert_only_app_governor(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_app_governor_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_app_governor(); + assert_panic_with_error(:result, expected_error: AccessErrors::ONLY_APP_GOVERNOR.describe()); +} + +// ── only_operator ── + +#[test] +fn test_only_operator_authorized() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.grant_role(Role::AppRoleAdmin, constants::APP_ROLE_ADMIN); + cheat_caller_address_once(contract_address: address, caller_address: constants::APP_ROLE_ADMIN); + dispatcher.grant_role(Role::Operator, constants::OPERATOR); + cheat_caller_address_once(contract_address: address, caller_address: constants::OPERATOR); + guard.assert_only_operator(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_operator_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_operator(); + assert_panic_with_error(:result, expected_error: AccessErrors::ONLY_OPERATOR.describe()); +} + +// ── only_token_admin ── + +#[test] +fn test_only_token_admin_authorized() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.grant_role(Role::AppRoleAdmin, constants::APP_ROLE_ADMIN); + cheat_caller_address_once(contract_address: address, caller_address: constants::APP_ROLE_ADMIN); + dispatcher.grant_role(Role::TokenAdmin, constants::TOKEN_ADMIN); + cheat_caller_address_once(contract_address: address, caller_address: constants::TOKEN_ADMIN); + guard.assert_only_token_admin(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_token_admin_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_token_admin(); + assert_panic_with_error(:result, expected_error: AccessErrors::ONLY_TOKEN_ADMIN.describe()); +} + +// ── only_upgrade_governor ── + +#[test] +fn test_only_upgrade_governor_authorized() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + grant(address, dispatcher, Role::UpgradeGovernor, constants::UPGRADE_GOVERNOR); + cheat_caller_address_once( + contract_address: address, caller_address: constants::UPGRADE_GOVERNOR, + ); + guard.assert_only_upgrade_governor(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_upgrade_governor_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_upgrade_governor(); + assert_panic_with_error( + :result, expected_error: AccessErrors::ONLY_UPGRADE_GOVERNOR.describe(), + ); +} + +// ── only_upgrader (upgrade_governor OR upgrade_agent) ── + +#[test] +fn test_only_upgrader_authorized_as_governor() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + grant(address, dispatcher, Role::UpgradeGovernor, constants::UPGRADE_GOVERNOR); + cheat_caller_address_once( + contract_address: address, caller_address: constants::UPGRADE_GOVERNOR, + ); + guard.assert_only_upgrader(); +} + +#[test] +fn test_only_upgrader_authorized_as_agent() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.grant_role(Role::AppRoleAdmin, constants::APP_ROLE_ADMIN); + cheat_caller_address_once(contract_address: address, caller_address: constants::APP_ROLE_ADMIN); + dispatcher.grant_role(Role::UpgradeAgent, constants::UPGRADE_AGENT); + cheat_caller_address_once(contract_address: address, caller_address: constants::UPGRADE_AGENT); + guard.assert_only_upgrader(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_upgrader_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_upgrader(); + assert_panic_with_error(:result, expected_error: AccessErrors::ONLY_UPGRADER.describe()); +} + +// ── only_security_admin ── + +#[test] +fn test_only_security_admin_authorized() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + grant(address, dispatcher, Role::SecurityAdmin, constants::SECURITY_ADMIN); + cheat_caller_address_once(contract_address: address, caller_address: constants::SECURITY_ADMIN); + guard.assert_only_security_admin(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_security_admin_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_security_admin(); + assert_panic_with_error(:result, expected_error: AccessErrors::ONLY_SECURITY_ADMIN.describe()); +} + +// ── only_security_agent ── + +#[test] +fn test_only_security_agent_authorized() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + grant(address, dispatcher, Role::SecurityAgent, constants::SECURITY_AGENT); + cheat_caller_address_once(contract_address: address, caller_address: constants::SECURITY_AGENT); + guard.assert_only_security_agent(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_security_agent_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_security_agent(); + assert_panic_with_error(:result, expected_error: AccessErrors::ONLY_SECURITY_AGENT.describe()); +} + +// ── only_security_governor ── + +#[test] +fn test_only_security_governor_authorized() { + let (address, dispatcher) = deploy(); + let guard = IGuardTestDispatcher { contract_address: address }; + grant(address, dispatcher, Role::SecurityGovernor, constants::SECURITY_GOVERNOR); + cheat_caller_address_once( + contract_address: address, caller_address: constants::SECURITY_GOVERNOR, + ); + guard.assert_only_security_governor(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_only_security_governor_unauthorized() { + let (address, _, guard) = setup_guard_test(); + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = guard.assert_only_security_governor(); + assert_panic_with_error( + :result, expected_error: AccessErrors::ONLY_SECURITY_GOVERNOR.describe(), + ); +} + +// ─── legacy role reclaim +// ────────────────────────────── + +fn deploy_legacy() -> (ContractAddress, ICommonRolesDispatcher, ILegacySetupDispatcher) { + let contract = *declare("LegacyCommonRolesMock").unwrap().contract_class(); + let (address, _) = contract.deploy(@array![constants::INITIAL_ROOT_ADMIN.into()]).unwrap(); + ( + address, + ICommonRolesDispatcher { contract_address: address }, + ILegacySetupDispatcher { contract_address: address }, + ) +} + +fn deploy_legacy_safe() -> (ContractAddress, ICommonRolesSafeDispatcher, ILegacySetupDispatcher) { + let (address, _, setup) = deploy_legacy(); + (address, ICommonRolesSafeDispatcher { contract_address: address }, setup) +} + +#[test] +#[feature("safe_dispatcher")] +fn test_reclaim_legacy_roles_disabled_after_initialize() { + // Fresh contracts that called initialize() must NOT allow reclaim. + let (_, safe) = deploy_safe(); + let result = safe.reclaim_legacy_roles(); + assert_panic_with_error( + :result, expected_error: AccessErrors::LEGACY_ROLE_RECLAIM_DISABLED.describe(), + ); +} + +#[test] +fn test_reclaim_legacy_roles_restores_membership() { + use starkware_utils::components::roles::interface::OPERATOR; + let (address, dispatcher, setup) = deploy_legacy(); + let account = constants::OPERATOR; + + assert!(!dispatcher.has_role(Role::Operator, account)); + setup.set_legacy_role(OPERATOR, account); + cheat_caller_address_once(contract_address: address, caller_address: account); + dispatcher.reclaim_legacy_roles(); + assert!(dispatcher.has_role(Role::Operator, account)); +} + +#[test] +fn test_reclaim_legacy_roles_clears_legacy_entry() { + use starkware_utils::components::roles::interface::OPERATOR; + let (address, dispatcher, setup) = deploy_legacy(); + let account = constants::OPERATOR; + + setup.set_legacy_role(OPERATOR, account); + cheat_caller_address_once(contract_address: address, caller_address: account); + dispatcher.reclaim_legacy_roles(); + // Second reclaim should be a no-op (legacy entry cleared). + cheat_caller_address_once(contract_address: address, caller_address: account); + dispatcher.reclaim_legacy_roles(); + assert!(dispatcher.has_role(Role::Operator, account)); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_reclaim_legacy_roles_for_accounts_requires_security_governor() { + let (address, _, _) = deploy_legacy(); + let safe = ICommonRolesSafeDispatcher { contract_address: address }; + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = safe.reclaim_legacy_roles_for_accounts(array![constants::OPERATOR].span()); + assert_panic_with_error( + :result, expected_error: AccessErrors::ONLY_SECURITY_GOVERNOR.describe(), + ); +} + +#[test] +fn test_reclaim_legacy_roles_for_accounts_restores_membership() { + use starkware_utils::components::roles::interface::OPERATOR; + let (address, dispatcher, setup) = deploy_legacy(); + let account = constants::OPERATOR; + + assert!(!dispatcher.has_role(Role::Operator, account)); + setup.set_legacy_role(OPERATOR, account); + + // Grant security_governor role to INITIAL_ROOT_ADMIN so they can call the function. + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.grant_role(Role::SecurityGovernor, constants::INITIAL_ROOT_ADMIN); + + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.reclaim_legacy_roles_for_accounts(array![account].span()); + assert!(dispatcher.has_role(Role::Operator, account)); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_disable_legacy_role_reclaim_requires_upgrade_governor() { + let (address, _, _) = deploy_legacy(); + let safe = ICommonRolesSafeDispatcher { contract_address: address }; + cheat_caller_address_once(contract_address: address, caller_address: constants::WRONG_ADMIN); + let result = safe.disable_legacy_role_reclaim(); + assert_panic_with_error( + :result, expected_error: AccessErrors::ONLY_UPGRADE_GOVERNOR.describe(), + ); +} + +#[test] +fn test_reclaim_legacy_roles_noop_when_no_legacy_roles() { + let (address, dispatcher, _) = deploy_legacy(); + // No legacy roles set — reclaim should succeed without granting anything. + cheat_caller_address_once(contract_address: address, caller_address: constants::OPERATOR); + dispatcher.reclaim_legacy_roles(); + assert!(!dispatcher.has_role(Role::Operator, constants::OPERATOR)); +} + +#[test] +fn test_revoke_then_regrant_role() { + let (address, dispatcher) = deploy(); + let account = constants::APP_ROLE_ADMIN; + let gov_admin = constants::INITIAL_ROOT_ADMIN; + + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.grant_role(Role::AppRoleAdmin, account); + assert!(dispatcher.has_role(Role::AppRoleAdmin, account)); + + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.revoke_role(Role::AppRoleAdmin, account); + assert!(!dispatcher.has_role(Role::AppRoleAdmin, account)); + + cheat_caller_address_once(contract_address: address, caller_address: gov_admin); + dispatcher.grant_role(Role::AppRoleAdmin, account); + assert!(dispatcher.has_role(Role::AppRoleAdmin, account)); +} + +#[test] +fn test_disable_legacy_role_reclaim_idempotent() { + let (address, dispatcher, _) = deploy_legacy(); + + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.grant_role(Role::UpgradeGovernor, constants::INITIAL_ROOT_ADMIN); + + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.disable_legacy_role_reclaim(); + // Second call should succeed (idempotent write). + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.disable_legacy_role_reclaim(); +} + +#[test] +#[feature("safe_dispatcher")] +fn test_disable_legacy_role_reclaim_prevents_reclaim() { + let (address, dispatcher, _) = deploy_legacy(); + let safe = ICommonRolesSafeDispatcher { contract_address: address }; + + // Grant upgrade_governor to INITIAL_ROOT_ADMIN. + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.grant_role(Role::UpgradeGovernor, constants::INITIAL_ROOT_ADMIN); + + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + dispatcher.disable_legacy_role_reclaim(); + + cheat_caller_address_once( + contract_address: address, caller_address: constants::INITIAL_ROOT_ADMIN, + ); + let result = safe.reclaim_legacy_roles(); + assert_panic_with_error( + :result, expected_error: AccessErrors::LEGACY_ROLE_RECLAIM_DISABLED.describe(), + ); +} diff --git a/packages/utils/src/components/pausable/pausable.cairo b/packages/utils/src/components/pausable/pausable.cairo index a18c0862..570d64f8 100644 --- a/packages/utils/src/components/pausable/pausable.cairo +++ b/packages/utils/src/components/pausable/pausable.cairo @@ -1,12 +1,12 @@ #[starknet::component] pub(crate) mod PausableComponent { - use RolesComponent::InternalTrait as RolesInternalTrait; + use CommonRolesComponent::InternalTrait as CommonRolesInternalTrait; use openzeppelin::access::accesscontrol::AccessControlComponent; use openzeppelin::introspection::src5::SRC5Component; use starknet::storage::{StoragePointerReadAccess, StoragePointerWriteAccess}; use starknet::{ContractAddress, get_caller_address}; + use starkware_utils::components::common_roles::CommonRolesComponent; use starkware_utils::components::pausable::interface::IPausable; - use starkware_utils::components::roles::RolesComponent; #[storage] pub struct Storage { @@ -42,7 +42,7 @@ pub(crate) mod PausableComponent { TContractState, +HasComponent, +Drop, - impl Roles: RolesComponent::HasComponent, + impl CommonRoles: CommonRolesComponent::HasComponent, +AccessControlComponent::HasComponent, +SRC5Component::HasComponent, > of IPausable> { @@ -59,8 +59,8 @@ pub(crate) mod PausableComponent { /// /// Emits a `Paused` event. fn pause(ref self: ComponentState) { - let roles = get_dep_component!(@self, Roles); - roles.only_security_agent(); + let common_roles = get_dep_component!(@self, CommonRoles); + common_roles.only_security_agent(); self.assert_not_paused(); self.paused.write(true); self.emit(Paused { account: get_caller_address() }); @@ -74,8 +74,8 @@ pub(crate) mod PausableComponent { /// /// Emits an `Unpaused` event. fn unpause(ref self: ComponentState) { - let roles = get_dep_component!(@self, Roles); - roles.only_security_governor(); + let common_roles = get_dep_component!(@self, CommonRoles); + common_roles.only_security_governor(); self.assert_paused(); self.paused.write(false); self.emit(Unpaused { account: get_caller_address() }); diff --git a/packages/utils/src/components/replaceability/mock.cairo b/packages/utils/src/components/replaceability/mock.cairo index 4b6d879c..9cafcd4f 100644 --- a/packages/utils/src/components/replaceability/mock.cairo +++ b/packages/utils/src/components/replaceability/mock.cairo @@ -1,15 +1,15 @@ #[starknet::contract] pub(crate) mod ReplaceabilityMock { - use RolesComponent::InternalTrait as RolesInternalTrait; + use CommonRolesComponent::InternalTrait as CommonRolesInternalTrait; use openzeppelin::access::accesscontrol::AccessControlComponent; use openzeppelin::introspection::src5::SRC5Component; use starknet::ContractAddress; + use starkware_utils::components::common_roles::CommonRolesComponent; use starkware_utils::components::replaceability::ReplaceabilityComponent; use starkware_utils::components::replaceability::ReplaceabilityComponent::InternalReplaceabilityTrait; - use starkware_utils::components::roles::RolesComponent; component!(path: ReplaceabilityComponent, storage: replaceability, event: ReplaceabilityEvent); - component!(path: RolesComponent, storage: roles, event: RolesEvent); + component!(path: CommonRolesComponent, storage: common_roles, event: CommonRolesEvent); component!(path: AccessControlComponent, storage: accesscontrol, event: AccessControlEvent); component!(path: SRC5Component, storage: src5, event: SRC5Event); @@ -18,7 +18,7 @@ pub(crate) mod ReplaceabilityMock { #[substorage(v0)] replaceability: ReplaceabilityComponent::Storage, #[substorage(v0)] - roles: RolesComponent::Storage, + common_roles: CommonRolesComponent::Storage, #[substorage(v0)] accesscontrol: AccessControlComponent::Storage, #[substorage(v0)] @@ -29,14 +29,14 @@ pub(crate) mod ReplaceabilityMock { #[derive(Drop, starknet::Event)] pub(crate) enum Event { ReplaceabilityEvent: ReplaceabilityComponent::Event, - RolesEvent: RolesComponent::Event, + CommonRolesEvent: CommonRolesComponent::Event, AccessControlEvent: AccessControlComponent::Event, SRC5Event: SRC5Component::Event, } #[constructor] fn constructor(ref self: ContractState, upgrade_delay: u64, governance_admin: ContractAddress) { - self.roles.initialize(:governance_admin); + self.common_roles.initialize(:governance_admin); self.replaceability.initialize(:upgrade_delay); } @@ -45,5 +45,5 @@ pub(crate) mod ReplaceabilityMock { ReplaceabilityComponent::ReplaceabilityImpl; #[abi(embed_v0)] - impl RolesImpl = RolesComponent::RolesImpl; + impl CommonRolesImpl = CommonRolesComponent::CommonRolesImpl; } diff --git a/packages/utils/src/components/replaceability/replaceability.cairo b/packages/utils/src/components/replaceability/replaceability.cairo index 977fb4ad..93f10c76 100644 --- a/packages/utils/src/components/replaceability/replaceability.cairo +++ b/packages/utils/src/components/replaceability/replaceability.cairo @@ -10,13 +10,13 @@ pub(crate) mod ReplaceabilityComponent { StoragePointerWriteAccess, }; use starknet::syscalls::{library_call_syscall, replace_class_syscall}; + use starkware_utils::components::common_roles::CommonRolesComponent; + use starkware_utils::components::common_roles::CommonRolesComponent::InternalTrait; use starkware_utils::components::replaceability::errors::ReplaceErrors; use starkware_utils::components::replaceability::interface::{ EIC_INITIALIZE_SELECTOR, IMPLEMENTATION_EXPIRATION, IReplaceable, ImplementationAdded, ImplementationData, ImplementationFinalized, ImplementationRemoved, ImplementationReplaced, }; - use starkware_utils::components::roles::RolesComponent; - use starkware_utils::components::roles::RolesComponent::InternalTrait; #[storage] @@ -53,7 +53,7 @@ pub(crate) mod ReplaceabilityComponent { pub impl Replaceability< TContractState, +HasComponent, - impl Roles: RolesComponent::HasComponent, + impl CommonRoles: CommonRolesComponent::HasComponent, +AccessControlComponent::HasComponent, +SRC5Component::HasComponent, +Drop, @@ -73,8 +73,8 @@ pub(crate) mod ReplaceabilityComponent { ref self: ComponentState, implementation_data: ImplementationData, ) { // The call is restricted to the upgrade governor. - let roles_comp = get_dep_component!(@self, Roles); - roles_comp.only_upgrade_governor(); + let common_roles = get_dep_component!(@self, CommonRoles); + common_roles.only_upgrade_governor(); let activation_time = get_block_timestamp() + self.get_upgrade_delay(); let expiration_time = activation_time + IMPLEMENTATION_EXPIRATION; @@ -89,8 +89,8 @@ pub(crate) mod ReplaceabilityComponent { ref self: ComponentState, implementation_data: ImplementationData, ) { // The call is restricted to the upgrade governor. - let roles_comp = get_dep_component!(@self, Roles); - roles_comp.only_upgrade_governor(); + let common_roles = get_dep_component!(@self, CommonRoles); + common_roles.only_upgrade_governor(); // Read implementation activation time. let impl_activation_time = self.get_impl_activation_time(:implementation_data); @@ -108,8 +108,8 @@ pub(crate) mod ReplaceabilityComponent { ref self: ComponentState, implementation_data: ImplementationData, ) { // The call is restricted to the upgrade agent or upgrade governor. - let roles_comp = get_dep_component!(@self, Roles); - roles_comp.only_upgrader(); + let common_roles = get_dep_component!(@self, CommonRoles); + common_roles.only_upgrader(); // Validate implementation is not finalized. assert!(!self.is_finalized(), "{}", ReplaceErrors::FINALIZED); diff --git a/packages/utils/src/components/replaceability/test.cairo b/packages/utils/src/components/replaceability/test.cairo index bf9571d4..c5f5018a 100644 --- a/packages/utils/src/components/replaceability/test.cairo +++ b/packages/utils/src/components/replaceability/test.cairo @@ -10,7 +10,7 @@ mod ReplaceabilityTests { use replaceability::mock::ReplaceabilityMock; use replaceability::test_utils::Constants::{ DEFAULT_UPGRADE_DELAY, DUMMY_FINAL_IMPLEMENTATION_DATA, DUMMY_NONFINAL_IMPLEMENTATION_DATA, - EIC_UPGRADE_DELAY_ADDITION, NOT_UPGRADE_GOVERNOR_ACCOUNT, + EIC_UPGRADE_DELAY_ADDITION, GOVERNANCE_ADMIN, NOT_UPGRADE_GOVERNOR_ACCOUNT, }; use replaceability::test_utils::{ assert_finalized_status, assert_implementation_finalized_event_emitted, @@ -24,6 +24,9 @@ mod ReplaceabilityTests { cheat_caller_address, get_class_hash, spy_events, }; use starkware_utils::components::replaceability; + use starkware_utils::components::roles::interface::{ + ICommonRolesDispatcher, ICommonRolesDispatcherTrait, Role, + }; use starkware_utils_testing::test_utils::cheat_caller_address_once; #[test] @@ -482,4 +485,24 @@ mod ReplaceabilityTests { // Should revert with FINALIZED as the implementation is already finalized. replaceable_dispatcher.replace_to(:implementation_data); } + + // ─── Replaceability role enforcement + // ────────────────────────────────────── + + #[test] + fn test_upgrade_governor_can_use_replaceability() { + let replaceable_dispatcher = deploy_replaceability_mock(); + let contract_address = replaceable_dispatcher.contract_address; + let account = NOT_UPGRADE_GOVERNOR_ACCOUNT; + + // Grant upgrade governor via ICommonRoles. + cheat_caller_address_once(:contract_address, caller_address: GOVERNANCE_ADMIN); + ICommonRolesDispatcher { contract_address } + .grant_role(role: Role::UpgradeGovernor, :account); + + // Verify the granted account can call add_new_implementation. + cheat_caller_address_once(:contract_address, caller_address: account); + replaceable_dispatcher + .add_new_implementation(implementation_data: DUMMY_NONFINAL_IMPLEMENTATION_DATA()); + } } diff --git a/packages/utils/src/components/replaceability/test_utils.cairo b/packages/utils/src/components/replaceability/test_utils.cairo index 619bf922..bb244586 100644 --- a/packages/utils/src/components/replaceability/test_utils.cairo +++ b/packages/utils/src/components/replaceability/test_utils.cairo @@ -8,7 +8,9 @@ use starkware_utils::components::replaceability::interface::{ ImplementationReplaced, }; use starkware_utils::components::replaceability::mock::ReplaceabilityMock; -use starkware_utils::components::roles::interface::{IRolesDispatcher, IRolesDispatcherTrait}; +use starkware_utils::components::roles::interface::{ + ICommonRolesDispatcher, ICommonRolesDispatcherTrait, Role, +}; use starkware_utils_testing::event_test_utils::is_emitted; use starkware_utils_testing::test_utils::cheat_caller_address_once; @@ -68,9 +70,9 @@ pub(crate) fn get_upgrade_governor_account(contract_address: ContractAddress) -> } fn set_caller_as_upgrade_governor(contract_address: ContractAddress, caller: ContractAddress) { - let roles_dispatcher = IRolesDispatcher { contract_address: contract_address }; + let dispatcher = ICommonRolesDispatcher { contract_address }; cheat_caller_address_once(:contract_address, caller_address: Constants::GOVERNANCE_ADMIN); - roles_dispatcher.register_upgrade_governor(account: caller); + dispatcher.grant_role(Role::UpgradeGovernor, caller); } pub(crate) fn dummy_final_implementation_data_with_class_hash( diff --git a/packages/utils/src/components/roles/errors.cairo b/packages/utils/src/components/roles/errors.cairo index 4d0d1d71..8478316b 100644 --- a/packages/utils/src/components/roles/errors.cairo +++ b/packages/utils/src/components/roles/errors.cairo @@ -18,7 +18,7 @@ pub enum AccessErrors { ONLY_SECURITY_GOVERNOR, ONLY_MINTER, ONLY_SELF_CAN_RENOUNCE, - GOV_ADMIN_CANNOT_RENOUNCE, + ROLE_CANNOT_BE_RENOUNCED, MISSING_ROLE, LEGACY_ROLE_RECLAIM_DISABLED, } @@ -42,7 +42,7 @@ impl DescribableError of Describable { AccessErrors::ONLY_SECURITY_GOVERNOR => "ONLY_SECURITY_GOVERNOR", AccessErrors::ONLY_MINTER => "MINTER_ONLY", AccessErrors::ONLY_SELF_CAN_RENOUNCE => "ONLY_SELF_CAN_RENOUNCE", - AccessErrors::GOV_ADMIN_CANNOT_RENOUNCE => "GOV_ADMIN_CANNOT_SELF_REMOVE", + AccessErrors::ROLE_CANNOT_BE_RENOUNCED => "ROLE_CANNOT_BE_RENOUNCED", AccessErrors::MISSING_ROLE => "Caller is missing role", AccessErrors::LEGACY_ROLE_RECLAIM_DISABLED => "LEGACY_ROLE_RECLAIM_DISABLED", } diff --git a/packages/utils/src/components/roles/interface.cairo b/packages/utils/src/components/roles/interface.cairo index f48b4a1c..7d32dfff 100644 --- a/packages/utils/src/components/roles/interface.cairo +++ b/packages/utils/src/components/roles/interface.cairo @@ -76,30 +76,12 @@ pub trait IRoles { fn remove_upgrade_agent(ref self: TContractState, account: ContractAddress); fn register_upgrade_governor(ref self: TContractState, account: ContractAddress); fn remove_upgrade_governor(ref self: TContractState, account: ContractAddress); - fn renounce(ref self: TContractState, role: RoleId); fn register_security_admin(ref self: TContractState, account: ContractAddress); fn remove_security_admin(ref self: TContractState, account: ContractAddress); fn register_security_agent(ref self: TContractState, account: ContractAddress); fn remove_security_agent(ref self: TContractState, account: ContractAddress); fn register_security_governor(ref self: TContractState, account: ContractAddress); fn remove_security_governor(ref self: TContractState, account: ContractAddress); - fn reclaim_legacy_roles(ref self: TContractState); - fn reclaim_legacy_roles_for_accounts(ref self: TContractState, accounts: Span); - fn disable_legacy_role_reclaim(ref self: TContractState); -} - -#[starknet::interface] -pub trait IMinimalRoles { - fn is_governance_admin(self: @TContractState, account: ContractAddress) -> bool; - fn is_upgrade_governor(self: @TContractState, account: ContractAddress) -> bool; - fn is_upgrade_agent(self: @TContractState, account: ContractAddress) -> bool; - fn register_governance_admin(ref self: TContractState, account: ContractAddress); - fn remove_governance_admin(ref self: TContractState, account: ContractAddress); - fn register_upgrade_governor(ref self: TContractState, account: ContractAddress); - fn remove_upgrade_governor(ref self: TContractState, account: ContractAddress); - fn register_upgrade_agent(ref self: TContractState, account: ContractAddress); - fn remove_upgrade_agent(ref self: TContractState, account: ContractAddress); - fn renounce(ref self: TContractState, role: RoleId); } #[derive(Copy, Drop, PartialEq, starknet::Event)] @@ -220,3 +202,150 @@ pub(crate) struct UpgradeAgentRemoved { pub removed_account: ContractAddress, pub removed_by: ContractAddress, } + +// ─── CommonRoles types +// ────────────────────────────────── + +#[derive(Copy, Drop, PartialEq)] +pub enum Role { + AppGovernor, + AppRoleAdmin, + GovernanceAdmin, + Operator, + TokenAdmin, + UpgradeAgent, + UpgradeGovernor, + SecurityAdmin, + SecurityAgent, + SecurityGovernor, +} + +pub impl RoleSerde of Serde { + fn serialize(self: @Role, ref output: Array) { + let role_id: RoleId = (*self).into(); + role_id.serialize(ref output); + } + + fn deserialize(ref serialized: Span) -> Option { + let role_id: RoleId = Serde::deserialize(ref serialized)?; + if role_id == APP_GOVERNOR { + return Option::Some(Role::AppGovernor); + } + if role_id == APP_ROLE_ADMIN { + return Option::Some(Role::AppRoleAdmin); + } + if role_id == GOVERNANCE_ADMIN { + return Option::Some(Role::GovernanceAdmin); + } + if role_id == OPERATOR { + return Option::Some(Role::Operator); + } + if role_id == TOKEN_ADMIN { + return Option::Some(Role::TokenAdmin); + } + if role_id == UPGRADE_AGENT { + return Option::Some(Role::UpgradeAgent); + } + if role_id == UPGRADE_GOVERNOR { + return Option::Some(Role::UpgradeGovernor); + } + if role_id == SECURITY_ADMIN { + return Option::Some(Role::SecurityAdmin); + } + if role_id == SECURITY_AGENT { + return Option::Some(Role::SecurityAgent); + } + if role_id == SECURITY_GOVERNOR { + return Option::Some(Role::SecurityGovernor); + } + Option::None + } +} + +pub impl RoleIntoRoleId of Into { + fn into(self: Role) -> RoleId { + match self { + Role::AppGovernor => APP_GOVERNOR, + Role::AppRoleAdmin => APP_ROLE_ADMIN, + Role::GovernanceAdmin => GOVERNANCE_ADMIN, + Role::Operator => OPERATOR, + Role::TokenAdmin => TOKEN_ADMIN, + Role::UpgradeAgent => UPGRADE_AGENT, + Role::UpgradeGovernor => UPGRADE_GOVERNOR, + Role::SecurityAdmin => SECURITY_ADMIN, + Role::SecurityAgent => SECURITY_AGENT, + Role::SecurityGovernor => SECURITY_GOVERNOR, + } + } +} + +pub fn is_renounceable(role: Role) -> bool { + match role { + Role::AppGovernor => true, + Role::AppRoleAdmin => true, + Role::GovernanceAdmin => false, + Role::Operator => true, + Role::TokenAdmin => true, + Role::UpgradeAgent => true, + Role::UpgradeGovernor => true, + Role::SecurityAdmin => false, + Role::SecurityAgent => true, + Role::SecurityGovernor => true, + } +} + +#[starknet::interface] +pub trait ICommonRoles { + fn grant_role(ref self: TState, role: Role, account: ContractAddress); + fn revoke_role(ref self: TState, role: Role, account: ContractAddress); + fn has_role(self: @TState, role: Role, account: ContractAddress) -> bool; + fn renounce(ref self: TState, role: Role); + // Always present — rescue path for contracts upgrading from legacy role storage. + fn reclaim_legacy_roles(ref self: TState); + fn reclaim_legacy_roles_for_accounts(ref self: TState, accounts: Span); + fn disable_legacy_role_reclaim(ref self: TState); +} + +// ─── Category-scoped role interfaces ───── + +#[starknet::interface] +pub trait ISecurityRoles { + fn is_security_admin(self: @TState, account: ContractAddress) -> bool; + fn register_security_admin(ref self: TState, account: ContractAddress); + fn remove_security_admin(ref self: TState, account: ContractAddress); + fn is_security_agent(self: @TState, account: ContractAddress) -> bool; + fn register_security_agent(ref self: TState, account: ContractAddress); + fn remove_security_agent(ref self: TState, account: ContractAddress); + fn is_security_governor(self: @TState, account: ContractAddress) -> bool; + fn register_security_governor(ref self: TState, account: ContractAddress); + fn remove_security_governor(ref self: TState, account: ContractAddress); +} + +#[starknet::interface] +pub trait IGovernanceRoles { + fn is_governance_admin(self: @TState, account: ContractAddress) -> bool; + fn register_governance_admin(ref self: TState, account: ContractAddress); + fn remove_governance_admin(ref self: TState, account: ContractAddress); + fn is_app_role_admin(self: @TState, account: ContractAddress) -> bool; + fn register_app_role_admin(ref self: TState, account: ContractAddress); + fn remove_app_role_admin(ref self: TState, account: ContractAddress); + fn is_upgrade_governor(self: @TState, account: ContractAddress) -> bool; + fn register_upgrade_governor(ref self: TState, account: ContractAddress); + fn remove_upgrade_governor(ref self: TState, account: ContractAddress); + fn is_upgrade_agent(self: @TState, account: ContractAddress) -> bool; + fn register_upgrade_agent(ref self: TState, account: ContractAddress); + fn remove_upgrade_agent(ref self: TState, account: ContractAddress); +} + +#[starknet::interface] +pub trait IAppRoles { + fn is_app_governor(self: @TState, account: ContractAddress) -> bool; + fn register_app_governor(ref self: TState, account: ContractAddress); + fn remove_app_governor(ref self: TState, account: ContractAddress); + fn is_operator(self: @TState, account: ContractAddress) -> bool; + fn register_operator(ref self: TState, account: ContractAddress); + fn remove_operator(ref self: TState, account: ContractAddress); + fn is_token_admin(self: @TState, account: ContractAddress) -> bool; + fn register_token_admin(ref self: TState, account: ContractAddress); + fn remove_token_admin(ref self: TState, account: ContractAddress); +} diff --git a/packages/utils/src/components/roles/mock_contract.cairo b/packages/utils/src/components/roles/mock_contract.cairo index ed7c9c6b..fbf55a22 100644 --- a/packages/utils/src/components/roles/mock_contract.cairo +++ b/packages/utils/src/components/roles/mock_contract.cairo @@ -3,10 +3,12 @@ pub mod MockContract { use openzeppelin::access::accesscontrol::AccessControlComponent; use openzeppelin::introspection::src5::SRC5Component; use starknet::ContractAddress; + use starkware_utils::components::common_roles::CommonRolesComponent; use starkware_utils::components::roles::RolesComponent; use starkware_utils::components::roles::RolesComponent::InternalTrait as RolesInternalTrait; component!(path: RolesComponent, storage: roles, event: RolesEvent); + component!(path: CommonRolesComponent, storage: common_roles, event: CommonRolesEvent); component!(path: AccessControlComponent, storage: access_control, event: AccessControlEvent); component!(path: SRC5Component, storage: src5, event: SRC5Event); @@ -15,6 +17,8 @@ pub mod MockContract { #[substorage(v0)] roles: RolesComponent::Storage, #[substorage(v0)] + common_roles: CommonRolesComponent::Storage, + #[substorage(v0)] access_control: AccessControlComponent::Storage, #[substorage(v0)] src5: SRC5Component::Storage, @@ -24,12 +28,15 @@ pub mod MockContract { #[derive(Drop, starknet::Event)] pub enum Event { RolesEvent: RolesComponent::Event, + CommonRolesEvent: CommonRolesComponent::Event, AccessControlEvent: AccessControlComponent::Event, SRC5Event: SRC5Component::Event, } #[abi(embed_v0)] impl RolesImpl = RolesComponent::RolesImpl; + #[abi(embed_v0)] + impl CommonRolesImpl = CommonRolesComponent::CommonRolesImpl; #[constructor] fn constructor(ref self: ContractState, governance_admin: ContractAddress) { diff --git a/packages/utils/src/components/roles/roles.cairo b/packages/utils/src/components/roles/roles.cairo index 87fe562e..a1c97117 100644 --- a/packages/utils/src/components/roles/roles.cairo +++ b/packages/utils/src/components/roles/roles.cairo @@ -1,39 +1,24 @@ #[starknet::component] pub(crate) mod RolesComponent { use RolesInterface::{ - APP_GOVERNOR, APP_ROLE_ADMIN, AppGovernorAdded, AppGovernorRemoved, AppRoleAdminAdded, - AppRoleAdminRemoved, GOVERNANCE_ADMIN, GovernanceAdminAdded, GovernanceAdminRemoved, IRoles, - OPERATOR, OperatorAdded, OperatorRemoved, RoleId, SECURITY_ADMIN, SECURITY_AGENT, - SECURITY_GOVERNOR, SecurityAdminAdded, SecurityAdminRemoved, SecurityAgentAdded, - SecurityAgentRemoved, SecurityGovernorAdded, SecurityGovernorRemoved, TOKEN_ADMIN, - TokenAdminAdded, TokenAdminRemoved, UPGRADE_AGENT, UPGRADE_GOVERNOR, UpgradeAgentAdded, + AppGovernorAdded, AppGovernorRemoved, AppRoleAdminAdded, AppRoleAdminRemoved, + GovernanceAdminAdded, GovernanceAdminRemoved, IAppRoles, IGovernanceRoles, IRoles, + ISecurityRoles, OperatorAdded, OperatorRemoved, Role, SecurityAdminAdded, + SecurityAdminRemoved, SecurityAgentAdded, SecurityAgentRemoved, SecurityGovernorAdded, + SecurityGovernorRemoved, TokenAdminAdded, TokenAdminRemoved, UpgradeAgentAdded, UpgradeAgentRemoved, UpgradeGovernorAdded, UpgradeGovernorRemoved, }; - use core::num::traits::Zero; - use starknet::storage::{ - StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess, - StoragePointerWriteAccess, - }; + use openzeppelin::access::accesscontrol::AccessControlComponent; + use openzeppelin::access::accesscontrol::AccessControlComponent::AccessControlImpl; + use openzeppelin::introspection::src5::SRC5Component; use starknet::{ContractAddress, get_caller_address}; + use starkware_utils::components::common_roles::CommonRolesComponent; + use starkware_utils::components::common_roles::CommonRolesComponent::InternalTrait as CommonRolesInternalTrait; use starkware_utils::components::roles::errors::AccessErrors; use starkware_utils::components::roles::interface as RolesInterface; - const ROLE_ADMIN_PAIRS: [(RoleId, RoleId); 10] = [ - (APP_GOVERNOR, APP_ROLE_ADMIN), (APP_ROLE_ADMIN, GOVERNANCE_ADMIN), - (GOVERNANCE_ADMIN, GOVERNANCE_ADMIN), (OPERATOR, APP_ROLE_ADMIN), - (TOKEN_ADMIN, APP_ROLE_ADMIN), (UPGRADE_AGENT, APP_ROLE_ADMIN), - (UPGRADE_GOVERNOR, GOVERNANCE_ADMIN), (SECURITY_ADMIN, SECURITY_ADMIN), - (SECURITY_AGENT, SECURITY_ADMIN), (SECURITY_GOVERNOR, SECURITY_ADMIN), - ]; - #[storage] - pub struct Storage { - // LEGACY: This is the old storage for role members. - // We need it to allow reclaim legacy roles. - role_members: starknet::storage::Map<(RoleId, ContractAddress), bool>, - // Flag to disable legacy role reclaim. Once set, legacy roles cannot be reclaimed. - legacy_role_reclaim_disabled: bool, - } + pub struct Storage {} #[event] #[derive(Copy, Drop, PartialEq, starknet::Event)] @@ -59,462 +44,840 @@ pub(crate) mod RolesComponent { UpgradeAgentAdded: UpgradeAgentAdded, UpgradeAgentRemoved: UpgradeAgentRemoved, } - use openzeppelin::access::accesscontrol::AccessControlComponent; - use openzeppelin::access::accesscontrol::AccessControlComponent::{ - AccessControlImpl, InternalTrait as AccessInternalTrait, - }; - use openzeppelin::interfaces::access::accesscontrol::IAccessControl; - use openzeppelin::introspection::src5::SRC5Component; + // ─── IRoles (full 30-EP interface) + // ──────────────────────────────────────── + // Each method either reads from or delegates writes to CommonRolesComponent. + // Reads go through `get_dep_component!(self, CommonRoles).has_role(...)`. + // Writes go through `_register_role` / `_remove_role` (see InternalTrait below), + // which call `CommonRolesComponent::InternalTrait::grant_role` / `revoke_role` + // and emit the corresponding named event only on an actual state change. #[embeddable_as(RolesImpl)] pub impl Roles< TContractState, +HasComponent, +Drop, impl Access: AccessControlComponent::HasComponent, + impl CommonRoles: CommonRolesComponent::HasComponent, +SRC5Component::HasComponent, > of IRoles> { fn is_app_governor( self: @ComponentState, account: ContractAddress, ) -> bool { - get_dep_component!(self, Access).has_role(role: APP_GOVERNOR, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::AppGovernor, :account) } fn is_app_role_admin( self: @ComponentState, account: ContractAddress, ) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: APP_ROLE_ADMIN, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::AppRoleAdmin, :account) } fn is_governance_admin( self: @ComponentState, account: ContractAddress, ) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: GOVERNANCE_ADMIN, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::GovernanceAdmin, :account) } fn is_operator(self: @ComponentState, account: ContractAddress) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: OPERATOR, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::Operator, :account) } fn is_security_admin( self: @ComponentState, account: ContractAddress, ) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: SECURITY_ADMIN, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::SecurityAdmin, :account) } fn is_security_agent( self: @ComponentState, account: ContractAddress, ) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: SECURITY_AGENT, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::SecurityAgent, :account) } fn is_security_governor( self: @ComponentState, account: ContractAddress, ) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: SECURITY_GOVERNOR, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::SecurityGovernor, :account) } fn is_token_admin(self: @ComponentState, account: ContractAddress) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: TOKEN_ADMIN, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::TokenAdmin, :account) } fn is_upgrade_agent( self: @ComponentState, account: ContractAddress, ) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: UPGRADE_AGENT, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::UpgradeAgent, :account) } fn is_upgrade_governor( self: @ComponentState, account: ContractAddress, ) -> bool { - let access_comp = get_dep_component!(self, Access); - access_comp.has_role(role: UPGRADE_GOVERNOR, :account) + get_dep_component!(self, CommonRoles).has_role(role: Role::UpgradeGovernor, :account) } fn register_app_governor( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::AppGovernorAdded( - AppGovernorAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: APP_GOVERNOR, :account, :event); + self + ._register_role( + Role::AppGovernor, + account, + Event::AppGovernorAdded( + AppGovernorAdded { added_account: account, added_by: get_caller_address() }, + ), + ); } fn remove_app_governor(ref self: ComponentState, account: ContractAddress) { - let event = Event::AppGovernorRemoved( - AppGovernorRemoved { removed_account: account, removed_by: get_caller_address() }, - ); - self._revoke_role_and_emit(role: APP_GOVERNOR, :account, :event); + self + ._remove_role( + Role::AppGovernor, + account, + Event::AppGovernorRemoved( + AppGovernorRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } fn register_app_role_admin( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::AppRoleAdminAdded( - AppRoleAdminAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: APP_ROLE_ADMIN, :account, :event); + self + ._register_role( + Role::AppRoleAdmin, + account, + Event::AppRoleAdminAdded( + AppRoleAdminAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } fn remove_app_role_admin( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::AppRoleAdminRemoved( - AppRoleAdminRemoved { removed_account: account, removed_by: get_caller_address() }, - ); - self._revoke_role_and_emit(role: APP_ROLE_ADMIN, :account, :event); + self + ._remove_role( + Role::AppRoleAdmin, + account, + Event::AppRoleAdminRemoved( + AppRoleAdminRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } fn register_security_admin( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::SecurityAdminAdded( - SecurityAdminAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: SECURITY_ADMIN, :account, :event); + self + ._register_role( + Role::SecurityAdmin, + account, + Event::SecurityAdminAdded( + SecurityAdminAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } fn remove_security_admin( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::SecurityAdminRemoved( - SecurityAdminRemoved { removed_account: account, removed_by: get_caller_address() }, - ); - self._revoke_role_and_emit(role: SECURITY_ADMIN, :account, :event); + let caller_address = get_caller_address(); + assert!(account != caller_address, "{}", AccessErrors::ROLE_CANNOT_BE_RENOUNCED); + self + ._remove_role( + Role::SecurityAdmin, + account, + Event::SecurityAdminRemoved( + SecurityAdminRemoved { + removed_account: account, removed_by: caller_address, + }, + ), + ); } fn register_security_agent( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::SecurityAgentAdded( - SecurityAgentAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: SECURITY_AGENT, :account, :event); + self + ._register_role( + Role::SecurityAgent, + account, + Event::SecurityAgentAdded( + SecurityAgentAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } fn remove_security_agent( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::SecurityAgentRemoved( - SecurityAgentRemoved { removed_account: account, removed_by: get_caller_address() }, - ); - self._revoke_role_and_emit(role: SECURITY_AGENT, :account, :event); + self + ._remove_role( + Role::SecurityAgent, + account, + Event::SecurityAgentRemoved( + SecurityAgentRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } fn register_security_governor( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::SecurityGovernorAdded( - SecurityGovernorAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: SECURITY_GOVERNOR, :account, :event); + self + ._register_role( + Role::SecurityGovernor, + account, + Event::SecurityGovernorAdded( + SecurityGovernorAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } fn remove_security_governor( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::SecurityGovernorRemoved( - SecurityGovernorRemoved { - removed_account: account, removed_by: get_caller_address(), - }, - ); - self._revoke_role_and_emit(role: SECURITY_GOVERNOR, :account, :event); + self + ._remove_role( + Role::SecurityGovernor, + account, + Event::SecurityGovernorRemoved( + SecurityGovernorRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } fn register_governance_admin( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::GovernanceAdminAdded( - GovernanceAdminAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: GOVERNANCE_ADMIN, :account, :event); + self + ._register_role( + Role::GovernanceAdmin, + account, + Event::GovernanceAdminAdded( + GovernanceAdminAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } fn remove_governance_admin( ref self: ComponentState, account: ContractAddress, ) { let caller_address = get_caller_address(); - - // Governance admin musn't remove itself, to avoid losing governance. - assert!(account != caller_address, "{}", AccessErrors::GOV_ADMIN_CANNOT_RENOUNCE); - let event = Event::GovernanceAdminRemoved( - GovernanceAdminRemoved { removed_account: account, removed_by: caller_address }, - ); - self._revoke_role_and_emit(role: GOVERNANCE_ADMIN, :account, :event); + assert!(account != caller_address, "{}", AccessErrors::ROLE_CANNOT_BE_RENOUNCED); + self + ._remove_role( + Role::GovernanceAdmin, + account, + Event::GovernanceAdminRemoved( + GovernanceAdminRemoved { + removed_account: account, removed_by: caller_address, + }, + ), + ); } fn register_operator(ref self: ComponentState, account: ContractAddress) { - let event = Event::OperatorAdded( - OperatorAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: OPERATOR, :account, :event); + self + ._register_role( + Role::Operator, + account, + Event::OperatorAdded( + OperatorAdded { added_account: account, added_by: get_caller_address() }, + ), + ); } fn remove_operator(ref self: ComponentState, account: ContractAddress) { - let event = Event::OperatorRemoved( - OperatorRemoved { removed_account: account, removed_by: get_caller_address() }, - ); - self._revoke_role_and_emit(role: OPERATOR, :account, :event); + self + ._remove_role( + Role::Operator, + account, + Event::OperatorRemoved( + OperatorRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } fn register_token_admin( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::TokenAdminAdded( - TokenAdminAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: TOKEN_ADMIN, :account, :event); + self + ._register_role( + Role::TokenAdmin, + account, + Event::TokenAdminAdded( + TokenAdminAdded { added_account: account, added_by: get_caller_address() }, + ), + ); } fn remove_token_admin(ref self: ComponentState, account: ContractAddress) { - let event = Event::TokenAdminRemoved( - TokenAdminRemoved { removed_account: account, removed_by: get_caller_address() }, - ); - self._revoke_role_and_emit(role: TOKEN_ADMIN, :account, :event); + self + ._remove_role( + Role::TokenAdmin, + account, + Event::TokenAdminRemoved( + TokenAdminRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } fn register_upgrade_agent( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::UpgradeAgentAdded( - UpgradeAgentAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: UPGRADE_AGENT, :account, :event); + self + ._register_role( + Role::UpgradeAgent, + account, + Event::UpgradeAgentAdded( + UpgradeAgentAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } fn remove_upgrade_agent( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::UpgradeAgentRemoved( - UpgradeAgentRemoved { removed_account: account, removed_by: get_caller_address() }, - ); - self._revoke_role_and_emit(role: UPGRADE_AGENT, :account, :event); + self + ._remove_role( + Role::UpgradeAgent, + account, + Event::UpgradeAgentRemoved( + UpgradeAgentRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } fn register_upgrade_governor( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::UpgradeGovernorAdded( - UpgradeGovernorAdded { added_account: account, added_by: get_caller_address() }, - ); - self._grant_role_and_emit(role: UPGRADE_GOVERNOR, :account, :event); + self + ._register_role( + Role::UpgradeGovernor, + account, + Event::UpgradeGovernorAdded( + UpgradeGovernorAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } fn remove_upgrade_governor( ref self: ComponentState, account: ContractAddress, ) { - let event = Event::UpgradeGovernorRemoved( - UpgradeGovernorRemoved { - removed_account: account, removed_by: get_caller_address(), - }, - ); - self._revoke_role_and_emit(role: UPGRADE_GOVERNOR, :account, :event); + self + ._remove_role( + Role::UpgradeGovernor, + account, + Event::UpgradeGovernorRemoved( + UpgradeGovernorRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); + } + } + + // ─── Category-scoped implementations + // ───────────────────────────────────── + // These three embeddable implementations cover ISecurityRoles / IGovernanceRoles / + // IAppRoles. A contract that does not need all 30 IRoles entry points can embed just + // the category implementation it needs instead of RolesImpl. + + #[embeddable_as(SecurityRolesImpl)] + pub impl SecurityRoles< + TContractState, + +HasComponent, + +Drop, + impl Access: AccessControlComponent::HasComponent, + impl CommonRoles: CommonRolesComponent::HasComponent, + +SRC5Component::HasComponent, + > of ISecurityRoles> { + fn is_security_admin( + self: @ComponentState, account: ContractAddress, + ) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::SecurityAdmin, :account) + } + + fn register_security_admin( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._register_role( + Role::SecurityAdmin, + account, + Event::SecurityAdminAdded( + SecurityAdminAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); + } + + fn remove_security_admin( + ref self: ComponentState, account: ContractAddress, + ) { + let caller_address = get_caller_address(); + assert!(account != caller_address, "{}", AccessErrors::ROLE_CANNOT_BE_RENOUNCED); + self + ._remove_role( + Role::SecurityAdmin, + account, + Event::SecurityAdminRemoved( + SecurityAdminRemoved { + removed_account: account, removed_by: caller_address, + }, + ), + ); + } + + fn is_security_agent( + self: @ComponentState, account: ContractAddress, + ) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::SecurityAgent, :account) + } + + fn register_security_agent( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._register_role( + Role::SecurityAgent, + account, + Event::SecurityAgentAdded( + SecurityAgentAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } - fn renounce(ref self: ComponentState, role: RoleId) { - assert!(role != GOVERNANCE_ADMIN, "{}", AccessErrors::GOV_ADMIN_CANNOT_RENOUNCE); - let mut access_comp = get_dep_component_mut!(ref self, Access); - access_comp.renounce_role(:role, account: get_caller_address()) + fn remove_security_agent( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._remove_role( + Role::SecurityAgent, + account, + Event::SecurityAgentRemoved( + SecurityAgentRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } - fn reclaim_legacy_roles(ref self: ComponentState) { - self.assert_role_reclaim_enabled(); - self._reclaim_legacy_roles_for_account(get_caller_address()); - let mut access_comp = get_dep_component_mut!(ref self, Access); - RolesInternalImpl::_set_role_admins(ref access_comp); + fn is_security_governor( + self: @ComponentState, account: ContractAddress, + ) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::SecurityGovernor, :account) } - fn reclaim_legacy_roles_for_accounts( - ref self: ComponentState, accounts: Span, + fn register_security_governor( + ref self: ComponentState, account: ContractAddress, ) { - self.only_security_governor(); - self.assert_role_reclaim_enabled(); - for account in accounts { - self._reclaim_legacy_roles_for_account(*account); - }; + self + ._register_role( + Role::SecurityGovernor, + account, + Event::SecurityGovernorAdded( + SecurityGovernorAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } - fn disable_legacy_role_reclaim(ref self: ComponentState) { - self.only_upgrade_governor(); - self.legacy_role_reclaim_disabled.write(true); + fn remove_security_governor( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._remove_role( + Role::SecurityGovernor, + account, + Event::SecurityGovernorRemoved( + SecurityGovernorRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } } - #[generate_trait] - pub impl ClaimRoleImpl< + #[embeddable_as(GovernanceRolesImpl)] + pub impl GovernanceRoles< TContractState, +HasComponent, +Drop, impl Access: AccessControlComponent::HasComponent, + impl CommonRoles: CommonRolesComponent::HasComponent, +SRC5Component::HasComponent, - > of ClaimRoleInternal { - // Reinstate role membership per legacy role membership: - // 1. If the account held the legacy role, it will be granted in the current realm. - // 2. Role membership in the current realm will not be cleared if no legacy member held. - // 3. Legacy membership is cleared after reading, so reclaim can be done effectively only - // once. - fn _reclaim_role( - ref self: ComponentState, role: RoleId, account: ContractAddress, - ) { - if self._has_legacy_role(account, role) { - // Clear legacy membership to prevent double claiming. - self.role_members.write((role, account), false); - let mut access_comp = get_dep_component_mut!(ref self, Access); - access_comp._grant_role(:role, :account); - } + > of IGovernanceRoles> { + fn is_governance_admin( + self: @ComponentState, account: ContractAddress, + ) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::GovernanceAdmin, :account) + } + + fn register_governance_admin( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._register_role( + Role::GovernanceAdmin, + account, + Event::GovernanceAdminAdded( + GovernanceAdminAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); } - fn _has_legacy_role( - self: @ComponentState, account: ContractAddress, role: RoleId, + fn remove_governance_admin( + ref self: ComponentState, account: ContractAddress, + ) { + let caller_address = get_caller_address(); + assert!(account != caller_address, "{}", AccessErrors::ROLE_CANNOT_BE_RENOUNCED); + self + ._remove_role( + Role::GovernanceAdmin, + account, + Event::GovernanceAdminRemoved( + GovernanceAdminRemoved { + removed_account: account, removed_by: caller_address, + }, + ), + ); + } + + fn is_app_role_admin( + self: @ComponentState, account: ContractAddress, ) -> bool { - self.role_members.read((role, account)) + get_dep_component!(self, CommonRoles).has_role(role: Role::AppRoleAdmin, :account) } - fn _reclaim_legacy_roles_for_account( + fn register_app_role_admin( ref self: ComponentState, account: ContractAddress, ) { - for (role, _) in ROLE_ADMIN_PAIRS.span() { - self._reclaim_role(role: *role, :account); - } + self + ._register_role( + Role::AppRoleAdmin, + account, + Event::AppRoleAdminAdded( + AppRoleAdminAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); + } + + fn remove_app_role_admin( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._remove_role( + Role::AppRoleAdmin, + account, + Event::AppRoleAdminRemoved( + AppRoleAdminRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); + } + + fn is_upgrade_governor( + self: @ComponentState, account: ContractAddress, + ) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::UpgradeGovernor, :account) + } + + fn register_upgrade_governor( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._register_role( + Role::UpgradeGovernor, + account, + Event::UpgradeGovernorAdded( + UpgradeGovernorAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); + } + + fn remove_upgrade_governor( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._remove_role( + Role::UpgradeGovernor, + account, + Event::UpgradeGovernorRemoved( + UpgradeGovernorRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); + } + + fn is_upgrade_agent( + self: @ComponentState, account: ContractAddress, + ) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::UpgradeAgent, :account) + } + + fn register_upgrade_agent( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._register_role( + Role::UpgradeAgent, + account, + Event::UpgradeAgentAdded( + UpgradeAgentAdded { + added_account: account, added_by: get_caller_address(), + }, + ), + ); + } + + fn remove_upgrade_agent( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._remove_role( + Role::UpgradeAgent, + account, + Event::UpgradeAgentRemoved( + UpgradeAgentRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); } } + #[embeddable_as(AppRolesImpl)] + pub impl AppRoles< + TContractState, + +HasComponent, + +Drop, + impl Access: AccessControlComponent::HasComponent, + impl CommonRoles: CommonRolesComponent::HasComponent, + +SRC5Component::HasComponent, + > of IAppRoles> { + fn is_app_governor( + self: @ComponentState, account: ContractAddress, + ) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::AppGovernor, :account) + } + + fn register_app_governor( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._register_role( + Role::AppGovernor, + account, + Event::AppGovernorAdded( + AppGovernorAdded { added_account: account, added_by: get_caller_address() }, + ), + ); + } + + fn remove_app_governor(ref self: ComponentState, account: ContractAddress) { + self + ._remove_role( + Role::AppGovernor, + account, + Event::AppGovernorRemoved( + AppGovernorRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); + } + + fn is_operator(self: @ComponentState, account: ContractAddress) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::Operator, :account) + } + + fn register_operator(ref self: ComponentState, account: ContractAddress) { + self + ._register_role( + Role::Operator, + account, + Event::OperatorAdded( + OperatorAdded { added_account: account, added_by: get_caller_address() }, + ), + ); + } + + fn remove_operator(ref self: ComponentState, account: ContractAddress) { + self + ._remove_role( + Role::Operator, + account, + Event::OperatorRemoved( + OperatorRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); + } + + fn is_token_admin(self: @ComponentState, account: ContractAddress) -> bool { + get_dep_component!(self, CommonRoles).has_role(role: Role::TokenAdmin, :account) + } + + fn register_token_admin( + ref self: ComponentState, account: ContractAddress, + ) { + self + ._register_role( + Role::TokenAdmin, + account, + Event::TokenAdminAdded( + TokenAdminAdded { added_account: account, added_by: get_caller_address() }, + ), + ); + } + + fn remove_token_admin(ref self: ComponentState, account: ContractAddress) { + self + ._remove_role( + Role::TokenAdmin, + account, + Event::TokenAdminRemoved( + TokenAdminRemoved { + removed_account: account, removed_by: get_caller_address(), + }, + ), + ); + } + } + + // ─── Internal helpers + // ───────────────────────────────────────────────────── #[generate_trait] pub impl RolesInternalImpl< TContractState, +HasComponent, +Drop, impl Access: AccessControlComponent::HasComponent, + impl CommonRoles: CommonRolesComponent::HasComponent, +SRC5Component::HasComponent, > of InternalTrait { - // TODO - change the fn name to _grant_role when we can have modularity. - fn _grant_role_and_emit( + // Idempotent grant: no-op (and no event) if `account` already holds `role`. + // The caller constructs the event before calling, so the only branching needed here + // is the early-return guard — avoids closures or runtime event dispatch. + fn _register_role( ref self: ComponentState, - role: RoleId, + role: Role, account: ContractAddress, event: Event, ) { - let mut access_comp = get_dep_component_mut!(ref self, Access); - if !access_comp.has_role(:role, :account) { - assert!(account.is_non_zero(), "{}", AccessErrors::ZERO_ADDRESS); - access_comp.grant_role(:role, :account); - self.emit(event); + let mut common_roles = get_dep_component_mut!(ref self, CommonRoles); + if common_roles.has_role(:role, :account) { + return; } + common_roles.grant_role(:role, :account); + self.emit(event); } - // TODO - change the fn name to _revoke_role when we can have modularity. - fn _revoke_role_and_emit( + // Idempotent revoke: no-op (and no event) if `account` does not hold `role`. + // Authorization (caller must be role admin) is enforced inside + // `CommonRolesComponent::InternalTrait::revoke_role` via OZ AccessControl. + fn _remove_role( ref self: ComponentState, - role: RoleId, + role: Role, account: ContractAddress, event: Event, ) { - let mut access_comp = get_dep_component_mut!(ref self, Access); - if access_comp.has_role(:role, :account) { - access_comp.revoke_role(:role, :account); - self.emit(event); + let mut common_roles = get_dep_component_mut!(ref self, CommonRoles); + if !common_roles.has_role(:role, :account) { + return; } - } - - fn assert_role_reclaim_enabled(self: @ComponentState) { - assert!( - !self.legacy_role_reclaim_disabled.read(), - "{}", - AccessErrors::LEGACY_ROLE_RECLAIM_DISABLED, - ); + common_roles.revoke_role(:role, :account); + self.emit(event); } // WARNING - // The following internal method is unprotected and should only be used from the containing - // contract's constructor (or, in context of tests, from the setup method). - // It should be called after the initialization of the access_control component. + // Unprotected — call only from a constructor (or test setup). + // Thin wrapper that forwards to CommonRolesComponent::initialize, which seeds + // GOVERNANCE_ADMIN and SECURITY_ADMIN and sets all role admin pairs. fn initialize(ref self: ComponentState, governance_admin: ContractAddress) { - let mut access_comp = get_dep_component_mut!(ref self, Access); - let un_initialized = access_comp.get_role_admin(role: GOVERNANCE_ADMIN).is_zero(); - assert!(un_initialized, "{}", AccessErrors::ALREADY_INITIALIZED); - access_comp.initializer(); - assert!(governance_admin.is_non_zero(), "{}", AccessErrors::ZERO_ADDRESS_GOV_ADMIN); - access_comp._grant_role(role: GOVERNANCE_ADMIN, account: governance_admin); - access_comp._grant_role(role: SECURITY_ADMIN, account: governance_admin); - self.legacy_role_reclaim_disabled.write(true); - Self::_set_role_admins(ref access_comp); - } - - // Recovers role_admin heirarcy. - // Set role_admin only if not set already. - fn _set_role_admins( - ref access_comp: AccessControlComponent::ComponentState, - ) { - for (role, admin_role) in ROLE_ADMIN_PAIRS.span() { - if access_comp.get_role_admin(role: *role).is_zero() { - access_comp.set_role_admin(role: *role, admin_role: *admin_role); - } - } + let mut common_roles = get_dep_component_mut!(ref self, CommonRoles); + common_roles.initialize(:governance_admin); } + // ─── Role guards (delegates to CommonRolesComponent) + // ───────────────── + // Embeddable methods on a sibling component are not directly callable via + // `get_dep_component!`, so each guard is re-exposed here as a thin wrapper. fn only_app_governor(self: @ComponentState) { - assert!( - self.is_app_governor(get_caller_address()), "{}", AccessErrors::ONLY_APP_GOVERNOR, - ); + get_dep_component!(self, CommonRoles).only_app_governor(); } fn only_operator(self: @ComponentState) { - assert!(self.is_operator(get_caller_address()), "{}", AccessErrors::ONLY_OPERATOR); + get_dep_component!(self, CommonRoles).only_operator(); } fn only_token_admin(self: @ComponentState) { - assert!( - self.is_token_admin(get_caller_address()), "{}", AccessErrors::ONLY_TOKEN_ADMIN, - ); + get_dep_component!(self, CommonRoles).only_token_admin(); } fn only_upgrade_governor(self: @ComponentState) { - assert!( - self.is_upgrade_governor(get_caller_address()), - "{}", - AccessErrors::ONLY_UPGRADE_GOVERNOR, - ); + get_dep_component!(self, CommonRoles).only_upgrade_governor(); } fn only_upgrader(self: @ComponentState) { - assert!( - self.is_upgrade_agent(get_caller_address()) - || self.is_upgrade_governor(get_caller_address()), - "{}", - AccessErrors::ONLY_UPGRADER, - ); + get_dep_component!(self, CommonRoles).only_upgrader(); } fn only_security_admin(self: @ComponentState) { - assert!( - self.is_security_admin(get_caller_address()), - "{}", - AccessErrors::ONLY_SECURITY_ADMIN, - ); + get_dep_component!(self, CommonRoles).only_security_admin(); } fn only_security_agent(self: @ComponentState) { - assert!( - self.is_security_agent(get_caller_address()), - "{}", - AccessErrors::ONLY_SECURITY_AGENT, - ); + get_dep_component!(self, CommonRoles).only_security_agent(); } fn only_security_governor(self: @ComponentState) { - assert!( - self.is_security_governor(get_caller_address()), - "{}", - AccessErrors::ONLY_SECURITY_GOVERNOR, - ); + get_dep_component!(self, CommonRoles).only_security_governor(); } } } diff --git a/packages/utils/src/components/roles/test.cairo b/packages/utils/src/components/roles/test.cairo index b2a1036f..3fce00ed 100644 --- a/packages/utils/src/components/roles/test.cairo +++ b/packages/utils/src/components/roles/test.cairo @@ -1,6 +1,10 @@ use core::num::traits::zero::Zero; use interface::{ - IRolesDispatcher, IRolesDispatcherTrait, IRolesSafeDispatcher, IRolesSafeDispatcherTrait, + IAppRolesDispatcher, IAppRolesDispatcherTrait, ICommonRolesDispatcher, + ICommonRolesDispatcherTrait, ICommonRolesSafeDispatcher, ICommonRolesSafeDispatcherTrait, + IGovernanceRolesDispatcher, IGovernanceRolesDispatcherTrait, IRolesDispatcher, + IRolesDispatcherTrait, IRolesSafeDispatcher, IRolesSafeDispatcherTrait, + ISecurityRolesDispatcher, ISecurityRolesDispatcherTrait, Role, }; use openzeppelin::access::accesscontrol::AccessControlComponent::Errors as OZAccessErrors; use roles::{event_test_utils, interface}; @@ -907,6 +911,21 @@ fn test_remove_security_admin() { assert!(!roles_dispatcher.is_security_admin(account: security_admin)); } +#[feature("safe_dispatcher")] +#[test] +fn test_remove_security_admin_self_panics() { + // A security admin cannot remove themselves via remove_security_admin. + let contract_address = test_utils::deploy_mock_contract(); + let roles_safe_dispatcher = IRolesSafeDispatcher { contract_address }; + let security_admin = constants::INITIAL_ROOT_ADMIN; + + cheat_caller_address_once(:contract_address, caller_address: security_admin); + let result = roles_safe_dispatcher.remove_security_admin(account: security_admin); + assert_panic_with_error( + :result, expected_error: AccessErrors::ROLE_CANNOT_BE_RENOUNCED.describe(), + ); +} + #[feature("safe_dispatcher")] #[test] @@ -914,22 +933,23 @@ fn test_renounce() { // Deploy mock contract. let contract_address = test_utils::deploy_mock_contract(); let roles_dispatcher = IRolesDispatcher { contract_address }; - let roles_safe_dispatcher = IRolesSafeDispatcher { contract_address }; + let common_roles_dispatcher = ICommonRolesDispatcher { contract_address }; + let common_roles_safe_dispatcher = ICommonRolesSafeDispatcher { contract_address }; let governance_admin = constants::INITIAL_ROOT_ADMIN; let app_role_admin = constants::APP_ROLE_ADMIN; // Try to renounce governance admin. // Note: the caller doesn't have to be a governance admin for this error as it's checked first. cheat_caller_address_once(:contract_address, caller_address: governance_admin); - let result = roles_safe_dispatcher.renounce(role: interface::GOVERNANCE_ADMIN); + let result = common_roles_safe_dispatcher.renounce(role: Role::GovernanceAdmin); assert_panic_with_error( - :result, expected_error: AccessErrors::GOV_ADMIN_CANNOT_RENOUNCE.describe(), + :result, expected_error: AccessErrors::ROLE_CANNOT_BE_RENOUNCED.describe(), ); // Renounce role without being registered. let mut spy = snforge_std::spy_events(); cheat_caller_address_once(:contract_address, caller_address: app_role_admin); - roles_dispatcher.renounce(role: interface::APP_ROLE_ADMIN); + common_roles_dispatcher.renounce(role: Role::AppRoleAdmin); let events = spy.get_events().emitted_by(:contract_address).events; assert_number_of_events( actual: events.len(), expected: 0, message: "test_remove_security_admin second", @@ -942,7 +962,7 @@ fn test_renounce() { // Renounce app role admin. let mut spy = snforge_std::spy_events(); cheat_caller_address_once(:contract_address, caller_address: app_role_admin); - roles_dispatcher.renounce(role: interface::APP_ROLE_ADMIN); + common_roles_dispatcher.renounce(role: Role::AppRoleAdmin); let events = spy.get_events().emitted_by(:contract_address).events; // We don't assert any specific event, because the only event emitted is by accesss control. @@ -950,3 +970,183 @@ fn test_renounce() { actual: events.len(), expected: 1, message: "test_remove_security_admin second", ); } + +// ─── Category interface parity tests ───── +// +// These tests verify that dispatching via a category interface (e.g. IUpgradeRoles) +// against a contract that embeds RolesComponent produces identical results to +// dispatching via IRoles — same state changes, same named events. + +// ── IUpgradeRoles ── + +#[test] +fn test_upgrade_roles_parity_register_via_category() { + let contract_address = test_utils::deploy_mock_contract(); + let roles = IRolesDispatcher { contract_address }; + let upgrade_roles = IGovernanceRolesDispatcher { contract_address }; + let governance_admin = constants::INITIAL_ROOT_ADMIN; + let account = constants::UPGRADE_GOVERNOR; + + assert!(!roles.is_upgrade_governor(:account)); + assert!(!upgrade_roles.is_upgrade_governor(:account)); + + let mut spy = snforge_std::spy_events(); + cheat_caller_address_once(:contract_address, caller_address: governance_admin); + upgrade_roles.register_upgrade_governor(:account); + + assert!(roles.is_upgrade_governor(:account)); + assert!(upgrade_roles.is_upgrade_governor(:account)); + let events = spy.get_events().emitted_by(:contract_address).events; + assert_number_of_events( + actual: events.len(), expected: 2, message: "upgrade_roles parity register", + ); + event_test_utils::assert_upgrade_governor_added_event( + events[1], added_account: account, added_by: governance_admin, + ); +} + +#[test] +fn test_upgrade_roles_parity_remove_via_category() { + let contract_address = test_utils::deploy_mock_contract(); + let roles = IRolesDispatcher { contract_address }; + let upgrade_roles = IGovernanceRolesDispatcher { contract_address }; + let governance_admin = constants::INITIAL_ROOT_ADMIN; + let account = constants::UPGRADE_GOVERNOR; + + cheat_caller_address_once(:contract_address, caller_address: governance_admin); + roles.register_upgrade_governor(:account); + + let mut spy = snforge_std::spy_events(); + cheat_caller_address_once(:contract_address, caller_address: governance_admin); + upgrade_roles.remove_upgrade_governor(:account); + + assert!(!roles.is_upgrade_governor(:account)); + assert!(!upgrade_roles.is_upgrade_governor(:account)); + let events = spy.get_events().emitted_by(:contract_address).events; + assert_number_of_events( + actual: events.len(), expected: 2, message: "upgrade_roles parity remove", + ); + event_test_utils::assert_upgrade_governor_removed_event( + events[1], removed_account: account, removed_by: governance_admin, + ); +} + +#[test] +fn test_upgrade_agent_parity_register_via_category() { + let contract_address = test_utils::deploy_mock_contract(); + let governance_admin = constants::INITIAL_ROOT_ADMIN; + let app_role_admin = constants::APP_ROLE_ADMIN; + let account = constants::UPGRADE_AGENT; + + cheat_caller_address_once(:contract_address, caller_address: governance_admin); + IRolesDispatcher { contract_address }.register_app_role_admin(account: app_role_admin); + + let roles = IRolesDispatcher { contract_address }; + let upgrade_roles = IGovernanceRolesDispatcher { contract_address }; + + assert!(!roles.is_upgrade_agent(:account)); + assert!(!upgrade_roles.is_upgrade_agent(:account)); + + let mut spy = snforge_std::spy_events(); + cheat_caller_address_once(:contract_address, caller_address: app_role_admin); + upgrade_roles.register_upgrade_agent(:account); + + assert!(roles.is_upgrade_agent(:account)); + assert!(upgrade_roles.is_upgrade_agent(:account)); + let events = spy.get_events().emitted_by(:contract_address).events; + assert_number_of_events( + actual: events.len(), expected: 2, message: "upgrade_agent parity register", + ); + event_test_utils::assert_upgrade_agent_added_event( + events[1], added_account: account, added_by: app_role_admin, + ); +} + +// ── ISecurityRoles ── + +#[test] +fn test_security_roles_parity_register_via_category() { + let contract_address = test_utils::deploy_mock_contract(); + let roles = IRolesDispatcher { contract_address }; + let security_roles = ISecurityRolesDispatcher { contract_address }; + let governance_admin = constants::INITIAL_ROOT_ADMIN; + let account = constants::SECURITY_AGENT; + + assert!(!roles.is_security_agent(:account)); + assert!(!security_roles.is_security_agent(:account)); + + let mut spy = snforge_std::spy_events(); + cheat_caller_address_once(:contract_address, caller_address: governance_admin); + security_roles.register_security_agent(:account); + + assert!(roles.is_security_agent(:account)); + assert!(security_roles.is_security_agent(:account)); + let events = spy.get_events().emitted_by(:contract_address).events; + assert_number_of_events( + actual: events.len(), expected: 2, message: "security_roles parity register", + ); + event_test_utils::assert_security_agent_added_event( + events[1], added_account: account, added_by: governance_admin, + ); +} + +// ── IGovernanceRoles ── + +#[test] +fn test_governance_roles_parity_register_via_category() { + let contract_address = test_utils::deploy_mock_contract(); + let roles = IRolesDispatcher { contract_address }; + let gov_roles = IGovernanceRolesDispatcher { contract_address }; + let governance_admin = constants::INITIAL_ROOT_ADMIN; + let account = constants::APP_ROLE_ADMIN; + + assert!(!roles.is_app_role_admin(:account)); + assert!(!gov_roles.is_app_role_admin(:account)); + + let mut spy = snforge_std::spy_events(); + cheat_caller_address_once(:contract_address, caller_address: governance_admin); + gov_roles.register_app_role_admin(:account); + + assert!(roles.is_app_role_admin(:account)); + assert!(gov_roles.is_app_role_admin(:account)); + let events = spy.get_events().emitted_by(:contract_address).events; + assert_number_of_events( + actual: events.len(), expected: 2, message: "governance_roles parity register", + ); + event_test_utils::assert_app_role_admin_added_event( + events[1], added_account: account, added_by: governance_admin, + ); +} + +// ── IAppRoles ── + +#[test] +fn test_app_roles_parity_register_via_category() { + let contract_address = test_utils::deploy_mock_contract(); + let governance_admin = constants::INITIAL_ROOT_ADMIN; + let app_role_admin = constants::APP_ROLE_ADMIN; + let account = constants::APP_GOVERNOR; + + cheat_caller_address_once(:contract_address, caller_address: governance_admin); + IRolesDispatcher { contract_address }.register_app_role_admin(account: app_role_admin); + + let roles = IRolesDispatcher { contract_address }; + let app_roles = IAppRolesDispatcher { contract_address }; + + assert!(!roles.is_app_governor(:account)); + assert!(!app_roles.is_app_governor(:account)); + + let mut spy = snforge_std::spy_events(); + cheat_caller_address_once(:contract_address, caller_address: app_role_admin); + app_roles.register_app_governor(:account); + + assert!(roles.is_app_governor(:account)); + assert!(app_roles.is_app_governor(:account)); + let events = spy.get_events().emitted_by(:contract_address).events; + assert_number_of_events( + actual: events.len(), expected: 2, message: "app_roles parity register", + ); + event_test_utils::assert_app_governor_added_event( + events[1], added_account: account, added_by: app_role_admin, + ); +}