Skip to content

Conversation

@Le-Caignec
Copy link
Contributor

@Le-Caignec Le-Caignec commented Oct 28, 2025

Gas Study

gas-comparison

Copilot AI review requested due to automatic review settings October 28, 2025 12:07
@Le-Caignec Le-Caignec self-assigned this Oct 28, 2025
Copy link
Contributor

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 pull request removes the IexecEscrowTokenSwapFacet and migrates the codebase from Solidity v0.6.0 to v0.8.0, making the v8 migration non-breaking by consolidating storage libraries and updating registry contracts to use OpenZeppelin v5.

Key changes:

  • Removed IexecEscrowTokenSwapFacet and related token swap functionality
  • Upgraded Solidity version from 0.6.0 to 0.8.0 across registry and test contracts
  • Consolidated PocoStorageLib.v8.sol into PocoStorageLib.sol
  • Updated registry contracts to use OpenZeppelin contracts v5 (ERC721Enumerable, Ownable)

Reviewed Changes

Copilot reviewed 73 out of 73 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
utils/proxy-tools.ts Removed imports and references to IexecEscrowTokenSwapFacet
scripts/tools/sol-to-uml.mjs Removed IexecEscrowTokenSwapFacet from UML diagram generation
hardhat.config.ts Removed exclusions for files that no longer exist
docs/solidity/index.md Regenerated documentation with updated structure and removed TokenSwap references
deploy/0_deploy.ts Fixed import path for Ownable factory and removed TokenSwap deployment comment
contracts/tools/testing/*.sol Upgraded Solidity version to 0.8.0 and removed unnecessary constructors
contracts/registries/**/*.sol Upgraded to Solidity 0.8.0, OpenZeppelin v5, and updated type casting for address-to-uint conversions
contracts/registries/proxy/*.sol Added proxy contracts copied from iexec-solidity with pragma updates
contracts/libs/PocoStorageLib.sol Consolidated v8 storage lib, updated imports, and fixed assembly syntax
contracts/interfaces/* Created IexecHubV3Interface, removed IexecEscrowTokenSwap, updated event declarations
contracts/facets/*.sol Updated all facets to use consolidated PocoStorageLib and replaced SafeMathExtended with native operators
abis/contracts/**/*.json Updated ABIs to reflect OpenZeppelin v5 error types and parameter name changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Le-Caignec Le-Caignec changed the base branch from main to chore/solidity-v8 October 28, 2025 12:10
@Le-Caignec Le-Caignec requested a review from Copilot October 28, 2025 12:10
Copy link
Contributor

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

Copilot reviewed 6 out of 6 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.

…y, and WorkerpoolRegistry for enhanced deployment handling
…y documentation for Iexec interfaces and methods
…f JSON files and updating initialization logic in Registry contract
…y documentation for new methods in Iexec contracts
Copilot AI review requested due to automatic review settings October 28, 2025 14:22
Copy link
Contributor

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

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings October 28, 2025 16:07
Copy link
Contributor

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

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.32%. Comparing base (4bab677) to head (31096ca).

Additional details and impacted files
@@                  Coverage Diff                  @@
##           chore/solidity-v8     #306      +/-   ##
=====================================================
+ Coverage              96.26%   96.32%   +0.06%     
=====================================================
  Files                     34       34              
  Lines                   1099     1117      +18     
  Branches                 203      209       +6     
=====================================================
+ Hits                    1058     1076      +18     
  Misses                    41       41              

☔ 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.

Copilot AI review requested due to automatic review settings October 28, 2025 16:27
@iExecBlockchainComputing iExecBlockchainComputing deleted a comment from Copilot AI Oct 28, 2025
Copy link
Contributor

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

Copilot reviewed 12 out of 15 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.

@Le-Caignec Le-Caignec marked this pull request as ready for review October 28, 2025 16:28
Copy link
Contributor

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

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 5 to 6

import {IexecERC20Common} from "./IexecERC20Common.sol";
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

The pragma experimental ABIEncoderV2; directive is unnecessary and deprecated in Solidity 0.8.0+. ABIEncoderV2 is enabled by default in Solidity 0.8.0 and later versions, so this pragma should be removed.

Suggested change
import {IexecERC20Common} from "./IexecERC20Common.sol";

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

possible to remove or should we consider to keep ?

Copy link
Contributor Author

@Le-Caignec Le-Caignec Oct 30, 2025

Choose a reason for hiding this comment

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

it have been done in this PR. #308
My goal here was simply to patch the functions to make the tests pass.
As discuss Timelock will be removed in a dedicated PR

@gfournieriExec
Copy link
Contributor

Are we sure we want to set
pragma solidity >=0.8.0 instead of pragma solidity ^0.8.0; ?

Copilot AI review requested due to automatic review settings October 30, 2025 14:48
Copy link
Contributor

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

Copilot reviewed 28 out of 28 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.

…to streamline deployment by consolidating migration checks into _mintCreate function. This change enhances backward compatibility and prepares for future updates.
…aset structures, update function signatures, and enhance clarity by removing outdated interfaces. This update includes the addition of new methods for managing categories, datasets, and worker pools, ensuring better organization and maintainability.
@Le-Caignec Le-Caignec requested a review from zguesmi October 31, 2025 14:27
Copilot AI review requested due to automatic review settings November 3, 2025 13:31
Copy link
Contributor

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

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…nd WorkerpoolRegistry to improve readability by directly returning the result of the _mintCreate function, eliminating intermediate variables.
…es, enhancing clarity and maintainability. This update introduces new methods for interacting with PoCo contracts, updates function signatures, and ensures backward compatibility with existing SDK references.
@Le-Caignec Le-Caignec requested a review from zguesmi November 3, 2025 13:46
Copilot AI review requested due to automatic review settings November 3, 2025 15:40
Copy link
Contributor

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

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@zguesmi zguesmi left a comment

Choose a reason for hiding this comment

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

Well done!

@zguesmi
Copy link
Member

zguesmi commented Nov 3, 2025

To not be merged yet!

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.

3 participants