gnosisguild / zodiac-modifier-roles

Smart account toolkit for role-based access control
https://roles.gnosisguild.org
GNU Lesser General Public License v3.0
80 stars 39 forks source link

Feat: Migration to Zodiac Core and Task Improvements in the Roles Mod #287

Closed juliopavila closed 1 month ago

juliopavila commented 2 months ago

Description:

This Pull Request (PR) centers around the integration of the new Zodiac Core package within the entire zodiac-modifier-roles project. The primary goal is to replace the previous Gnosis Zodiac dependencies with the recently established Zodiac Core, thereby improving modularity, code maintainability, and reducing the dependencies across the system.

Key Changes:

  1. Migration to Zodiac Core:

    • All previous imports from @gnosis.pm/zodiac have been replaced with @gnosis-guild/zodiac-core.
    • Updated contracts such as Core.sol, PermissionLoader.sol, and other related files to reflect these changes.
    • Replaced usage of @ethersproject libraries with ethers in the relevant modules.
  2. Upgrade to Ethers v6:

    • The entire roles monorepo has been upgraded to use ethers v6 due to an issue with the outdated reservedKeywords set.
    • The reservedKeywords set was outdated, containing ethers v5 contract property names that no longer exist in v6 (e.g., provider) and missing some v6 property names (e.g., target).
    • Instead of manually updating the reservedKeywords list, the solution now involves directly reading the list of built-in properties from the BaseContract class in ethers v6.
    • Special handling was added for the then property, which is not a property of BaseContract but is kept in a special passProperties list by ethers v6 to ensure it won't be passed through unintentionally.
  3. Task Enhancements:

    • Introduced new Hardhat tasks to support the deployment and verification of mastercopies:
      • deploy:mastercopies: Deploys all mastercopies across networks.
      • deploy:mastercopy --contract-version <version>: Deploys a specific version of a mastercopy.
      • verify:mastercopies: Verifies all mastercopies across networks.
      • verify:mastercopy --contract-version <version>: Verifies a specific version of a mastercopy.
    • Refined the task scripts to improve readability and maintainability.
  4. Configuration Updates:

    • Hardhat configuration has been updated to include setups for the new tasks and dependencies, specifically integrating the latest version of Typechain targeting ethers-v6.
    • Included configurations for new network additions such as Sepolia.
  5. Dependency and Package Upgrades:

    • Upgraded multiple npm dependencies to their latest versions, including Hardhat plugins and TypeScript.
    • Integrated the latest version of @gnosis-guild/zodiac-core, eliminating the older Zodiac dependencies.
  6. Test Suite Refactoring:

    • Modified existing tests to ensure compatibility with the new Zodiac Core structure.
    • Updated tests like Modifier.spec.ts and Integrity.spec.ts to align with the refactored logic and changes introduced in the contracts.

Solution:

The solution involved refactoring the project to utilize the Zodiac Core, which significantly enhances the modularity and reduces dependency issues, making the system more maintainable. Additionally, the upgrade to ethers v6 resolves previous issues with outdated reserved keywords by dynamically reading built-in properties from the BaseContract class. This approach also brings clarity to the codebase, facilitating easier future updates and debugging for developers.

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
zodiac-roles-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 30, 2024 5:50pm
zodiac-roles-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 30, 2024 5:50pm
github-actions[bot] commented 2 months ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

juliopavila commented 2 months ago

I have read the CLA Document and I hereby sign the CLA

github-actions[bot] commented 2 months ago

Pull Request Test Coverage Report for Build 10636221272

Details


Totals Coverage Status
Change from base Build 10495136937: 0.0%
Covered Lines: 665
Relevant Lines: 666

💛 - Coveralls
jfschwarz commented 2 months ago

@samepant you need to run yarn prepare

cristovaoth commented 1 month ago

We still need to include the master copies for v1.0.0. Since the current PR is already large, we could merge it and include the master copies in a follow up PR

jfschwarz commented 1 month ago

We still need to include the master copies for v1.0.0. Since the current PR is already large, we could merge it and include the master copies in a follow up PR

would probably make sense to add the v1 master copies over at https://github.com/gnosisguild/zodiac-modifier-roles-v1. (so that repo should be added to the migration backlog as well)