Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: improve v2 coverage #294

Merged
merged 18 commits into from
Aug 9, 2024
Merged

test: improve v2 coverage #294

merged 18 commits into from
Aug 9, 2024

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Aug 7, 2024

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

  • New Features

    • Introduced a new role, TSS_ROLE, for improved access control across multiple contracts.
    • Enhanced error handling with new error types like ConnectorInitialized and InsufficientZetaAmount.
    • Updated ABI and functionality for various contracts, including ERC20Custody, GatewayEVM, GatewayZEVM, and ZetaConnectorBase.
    • Implemented input validation checks for critical functions across multiple contracts to prevent invalid address usage.
  • Bug Fixes

    • Added checks to prevent zero addresses in critical transactional functions, enhancing security against invalid inputs.
  • Tests

    • Introduced comprehensive testing contracts for ERC20Custody, GatewayEVM, and GatewayZEVM to validate functionalities and edge cases.

Copy link
Contributor

coderabbitai bot commented Aug 7, 2024

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
Loading

🐇 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
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • 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 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 for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2024

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 ⚠️
v2/src/zevm/GatewayZEVM.sol 95.00% 1 Missing ⚠️
Additional details and impacted files
@@             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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@skosito skosito linked an issue Aug 7, 2024 that may be closed by this pull request
@skosito skosito marked this pull request as ready for review August 7, 2024 22:28
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (2)
v2/pkg/erc20custody.sol/erc20custody.go (2)

Line range hint 1-3:
Ensure adherence to Go best practices.

The generated code should adhere to Go best practices, including proper error handling and naming conventions. Review the code for any potential improvements or refactoring opportunities.

Consider reviewing the generated code for adherence to Go best practices, such as error handling and naming conventions. While the code is generated, ensuring best practices can improve maintainability and readability.


Minimize usage of deprecated variables.

The deprecated variables ERC20CustodyABI and ERC20CustodyBin are still being used in the v2/pkg/erc20custody.sol/erc20custody.go file. Consider replacing them with ERC20CustodyMetaData.ABI and ERC20CustodyMetaData.Bin to adhere to the deprecation notice.

  • File: v2/pkg/erc20custody.sol/erc20custody.go
    • Usage of ERC20CustodyBin in contract deployment.
Analysis chain

Line range hint 39-43:
Minimize usage of deprecated variables.

The variables ERC20CustodyABI and ERC20CustodyBin are marked as deprecated. Ensure that their usage is minimized and replaced by ERC20CustodyMetaData.ABI and ERC20CustodyMetaData.Bin wherever possible.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of deprecated variables.

# Test: Check for occurrences of deprecated variables in the codebase.
# Expect: Minimal usage or replacement with recommended alternatives.

rg --type go 'ERC20CustodyABI|ERC20CustodyBin'

Length of output: 1314

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between eff74bf and c698398.

Files selected for processing (36)
  • v2/pkg/erc20custody.sol/erc20custody.go (1 hunks)
  • v2/pkg/gatewayevm.sol/gatewayevm.go (1 hunks)
  • v2/pkg/gatewayevmupgradetest.sol/gatewayevmupgradetest.go (1 hunks)
  • v2/pkg/gatewayzevm.sol/gatewayzevm.go (1 hunks)
  • v2/pkg/igatewayevm.sol/igatewayevmerrors.go (1 hunks)
  • v2/pkg/igatewayzevm.sol/igatewayzevmerrors.go (1 hunks)
  • v2/pkg/senderzevm.sol/senderzevm.go (1 hunks)
  • v2/pkg/zetaconnectorbase.sol/zetaconnectorbase.go (2 hunks)
  • v2/pkg/zetaconnectornative.sol/zetaconnectornative.go (2 hunks)
  • v2/pkg/zetaconnectornonnative.sol/zetaconnectornonnative.go (2 hunks)
  • v2/src/evm/GatewayEVM.sol (9 hunks)
  • v2/src/evm/ZetaConnectorBase.sol (2 hunks)
  • v2/src/evm/ZetaConnectorNonNative.sol (5 hunks)
  • v2/src/evm/interfaces/IGatewayEVM.sol (1 hunks)
  • v2/src/zevm/GatewayZEVM.sol (11 hunks)
  • v2/src/zevm/interfaces/IGatewayZEVM.sol (1 hunks)
  • v2/test/ERC20Custody.t.sol (1 hunks)
  • v2/test/GatewayEVM.t.sol (17 hunks)
  • v2/test/GatewayZEVM.t.sol (13 hunks)
  • v2/test/ZRC20.t.sol (7 hunks)
  • v2/test/ZetaConnectorNative.t.sol (2 hunks)
  • v2/test/ZetaConnectorNonNative.t.sol (3 hunks)
  • v2/typechain-types/ZetaConnectorBase.ts (5 hunks)
  • v2/typechain-types/ZetaConnectorNative.ts (5 hunks)
  • v2/typechain-types/ZetaConnectorNonNative.ts (5 hunks)
  • v2/typechain-types/factories/ERC20Custody__factory.ts (1 hunks)
  • v2/typechain-types/factories/GatewayEVMEchidnaTest__factory.ts (2 hunks)
  • v2/typechain-types/factories/GatewayEVMUpgradeTest__factory.ts (2 hunks)
  • v2/typechain-types/factories/GatewayEVM__factory.ts (2 hunks)
  • v2/typechain-types/factories/GatewayZEVM__factory.ts (2 hunks)
  • v2/typechain-types/factories/IGatewayEVM.sol/IGatewayEVMErrors__factory.ts (1 hunks)
  • v2/typechain-types/factories/IGatewayZEVM.sol/IGatewayZEVMErrors__factory.ts (1 hunks)
  • v2/typechain-types/factories/SenderZEVM__factory.ts (1 hunks)
  • v2/typechain-types/factories/ZetaConnectorBase__factory.ts (1 hunks)
  • v2/typechain-types/factories/ZetaConnectorNative__factory.ts (2 hunks)
  • v2/typechain-types/factories/ZetaConnectorNonNative__factory.ts (2 hunks)
Additional comments not posted (156)
v2/typechain-types/factories/IGatewayEVM.sol/IGatewayEVMErrors__factory.ts (1)

17-21: Addition of ConnectorInitialized error type approved.

The new error type ConnectorInitialized has been added to the ABI, enhancing error handling. This change is consistent with the existing error definitions.

v2/typechain-types/factories/IGatewayZEVM.sol/IGatewayZEVMErrors__factory.ts (1)

32-36: Addition of InsufficientZetaAmount error type approved.

The new error type InsufficientZetaAmount enhances the error handling by providing more specific error reporting. This addition is consistent with the existing error definitions.

v2/src/evm/ZetaConnectorBase.sol (1)

31-32: Addition of TSS_ROLE and its assignment in the constructor approved.

The new role identifier TSS_ROLE enhances the role management functionality. The constructor correctly assigns this role to tssAddress_, ensuring proper role-based access control.

Ensure that tests cover the role assignment logic to verify correct behavior.

Also applies to: 45-45

Verification successful

