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

test: initialize echidna for fuzz testing #208

Closed skosito closed 4 months ago

skosito commented 4 months ago

to test:

Setup echidna

brew install echidna
solc-select use 0.8.7

Execute contract tests

echidna test/fuzz/ERC20CustodyNewEchidnaTest.sol --contract ERC20CustodyNewEchidnaTest  --config echidna.yaml
echidna test/fuzz/GatewayEVMEchidnaTest.sol --contract GatewayEVMEchidnaTest  --config echidna.yaml

just simple tests for now, and instructions on how to run, once we agree on approach we can open more issues for adding more fuzz tests same way its started here

alternatives: foundry has built in fuzz testing, and bunch of tools for better testing in general (for example cheatcodes: https://book.getfoundry.sh/forge/cheatcodes)

Summary by CodeRabbit

coderabbitai[bot] commented 4 months ago

Walkthrough

This update primarily enhances testing and code management. It introduces .eslintignore and .gitignore updates for ignoring the crytic-export directory, changes function visibility in ERC20CustodyNew and GatewayEVM contracts, adds a new error type, and includes reinitialization checks. Additionally, it provides configurations for Echidna testing and integrates new test files for fuzz testing mechanisms.

Changes

Files/Paths Change Summary
.eslintignore, .gitignore Added crytic-export to ignore list, preserving other existing entries.
contracts/.../ERC20CustodyNew.sol Changed withdrawAndCall function visibility from external to public.
contracts/.../GatewayEVM.sol Added new error type CustodyInitialized and added reinitialization check in setCustody function.
echidna.yaml Introduced new file with configurations for crytic-compile.
test/fuzz/ERC20CustodyNewEchidnaTest.sol New file to extend ERC20CustodyNew for testing withdrawal and calling functionality.
test/fuzz/GatewayEVMEchidnaTest.sol New file to extend GatewayEVM for testing token execution with balance assertions.
test/fuzz/readme.md Added setup instructions for Echidna, including installation, Solidity version configuration, and running tests.

Sequence Diagram(s)

sequenceDiagram
    participant Developer
    participant ERC20CustodyNew
    participant GatewayEVM
    Note right of Developer: Setup Echidna Testing
    Developer->>ERC20CustodyNew: Call withdrawAndCall as public
    Developer->>GatewayEVM: Deploy and setup initial custody
    GatewayEVM-->Developer: Error if custody already initialized
    Developer->>Echidna: Configure and run tests
    Echidna->>ERC20CustodyNew&GatewayEVM: Test fuzz cases

Poem

In fields of code where changes flow,
Functions now more public show,
Echidna tests run bright and fast,
Ensuring our contracts last.
Ignoring files both here and there,
Protects our repo, light as air.
With checks and balances now in place,
Our smart contracts bask in grace.
🐇✨


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 as 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. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### 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.
skosito commented 4 months ago

@lumtis @fadeev @fbac could you please check this draft and let me know your thoughts, especially on points in PR description? thanks

skosito commented 4 months ago

@coderabbitai full review

coderabbitai[bot] commented 4 months ago
Actions performed Full review triggered.
codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 48.09%. Comparing base (b0fbc16) to head (4ec838c).

Files Patch % Lines
contracts/prototypes/evm/ERC20CustodyNew.sol 0.00% 1 Missing :warning:
contracts/prototypes/evm/GatewayEVM.sol 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #208 +/- ## =========================================== - Coverage 59.25% 48.09% -11.17% =========================================== Files 31 31 Lines 1048 1048 Branches 262 263 +1 =========================================== - Hits 621 504 -117 - Misses 427 544 +117 ```

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

fadeev commented 4 months ago

I think using Foundry more is a good idea. What I still like about Hardhat is the ability to import and use TypeScript tools, we're using them for everything in tutorials: balances, faucet, transaction tracking, code generation. But I think Hardhat-Foundry plugin is a good middle-ground.

fbac commented 4 months ago

I don't have enough experience with Echidna yet to make a proper statement. Regarding hardhat vs. foundry I'd say I prefer foundry because it allows native solidity testing, fuzzing and in general better it's a better testing suite than hh is. On the other hand, I get that it can be hard to fully migrate.

I agree with the proposed hardhat-foundry usage, it's a good middle ground and the full migration can happen if everyone agrees about foundry being the best solution.

Also, I'll be studying echidna so I can have informed opinions on it.

skosito commented 4 months ago

Looks good to me.

We don't need to be complete in this PR, just write the base.

foundry instead of hardhat: too much effort atm to migrate

What concrete work to migrate outside of rewriting the test and deployment scripts? Also myaybe we can have both supported atm?

i think tests and scripts, probably not that much effort if we only do it for v2 i will convert this PR to ready for review and try hadhat-foundry plugin in next PR (https://github.com/zeta-chain/protocol-contracts/issues/210), so PRs are smaller and more focused, because even if we move to foundry having echidna setup might still be beneficial

lumtis commented 4 months ago

i think tests and scripts, probably not that much effort if we only do it for v2

We could also consider create entire new repo for v2 as the env is just completely different

skosito commented 4 months ago

@skosito can we add instructions in the PR how it can be tested?

yes, i added a readme in tests/fuzz/readme.md, but also copied to PR description

skosito commented 4 months ago

Tested the commands, looking good to me

thank you, @fbac @fadeev could you please review as well, need 1 more review to close this one?