feat: clean OpenZeppelin dual-version setup #308
Conversation
…ns and adding new contract interfaces for improved clarity and organization
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades OpenZeppelin Contracts from version 3.3.0 to 5.4.0 by removing the dual-version setup (@openzeppelin/contracts v3.3.0 and @openzeppelin/contracts-v5 alias) in favor of a single v5 installation. The change removes a custom TimelockController implementation and adopts the official OpenZeppelin v5 TimelockController contract.
Key changes:
- Replaced dual OpenZeppelin package setup with single v5.4.0 version
- Updated all import paths from
@openzeppelin/contracts-v5to@openzeppelin/contracts - Removed custom TimelockController contract (Solidity 0.6) in favor of OpenZeppelin's v5 implementation
- Updated hardhat config to compile OpenZeppelin's TimelockController for deployment
Reviewed Changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated OpenZeppelin dependency from dual v3.3.0/v5.0.2 setup to single v5.4.0, removed TimelockController exclusion |
| package-lock.json | Updated lock file to reflect new OpenZeppelin v5.4.0 dependency resolution |
| hardhat.config.ts | Updated import paths and added OpenZeppelin TimelockController to external compilation list |
| contracts/tools/TimelockController.sol | Removed custom Solidity 0.6 implementation |
| contracts/registries/*.sol | Updated import paths from @openzeppelin/contracts-v5 to @openzeppelin/contracts |
| contracts/libs/PocoStorageLib.sol | Updated IERC20 import path |
| contracts/facets/*.sol | Updated all OpenZeppelin import paths and reformatted IexecPoco1Facet contract declaration |
| contracts/tools/testing/*.sol | Updated mock contract import paths |
| abis/contracts/registries/*.json | Updated error definitions to match OpenZeppelin v5 (renamed Create2* errors to match new naming) |
| abis/contracts/facets/IexecPoco2Facet.json | Removed MathOverflowedMulDiv error (no longer exposed in v5) |
| docs/solidity/index.md | Reorganized documentation sections (content unchanged, only ordering) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rations for enhanced security in maintenance tasks
… proper formatting
Le-Caignec
changed the title
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…improved error handling
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feature/make-v8-migration-non-breaking #308 +/- ##
=======================================================================
Coverage 96.32% 96.32%
=======================================================================
Files 34 34
Lines 1117 1117
Branches 209 209
=======================================================================
Hits 1076 1076
Misses 41 41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…eaking' into feat/clean-dep
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| address[] memory proposers, | ||
| address[] memory executors | ||
| ) public { | ||
| ) { |
There was a problem hiding this comment.
[nitpick] The constructor visibility should be explicitly declared as public for clarity and consistency with Solidity best practices, even though constructors are implicitly public in Solidity 0.8.0+. This maintains compatibility with OpenZeppelin's TimelockController pattern and improves readability.
| ) { | |
| ) public { |
…consistency across multiple contracts
…nd organization, consolidating interfaces and updating function signatures for improved readability
…ing in IexecERC20Core contract for better readability and maintainability
…solidate function signatures, and enhance clarity by introducing new methods for managing categories, datasets, and worker pools. This update also includes breaking changes to improve organization and maintainability.
…ate package.json to exclude TimelockController from build artifacts
…ry structure, consolidate interfaces, and enhance function signatures for better clarity and organization. This update includes breaking changes and new methods for managing categories, datasets, and worker pools.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 35 out of 36 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 35 out of 36 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "version": "5.0.2", | ||
| "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-5.0.2.tgz", | ||
| "integrity": "sha512-ytPc6eLGcHHnapAZ9S+5qsdomhjo6QBHTDRRBFfTxXIpsicMhVPouPgmUPebZZZGX7vt9USA+Z+0M0dSVtSUEA==" | ||
| "version": "5.4.0", |
There was a problem hiding this comment.
maybe issue here ? instead of "version": "5.0.2",
There was a problem hiding this comment.
I removed it and regenerated it from scratch, and I got the same file. What about you?
It seems correct to me.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 36 out of 37 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eaking' into feat/clean-dep
…ith new error handling and function signatures. Add new error definitions in Errors.sol and Strings.sol. Introduce IERC165 and ERC165 interfaces. Update ERC20, ERC721, and related interfaces to reflect current standards. Enhance error handling in various contracts, including Create2 and ECDSA.
…arious platforms including solidity analyzers for darwin, freebsd, linux, and win32 environments
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 49 out of 67 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 participants
This PR upgrades OpenZeppelin Contracts from version 3.3.0 to 5.4.0 by removing the dual-version setup (@openzeppelin/contracts v3.3.0 and @openzeppelin/contracts-v5 alias) in favor of a single v5 installation.