-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #127 +/- ##
=======================================
Coverage ? 89.36%
=======================================
Files ? 43
Lines ? 2464
Branches ? 0
=======================================
Hits ? 2202
Misses ? 262
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7ecec92 to
287d845
Compare
RoeeGross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RoeeGross reviewed 1 of 1 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @remollemo)
5e19946 to
d85b11e
Compare
remollemo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 3 of 5 files reviewed, all discussions resolved (waiting on @remollemo)
146b241 to
e919b5b
Compare
e919b5b to
33c582d
Compare
remollemo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@remollemo reviewed 1 of 1 files at r2, 2 of 2 files at r3, 1 of 2 files at r4, 1 of 2 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @remollemo)
57d5fa3 to
c4cf173
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Reclaim Legacy Roles: Missing UPGRADE_AGENT and SECURITY_GOVERNOR
The reclaim_legacy_roles function is missing calls to reclaim the newly added UPGRADE_AGENT and SECURITY_GOVERNOR roles. Users who had these roles in the legacy system will not be able to reclaim them when calling this function. The function should include self._reclaim_role(role: UPGRADE_AGENT, :account); and self._reclaim_role(role: SECURITY_GOVERNOR, :account); to be consistent with the other roles.
packages/utils/src/components/roles/roles.cairo#L323-L334
starkware-starknet-utils/packages/utils/src/components/roles/roles.cairo
Lines 323 to 334 in c4cf173
| } | |
| fn reclaim_legacy_roles(ref self: ComponentState<TContractState>) { | |
| let account = get_caller_address(); | |
| self._reclaim_role(role: GOVERNANCE_ADMIN, :account); | |
| self._reclaim_role(role: APP_GOVERNOR, :account); | |
| self._reclaim_role(role: APP_ROLE_ADMIN, :account); | |
| self._reclaim_role(role: OPERATOR, :account); | |
| self._reclaim_role(role: TOKEN_ADMIN, :account); | |
| self._reclaim_role(role: UPGRADE_GOVERNOR, :account); | |
| self._reclaim_role(role: SECURITY_ADMIN, :account); | |
| self._reclaim_role(role: SECURITY_AGENT, :account); | |
| } |
remollemo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@remollemo reviewed 3 of 3 files at r7, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @remollemo)
c4cf173 to
02f7255
Compare
remollemo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@remollemo reviewed 1 of 1 files at r8, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @remollemo)
This change is
Note
Introduces
UPGRADE_AGENTrole with APIs, events, guard, and tests; also addsSECURITY_GOVERNORrole support and updates admin mappings and error codes.roles.cairo):is_upgrade_agent,register_upgrade_agent,remove_upgrade_agentand correspondingUpgradeAgentAdded/Removedevents.is_security_governor,register_security_governor,remove_security_governorandSecurityGovernorAdded/Removedevents.only_upgrader(allowsUPGRADE_AGENTorUPGRADE_GOVERNOR) andonly_security_governor.initializeto set admins:UPGRADE_AGENT -> APP_ROLE_ADMIN,SECURITY_GOVERNOR -> SECURITY_ADMIN.interface.cairo):UPGRADE_AGENT,SECURITY_GOVERNOR.IRoles/IMinimalRoleswithis_*,register_*,remove_*for upgrade agent and security governor; add related event structs.errors.cairo): AddONLY_UPGRADER,ONLY_SECURITY_GOVERNORdescribable errors.UPGRADE_AGENT.UpgradeAgentAdded/Removed.register/remove_upgrade_agentcases verifying role checks and event emission.Written by Cursor Bugbot for commit 02f7255. This will update automatically on new commits. Configure here.