-
Notifications
You must be signed in to change notification settings - Fork 14
feat: replace erc1538 naming to diamond proxy naming - part 1 #229
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
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.
Pull Request Overview
This PR updates naming throughout the codebase to replace "ERC1538" references with "Diamond proxy" terminology, reflecting a migration from ERC1538 proxy pattern to Diamond proxy pattern. This appears to be part 1 of a larger refactoring effort.
- Replace all occurrences of "ERC1538" with "Diamond" or "DiamondProxy" in variable names, comments, and configuration
- Update deployment and configuration scripts to use new Diamond proxy naming
- Maintain backward compatibility by keeping deprecated ERC1538Proxy config field
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
utils/proxy-tools.ts | Update comments to reference Diamond proxy instead of ERC1538 |
utils/config.ts | Add DiamondProxy config field while keeping deprecated ERC1538Proxy |
test/utils/hardhat-fixture-deployer.ts | Replace ERC1538Proxy references with DiamondProxy in test setup |
test/utils/fixture-helpers.ts | Update ownership transfer functions to use DiamondProxy |
scripts/upgrades/upgrade-helper.ts | Rename variables and comments from erc1538 to diamond terminology |
scripts/tools/update-config.ts | Update deployment getter to use DiamondProxy instead of ERC1538Proxy |
scripts/sponsoring/1_add-modules-to-proxy.ts | Replace ERC1538Proxy variables with DiamondProxy throughout |
scripts/set-callback-gas.ts | Update deployment getter to use DiamondProxy |
scripts/boost/1_add-modules-to-proxy.ts | Replace ERC1538Proxy references with DiamondProxy |
deploy/0_deploy.ts | Update variable names and comments from erc1538 to diamond terminology |
config/config.json | Replace ERC1538Proxy key with DiamondProxy in configuration |
.github/workflows/deploy-smart-contracts.yml | Update workflow comment to reference Diamond proxy |
Comments suppressed due to low confidence (1)
scripts/upgrades/upgrade-helper.ts:33
- [nitpick] The variable name 'diamondQueryInstance' suggests it should be a Diamond-related type, but it's still using 'ERC1538Query'. Consider updating the type name to match the Diamond pattern or using a more generic name like 'queryInstance'.
const diamondQueryInstance: ERC1538Query = ERC1538Query__factory.connect(
Co-authored-by: Zied Guesmi <26070035+zguesmi@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/diamond #229 +/- ##
================================================
Coverage 83.85% 83.85%
================================================
Files 36 36
Lines 1109 1109
Branches 225 225
================================================
Hits 930 930
Misses 179 179 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Robin Le Caignec <72495599+Le-Caignec@users.noreply.github.com>
Split de la PR #226 : #226 en 3