feat: add gha upgrade facet #289
Conversation
gfournierPro
commented
Oct 15, 2025
gfournierPro
commented
- added verification to upgrade script too
There was a problem hiding this comment.
Pull Request Overview
Adds a reusable verification utility and integrates contract verification into both initial deployment and facet upgrade flows, plus introduces a GitHub Actions workflow for automated facet upgrades.
- Refactors verify script into reusable functions with optional targeted verification.
- Integrates verification into deploy and upgrade scripts.
- Adds CI workflow to run dry-run or live facet upgrades and persist artifacts.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| scripts/verify.ts | Refactors and generalizes contract verification logic. |
| scripts/upgrades/deploy-and-update-some-facet.ts | Invokes new verify utility after facet upgrade. |
| deploy/0_deploy.ts | Replaces inline verification logic with centralized verify() call. |
| .github/workflows/upgrade-facets.yml | Adds workflow to automate facet upgrade (dry run or live) and artifact persistence. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #289 +/- ##
=======================================
Coverage 84.85% 84.85%
=======================================
Files 37 37
Lines 1241 1241
Branches 235 235
=======================================
Hits 1053 1053
Misses 188 188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| const contractsToVerify = | ||
| contracts && contracts.length > 0 ? contracts : await getContractsFromDeployments(); |
| error.message.includes('has') && | ||
| error.message.includes('parameters but') && | ||
| error.message.includes('arguments were provided') |
There was a problem hiding this comment.
Are you sure we’ve covered all possible scenarios? If the logs change, those lines will break. Maybe it’s not a good idea.
There was a problem hiding this comment.
goona dig on that
But that's the from previous work, I did not change anything here
There was a problem hiding this comment.
But here it only concerns constructor arguments
There was a problem hiding this comment.
Since in line 77 we already log the error
I can say that those 3 checks and the other console.error are redondant
There was a problem hiding this comment.
Yes we can keep only the above error message
| error.message.includes('has') && | ||
| error.message.includes('parameters but') && | ||
| error.message.includes('arguments were provided') |
There was a problem hiding this comment.
Yes we can keep only the above error message
| run: npm run build | ||
|
|
||
| - name: Run fork test (dry run) | ||
| if: inputs.dry_run == true |
… better encapsulation
…ucture and clarity
| * @param contracts - Optional array of specific contracts to verify. If not provided, | ||
| * will verify all contracts from the deployments/{network} directory. | ||
| */ | ||
| async function verify(contracts?: ContractToVerify[]): Promise<void> { |
There was a problem hiding this comment.
| async function verify(contracts?: ContractToVerify[]): Promise<void> { | |
| export default async function verify(contracts?: ContractToVerify[]): Promise<void> { |
scripts/verify.ts
Outdated
| * | ||
| * @param contracts - Optional array of specific contracts to verify. | ||
| */ | ||
| async function tryVerify(contracts?: ContractToVerify[]): Promise<void> { |
There was a problem hiding this comment.
| async function tryVerify(contracts?: ContractToVerify[]): Promise<void> { | |
| export async function tryVerify(contracts?: ContractToVerify[]): Promise<void> { |
scripts/verify.ts
Outdated
| export default verify; | ||
| export { tryVerify }; |
There was a problem hiding this comment.
| export default verify; | |
| export { tryVerify }; |
