Skip to content

feat(roles): add upgrade_agent #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
remollemo merged 3 commits into from
Nov 9, 2025
Merged

feat(roles): add upgrade_agent #127

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c616533
feat(roles): add upgrade_agent
remollemo Nov 3, 2025
33c582d
tests for upgrade_agnet
remollemo Nov 4, 2025
02f7255
sec gov
remollemo Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/testing/src/constants.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub const APP_ROLE_ADMIN: ContractAddress = 'APP_ROLE_ADMIN'.try_into().unwrap()
pub const APP_GOVERNOR: ContractAddress = 'APP_GOVERNOR'.try_into().unwrap();
pub const OPERATOR: ContractAddress = 'OPERATOR'.try_into().unwrap();
pub const TOKEN_ADMIN: ContractAddress = 'TOKEN_ADMIN'.try_into().unwrap();
pub const UPGRADE_AGENT: ContractAddress = 'UPGRADE_AGENT'.try_into().unwrap();
pub const UPGRADE_GOVERNOR: ContractAddress = 'UPGRADE_GOVERNOR'.try_into().unwrap();
pub const SECURITY_AGENT: ContractAddress = 'SECURITY_AGENT'.try_into().unwrap();
pub const DUMMY_ADDRESS: ContractAddress = 'DUMMY_ADDRESS'.try_into().unwrap();
2 changes: 2 additions & 0 deletions packages/utils/src/components/roles/errors.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub enum AccessErrors {
ONLY_OPERATOR,
ONLY_TOKEN_ADMIN,
ONLY_UPGRADE_GOVERNOR,
ONLY_UPGRADER,
ONLY_SECURITY_ADMIN,
ONLY_SECURITY_AGENT,
ONLY_MINTER,
Expand All @@ -33,6 +34,7 @@ impl DescribableError of Describable<AccessErrors> {
AccessErrors::ONLY_OPERATOR => "ONLY_OPERATOR",
AccessErrors::ONLY_TOKEN_ADMIN => "ONLY_TOKEN_ADMIN",
AccessErrors::ONLY_UPGRADE_GOVERNOR => "ONLY_UPGRADE_GOVERNOR",
AccessErrors::ONLY_UPGRADER => "ONLY_UPGRADER",
AccessErrors::ONLY_SECURITY_ADMIN => "ONLY_SECURITY_ADMIN",
AccessErrors::ONLY_SECURITY_AGENT => "ONLY_SECURITY_AGENT",
AccessErrors::ONLY_MINTER => "MINTER_ONLY",
Expand Down
44 changes: 44 additions & 0 deletions packages/utils/src/components/roles/event_test_utils.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,50 @@ pub(crate) fn assert_upgrade_governor_added_event(
}
}

pub(crate) fn assert_upgrade_agent_added_event(
spied_event: @(ContractAddress, Event),
added_account: ContractAddress,
added_by: ContractAddress,
) {
let expected_event = @MockContract::Event::RolesEvent(
RolesEvent::UpgradeAgentAdded(
RolesInterface::UpgradeAgentAdded { added_account, added_by },
),
);
let (expected_emitted_by, raw_event) = spied_event;
let wrapped_spied_event = Events { events: array![(*expected_emitted_by, raw_event.clone())] };
let emitted = is_emitted(self: @wrapped_spied_event, :expected_emitted_by, :expected_event);
if !emitted {
let details = format!(
"UpgradeAgentAdded{{added_account: {:?}, added_by: {:?}}}", added_account, added_by,
);
panic_with_event_details(:expected_emitted_by, :details);
}
}

pub(crate) fn assert_upgrade_agent_removed_event(
spied_event: @(ContractAddress, Event),
removed_account: ContractAddress,
removed_by: ContractAddress,
) {
let expected_event = @MockContract::Event::RolesEvent(
RolesEvent::UpgradeAgentRemoved(
RolesInterface::UpgradeAgentRemoved { removed_account, removed_by },
),
);
let (expected_emitted_by, raw_event) = spied_event;
let wrapped_spied_event = Events { events: array![(*expected_emitted_by, raw_event.clone())] };
let emitted = is_emitted(self: @wrapped_spied_event, :expected_emitted_by, :expected_event);
if !emitted {
let details = format!(
"UpgradeAgentRemoved{{removed_account: {:?}, removed_by: {:?}}}",
removed_account,
removed_by,
);
panic_with_event_details(:expected_emitted_by, :details);
}
}

pub(crate) fn assert_upgrade_governor_removed_event(
spied_event: @(ContractAddress, Event),
removed_account: ContractAddress,
Expand Down
22 changes: 22 additions & 0 deletions packages/utils/src/components/roles/interface.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub type RoleId = felt252;
// ----------------------------------------
// GOVERNANCE_ADMIN | GOVERNANCE_ADMIN
// UPGRADE_GOVERNOR | GOVERNANCE_ADMIN
// UPGRADE_AGENT | APP_ROLE_ADMIN
// APP_ROLE_ADMIN | GOVERNANCE_ADMIN
// APP_GOVERNOR | APP_ROLE_ADMIN
// OPERATOR | APP_ROLE_ADMIN
Expand All @@ -29,6 +30,9 @@ pub const OPERATOR: RoleId = 0x023edb77f7c8cc9e38e8afe78954f703aeeda7fffe014eeb6
// int.from_bytes(Web3.keccak(text="ROLE_TOKEN_ADMIN"), "big") & MASK_250 .
pub const TOKEN_ADMIN: RoleId = 0x0128d63adbf6b09002c26caf55c47e2f26635807e3ef1b027218aa74c8d61a3e;

// int.from_bytes(Web3.keccak(text="ROLE_UPGRADE_AGENT"), "big") & MASK_250 .
pub const UPGRADE_AGENT: RoleId = 0x1d8034a6db21585e9d97ca912eb8113361e6858f64c45c9b321a4d01e949484;

// int.from_bytes(Web3.keccak(text="ROLE_UPGRADE_GOVERNOR"), "big") & MASK_250 .
pub const UPGRADE_GOVERNOR: RoleId =
0x251e864ca2a080f55bce5da2452e8cfcafdbc951a3e7fff5023d558452ec228;
Expand All @@ -48,6 +52,7 @@ pub trait IRoles<TContractState> {
fn is_governance_admin(self: @TContractState, account: ContractAddress) -> bool;
fn is_operator(self: @TContractState, account: ContractAddress) -> bool;
fn is_token_admin(self: @TContractState, account: ContractAddress) -> bool;
fn is_upgrade_agent(self: @TContractState, account: ContractAddress) -> bool;
fn is_upgrade_governor(self: @TContractState, account: ContractAddress) -> bool;
fn is_security_admin(self: @TContractState, account: ContractAddress) -> bool;
fn is_security_agent(self: @TContractState, account: ContractAddress) -> bool;
Expand All @@ -61,6 +66,8 @@ pub trait IRoles<TContractState> {
fn remove_operator(ref self: TContractState, account: ContractAddress);
fn register_token_admin(ref self: TContractState, account: ContractAddress);
fn remove_token_admin(ref self: TContractState, account: ContractAddress);
fn register_upgrade_agent(ref self: TContractState, account: ContractAddress);
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);
Expand All @@ -76,10 +83,13 @@ pub trait IRoles<TContractState> {
pub trait IMinimalRoles<TContractState> {
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);
}

Expand Down Expand Up @@ -177,3 +187,15 @@ pub(crate) struct UpgradeGovernorRemoved {
pub removed_account: ContractAddress,
pub removed_by: ContractAddress,
}

#[derive(Copy, Drop, PartialEq, starknet::Event)]
pub(crate) struct UpgradeAgentAdded {
pub added_account: ContractAddress,
pub added_by: ContractAddress,
}

#[derive(Copy, Drop, PartialEq, starknet::Event)]
pub(crate) struct UpgradeAgentRemoved {
pub removed_account: ContractAddress,
pub removed_by: ContractAddress,
}
44 changes: 42 additions & 2 deletions packages/utils/src/components/roles/roles.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ pub(crate) mod RolesComponent {
AppRoleAdminRemoved, GOVERNANCE_ADMIN, GovernanceAdminAdded, GovernanceAdminRemoved, IRoles,
OPERATOR, OperatorAdded, OperatorRemoved, RoleId, SECURITY_ADMIN, SECURITY_AGENT,
SecurityAdminAdded, SecurityAdminRemoved, SecurityAgentAdded, SecurityAgentRemoved,
TOKEN_ADMIN, TokenAdminAdded, TokenAdminRemoved, UPGRADE_GOVERNOR, UpgradeGovernorAdded,
UpgradeGovernorRemoved,
TOKEN_ADMIN, TokenAdminAdded, TokenAdminRemoved, UPGRADE_AGENT, UPGRADE_GOVERNOR,
UpgradeAgentAdded, UpgradeAgentRemoved, UpgradeGovernorAdded, UpgradeGovernorRemoved,
};
use core::num::traits::Zero;
use starknet::storage::StorageMapReadAccess;
Expand Down Expand Up @@ -40,6 +40,8 @@ pub(crate) mod RolesComponent {
TokenAdminRemoved: TokenAdminRemoved,
UpgradeGovernorAdded: UpgradeGovernorAdded,
UpgradeGovernorRemoved: UpgradeGovernorRemoved,
UpgradeAgentAdded: UpgradeAgentAdded,
UpgradeAgentRemoved: UpgradeAgentRemoved,
}
use openzeppelin::access::accesscontrol::AccessControlComponent;
use openzeppelin::access::accesscontrol::AccessControlComponent::{
Expand Down Expand Up @@ -100,6 +102,13 @@ pub(crate) mod RolesComponent {
access_comp.has_role(role: TOKEN_ADMIN, :account)
}

fn is_upgrade_agent(
self: @ComponentState<TContractState>, account: ContractAddress,
) -> bool {
let access_comp = get_dep_component!(self, Access);
access_comp.has_role(role: UPGRADE_AGENT, :account)
}

fn is_upgrade_governor(
self: @ComponentState<TContractState>, account: ContractAddress,
) -> bool {
Expand Down Expand Up @@ -230,6 +239,24 @@ pub(crate) mod RolesComponent {
self._revoke_role_and_emit(role: TOKEN_ADMIN, :account, :event);
}

fn register_upgrade_agent(
ref self: ComponentState<TContractState>, 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);
}

fn remove_upgrade_agent(
ref self: ComponentState<TContractState>, 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);
}

fn register_upgrade_governor(
ref self: ComponentState<TContractState>, account: ContractAddress,
) {
Expand Down Expand Up @@ -349,6 +376,7 @@ pub(crate) mod RolesComponent {
access_comp.set_role_admin(role: GOVERNANCE_ADMIN, admin_role: GOVERNANCE_ADMIN);
access_comp.set_role_admin(role: OPERATOR, admin_role: APP_ROLE_ADMIN);
access_comp.set_role_admin(role: TOKEN_ADMIN, admin_role: APP_ROLE_ADMIN);
access_comp.set_role_admin(role: UPGRADE_AGENT, admin_role: APP_ROLE_ADMIN);
access_comp.set_role_admin(role: UPGRADE_GOVERNOR, admin_role: GOVERNANCE_ADMIN);

access_comp._grant_role(role: SECURITY_ADMIN, account: governance_admin);
Expand All @@ -361,14 +389,17 @@ pub(crate) mod RolesComponent {
self.is_app_governor(get_caller_address()), "{}", AccessErrors::ONLY_APP_GOVERNOR,
);
}

fn only_operator(self: @ComponentState<TContractState>) {
assert!(self.is_operator(get_caller_address()), "{}", AccessErrors::ONLY_OPERATOR);
}

fn only_token_admin(self: @ComponentState<TContractState>) {
assert!(
self.is_token_admin(get_caller_address()), "{}", AccessErrors::ONLY_TOKEN_ADMIN,
);
}

fn only_upgrade_governor(self: @ComponentState<TContractState>) {
assert!(
self.is_upgrade_governor(get_caller_address()),
Expand All @@ -377,6 +408,15 @@ pub(crate) mod RolesComponent {
);
}

fn only_upgrader(self: @ComponentState<TContractState>) {
assert!(
self.is_upgrade_agent(get_caller_address())
|| self.is_upgrade_governor(get_caller_address()),
"{}",
AccessErrors::ONLY_UPGRADER,
);
}

fn only_security_admin(self: @ComponentState<TContractState>) {
assert!(
self.is_security_admin(get_caller_address()),
Expand Down
104 changes: 104 additions & 0 deletions packages/utils/src/components/roles/test.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,110 @@ fn test_remove_upgrade_governor() {
assert!(!roles_dispatcher.is_upgrade_governor(account: upgrade_governor));
}

#[feature("safe_dispatcher")]
#[test]
fn test_register_upgrade_agent() {
// 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 governance_admin = constants::INITIAL_ROOT_ADMIN;
let app_role_admin = constants::APP_ROLE_ADMIN;
let upgrade_agent = constants::UPGRADE_AGENT;
let wrong_admin = constants::WRONG_ADMIN;

// Register app role admin.
cheat_caller_address_once(:contract_address, caller_address: governance_admin);
roles_dispatcher.register_app_role_admin(account: app_role_admin);

// Try to add zero address as upgrade governor.
cheat_caller_address_once(:contract_address, caller_address: app_role_admin);
let result = roles_safe_dispatcher.register_upgrade_agent(account: Zero::zero());
assert_panic_with_error(:result, expected_error: AccessErrors::ZERO_ADDRESS.describe());

// Try to add upgrade governor with unqualified caller.
cheat_caller_address_once(:contract_address, caller_address: wrong_admin);
let result = roles_safe_dispatcher.register_upgrade_agent(account: upgrade_agent);
assert_panic_with_felt_error(:result, expected_error: OZAccessErrors::MISSING_ROLE);

// Register upgrade governor and perform the corresponding checks.
let mut spy = snforge_std::spy_events();
assert!(!roles_dispatcher.is_upgrade_agent(account: upgrade_agent));
cheat_caller_address_once(:contract_address, caller_address: app_role_admin);
roles_dispatcher.register_upgrade_agent(account: upgrade_agent);

let events = spy.get_events().emitted_by(:contract_address).events;
// We only check events[1] because events[0] is an event emitted by OZ AccessControl.
assert_number_of_events(
actual: events.len(), expected: 2, message: "test_register_upgrade_agent first",
);
event_test_utils::assert_upgrade_agent_added_event(
events[1], added_account: upgrade_agent, added_by: app_role_admin,
);
assert!(roles_dispatcher.is_upgrade_agent(account: upgrade_agent));

// Register upgrade governor that is already registered (should not emit events).
let mut spy = snforge_std::spy_events();
cheat_caller_address_once(:contract_address, caller_address: app_role_admin);
roles_dispatcher.register_upgrade_agent(account: upgrade_agent);
let events = spy.get_events().emitted_by(:contract_address).events;
assert_number_of_events(
actual: events.len(), expected: 0, message: "test_register_upgrade_agent second",
);
}


#[feature("safe_dispatcher")]
#[test]
fn test_remove_upgrade_agent() {
// 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 governance_admin = constants::INITIAL_ROOT_ADMIN;
let app_role_admin = constants::APP_ROLE_ADMIN;
let upgrade_agent = constants::UPGRADE_AGENT;
let wrong_admin = constants::WRONG_ADMIN;

// Register app role admin.
cheat_caller_address_once(:contract_address, caller_address: governance_admin);
roles_dispatcher.register_app_role_admin(account: app_role_admin);

// Remove upgrade governor that was not registered (should not emit events).
assert!(!roles_dispatcher.is_upgrade_agent(account: upgrade_agent));
let mut spy = snforge_std::spy_events();
cheat_caller_address_once(:contract_address, caller_address: app_role_admin);
roles_dispatcher.remove_upgrade_agent(account: upgrade_agent);
let events = spy.get_events().emitted_by(:contract_address).events;
assert_number_of_events(
actual: events.len(), expected: 0, message: "test_remove_upgrade_agent first",
);

// Register upgrade governor.
cheat_caller_address_once(:contract_address, caller_address: app_role_admin);
roles_dispatcher.register_upgrade_agent(account: upgrade_agent);

// Try to remove upgrade governor with unqualified caller.
cheat_caller_address_once(:contract_address, caller_address: wrong_admin);
let result = roles_safe_dispatcher.remove_upgrade_agent(account: upgrade_agent);
assert_panic_with_felt_error(:result, expected_error: OZAccessErrors::MISSING_ROLE);

// Remove upgrade governor and perform the corresponding checks.
assert!(roles_dispatcher.is_upgrade_agent(account: upgrade_agent));
let mut spy = snforge_std::spy_events();
cheat_caller_address_once(:contract_address, caller_address: app_role_admin);
roles_dispatcher.remove_upgrade_agent(account: upgrade_agent);
let events = spy.get_events().emitted_by(:contract_address).events;
// We only check events[1] because events[0] is an event emitted by OZ AccessControl.
assert_number_of_events(
actual: events.len(), expected: 2, message: "test_remove_upgrade_agent second",
);
event_test_utils::assert_upgrade_agent_removed_event(
events[1], removed_account: upgrade_agent, removed_by: app_role_admin,
);
assert!(!roles_dispatcher.is_upgrade_agent(account: upgrade_agent));
}


#[test]
#[feature("safe_dispatcher")]
Expand Down