Verification of TSS_ROLE Assignment in Constructor

The constructor in ZetaConnectorBase.sol correctly assigns the TSS_ROLE to tssAddress_, ensuring proper role-based access control as intended. The TSS_ROLE is defined and used appropriately in the file.

  • Constructor Role Assignment: The constructor in v2/src/evm/ZetaConnectorBase.sol includes _grantRole(TSS_ROLE, tssAddress_);, confirming the role assignment logic is implemented.
  • TSS_ROLE Definition: The TSS_ROLE is defined as bytes32 public constant TSS_ROLE = keccak256("TSS_ROLE"); in the same file.

The role assignment logic appears to be implemented correctly. Ensure that tests cover this logic to verify correct behavior.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the role assignment logic in the constructor.

# Test: Search for constructor usage and role assignment. Expect: Correct assignment of TSS_ROLE.
rg --type solidity -A 10 'constructor(address gateway_, address zetaToken_, address tssAddress_, address admin_)'

Length of output: 148


Script:

#!/bin/bash
# Use ast-grep to find the constructor in the Solidity file and check for TSS_ROLE assignment
ast-grep --lang solidity --pattern $'constructor(address gateway_, address zetaToken_, address tssAddress_, address admin_) { $$$ }'

Length of output: 250


Script:

#!/bin/bash
# Search for the assignment of TSS_ROLE in the Solidity file
ast-grep --lang solidity --pattern $'TSS_ROLE = keccak256("TSS_ROLE");'

Length of output: 189


Script:

#!/bin/bash
# Use grep to find the constructor in the Solidity file
grep -A 10 'constructor(address gateway_, address zetaToken_, address tssAddress_, address admin_)' $(fd --type f --extension sol)

Length of output: 4193


Script:

#!/bin/bash
# Use grep to search for the assignment of TSS_ROLE in the Solidity file
grep 'TSS_ROLE = keccak256("TSS_ROLE");' $(fd --type f --extension sol)

Length of output: 3819

v2/src/evm/ZetaConnectorNonNative.sol (2)

118-121: LGTM! The _mintTo function improves modularity.

The refactoring centralizes the minting logic, enhancing code modularity and maintainability.


32-32: Verify the appropriateness of TSS_ROLE for setMaxSupply.

The role requirement has been changed from WITHDRAWER_ROLE to TSS_ROLE. Ensure that TSS_ROLE is the correct role for managing the maximum supply.

Verification successful

TSS_ROLE is appropriate for setMaxSupply.

The role TSS_ROLE is consistently used for access control across multiple contracts, indicating its suitability for managing permissions related to setMaxSupply. This aligns with its intended purpose in the codebase.

  • v2/src/evm/ZetaConnectorBase.sol: Definition of TSS_ROLE.
  • v2/src/evm/ZetaConnectorNonNative.sol: Usage in setMaxSupply.
  • v2/src/evm/GatewayEVM.sol: Similar access control usage.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the appropriateness of `TSS_ROLE` for `setMaxSupply`.

# Test: Search for the definition and usage of `TSS_ROLE` to ensure it's suitable for managing max supply.
rg --type solidity 'TSS_ROLE'

Length of output: 62


Script:

#!/bin/bash
# Description: Verify the appropriateness of `TSS_ROLE` for `setMaxSupply`.

# Corrected Test: Search for the definition and usage of `TSS_ROLE` in Solidity files.
rg --type-add 'sol:.sol' --type sol 'TSS_ROLE'

Length of output: 193


Script:

#!/bin/bash
# Description: List Solidity files and search for `TSS_ROLE`.

# List Solidity files to ensure they are present.
fd --extension sol

# Search for the definition and usage of `TSS_ROLE` in Solidity files.
rg 'TSS_ROLE' --glob '*.sol'

Length of output: 19610

v2/src/evm/interfaces/IGatewayEVM.sol (1)

73-73: Verify the usage context of ConnectorInitialized.

The new error ConnectorInitialized enhances error handling. Ensure it is appropriately used in the contract logic.

Verification successful

ConnectorInitialized is used correctly in the contract logic.

The ConnectorInitialized error is defined in IGatewayEVM.sol and is used in GatewayEVM.sol to revert the transaction if zetaConnector is already set. Additionally, the error is tested in GatewayEVM.t.sol, ensuring that the contract behaves as expected when this error condition is met.

  • Usage in Contract Logic: v2/src/evm/GatewayEVM.sol
  • Test Coverage: v2/test/GatewayEVM.t.sol
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage context of `ConnectorInitialized`.

# Test: Search for occurrences of `ConnectorInitialized` to ensure it's used appropriately.
rg --type solidity 'ConnectorInitialized'

Length of output: 74


Script:

#!/bin/bash
# Description: Search for occurrences of `ConnectorInitialized` in Solidity files.

# Test: Use `rg` to find occurrences of `ConnectorInitialized` in all files.
rg 'ConnectorInitialized' --glob '*.sol' -A 5

Length of output: 1095

v2/src/zevm/interfaces/IGatewayZEVM.sol (1)

160-160: Verify the usage context of InsufficientZetaAmount.

The new error InsufficientZetaAmount enhances error handling. Ensure it is appropriately used in the contract logic.

Verification successful

Usage of InsufficientZetaAmount is Verified

The InsufficientZetaAmount error is used in the GatewayZEVM.sol contract to handle cases where the amount is zero, ensuring that the contract logic appropriately reverts when the condition is met. Additionally, it is tested in GatewayZEVM.t.sol, confirming its integration into the contract's error handling mechanisms.

  • Code Locations:
    • v2/src/zevm/GatewayZEVM.sol: Lines where InsufficientZetaAmount is used to revert transactions.
    • v2/test/GatewayZEVM.t.sol: Lines where InsufficientZetaAmount is tested.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage context of `InsufficientZetaAmount`.

# Test: Search for occurrences of `InsufficientZetaAmount` to ensure it's used appropriately.
rg --type solidity 'InsufficientZetaAmount'

Length of output: 76


Script:

#!/bin/bash
# Description: Verify the usage context of `InsufficientZetaAmount`.

# Test: Search for occurrences of `InsufficientZetaAmount` in the codebase.
rg 'InsufficientZetaAmount'

Length of output: 60977

v2/typechain-types/factories/SenderZEVM__factory.ts (1)

123-123: Verify the bytecode update.

The _bytecode variable has been updated. Ensure that this change is intentional and that any dependent systems or tests are updated to reflect this change.

v2/pkg/igatewayevm.sol/igatewayevmerrors.go (1)

34-34: Ensure handling of the new "ConnectorInitialized" error.

The ABI now includes the "ConnectorInitialized" error. Verify that this error is properly handled in the contract logic and tests.

Verification successful

Handling of "ConnectorInitialized" Error Verified

The "ConnectorInitialized" error is properly handled in both the contract logic and the test suite. The error is used in GatewayEVM.sol and is expected in tests such as GatewayEVM.t.sol. No further action is required.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the handling of the "ConnectorInitialized" error in the codebase.

