Closed skosito closed 2 months ago
[!WARNING]
Rate limit exceeded
@skosito has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 50 seconds before requesting another review.
How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the `@coderabbitai review` command as a PR comment. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit.How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our [FAQ](https://coderabbit.ai/docs/faq) for further information.Commits
Files that changed from the base of the PR and between ac14eba0e12c3012a0626b5bd50e580e37a697cd and 264dda67eff7d15dd6077aca1b4c734f7d290afe.
The recent updates encompass several enhancements and refinements across multiple files, mainly focusing on upgrading the Node.js version from 16 to 18 within GitHub workflows, adding Foundry installation steps, refining Solidity contracts and tests, and updating .gitignore
and .eslintignore
files. Additionally, new configurations for Foundry and Echidna, as well as the integration of forge-std
, were added to improve the development and testing environment.
File/Path | Change Summary |
---|---|
.eslintignore |
Added crytic-export to ignored directories. |
.gitignore |
Added crytic-export , out , and cache_forge to ignored directories. |
.gitmodules |
Added a submodule entry for "lib/forge-std". |
.github/workflows/... |
Updated Node.js version to 18, added Foundry installation steps, and introduced new test steps. |
contracts/prototypes/evm/... |
Changed visibility of functions, added interfaces, and improved error and event handling. |
contracts/prototypes/test/... |
Added new test contracts and functions for interaction testing. |
contracts/prototypes/zevm/... |
Implemented new interfaces, added events and errors, and modified contract inheritance. |
contracts/zevm/ZRC20.sol |
Updated imports, adjusted contract inheritance, and added overrides for variables. |
contracts/zevm/ZRC20New.sol |
Similar updates to ZRC20.sol , but specific to the ZRC20New contract. |
contracts/zevm/interfaces/... |
Introduced new interfaces and enums for system and ZRC20 contracts. |
echidna.yaml |
Added configuration for crytic-compile , test limits, and contract inclusion. |
foundry.toml |
Introduced Foundry configuration settings for profiles, directories, and optimizations. |
hardhat.config.ts |
Added an import statement for @nomicfoundation/hardhat-foundry . |
lib/forge-std |
Integrated a subproject commit for forge-std . |
In the meadow of code, updates bloom bright,
Node leapt to eighteen, a soaring new height. 🌿
Foundry's now in, for tests that ignite,
Solidity crafted, contracts just right.
Git's paths are cleaner, ignored in the night,
With each line of code, a future in sight. 💻
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?
Why do we need Foundry as a git submodule? Isn't it something developers can install with cargo
, instead?
Why do we need Foundry as a git submodule? Isn't it something developers can install with
cargo
, instead?
this is import to be used in project, not a tool itself, we still need to install it as tool
this is automatically set up with hardhat-foundry plugin, and also what happens if you run forge init
alternative is to use npm package, but i thought to not do many modifications to how plugin is supposed to work
NOTE: too many generated files, maybe we can consider only generating go and ts bindings for things that are needed? these new forge lib sol files dont need all these generated bindings, but lets clear it up in separate issue (add ignore folders to generate_go script and find a way to ignore contracts in typechain)
Yeah I think we can skip typechain generation for everything
forge-std
relatedIf this is not too complex I suggest to migrate all tests into Solidity for V2 these look much better than with Hardhat
The PR looks good to me right now
thanks for checking, i will handle both tests migration and generate files removal in separate PRs
issues: https://github.com/zeta-chain/protocol-contracts/issues/213 https://github.com/zeta-chain/protocol-contracts/issues/218
introduced hardhat-foundry plugin (https://hardhat.org/hardhat-runner/plugins/nomicfoundation-hardhat-foundry)
bumped node to 18 for new dependencies
fixed CI to include foundry, but also test:prototypes which were missing and new forge tests
migrated 2 tests from hardhat to foundry, for illustration how tests look like in foundry, how cheatcodes are used etc, if direction is ok, separate issue to migrate all v2 tests, should be same as these 2 and fairly straightforward (https://github.com/zeta-chain/protocol-contracts/issues/213)
NOTE: too many generated files, maybe we can consider only generating go and ts bindings for things that are needed? these new forge lib sol files dont need all these generated bindings, but lets clear it up in separate issue (add ignore folders to generate_go script and find a way to ignore contracts in typechain)
Summary by CodeRabbit
New Features
GatewayEVMZEVMTest
to facilitate interactions between EVM and ZEVM components.echidna
andfoundry
to streamline testing and build processes.forge-std
submodule for additional functionality.Bug Fixes
GatewayZEVM
andERC20CustodyNew
contracts.Refactor
Chores
.gitignore
and.eslintignore
to include new directories likecrytic-export
andcache_forge
.zevm
andevm
contract directories.