Replace current proxy with ERC-2535 diamond proxy #213
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #213 +/- ##
===========================================
+ Coverage 84.50% 84.83% +0.33%
===========================================
Files 35 36 +1
Lines 1084 1108 +24
Branches 221 224 +3
===========================================
+ Hits 916 940 +24
Misses 168 168 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
james-toussaint
force-pushed
the
feature/erc-2535-diamond-proxy
branch
from
fbf7626 to
d879576
Compare
| pragma solidity >=0.6.0 <0.9.0; | ||
|
|
||
| abstract contract OwnableDiamondStore { | ||
| // Add storage slot padding for now to avoid updating the few tests using |
There was a problem hiding this comment.
add a todo remove ?
| // `receive` & `fallback` functions must be respectively added to diamond with | ||
| // 0x00000000 & 0xffffffff selectors. |
There was a problem hiding this comment.
this comments should be set few lines before (?)
There was a problem hiding this comment.
or it's a future modification that we need to be aware of ? like https://github.com/iExecBlockchainComputing/iexec-solidity/blob/fec925af8201943d0d16de23b1c765e0b9a0bf83/contracts/ERC1538/ERC1538Core.sol#L36-L37
| } | ||
| address facet = ds.selectorToFacetAndPosition[msg.sig].facetAddress; | ||
| if(facet == address(0)){ | ||
| facet = ds.selectorToFacetAndPosition[0xffffffff].facetAddress; // fallback |
There was a problem hiding this comment.
I don't really get this line? What should be the output the address of the entrypoint ?
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the project from ERC-1538 proxy pattern to ERC-2535 Diamond proxy pattern, implementing a more standardized and flexible proxy architecture. The Diamond pattern provides better modularity and gas efficiency for upgradeable contracts.
Key changes include:
- Replacement of ERC-1538 proxy with ERC-2535 Diamond proxy implementation
- Migration of contract linking logic to use Diamond facets instead of ERC-1538 modules
- Update of base contracts and storage patterns to support Diamond architecture
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/proxy-tools.ts | Replaces ERC-1538 linking logic with Diamond facet management |
| deploy/0_deploy.ts | Updates deployment script to use Diamond proxy instead of ERC-1538 |
| contracts/Diamond.sol | Adds new Diamond proxy contract implementation |
| contracts/OwnableDiamondStore.sol | Introduces Diamond-compatible storage with ownership functionality |
| contracts/modules/DelegateBase.sol | Removes ERC-1538 module inheritance |
| contracts/Store.sol | Updates storage base class to use Diamond pattern |
| package.json | Adds Diamond library dependency |
Comments suppressed due to low confidence (1)
deploy/0_deploy.ts:86
- The transferOwnershipCall variable is prepared but not used in the Diamond constructor. This appears to be dead code that should be removed.
throw new Error('Failed to prepare transferOwnership data');
|
Closed in favor of #241 |
Diamonddiff with https://github.com/mudgen/diamond-3-hardhat/blob/08d415c7e1a27c811c34875d9263a90bc719cee3/contracts/Diamond.solIexecPoco1Delegateisn't verified)