# Test: Search for "ConnectorInitialized" usage in the codebase. Expect: Proper handling in contract logic and tests.
rg 'ConnectorInitialized'

Length of output: 153897

v2/pkg/igatewayzevm.sol/igatewayzevmerrors.go (1)

34-34: Ensure handling of the new "InsufficientZetaAmount" error.

The ABI now includes the "InsufficientZetaAmount" error. Verify that this error is properly handled in the contract logic and tests.

Verification successful

Handling of "InsufficientZetaAmount" Error Verified

The "InsufficientZetaAmount" error is properly handled in the contract logic and is tested in the codebase.

  • Contract Logic: The error is triggered in GatewayZEVM.sol when the amount is zero.
  • Tests: The error is expected and tested in GatewayZEVM.t.sol.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the handling of the "InsufficientZetaAmount" error in the codebase.

# Test: Search for "InsufficientZetaAmount" usage in the codebase. Expect: Proper handling in contract logic and tests.
rg 'InsufficientZetaAmount'

Length of output: 60977

v2/typechain-types/factories/ZetaConnectorBase__factory.ts (1)

38-50: Addition of TSS_ROLE is consistent and appropriate.

The addition of the TSS_ROLE function to the ABI is consistent with other role declarations and correctly defined as a view function. This enhances the role-based access control mechanism.

v2/test/ZRC20.t.sol (9)

66-75: Comprehensive transfer test coverage.

The testTransfer function effectively verifies the transfer functionality by checking the recipient's balance before and after the transfer.


77-85: Robust testing for transfer failure due to insufficient balance.

The testTransferFailsIfNoBalance function appropriately tests the failure scenario when the sender's balance is insufficient.


87-92: Validation of zero address recipient in transfer.

The testTransferFailsIfRecipientIsZeroAddress function correctly checks for reverts when attempting to transfer to a zero address.


158-161: Approval rejection for zero address recipient.

The testApproveFailsIfRecipientIsZeroAddress function ensures that approving a zero address results in a revert, which is a critical security check.


179-183: Deposit failure test for zero address recipient.

The testDepositFailsIfRecipientIsZeroAddress function effectively tests the scenario where a deposit to a zero address should fail.


245-259: Withdrawal failure due to insufficient balance.

The testWithdrawFailsIfNoBalance function accurately tests the failure condition when withdrawing with insufficient balance.


261-275: Withdrawal failure due to no allowance.

The testWithdrawFailsIfNoAllowance function correctly tests the scenario where a withdrawal fails due to lack of allowance.


288-292: System contract address update failure for zero address.

The testUpdateSystemContractAddressFailsIfZeroAddress function ensures that updating the system contract address to a zero address fails, which is a necessary validation.


305-309: Gateway address update failure for zero address.

The testUpdateGatewayAddressFailsIfZeroAddress function appropriately tests the failure scenario for updating the gateway address to a zero address.

v2/src/zevm/GatewayZEVM.sol (12)

55-55: Initialization input validation is robust.

The added checks for zero addresses in the initialize function enhance the contract's robustness by ensuring valid inputs during initialization.


121-123: Receiver and amount validation in withdrawal.

The withdraw function now includes essential checks for the receiver's length and amount, preventing invalid operations.


143-145: Enhanced validation in withdrawAndCall.

The withdrawAndCall function now correctly validates the receiver and amount, improving security.


153-154: Amount validation in ZETA withdrawal.

The withdraw function includes a necessary check for non-zero amounts, ensuring valid withdrawals.


174-175: Amount validation in withdrawAndCall.

The withdrawAndCall function now includes a check for non-zero amounts, enhancing reliability.


186-187: Receiver validation in call function.

The call function now includes a check for the receiver's length, preventing invalid calls.


196-198: Input validation in deposit function.

The deposit function now includes checks for zero addresses and non-zero amounts, improving security.


221-223: Input validation in execute function.

The execute function now includes checks for zero addresses and non-zero amounts, enhancing robustness.


244-245: Input validation in depositAndCall.

The depositAndCall function now includes checks for zero addresses and non-zero amounts, improving reliability.


267-268: Target validation in ZETA deposit and call.

The depositAndCall function now includes a check for a zero target address, preventing invalid operations.


292-294: Input validation in executeRevert.

The executeRevert function now includes checks for zero addresses and non-zero amounts, enhancing security.


315-316: Input validation in depositAndRevert.

The depositAndRevert function now includes checks for zero addresses and non-zero amounts, improving robustness.

v2/test/ZetaConnectorNative.t.sol (3)

43-43: Introduction of EnforcedPause Error is Appropriate.

The EnforcedPause error effectively handles scenarios where operations are attempted while the contract is paused. This enhances security by preventing unauthorized actions during a paused state.


47-47: Introduction of PAUSER_ROLE Constant is Appropriate.

The addition of the PAUSER_ROLE constant aligns with access control best practices, allowing designated roles to manage the contract's pause state effectively.


93-126: Comprehensive Testing of Pause Functionality.

The testWithdrawTogglePause function effectively tests the pause and unpause functionality, ensuring only authorized accounts can perform these actions and that withdrawals are blocked when paused.

Consider verifying its integration with the rest of the test suite to ensure comprehensive coverage.

v2/src/evm/GatewayEVM.sol (7)

133-133: Zero Address Check for destination Enhances Security.

The addition of a zero address check for the destination parameter prevents invalid interactions, thereby enhancing the security of the execute function.


160-160: Zero Address Check for to Parameter is Necessary.

The zero address check for the to parameter is a necessary validation step to prevent invalid token transfers, enhancing the function's robustness.


197-197: Zero Address Check for to Parameter Enhances Security.

The addition of a zero address check for the to parameter enhances security by preventing invalid interactions in the revertWithERC20 function.


209-209: Zero Address Check for receiver Parameter is Crucial.

The zero address check for the receiver parameter is crucial for ensuring valid deposit operations, enhancing the function's reliability.


236-236: Zero Address Check for receiver Parameter is Essential.

The zero address check for the receiver parameter is essential for preventing invalid interactions during deposit and call operations, thereby enhancing security.


279-279: Reordering Checks in setCustody Streamlines Control Flow.

Reordering the checks to place the zero address check before the custody initialization check ensures that invalid addresses are caught early, streamlining the control flow.


289-289: Reordering Checks in setConnector Improves Robustness.

Reordering the checks to place the zero address check before the connector initialization check ensures early detection of invalid addresses, improving the function's robustness.

v2/test/ZetaConnectorNonNative.t.sol (9)

44-44: Introduction of ExceedsMaxSupply Error is Appropriate.

The ExceedsMaxSupply error effectively handles scenarios where attempts are made to exceed the maximum supply, enforcing supply limits and ensuring contract constraints are respected.


45-45: Introduction of EnforcedPause Error is Appropriate.

The EnforcedPause error effectively handles scenarios where operations are attempted while the contract is paused, enhancing security by preventing unauthorized actions during a paused state.


48-48: Introduction of TSS_ROLE Constant is Appropriate.

The addition of the TSS_ROLE constant aligns with access control best practices, allowing designated roles to manage TSS-related operations effectively.


