Create pre deployment addition script - #1290
Conversation
- Add scripts/add-predeployment.ts to create pre-deployment artifacts - Add GitHub Action workflow callable from UI and other workflows - Always creates a PR for review - Runs prettier after adding artifact via npm post script
feat: add pre-deployment workflow for OP Stack and ZKsync networks
- Add scripts/add-predeployment.ts to create pre-deployment artifacts - Add GitHub Action workflow callable from UI and other workflows - Always creates a PR for review - Runs prettier after adding artifact via npm post script
feat: add pre-deployment workflow for OP Stack and ZKsync networks
- Adjusted error message formatting for consistency and clarity in the PredeploymentError class. - Ensured proper line breaks in error details for better readability.
- Enhanced consistency and readability of error messages in the PredeploymentError class. - Adjusted line breaks for better clarity in error details.
| env: | ||
| CHAIN_ID: ${{ inputs.chain_id }} | ||
| RPC: ${{ inputs.rpc_url }} | ||
| SKIP_CHAINLIST_CHECK: ${{ inputs.skip_chainlist_check }} |
There was a problem hiding this comment.
I don't think we should have this. We currently have a policy "no non-chainlist networks", and I don't think this CI workflow and script should support it.
| "add:predeployment": "ts-node scripts/add-predeployment.ts", | ||
| "postadd:predeployment": "npm run fmt", |
There was a problem hiding this comment.
Is formatting really necessary here? Can't we just write a correctly formatted file and simplify the process? (IMO, its different to the safe-deployments case where you are editing a JSON file - here you are just writing a pre-baked file).
| "compile:zk": "hardhat compile && hardhat deploy-zksync --script compile-zk.ts", | ||
| "verify": "ts-node scripts/verify.ts", | ||
| "verify:new-chain-request": "ts-node scripts/new-chain.ts", | ||
| "add:predeployment": "ts-node scripts/add-predeployment.ts", |
There was a problem hiding this comment.
ubernit:
| "add:predeployment": "ts-node scripts/add-predeployment.ts", | |
| "add-predeployment": "ts-node scripts/add-predeployment.ts", |
| "Chain ID not provided", | ||
| `**⛔️ Error:**<br>` + | ||
| `Chain ID not provided. Please set the CHAIN_ID environment variable.`, |
There was a problem hiding this comment.
These errors with formatting and emojis seem unnecessary to me, and IMO add quite a bit of "noise" to the script.
They were there for the new chain script because these are formatted messages for posting to GitHub issues, but here they are just echo-ed to the GitHub action console. In fact, I don't know if the <br> or the ** bold markers even render correctly.
| } | ||
|
|
||
| // Verify chain is in chainlist (optional) | ||
| const skipChainlistCheck = process.env.SKIP_CHAINLIST_CHECK === "true"; |
There was a problem hiding this comment.
This is against the policy of the repo, we should not include it here.
| await verifyPredeployment(rpcUrl, chainId); | ||
| } else { | ||
| console.log( | ||
| "Warning: No RPC URL provided. Skipping on-chain verification.", |
There was a problem hiding this comment.
We should get the RPC from the Chainlist rocs.json no?
|
|
||
| fs.writeFileSync( | ||
| artifactPath, | ||
| JSON.stringify(PREDEPLOYMENT_ARTIFACT, null, "\t") + "\n", |
There was a problem hiding this comment.
This makes me think that we don't need to run npm run fmt after execution.
- Remove SKIP_CHAINLIST_CHECK option (repo policy: no non-chainlist networks) - Auto-fetch RPC URL from chainlist rpcs.json - Simplify error messages (remove markdown formatting and emojis) - Rename script from add:predeployment to add-predeployment - Remove postadd:predeployment script (file already correctly formatted) - Remove rpc_url and skip_chainlist_check inputs from workflow - Update workflow to use new script name and simplified outputs
- Changed npm install command from 'npm ci --ignore-scripts' to 'npm ci' for consistency and to ensure all scripts are executed during installation.
| class PredeploymentError extends Error { | ||
| constructor(message: string) { | ||
| super(message); | ||
| this.name = "PredeploymentError"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we just use a regular Error? I don't think the additional type gives us anything.
- Removed unused RPC URL fetching and verification logic from the add-predeployment script. - Streamlined error handling for chainlist verification. - Updated type definitions for better clarity and removed unnecessary
2 participants
No description provided.