zeta-chain / protocol-contracts

Protocol contracts implementing the core logic of the protocol, deployed on ZetaChain and on connected chains
MIT License
70 stars 58 forks source link

refactor: use soldeer instead of git submodules #337

Closed skosito closed 2 months ago

skosito commented 3 months ago

Summary by CodeRabbit

coderabbitai[bot] commented 3 months ago

[!IMPORTANT]

Review skipped

Review was skipped due to path filters

Files ignored due to path filters (90) * `v2/pkg/address.sol/address.go` is excluded by `!v2/pkg/**` * `v2/pkg/beaconproxy.sol/beaconproxy.go` is excluded by `!v2/pkg/**` * `v2/pkg/console.sol/console.go` is excluded by `!v2/pkg/**` * `v2/pkg/core.sol/core.go` is excluded by `!v2/pkg/**` * `v2/pkg/defender.sol/defender.go` is excluded by `!v2/pkg/**` * `v2/pkg/defenderdeploy.sol/defenderdeploy.go` is excluded by `!v2/pkg/**` * `v2/pkg/erc1967proxy.sol/erc1967proxy.go` is excluded by `!v2/pkg/**` * `v2/pkg/erc1967utils.sol/erc1967utils.go` is excluded by `!v2/pkg/**` * `v2/pkg/erc20custody.sol/erc20custody.go` is excluded by `!v2/pkg/**` * `v2/pkg/erc20custody.t.sol/erc20custodytest.go` is excluded by `!v2/pkg/**` * `v2/pkg/erc20custodyechidnatest.sol/erc20custodyechidnatest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayevm.sol/gatewayevm.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayevm.t.sol/gatewayevminboundtest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayevm.t.sol/gatewayevmtest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayevmechidnatest.sol/gatewayevmechidnatest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayevmupgrade.t.sol/gatewayevmuupsupgradetest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayevmupgradetest.sol/gatewayevmupgradetest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayevmzevm.t.sol/gatewayevmzevmtest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayzevm.sol/gatewayzevm.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayzevm.t.sol/gatewayzevminboundtest.go` is excluded by `!v2/pkg/**` * `v2/pkg/gatewayzevm.t.sol/gatewayzevmoutboundtest.go` is excluded by `!v2/pkg/**` * `v2/pkg/math.sol/math.go` is excluded by `!v2/pkg/**` * `v2/pkg/mockerc20.sol/mockerc20.go` is excluded by `!v2/pkg/**` * `v2/pkg/mockerc721.sol/mockerc721.go` is excluded by `!v2/pkg/**` * `v2/pkg/proxyadmin.sol/proxyadmin.go` is excluded by `!v2/pkg/**` * `v2/pkg/receiverevm.sol/receiverevm.go` is excluded by `!v2/pkg/**` * `v2/pkg/safeconsole.sol/safeconsole.go` is excluded by `!v2/pkg/**` * `v2/pkg/safeerc20.sol/safeerc20.go` is excluded by `!v2/pkg/**` * `v2/pkg/senderzevm.sol/senderzevm.go` is excluded by `!v2/pkg/**` * `v2/pkg/signedmath.sol/signedmath.go` is excluded by `!v2/pkg/**` * `v2/pkg/src/strings.sol/strings.go` is excluded by `!v2/pkg/**` * `v2/pkg/stderror.sol/stderror.go` is excluded by `!v2/pkg/**` * `v2/pkg/stdjson.sol/stdjson.go` is excluded by `!v2/pkg/**` * `v2/pkg/stdmath.sol/stdmath.go` is excluded by `!v2/pkg/**` * `v2/pkg/stdstorage.sol/stdstorage.go` is excluded by `!v2/pkg/**` * `v2/pkg/stdstorage.sol/stdstoragesafe.go` is excluded by `!v2/pkg/**` * `v2/pkg/stdstyle.sol/stdstyle.go` is excluded by `!v2/pkg/**` * `v2/pkg/stdtoml.sol/stdtoml.go` is excluded by `!v2/pkg/**` * `v2/pkg/storageslot.sol/storageslot.go` is excluded by `!v2/pkg/**` * `v2/pkg/strings.sol/strings.go` is excluded by `!v2/pkg/**` * `v2/pkg/systemcontract.sol/systemcontract.go` is excluded by `!v2/pkg/**` * `v2/pkg/systemcontractmock.sol/systemcontractmock.go` is excluded by `!v2/pkg/**` * `v2/pkg/testerc20.sol/testerc20.go` is excluded by `!v2/pkg/**` * `v2/pkg/testuniversalcontract.sol/testuniversalcontract.go` is excluded by `!v2/pkg/**` * `v2/pkg/transparentupgradeableproxy.sol/transparentupgradeableproxy.go` is excluded by `!v2/pkg/**` * `v2/pkg/upgradeablebeacon.sol/upgradeablebeacon.go` is excluded by `!v2/pkg/**` * `v2/pkg/upgrades.sol/unsafeupgrades.go` is excluded by `!v2/pkg/**` * `v2/pkg/upgrades.sol/upgrades.go` is excluded by `!v2/pkg/**` * `v2/pkg/utils.sol/utils.go` is excluded by `!v2/pkg/**` * `v2/pkg/versions.sol/versions.go` is excluded by `!v2/pkg/**` * `v2/pkg/wzeta.sol/weth9.go` is excluded by `!v2/pkg/**` * `v2/pkg/zeta.non-eth.sol/zetanoneth.go` is excluded by `!v2/pkg/**` * `v2/pkg/zetaconnectornative.sol/zetaconnectornative.go` is excluded by `!v2/pkg/**` * `v2/pkg/zetaconnectornative.t.sol/zetaconnectornativetest.go` is excluded by `!v2/pkg/**` * `v2/pkg/zetaconnectornonnative.sol/zetaconnectornonnative.go` is excluded by `!v2/pkg/**` * `v2/pkg/zetaconnectornonnative.t.sol/zetaconnectornonnativetest.go` is excluded by `!v2/pkg/**` * `v2/pkg/zrc20.sol/zrc20.go` is excluded by `!v2/pkg/**` * `v2/pkg/zrc20.t.sol/zrc20test.go` is excluded by `!v2/pkg/**` * `v2/soldeer.lock` is excluded by `!**/*.lock` * `v2/types/factories/Address__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/BeaconProxy__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ERC1967Proxy__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ERC1967Utils__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ERC20CustodyEchidnaTest__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ERC20Custody__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/GatewayEVMEchidnaTest__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/GatewayEVMUpgradeTest__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/GatewayEVM__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/GatewayZEVM__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/Math__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/MockERC20__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/MockERC721__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ProxyAdmin__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ReceiverEVM__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/SafeERC20__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/SenderZEVM__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/StdError.sol/StdError__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/StdStorage.sol/StdStorageSafe__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/Strings__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/SystemContract.sol/SystemContract__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/SystemContractMock.sol/SystemContractMock__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/TestERC20__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/TestUniversalContract__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/TransparentUpgradeableProxy.sol/TransparentUpgradeableProxy__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/UpgradeableBeacon__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/WZETA.sol/WETH9__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ZRC20.sol/ZRC20__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/Zeta.non-eth.sol/ZetaNonEth__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ZetaConnectorNative__factory.ts` is excluded by `!v2/types/**` * `v2/types/factories/ZetaConnectorNonNative__factory.ts` is excluded by `!v2/types/**`

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough ## Walkthrough The recent updates refine the GitHub Actions workflows by eliminating recursive submodule checkouts and improving dependency management through additional installation commands. These modifications simplify the build process while ensuring that necessary tools are current. Documentation enhancements provide clearer installation instructions and updated links, thereby facilitating a smoother onboarding experience for developers engaging with the new smart contract architecture. ## Changes | Files | Change Summary | |-----------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `.github/workflows/generated-files_v2.yaml`, `.github/workflows/lint_v2.yaml`, `.github/workflows/slither_v2.yaml`, `.github/workflows/test_v2.yaml` | Removed recursive submodule checkout from workflow configurations; enhanced dependency installation with `forge soldeer update` after `yarn install`. | | `.github/workflows/publish-npm_v2.yaml` | Updated dependency installation to include `forge soldeer update` following `yarn install`. | | `.gitignore` | Added `dependencies/` to the ignore list and provided comments on ignored files for better clarity. | | `v2/README.md`, `v2/docs/src/README.md` | Updated documentation to include installation instructions for dependencies using `yarn` and `forge soldeer update`, and modified links for improved accessibility. | | `v2/docs/.../*.md` (multiple files) | Updated Git source links to point to new commits for clarity, without affecting functionality or structure. | | `v2/package.json` | Modified "generate" script to remove automatic documentation generation step, indicating a shift in build process. | | `v2/scripts/generate_go.sh` | Added commands to output versions of `abigen` and `solc` tools within the `process_file` function for enhanced environment verification. | ## Possibly related PRs - #338: The changes in this PR involve the removal of a check in the ZRC20 constructor, which aligns with the main PR's focus on simplifying processes in the GitHub Actions workflow, particularly regarding the handling of submodules and naming conventions.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
codecov-commenter commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.37%. Comparing base (4fed7cb) to head (ac15a64).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #337 +/- ## ======================================= Coverage 97.37% 97.37% ======================================= Files 7 7 Lines 305 305 Branches 98 98 ======================================= Hits 297 297 Misses 8 8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

