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: improve v2 coverage #294

Closed skosito closed 3 months ago

skosito commented 3 months ago

bumps v2 coverage from ~70% to ~80%, fixes some smaller issues, extends simple checks of external functions a bit

@lumtis question about more tests - the only protocol contract that is not tested is SystemContract, simply because we are not using full functionality, there are no tests for it in v1, and it will be replaced by some sort of registry contract, so no point to add tests for whole contract now

this contract is currently under test/utils so it's not impacting coverage, should we move it under src/zevm and exclude from coverage?

Summary by CodeRabbit

coderabbitai[bot] commented 3 months ago

[!CAUTION]

Review failed

The pull request is closed.

Walkthrough

The recent updates across several smart contracts introduce enhanced functionality and improved error handling. Key changes include the addition of the TSS_ROLE for refined access control, rigorous input validation checks, and new error types for better debugging and user experience. These modifications aim to create a more robust and versatile framework for interactions within the contracts, ultimately boosting their security and operational capabilities.

Changes

Files Change Summary
v2/pkg/erc20custody.sol/... Updated ABI and binary representation for ERC20Custody, modifying function signatures and adding new error types.
v2/pkg/gatewayevm.sol/... Updated ABI for GatewayEVM, reflecting changes in function definitions and state mutability.
v2/pkg/gatewayevmupgradetest.sol/... Modifications to ABI and binary for GatewayEVMUpgradeTest, enhancing functionality with new methods.
v2/pkg/gatewayzevm.sol/... Updated ABI and binary, introducing new functions and events in GatewayZEVM.
v2/pkg/igatewayevm.sol/... Added new error type ConnectorInitialized to IGatewayEVMErrorsMetaData.
v2/pkg/igatewayzevm.sol/... Included new error type InsufficientZetaAmount in IGatewayZEVMErrorsMetaData.
v2/pkg/senderzevm.sol/... Updated Bin field in SenderZEVMMetaData, indicating changes in bytecode.
v2/pkg/zetaconnectorbase.sol/... Introduced TSS_ROLE and new methods for role retrieval in ZetaConnectorBase.
v2/pkg/zetaconnectornative.sol/... Added TSS_ROLE and new methods for accessing this role in ZetaConnectorNative.
v2/pkg/zetaconnectornonnative.sol/... Introduced TSS_ROLE function for role management in ZetaConnectorNonNative.
v2/src/evm/GatewayEVM.sol Implemented zero address checks across multiple functions for improved security.
v2/src/evm/ZetaConnectorBase.sol Added TSS_ROLE constant to enhance access control.
v2/src/evm/ZetaConnectorNonNative.sol Updated setMaxSupply function to require TSS_ROLE for permissions.
v2/src/evm/interfaces/IGatewayEVM.sol Introduced ConnectorInitialized error for detailed error handling.
v2/src/zevm/GatewayZEVM.sol Added input validation checks to various functions to prevent invalid address usage.
v2/src/zevm/interfaces/IGatewayZEVM.sol Included error for insufficient Zeta amounts.
v2/test/ERC20Custody.t.sol New test contract ERC20CustodyTest introduced for validating functionality of ERC20Custody.
v2/test/GatewayEVM.t.sol Updated tests to use specific non-native Zeta token implementation, enhancing coverage.
v2/test/GatewayZEVM.t.sol Enhanced error handling with new error types in tests, focusing on input validation.
v2/test/ZRC20.t.sol Added new test functions to improve coverage of ZRC20 functionalities.
v2/test/ZetaConnectorNative.t.sol Introduced EnforcedPause error and testing for paused contract behavior.
v2/test/ZetaConnectorNonNative.t.sol Added new error types related to access control and supply management.
v2/typechain-types/ZetaConnectorBase.ts Updated interfaces to include TSS_ROLE.
v2/typechain-types/ZetaConnectorNative.ts Included TSS_ROLE in interface definitions.
v2/typechain-types/ZetaConnectorNonNative.ts Added TSS_ROLE to manage new role functionality.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Contract
    participant RoleManagement

    User->>Contract: Call function (e.g., deposit)
    Contract->>RoleManagement: Check for valid role (TSS_ROLE)
    RoleManagement-->>Contract: Role validated
    Contract->>Contract: Execute function logic
    Contract-->>User: Return result

🐇 In the meadow, a change so bright,
New roles and checks take glorious flight.
TSS_ROLE hops in, with a skip and a cheer,
Validations in place, we’ve nothing to fear!
Contracts now flourish, secure and keen,
In our blockchain garden, all is serene! 🌼


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 Configuration 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.
codecov-commenter commented 3 months ago

Codecov Report

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

Project coverage is 82.65%. Comparing base (43176d2) to head (8fcc85b). Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
v2/src/evm/ZetaConnectorBase.sol 0.00% 1 Missing :warning:
v2/src/zevm/GatewayZEVM.sol 95.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #294 +/- ## =========================================== + Coverage 68.97% 82.65% +13.67% =========================================== Files 7 7 Lines 245 271 +26 Branches 61 85 +24 =========================================== + Hits 169 224 +55 + Misses 76 47 -29 ```

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

lumtis commented 3 months ago

this contract is currently under test/utils so it's not impacting coverage, should we move it under src/zevm and exclude from coverage?

We should exclude it from coverage yes