feat: Multisig migration script#1433
Conversation
tbwebb22
requested review from
bmzig,
mrice32,
nicholaspai and
pxrl
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 781079dc31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function resolveProvider(chainId: number): ethers.providers.JsonRpcProvider | undefined { | ||
| try { | ||
| const url = getNodeUrl(chainId); | ||
| if (!url) return undefined; | ||
| return new ethers.providers.JsonRpcProvider(url); |
There was a problem hiding this comment.
When a developer has only CUSTOM_NODE_URL in .env (a common setup for single-chain scripts), getNodeUrl(chainId) returns that same RPC for every row this report builds, and this code never checks provider.getNetwork().chainId before querying contract addresses. In that scenario the multi-chain report silently reads owners from the wrong network for all other chains and can mark migrations green/red based on unrelated contracts; require per-chain RPCs here or verify the provider chain ID before using it.
Useful? React with 👍 / 👎.
| // DonationBox variants use AccessControl, so "ownership" = DEFAULT_ADMIN_ROLE membership. | ||
| const DONATION_BOX_NAMES = ["DonationBox", "DonationBox_CCTP", "DonationBox_OFT"]; |
There was a problem hiding this comment.
The deployed-addresses artifact currently stores DonationBox deployments with flow/token suffixes such as DonationBox_CCTP_USDC, DonationBox_CCTP_USDC_EVM, and DonationBox_OFT_USDT, so this exact-name allowlist never finds them. As a result, chains that do have DonationBoxes render DonationBox Admin as — and those admin migrations are omitted from the progress count; match by prefix or include the actual suffixed names.
Useful? React with 👍 / 👎.
fusmanii
left a comment
fusmanii
left a comment
There was a problem hiding this comment.
the md file looks nice, easy to spot check at a glance.
| "extract-addresses": "./script/utils/extract_foundry_addresses.sh", | ||
| "generate-constants-json": "ts-node ./script/utils/GenerateConstantsJson.ts", | ||
| "deploy-safe": "ts-node ./script/safe-multisig/deploySafe.ts", | ||
| "list-multisigs": "ts-node ./script/safe-multisig/generateMultisigList.ts", |
There was a problem hiding this comment.
| "list-multisigs": "ts-node ./script/safe-multisig/generateMultisigList.ts", | |
| "list-multisigs": "ts-node ./script/safe-multisig/generateMultisigList.ts && yarn prettier --write script/safe-multisig/MULTISIGS.md", |
so that it gets formatted
|
|
||
| **Migration progress: 9.1%** — 6 of 66 checks pass (60 outstanding). | ||
|
|
||
| | Chain ID | Chain | Ops Multisig Deployed | Universal SpokePool Owner | Counterfactual WithdrawManager Owner | Counterfactual WithdrawManager directWithdrawer | Sponsored CCTP Periphery Owner | Sponsored OFT Periphery Owner | DonationBox Admin | |
There was a problem hiding this comment.
I noticed that the donation boxes are missing on all chains
There was a problem hiding this comment.
good catch! Updated the script and re-ran it - Donation boxes populated now. Since the DonationBox uses AccessControl (non-enumerable) - the table will just show "No" if the admin isn't one of the multisigs or fallback EOA
There was a problem hiding this comment.
also just realized we probably need two columns for DonationBox - some chains have CCTP and OFT deployments of it
There was a problem hiding this comment.
ok made this update - if there are multiple DonationBoxes for a chain, it will show each of them in the DonationBox column
tbwebb22
force-pushed
the
taylor/multisig-migration-script
branch
from
4e22148 to
e6c6e41
Compare
2 participants
Open
MULTISIGS.mdin preview mode to see the formatted tableCloses ACP-142