49-49: Introduction of PAUSER_ROLE Constant is Appropriate.

The addition of the PAUSER_ROLE constant aligns with access control best practices, allowing designated roles to manage the contract's pause state effectively.


311-320: Testing Exceeding Max Supply is Well-Structured.

The testWithdrawAndFailsIfMaxSupplyIsReached function is well-structured, ensuring that attempts to exceed the maximum supply trigger the ExceedsMaxSupply error.


322-332: Testing Exceeding Max Supply in Withdraw and Call is Well-Structured.

The testWithdrawAndCallFailsIfMaxSupplyIsReached function is well-structured, ensuring that attempts to exceed the maximum supply trigger the ExceedsMaxSupply error.


334-344: Testing Exceeding Max Supply in Withdraw and Revert is Well-Structured.

The testWithdrawAndRevertFailsIfMaxSupplyIsReached function is well-structured, ensuring that attempts to exceed the maximum supply trigger the ExceedsMaxSupply error.


346-350: Testing Unauthorized Max Supply Setting is Well-Structured.

The testSexMaxSupplyFailsIfSenderIsNotTss function is well-structured, ensuring that unauthorized attempts to set the maximum supply trigger the AccessControlUnauthorizedAccount error.


96-130: Comprehensive Testing of Pause Functionality.

The testWithdrawTogglePause function effectively tests the pause and unpause functionality, ensuring only authorized accounts can perform these actions and that withdrawals are blocked when paused.

Consider verifying its integration with the rest of the test suite to ensure comprehensive coverage.

Verification successful

Function testWithdrawTogglePause is Present in the Test File

The testWithdrawTogglePause function is correctly defined in the v2/test/ZetaConnectorNonNative.t.sol file. This confirms its presence in the test suite.

  • File: v2/test/ZetaConnectorNonNative.t.sol
  • Function: testWithdrawTogglePause
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of `testWithdrawTogglePause` with the rest of the test suite.

# Test: Run the test suite to ensure `testWithdrawTogglePause` is included and passes.
forge test --match-path v2/test/ZetaConnectorNonNative.t.sol

Length of output: 135


Script:

#!/bin/bash
# Verify the presence and definition of the `testWithdrawTogglePause` function in the test file.

ast-grep --lang solidity --pattern $'function testWithdrawTogglePause() public { $$$ }' v2/test/ZetaConnectorNonNative.t.sol

Length of output: 242


Script:

#!/bin/bash
# Use rg to search for the definition of the `testWithdrawTogglePause` function in the test file.

rg 'function testWithdrawTogglePause\(\) public' v2/test/ZetaConnectorNonNative.t.sol

Length of output: 135

v2/test/GatewayEVM.t.sol (17)

33-33: LGTM: Updated zeta to ZetaNonEth.

The change from TestERC20 to ZetaNonEth is appropriate for testing non-native token functionalities.


78-83: LGTM: Access control test for custody.

The test correctly checks that only an admin can set custody.


86-89: LGTM: Custody initialization test.

The test correctly ensures that custody cannot be set again once initialized.


92-95: LGTM: Zero address check for custody.

The test correctly ensures that custody cannot be set to a zero address.


98-103: LGTM: Access control test for connector.

The test correctly checks that only an admin can set the connector.


106-109: LGTM: Connector initialization test.

The test correctly ensures that the connector cannot be set again once initialized.


112-115: LGTM: Zero address check for connector.

The test correctly ensures that the connector cannot be set to a zero address.


181-186: LGTM: Zero address execution test.

The test correctly ensures that execution cannot proceed if the destination is a zero address.


Line range hint 219-224: LGTM: Role-based access control for ERC20 execution.

The test correctly ensures that only authorized roles can execute with ERC20.


Line range hint 229-234: LGTM: Role-based access control for ERC20 revert.

The test correctly ensures that only authorized roles can revert with ERC20.


267-273: LGTM: Zero address revert test.

The test correctly ensures that execution revert cannot proceed if the receiver is a zero address.


285-285: LGTM: Updated zeta to ZetaNonEth.

The change from TestERC20 to ZetaNonEth is appropriate for testing non-native token functionalities.


338-347: LGTM: Deposit Zeta to connector test.

The test correctly verifies the deposit of Zeta tokens to the connector.


359-362: LGTM: Zero address deposit test for ERC20.

The test correctly ensures that depositing to a zero address fails.


384-388: LGTM: Zero address deposit test for ETH.

The test correctly ensures that depositing ETH to a zero address fails.


420-426: LGTM: Zero address deposit with payload test for ERC20.

The test correctly ensures that depositing with payload to a zero address fails.


450-455: LGTM: Zero address deposit with payload test for ETH.

The test correctly ensures that depositing with payload to a zero address fails.

v2/pkg/senderzevm.sol/senderzevm.go (1)

35-35: Verify the impact of the bytecode change.

The Bin field in SenderZEVMMetaData has been updated, indicating a change in the contract's bytecode. Ensure that this change aligns with the intended functionality and does not introduce any issues.

Verification successful

No impact on current functionality due to bytecode change

The change in the SenderZEVMMetaData.Bin field does not affect other parts of the codebase, as it is only referenced in a deprecated variable. Ensure that any intended changes in functionality due to the bytecode update are documented or tested separately.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the impact of the bytecode change in SenderZEVMMetaData.

# Test: Search for references to SenderZEVMMetaData in the codebase.
# Expect: Ensure that the changes align with the intended functionality.
rg --type go 'SenderZEVMMetaData'

Length of output: 719


Script:

#!/bin/bash
# Description: Search for usage of SenderZEVMMetaData.Bin in the codebase to verify the impact of the bytecode change.

# Test: Search for references to SenderZEVMMetaData.Bin in the codebase.
rg --type go 'SenderZEVMMetaData\.Bin'

Length of output: 206

v2/typechain-types/ZetaConnectorBase.ts (1)

31-31: Verify integration of TSS_ROLE.

The addition of TSS_ROLE enhances role management capabilities. Ensure that the integration is consistent and correctly utilized in the contract's logic.

Also applies to: 70-70, 129-129, 347-347, 437-439

Verification successful

Integration of TSS_ROLE is consistent and correct.

The TSS_ROLE is integrated across various parts of the codebase, including role management functions such as getRoleAdmin, grantRole, and hasRole. It is consistently used in both contract logic and associated factory files, indicating proper integration. No issues were found in its usage.

  • Files where TSS_ROLE is integrated:
    • v2/typechain-types/ZetaConnectorBase.ts
    • v2/typechain-types/ZetaConnectorNative.ts
    • v2/typechain-types/ZetaConnectorNonNative.ts
    • Various factory and test files
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration and usage of TSS_ROLE in the ZetaConnectorBase interfaces.

# Test: Search for references to TSS_ROLE in the TypeScript codebase.
# Expect: Ensure that TSS_ROLE is correctly integrated and utilized.
rg --type ts 'TSS_ROLE'

Length of output: 3347


Script:

#!/bin/bash
# Description: Search for the usage of TSS_ROLE in the logic of the contracts to verify its integration.

# Test: Search for TSS_ROLE usage in contract logic.
# Expect: Ensure that TSS_ROLE is correctly integrated and utilized in contract logic.
rg --type ts 'TSS_ROLE' -A 5

Length of output: 16528

v2/typechain-types/ZetaConnectorNative.ts (5)

31-31: Addition of TSS_ROLE to getFunction.

The inclusion of TSS_ROLE in the getFunction method enhances the role management capabilities by allowing dynamic retrieval of function fragments associated with this role. This change is consistent with the introduction of the new role.


70-70: Addition of encodeFunctionData for TSS_ROLE.

The encodeFunctionData method for TSS_ROLE is correctly added, enabling the encoding of function calls related to this role. This is a necessary addition for handling the new role.


129-129: Addition of decodeFunctionResult for TSS_ROLE.

The decodeFunctionResult method for TSS_ROLE is properly added, facilitating the decoding of results from function calls associated with this role. This ensures correct interpretation of responses involving TSS_ROLE.


347-347: Exposure of TSS_ROLE as a TypedContractMethod.

The addition of TSS_ROLE as a TypedContractMethod in the ZetaConnectorNative interface allows for structured access to this role. This enhances the contract's interaction capabilities.


437-439: Addition of getFunction for TSS_ROLE.

The getFunction method for TSS_ROLE is correctly added, enabling dynamic retrieval of the function associated with this role. This is consistent with the role's integration into the contract.

v2/test/ERC20Custody.t.sol (18)

1-44: Comprehensive Test Setup and Imports.

The test setup includes necessary imports and constants for role management. The use of forge-std for testing utilities is appropriate. The constants for roles are well-defined, ensuring clarity in access control tests.


45-72: Effective setUp Method Implementation.

The setUp method effectively initializes the test environment, deploying necessary contracts and setting initial conditions. The use of vm.deal and vm.startPrank is correct for setting up account balances and simulating transactions.


74-85: Test for Zero Address Handling.

The test testNewCustodyFailsIfAddressesAreZero appropriately checks for zero address inputs, ensuring that the contract reverts as expected. This is a crucial test for input validation.


87-119: Test for Token Transfer and Event Emission.

The test testForwardCallToReceiveERC20ThroughCustody verifies token transfer logic and event emission. The use of vm.expectEmit ensures that events are emitted as expected, providing robust test coverage.


121-172: Test for Pause Functionality and Reversion.

The test testForwardCallToReceiveERC20ThroughCustodyTogglePause effectively checks the pause and unpause functionality, ensuring that operations revert when paused. This is crucial for maintaining contract state integrity.


174-182: Test for Unauthorized Access.

The test testForwardCallToReceiveERC20ThroughCustodyFailsIfSenderIsNotWithdrawer correctly verifies that only authorized accounts can perform certain actions, reinforcing role-based access control.


184-192: Test for Zero Amount Handling.

The test testForwardCallToReceiveERC20ThroughCustodyFailsIfAmountIs0 ensures that the contract reverts when attempting to transfer zero tokens, which is important for preventing unnecessary operations.


194-202: Test for Zero Address Receiver.

The test testForwardCallToReceiveERC20ThroughCustodyFailsIfReceiverIsZeroAddress checks for zero address handling in the receiver parameter, ensuring robust input validation.


204-236: Test for Partial Token Transfer.

The test testForwardCallToReceiveERC20PartialThroughCustody verifies the logic for partial token transfers, ensuring correct handling of token amounts and event emissions.


238-246: Test for Unauthorized Partial Transfer.

The test testForwardCallToReceiveERC20PartialThroughCustodyFailsIfSenderIsNotWithdrawer reinforces access control by ensuring unauthorized accounts cannot initiate partial transfers.


248-256: Test for Zero Amount in Partial Transfer.

The test testForwardCallToReceiveERC20PartialThroughCustodyFailsIfAmountIs0 ensures that partial transfers with zero amount revert, maintaining logical consistency in token operations.


258-289: Test for No Parameter Function Call.

The test testForwardCallToReceiveNoParamsThroughCustody checks the behavior of functions with no parameters, ensuring correct event emissions and state changes.


291-315: Test for Direct Withdrawal.

The test testWithdrawThroughCustody verifies direct token withdrawal logic, ensuring tokens are transferred correctly and events are emitted as expected.


317-323: Test for Unauthorized Withdrawal.

The test testWithdrawThroughCustodyFailsIfSenderIsNotWithdrawer ensures that only authorized accounts can perform withdrawals, reinforcing security through access control.


325-359: Test for Withdraw and Revert Logic.

The test testWithdrawAndRevertThroughCustody checks the logic for withdrawing and reverting, ensuring correct event emissions and token transfers.


361-368: Test for Unauthorized Withdraw and Revert.

The test testWithdrawAndRevertThroughCustodyFailsIfSenderIsNotWithdrawer ensures unauthorized accounts cannot perform withdraw and revert operations, maintaining strict access control.


370-377: Test for Zero Amount in Withdraw and Revert.

The test testWithdrawAndRevertThroughCustodyFailsIfAmountIs0 ensures operations revert when attempting to withdraw and revert with zero amount, maintaining logical consistency.


379-386: Test for Zero Address in Withdraw and Revert.

The test testWithdrawAndRevertThroughCustodyFailsIfReceiverIsZeroAddress checks for zero address handling in the receiver parameter, ensuring robust input validation.

v2/typechain-types/ZetaConnectorNonNative.ts (5)

31-31: Addition of TSS_ROLE to getFunction.

The inclusion of TSS_ROLE in the getFunction method enhances the role management capabilities by allowing dynamic retrieval of function fragments associated with this role. This change is consistent with the introduction of the new role.


73-73: Addition of encodeFunctionData for TSS_ROLE.

The encodeFunctionData method for TSS_ROLE is correctly added, enabling the encoding of function calls related to this role. This is a necessary addition for handling the new role.


137-137: Addition of decodeFunctionResult for TSS_ROLE.

The decodeFunctionResult method for TSS_ROLE is properly added, facilitating the decoding of results from function calls associated with this role. This ensures correct interpretation of responses involving TSS_ROLE.


372-372: Exposure of TSS_ROLE as a TypedContractMethod.

The addition of TSS_ROLE as a TypedContractMethod in the ZetaConnectorNonNative interface allows for structured access to this role. This enhances the contract's interaction capabilities.


470-472: Addition of getFunction for TSS_ROLE.

The getFunction method for TSS_ROLE is correctly added, enabling dynamic retrieval of the function associated with this role. This is consistent with the role's integration into the contract.

v2/typechain-types/factories/ERC20Custody__factory.ts (1)

585-585: Verify thorough testing of updated bytecode.

The _bytecode constant has been entirely replaced, indicating significant changes to the contract's logic or optimizations. Ensure that these changes are thoroughly tested to confirm the contract behaves as expected.

Verification successful

Updated bytecode is covered by existing test cases.

The ERC20Custody contract is referenced in multiple test files, indicating that the updated bytecode is being tested. Ensure these tests are comprehensive and cover all new functionalities or changes.

  • Test files include v2/test/ZetaConnectorNonNative.t.sol, v2/test/GatewayEVM.t.sol, and v1/test/ERC20Custody.spec.ts.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Ensure the updated bytecode is thoroughly tested.

# Test: Search for test cases involving `ERC20Custody`.
rg --type javascript --type typescript --type solidity 'ERC20Custody'

Length of output: 104


Script:

#!/bin/bash
# Description: Ensure the updated bytecode is thoroughly tested.

# Test: Search for test cases involving `ERC20Custody`.
rg --type js --type ts --type sol 'ERC20Custody'

Length of output: 76


Script:

#!/bin/bash
# Description: Ensure the updated bytecode is thoroughly tested.

# Test: Search for test cases involving `ERC20Custody` with custom file type definition for Solidity.
rg --type-add 'sol:*.sol' --type js --type ts --type sol 'ERC20Custody'

Length of output: 20389

v2/typechain-types/factories/ZetaConnectorNonNative__factory.ts (2)

620-620: Verify thorough testing of updated bytecode.

The _bytecode constant has been updated, indicating changes to the contract's logic or optimizations. Ensure these changes are thoroughly tested to confirm the contract behaves as expected.

Verification successful

The updated bytecode is likely tested.

The ZetaConnectorNonNative contract is referenced in several test files, indicating that the updated bytecode is likely covered by tests. Ensure these tests are comprehensive and cover all new or modified functionalities.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Ensure the updated bytecode is thoroughly tested.

# Test: Search for test cases involving `ZetaConnectorNonNative`.
rg --type javascript --type typescript --type solidity 'ZetaConnectorNonNative'

Length of output: 114


Script:

#!/bin/bash
# Description: Ensure the updated bytecode is thoroughly tested.

# Test: Search for test cases involving `ZetaConnectorNonNative`.
rg --type js --type ts --type solidity 'ZetaConnectorNonNative'

Length of output: 96


Script:

#!/bin/bash
# Description: Ensure the updated bytecode is thoroughly tested.

# Test: Search for test cases involving `ZetaConnectorNonNative`.
rg --type js --type ts 'ZetaConnectorNonNative'

Length of output: 2652


Script:

#!/bin/bash
# Description: Search for test files or functions involving `ZetaConnectorNonNative`.

# Test: Look for test files or functions that reference `ZetaConnectorNonNative`.
rg 'ZetaConnectorNonNative' --type js --type ts --glob '**/test/**' --glob '**/*.test.*'

Length of output: 1776


75-87: Verify integration and testing of TSS_ROLE.

The TSS_ROLE function has been added to the ABI to enhance role management. Ensure this function is integrated into the contract's role management logic and is thoroughly tested.

v2/typechain-types/factories/ZetaConnectorNative__factory.ts (2)

614-614: Verify thorough testing of updated bytecode.

The _bytecode constant has been updated, indicating changes to the contract's logic or optimizations. Ensure these changes are thoroughly tested to confirm the contract behaves as expected.


75-87: Verify integration and testing of TSS_ROLE.

The TSS_ROLE function has been added to the ABI to enhance role management. Ensure this function is integrated into the contract's role management logic and is thoroughly tested.

Verification successful

TSS_ROLE Integration and Testing Verified

The TSS_ROLE constant is integrated into the role management logic across multiple Solidity contracts and is referenced in several test files, indicating that it is being tested. This confirms that TSS_ROLE is both integrated and tested within the codebase.

  • Files with TSS_ROLE integration:

    • v2/src/evm/ZetaConnectorNonNative.sol
    • v2/src/evm/ZetaConnectorBase.sol
    • v2/src/evm/GatewayEVM.sol
  • Test files referencing TSS_ROLE:

    • v2/test/ZetaConnectorNonNative.t.sol
    • v2/test/GatewayEVM.t.sol
    • v2/test/utils/GatewayEVMUpgradeTest.sol
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration and testing of `TSS_ROLE`.

# Test: Search for test cases involving `TSS_ROLE`.
rg --type javascript --type typescript --type solidity 'TSS_ROLE'

Length of output: 100


Script:

#!/bin/bash
# Description: Verify the integration and testing of `TSS_ROLE`.

# Test: Search for test cases involving `TSS_ROLE` using correct file types.
rg --type js --type ts --type sol 'TSS_ROLE'

Length of output: 72


Script:

#!/bin/bash
# Description: Verify the integration and testing of `TSS_ROLE`.

# Test: Search for test cases involving `TSS_ROLE` in JavaScript and TypeScript files.
rg --type js --type ts 'TSS_ROLE'

# Test: Search for test cases involving `TSS_ROLE` in Solidity files by extension.
rg --glob '*.sol' 'TSS_ROLE'

Length of output: 5034

v2/test/GatewayZEVM.t.sol (28)

79-89: Test case for low balance: Comprehensive coverage.

The test case testWithdrawZRC20FailsIfNoBalanceForGasFee correctly tests the LowBalance error when there is insufficient balance for gas fees. Ensure similar coverage for other scenarios.


91-98: Test case for low balance: Comprehensive coverage.

The test case testWithdrawZRC20FailsIfNoBalanceForTransfer correctly tests the LowBalance error when there is insufficient balance for transfer. This complements the previous test case.


100-103: Test case for zero amount: Correct error handling.

The test case testWithdrawZRC20FailsIsAmountIs0 verifies the contract's behavior when attempting to withdraw a zero amount, expecting the InsufficientZRC20Amount error. This is a good practice for input validation.


105-108: Test case for zero address: Correct error handling.

The test case testWithdrawZRC20FailsIfReceiverIsZeroAddress ensures that the contract reverts with the ZeroAddress error when the receiver is a zero address. This is essential for preventing invalid transactions.


126-130: Test case for withdraw and call with zero address: Correct error handling.

The test case testWithdrawAndCallZRC20FailsIfReceiverIsZeroAddress checks for the ZeroAddress error when using withdrawAndCall with a zero address. This maintains consistency with other zero address checks.


132-136: Test case for withdraw and call with zero amount: Correct error handling.

The test case testWithdrawAndCallZRC20FailsIfAmountIsZero verifies the InsufficientZRC20Amount error when attempting to withdraw zero amount with a call. This ensures robust input validation.


170-173: Test case for zero amount withdrawal: Correct error handling.

The test case testWithdrawZETAFailsIfAmountIsZero correctly expects the InsufficientZetaAmount error when withdrawing a zero amount. This aligns with the pattern of handling zero value transactions.


175-179: Test case for withdraw and call with zero amount: Correct error handling.

The test case testWithdrawAndcallZETAFailsIfAmountIsZero ensures the InsufficientZetaAmount error is triggered for zero amount withdrawals with a call. This is consistent with other zero amount checks.


226-234: Test case for no balance: Ensure correct error handling.

The test case testWithdrawZETAFailsIfNoBalance expects a revert when there is no balance to withdraw. Ensure that the contract logic correctly reverts in this scenario.


283-287: Test case for call with zero address: Correct error handling.

The test case testCallFailsIfReceiverIsZeroAddress ensures that the contract reverts with the ZeroAddress error when the receiver is a zero address. This is consistent with other zero address checks.


354-358: Test case for deposit with zero ZRC20 address: Correct error handling.

The test case testDepositFailsIfZrc20IsZeroAddress correctly expects the ZeroAddress error when the ZRC20 address is zero. This is crucial for maintaining valid contract interactions.


360-364: Test case for deposit with zero target address: Correct error handling.

The test case testDepositFailsIfTargetIsZeroAddress ensures the ZeroAddress error is triggered when the target address is zero. This aligns with best practices for input validation.


366-370: Test case for deposit with zero amount: Correct error handling.

The test case testDepositFailsIfAmountIs0 correctly expects the InsufficientZRC20Amount error for zero amount deposits. This ensures robust input validation.


443-451: Test case for execute with zero ZRC20 address: Correct error handling.

The test case testExecuteFailsIfZrc20IsZeroAddress ensures the ZeroAddress error is triggered when the ZRC20 address is zero. This is consistent with other zero address checks.


453-461: Test case for execute with zero target address: Correct error handling.

The test case testExecuteFailsIfTargetIsZeroAddress checks for the ZeroAddress error when the target address is zero. This maintains consistency with other zero address checks.


463-471: Test case for execute with zero amount: Correct error handling.

The test case testExecuteFailsIfAmountIsZero verifies the InsufficientZRC20Amount error for zero amount executions. This ensures robust input validation.


494-502: Test case for execute revert with zero ZRC20 address: Correct error handling.

The test case testExecuteRevertZContractFailsIfZrc20IsZeroAddress ensures the ZeroAddress error is triggered when the ZRC20 address is zero during a revert. This is consistent with other zero address checks.


504-512: Test case for execute revert with zero target address: Correct error handling.

The test case testExecuteRevertZContractFailsIfTargetIsZeroAddress checks for the ZeroAddress error when the target address is zero during a revert. This maintains consistency with other zero address checks.


514-522: Test case for execute revert with zero amount: Correct error handling.

The test case testExecuteRevertZContractFailsIfAmountIsZero verifies the InsufficientZRC20Amount error for zero amount reverts. This ensures robust input validation.


545-553: Test case for deposit and call with zero ZRC20 address: Correct error handling.

The test case testDepositZRC20AndCallZContractFailsIfZrc20IsZeroAddress ensures the ZeroAddress error is triggered when the ZRC20 address is zero during a deposit and call. This is consistent with other zero address checks.


555-563: Test case for deposit and call with zero target address: Correct error handling.

The test case testDepositZRC20AndCallZContractFailsIfTargetIsZeroAddress checks for the ZeroAddress error when the target address is zero during a deposit and call. This maintains consistency with other zero address checks.


565-573: Test case for deposit and call with zero amount: Correct error handling.

The test case testDepositZRC20AndCallZContractFailsIfAmountIsZero verifies the InsufficientZRC20Amount error for zero amount deposits and calls. This ensures robust input validation.


622-630: Test case for deposit and revert with zero ZRC20 address: Correct error handling.

The test case testDepositAndRevertZRC20AndCallZContractFailsIfZrc20IsZeroAddress ensures the ZeroAddress error is triggered when the ZRC20 address is zero during a deposit and revert. This is consistent with other zero address checks.


632-640: Test case for deposit and revert with zero target address: Correct error handling.

The test case testDepositAndRevertZRC20AndCallZContractFailsIfTargetIsZeroAddress checks for the ZeroAddress error when the target address is zero during a deposit and revert. This maintains consistency with other zero address checks.


642-650: Test case for deposit and revert with zero amount: Correct error handling.

The test case testDepositAndRevertZRC20AndCallZContractFailsIfAmountIsZero verifies the InsufficientZRC20Amount error for zero amount deposits and reverts. This ensures robust input validation.


699-707: Test case for deposit ZETA and call with zero target address: Correct error handling.

The test case testDepositZETAAndCallZContractFailsIfTargetIsZeroAddress ensures the ZeroAddress error is triggered when the target address is zero during a ZETA deposit and call. This is consistent with other zero address checks.


709-717: Test case for deposit ZETA and call with zero amount: Correct error handling.

The test case testDepositZETAAndCallZContractFailsIfTargetIsAmountIsZero verifies the InsufficientZetaAmount error for zero amount ZETA deposits and calls. This ensures robust input validation.


30-31: New error declarations: Ensure consistent usage.

The ZeroAddress and LowBalance errors are newly declared. Ensure these are consistently used across the test cases and that they match the error conditions in the contract under test.

Verification successful

Consistent Usage of ZeroAddress and LowBalance Errors Verified

The ZeroAddress and LowBalance errors are consistently used across the codebase in both contracts and test files. They are declared and utilized in various scenarios, ensuring that error conditions are correctly handled and tested.

  • Files with ZeroAddress usage:

    • v2/src/zevm/GatewayZEVM.sol
    • v2/src/zevm/ZRC20.sol
    • Various test files including v2/test/GatewayZEVM.t.sol
  • Files with LowBalance usage:

    • v2/src/zevm/ZRC20.sol
    • Test files such as v2/test/ZRC20.t.sol

These errors are integrated into the logic of the contracts and are verified through comprehensive testing.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify usage of ZeroAddress and LowBalance errors in the contract.
ast-grep --lang solidity --pattern $'error ZeroAddress();' --pattern $'error LowBalance();'

Length of output: 208


Script:

#!/bin/bash
# Search for occurrences of ZeroAddress and LowBalance in Solidity files to verify their usage.
rg 'ZeroAddress' --glob '*.sol' -A 5
rg 'LowBalance' --glob '*.sol' -A 5

Length of output: 48594

v2/typechain-types/factories/GatewayEVMUpgradeTest__factory.ts (2)

935-939: New error declaration: Ensure correct integration.

The ConnectorInitialized error is added to the ABI. Ensure that this error is correctly integrated and used in the contract logic where applicable.

Verification successful

Integration of ConnectorInitialized Error Verified

The ConnectorInitialized error is correctly integrated into the contract logic and is being used as intended. It is declared in the IGatewayEVM.sol interface, utilized in the GatewayEVM.sol contract, and tested in GatewayEVM.t.sol. No issues found with its integration.

  • v2/src/evm/interfaces/IGatewayEVM.sol: Declaration of ConnectorInitialized.
  • v2/src/evm/GatewayEVM.sol: Usage in contract logic.
  • v2/test/GatewayEVM.t.sol: Test case for the error.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify usage of ConnectorInitialized error in the contract.
ast-grep --lang solidity --pattern $'error ConnectorInitialized();'

Length of output: 185


Script:

#!/bin/bash
# Search for the declaration and usage of ConnectorInitialized error in Solidity files.
rg 'ConnectorInitialized' --glob '*.sol' -A 5

Length of output: 1095


1046-1046: Bytecode update: Verify consistency with contract logic.

The _bytecode variable has been updated. Ensure that the changes in the bytecode reflect the intended logic and functionality of the contract.

v2/typechain-types/factories/GatewayEVM__factory.ts (2)

907-911: Addition of ConnectorInitialized error type.

The addition of the ConnectorInitialized error type to the ABI enhances the contract's ability to handle specific error conditions related to connector initialization. This is a positive change for improving error handling and debugging.


1018-1018: Modification of GatewayEVMConstructorParams type declaration.

The change in the GatewayEVMConstructorParams type declaration to include a tuple with an optional Signer is a good approach for flexibility in constructor parameters. It allows for more versatile contract deployment scenarios.

v2/typechain-types/factories/GatewayZEVM__factory.ts (3)

960-964: Addition of InsufficientZetaAmount error type looks good.

The new error type enhances error handling by providing specific feedback related to Zeta token transactions. This aligns with existing patterns in the file.


1034-1034: Verify the updated bytecode.

The change in the _bytecode string reflects updates in the contract's compiled logic. Ensure that these changes are tested and verified for correctness.


1036-1036: Change to GatewayZEVMConstructorParams is approved.

The inclusion of an optional signer adds flexibility to the constructor parameters. Ensure compatibility with existing code.

Consider verifying that all constructor usages are updated to handle the new parameter option.

v2/typechain-types/factories/GatewayEVMEchidnaTest__factory.ts (2)

959-963: Addition of ConnectorInitialized error improves error handling.

The inclusion of the ConnectorInitialized error in the ABI enhances the contract's error handling capabilities by providing a specific error type for connector initialization issues. This can improve debugging and user experience.


1070-1070: Constructor parameter change enhances flexibility.

The addition of an optional signer parameter to the constructor allows for more flexible instantiation of the contract factory. It supports scenarios with and without a signer, enhancing usability.

v2/pkg/zetaconnectorbase.sol/zetaconnectorbase.go (4)

34-34: ABI Update Approved.

The ABI string has been correctly updated to include the new TSS_ROLE function. This enhances the role management capabilities of the contract.


248-260: Addition of TSSROLE Function Approved.

The TSSROLE function in ZetaConnectorBaseCaller is correctly implemented and consistent with other role retrieval methods.


265-267: Addition of TSSROLE Function in Session Approved.

The TSSROLE function in ZetaConnectorBaseSession is correctly implemented and consistent with the session-based access pattern.


272-274: Addition of TSSROLE Function in Caller Session Approved.

The TSSROLE function in ZetaConnectorBaseCallerSession is correctly implemented and consistent with the caller session pattern.

v2/pkg/erc20custody.sol/erc20custody.go (1)

35-35: Verify ABI and binary consistency.

Ensure that the updated ABI accurately reflects the intended function signatures and error types. The binary representation should be consistent with these changes.

v2/pkg/zetaconnectornative.sol/zetaconnectornative.go (3)

34-34: ABI Update: Inclusion of TSS_ROLE.

The ABI has been updated to include the TSS_ROLE function, enhancing the contract's role management capabilities.


270-282: Addition of TSSROLE function in ZetaConnectorNativeCaller.

The TSSROLE function has been added to the ZetaConnectorNativeCaller struct, allowing external calls to retrieve the TSS_ROLE. The implementation is consistent with other role retrieval functions.


287-296: Addition of TSSROLE functions in session structs.

The TSSROLE functions have been added to both ZetaConnectorNativeSession and ZetaConnectorNativeCallerSession, enabling session-based calls to retrieve the TSS_ROLE. The implementation is consistent with other session-based role retrieval functions.

v2/pkg/zetaconnectornonnative.sol/zetaconnectornonnative.go (3)

270-282: LGTM: Consistent implementation for TSSROLE in ZetaConnectorNonNativeCaller.

The function follows the existing pattern for role retrieval functions, ensuring consistency and correctness.


287-289: LGTM: Consistent implementation for TSSROLE in ZetaConnectorNonNativeSession.

This function correctly uses the session's call options to retrieve the TSS_ROLE.


294-296: LGTM: Consistent implementation for TSSROLE in ZetaConnectorNonNativeCallerSession.

This function aligns well with the existing caller session pattern, ensuring proper use of session call options.

v2/pkg/gatewayzevm.sol/gatewayzevm.go (3)

Line range hint 111-118:
Verify GatewayZEVM struct and method updates.

Ensure that the GatewayZEVM struct and related methods correctly reflect the updated contract interface and handle any deprecated fields appropriately.


48-49: Verify ABI and Bin updates in Go bindings.

The ABI and Bin have been updated with new functions, events, and error types. Ensure that these changes are correctly reflected in the Go bindings and that any deprecated fields are appropriately handled.

Verification successful

ABI and Bin updates are correctly reflected in Go bindings.

The Go bindings for the GatewayZEVM contract have been updated to include the new ABI and Bin. The functions DepositAndCall, WithdrawAndCall, and the error InsufficientZetaAmount are correctly referenced in the codebase, confirming that the updates are reflected. No deprecated fields are being used.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the Go bindings reflect the updated ABI and Bin.

# Test: Check for usage of deprecated fields. Expect: No usage of deprecated fields.
rg --type go 'GatewayZEVMABI|GatewayZEVM[AB]in'

# Test: Check for references to updated functions, events, and errors in the Go codebase.
rg --type go 'DepositAndCall|WithdrawAndCall|InsufficientZetaAmount'

Length of output: 278194


Line range hint 97-108:
Verify ABI parsing in DeployGatewayZEVM.

Ensure that the ABI parsing in DeployGatewayZEVM aligns with the updated ABI structure. This is crucial for correct contract deployment and interaction.

v2/pkg/gatewayevm.sol/gatewayevm.go (1)

34-35: Verify ABI and Binary Consistency.

The ABI and binary values have been updated. Ensure that these changes accurately reflect the intended contract functionality and that the binary is consistent with the updated ABI.

v2/pkg/gatewayevmupgradetest.sol/gatewayevmupgradetest.go (1)

34-35: Verify ABI and binary consistency.

The ABI and binary representation have been updated. Ensure that these changes align with the intended functionality of the contract and that the new functions and events are adequately tested.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c698398 and 2af9c3c.

Files selected for processing (3)
  • v2/pkg/gatewayevm.sol/gatewayevm.go (1 hunks)
  • v2/typechain-types/factories/GatewayEVMEchidnaTest__factory.ts (2 hunks)
  • v2/typechain-types/factories/GatewayEVM__factory.ts (2 hunks)
Files skipped from review as they are similar to previous changes (3)
  • v2/pkg/gatewayevm.sol/gatewayevm.go
  • v2/typechain-types/factories/GatewayEVMEchidnaTest__factory.ts
  • v2/typechain-types/factories/GatewayEVM__factory.ts

@lumtis
Copy link
Member

lumtis commented Aug 8, 2024

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

@skosito skosito merged commit da9ec04 into main Aug 9, 2024
9 checks passed
@skosito skosito deleted the improve-v2-coverage branch August 9, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve v2 coverage
4 participants