skosito commented 3 months ago

@fadeev do we need to run yarn docs with yarn generate? unfortunately this breaks the build every time because of changes in md files like:

[Git Source](https://github.com/zeta-chain/protocol-contracts/blob/9c4e8cd193f3cb13413316a8459936d5fae7dd6c/contracts/zevm/interfaces/IGatewayZEVM.sol)

that are updated with every commit .../blob/<COMMIT_ID>/... so CI reports diff and fails the workflow

fadeev commented 3 months ago

@skosito let's remove yarn docs from generate.

skosito commented 2 months ago

@skosito let's remove yarn docs from generate.

hey @fadeev could you please recheck this one? thanks

fadeev commented 2 months ago

@skosito let's remove yarn docs from generate.

hey @fadeev could you please recheck this one? thanks

I've ran yarn generate, this should satisfy the check. It wasn't triggered for some reason. Conflicts with main can be solved with git merge --strategy-option theirs (they are only touching docs).

skosito commented 2 months ago

@skosito let's remove yarn docs from generate.

hey @fadeev could you please recheck this one? thanks

I've ran yarn generate, this should satisfy the check. It wasn't triggered for some reason. Conflicts with main can be solved with git merge --strategy-option theirs (they are only touching docs).

i was more referring to PR as a whole, if you could review it it would be great, to see if this is ok from package manager perspective, npm publishing and so on

i will resolve conflicts

fadeev commented 2 months ago

@skosito I think this is good as long as we use Soldeer only for "development dependencies", so for scripts, tests, etc. (which I believe is what we're doing right now with forge-std and openzeppelin-foundry-upgrades). Everything else we install from npm to keep compatibility with tools relying on npm.

fadeev commented 2 months ago

Should we remove lib/?

skosito commented 2 months ago

Should we remove lib/?

yes initially i removed it in first commit https://github.com/zeta-chain/protocol-contracts/pull/337/commits/cb0d2663add9a3ff8d95df759a2b13ec2b84a83b#diff-397b1d5f636e2fc49d5a7c17abf7506455208f62a7415abd09fb7e5cff7ff22b but it is reverted back probably in merge commit, looking into it