Skip to content

update grant role to revoke personal deployer admin#56

Open
nguyennk92 wants to merge 2 commits intomainfrom
revoke-personal-wallet
Open

update grant role to revoke personal deployer admin#56
nguyennk92 wants to merge 2 commits intomainfrom
revoke-personal-wallet

Conversation

@nguyennk92
Copy link
Member

@nguyennk92 nguyennk92 commented Feb 5, 2026

Note

Medium Risk
Touches access-control administration by granting/revoking DEFAULT_ADMIN_ROLE/ADMIN_ROLE; a misconfigured MULTISIG or execution on the wrong deployment address could lock out admin control.

Overview
Moves post-deploy role management toward multisig custody by updating GrantRole.s.sol to grant DEFAULT_ADMIN_ROLE/ADMIN_ROLE to MULTISIG and revoke those roles from the deployer address (derived from PRIVATE_KEY), and adds a new V3UtilsGrantRoleScript alongside the existing V3Automation flow.

Updates tooling/config to support this: the Makefile now exposes grant-role-v3automation and grant-role-v3utils targets, sample.env adds MULTISIG, and contracts.json adds additional chain-id entries (130, 999, 43114).

Written by Cursor Bugbot for commit 1457ea3. This will update automatically on new commits. Configure here.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

v3automation.grantRole(v3automation.DEFAULT_ADMIN_ROLE(), multisig);
v3automation.grantRole(v3automation.ADMIN_ROLE(), multisig);
v3automation.revokeRole(v3automation.ADMIN_ROLE(), deployerAddress);
v3automation.revokeRole(v3automation.DEFAULT_ADMIN_ROLE(), deployerAddress);
Copy link

Choose a reason for hiding this comment

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

Script revokes from wrong address, causing failure

High Severity

The scripts try to revoke roles from deployerAddress (derived from PRIVATE_KEY) and assume this address has DEFAULT_ADMIN_ROLE. However, during initialization in Init.s.sol, roles are granted to the WITHDRAWER address (via admin in CommonScript), not the deployer. If these addresses differ, the grantRole calls will revert because deployerAddress lacks admin permissions, and even if they succeed, the revokeRole calls would revoke from the wrong account—failing to remove the actual admin's privileges.

Additional Locations (1)

Fix in Cursor Fix in Web

grant-role-v3automation: v3automation
forge script script/GrantRole.s.sol:V3AutomationGrantRoleScript --rpc-url $(RPC_URL) --broadcast --legacy --gas-price 0
grant-role-v3utils: v3utils
forge script script/GrantRole.s.sol:V3UtilsGrantRoleScript --rpc-url $(RPC_URL) --broadcast --legacy --gas-price 0
Copy link

Choose a reason for hiding this comment

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

Makefile references removed grant-role target

Medium Severity

The deploy-everything target calls make grant-role on line 62, but this target was renamed to grant-role-v3automation in the same diff. Running make deploy-everything will fail at this step. Additionally, the new grant-role-v3utils target is not included, so even if corrected to call only grant-role-v3automation, the V3Utils contract wouldn't have its roles configured.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments