-
Notifications
You must be signed in to change notification settings - Fork 14
Fix fallback and receive functions in Diamond contract #223
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/diamond #223 +/- ##
================================================
Coverage 84.35% 84.35%
================================================
Files 37 37
Lines 1112 1112
Branches 225 225
================================================
Hits 938 938
Misses 174 174 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 implements fixes for the fallback and receive functions in the Diamond contract as part of migrating from ERC-1538 to ERC-2535 Diamond pattern. The changes remove unused utility functions and clean up deployment configuration files while updating test storage slot references to match the new storage layout.
- Removal of unused code comment blocks and utility functions
- Update of Diamond dependency configuration in Hardhat
- Storage slot corrections for test cases to align with new Diamond implementation
Reviewed Changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
utils/proxy-tools.ts | Removes unused function and commented code blocks |
test/byContract/IexecEscrow/IexecEscrowNative.test.ts | Updates storage slot index from 0x0c to 0x07 for m_totalSupply |
hardhat.config.ts | Comments out Diamond.sol dependency compilation |
deployments/arbitrumSepolia/*.json | Updates contract addresses and deployment counts for new deployment |
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.
LGTM
Good job 👍🏻
Migration steps:
receive
andfallback
behavior inDiamond.sol
Module
toFacet
in contracts namesDiamond.sol
Important diff:
The diff between Nick Mudge's
Diamond.sol
and ours can be seen in this commit: Adapt Diamond contractArbitrum Sepolia deployment:
Proxy deployed and verified on Arbitrum Sepolia: 0x83AB1D51195894E402D0dE895e0141b2a1E4f9E8
Deployment logs: