Skip to content

Conversation

gfournieriExec
Copy link
Contributor

@gfournieriExec gfournieriExec commented Jul 24, 2025

Migration steps:

  • Migrate src files (minimal changes)
  • Modify receive and fallback behavior in Diamond.sol
  • Rename all 1538 to 2535 in contracts and rename Module to Facet in contracts names
  • Update all facets to use namespaces app storage.
  • Remove ENSIntegration modules/functions
  • Update docs (soldoc, diagrams, ...)
  • Add unit tests for Diamond.sol

Copy link

codecov bot commented Jul 24, 2025

Codecov Report

❌ Patch coverage is 55.81395% with 152 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.36%. Comparing base (287cc9c) to head (4da1c56).
⚠️ Report is 7 commits behind head on feature/diamond.

Files with missing lines Patch % Lines
...racts/modules/facets/IexecEscrowTokenSwapFacet.sol 0.00% 66 Missing ⚠️
contracts/modules/facets/SignatureVerifier.sol 0.00% 50 Missing ⚠️
...ontracts/modules/facets/IexecEscrowNativeFacet.sol 0.00% 35 Missing ⚠️
contracts/modules/facets/IexecMaintenanceFacet.sol 96.66% 1 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##           feature/diamond     #226      +/-   ##
===================================================
- Coverage            83.85%   79.36%   -4.50%     
===================================================
  Files                   36       37       +1     
  Lines                 1109     1173      +64     
  Branches               225      236      +11     
===================================================
+ Hits                   930      931       +1     
- Misses                 179      242      +63     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gfournieriExec gfournieriExec requested a review from Copilot July 25, 2025 06:32
Copy link
Contributor

@Copilot Copilot AI left a 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 migrates the codebase from ERC1538 naming conventions to Diamond proxy (ERC2535) naming conventions. The migration updates contract references, module names, configuration keys, and deployment scripts to reflect the transition from "modules/delegates" to "facets" and from "ERC1538Proxy" to "DiamondProxy".

  • Renames all contract modules from "Delegate" to "Facet" (e.g., IexecMaintenanceDelegateIexecMaintenanceFacet)
  • Updates configuration keys from ERC1538Proxy to DiamondProxy while maintaining backward compatibility
  • Migrates base contracts from DelegateBase to FacetBase

Reviewed Changes

Copilot reviewed 54 out of 54 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
utils/proxy-tools.ts Updated JSDoc comments to reference Diamond proxy instead of ERC1538
utils/config.ts Added DiamondProxy configuration while keeping ERC1538Proxy as deprecated
test/utils/*.ts Updated test utilities to use DiamondProxy configuration and naming
test/byContract/IexecPocoBoost/*.ts Migrated test files to use new Facet naming conventions
scripts/*.ts Updated deployment and configuration scripts to use Diamond proxy naming
contracts/modules/facets/*.sol New facet contracts replacing delegate contracts with updated inheritance
contracts/modules/delegates/*.sol Removed old delegate contracts
contracts/modules/FacetBase*.sol Updated base contracts with new naming and documentation
hardhat.config.ts Updated soldoc exclusion paths to use facets instead of delegates
deploy/*.ts Updated deployment scripts to use Diamond proxy and facet naming
.github/workflows/*.yml Updated CI workflow comments to reference DiamondProxy
Comments suppressed due to low confidence (3)

test/utils/hardhat-fixture-deployer.ts:62

  • The deployment name 'Diamond' is inconsistent with the variable name 'newProxyAddress' and the log message 'DiamondProxy'. Consider using 'DiamondProxy' as the deployment name for consistency.
        const newProxyAddress = (await deployments.get('Diamond')).address;

scripts/set-callback-gas.ts:15

  • The deployment name 'Diamond' is inconsistent with the variable name 'diamondProxyAddress'. Consider using 'DiamondProxy' as the deployment name for consistency throughout the codebase.
    const diamondProxyAddress = (await deployments.get('Diamond')).address;

deploy/1_deploy-ens.ts:29

  • The deployment name 'Diamond' is inconsistent with the variable name 'diamondProxyAddress'. Consider using 'DiamondProxy' as the deployment name for consistency with other parts of the codebase.
    const diamondProxyAddress = (await deployments.get('Diamond')).address;

Copy link
Contributor

@Le-Caignec Le-Caignec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t it a facet as well?

@gfournieriExec gfournieriExec marked this pull request as ready for review July 25, 2025 10:26
@gfournieriExec
Copy link
Contributor Author

Replaced by PR #229, PR #230, PR #234

@gfournieriExec gfournieriExec deleted the feature/rename-to-diamond branch July 29, 2025 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants