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

fix: missing receiver in zevm withdraw functions #301

Merged
merged 23 commits into from
Aug 9, 2024

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Aug 8, 2024

closes: #300

  • also adds receiver to withdraw function, seems like it was missing there as well
  • fixes couple localnet issues because of recent changes

Summary by CodeRabbit

  • New Features

    • Enhanced GatewayEVM and GatewayZEVM contracts with new functions for improved withdrawal and custody management.
    • Added functionality to the EvmDepositAndCallScript for managing ERC20 tokens with custody operations.
  • Bug Fixes

    • Implemented checks to prevent transactions to zero addresses, enhancing contract safety.
  • Tests

    • Introduced new test cases to ensure contracts handle zero address and amount scenarios appropriately.
  • Documentation

    • Updated comments in the interfaces to reflect new function signatures and parameters.

Copy link
Contributor

coderabbitai bot commented Aug 8, 2024

Walkthrough

The recent updates introduce significant enhancements to the GatewayEVM and GatewayZEVM contracts, improving transaction handling and security. Key changes include the addition of new parameters in function signatures, enhanced error checks, and new functionalities like ERC20 custody management. Updates to scripts and tests ensure comprehensive validation of these features, promoting better contract interaction and usability.

Changes

Files Change Summary
v2/pkg/gatewayevm.sol/gatewayevm.go, ...gatewayzevm.go Updated ABIs with new and modified function signatures, adding parameters to improve transaction handling.
v2/pkg/igatewayzevm.sol/igatewayzevm.go Modifications to function signatures in the ABI, incorporating a new receiver parameter.
v2/src/evm/GatewayEVM.sol, ...GatewayZEVM.sol Enhanced call, withdraw, and withdrawAndCall functions with checks for zero addresses and new parameters.
v2/typechain-types/*.ts Updated TypeScript interfaces with new receiver parameters for withdraw and withdrawAndCall functions.
v2/test/GatewayEVM.t.sol, ...GatewayZEVM.t.sol Added tests for new functionalities, ensuring correct behavior with zero addresses and amounts.
v2/scripts/localnet/*.s.sol Integrated ERC20Custody in the script to manage tokens effectively, adding new variables and logic.
v2/typechain-types/factories/*.ts Modifications to bytecode strings and constructor parameters reflecting changes in contract logic.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant GatewayEVM
    participant GatewayZEVM
    participant ERC20Custody

    User->>GatewayEVM: call(receiver, payload)
    GatewayEVM-->>User: Check receiver not zero
    GatewayEVM->>GatewayZEVM: withdraw(receiver, amount, chainId)
    GatewayZEVM-->>User: Emit withdrawal event
    User->>ERC20Custody: whitelist(erc20Address)
    ERC20Custody-->>User: Confirm whitelisting
Loading

🐇 In a meadow lush and bright,
The contracts dance in morning light.
New functions added, oh what glee,
With checks for safety, wild and free!
Tokens flowing, like a stream,
In code we trust, in code we dream! 🌼


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.

@skosito skosito linked an issue Aug 8, 2024 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.27%. Comparing base (f975656) to head (3768305).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #301      +/-   ##
==========================================
+ Coverage   83.09%   83.27%   +0.17%     
==========================================
  Files           7        7              
  Lines         284      287       +3     
  Branches       92       95       +3     
==========================================
+ Hits          236      239       +3     
  Misses         48       48              

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

Base automatically changed from whitelist-custody to main August 9, 2024 13:49
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 f975656 and 96e69ad.

Files selected for processing (18)
  • v2/pkg/gatewayevm.sol/gatewayevm.go (1 hunks)
  • v2/pkg/gatewayzevm.sol/gatewayzevm.go (3 hunks)
  • v2/pkg/igatewayzevm.sol/igatewayzevm.go (3 hunks)
  • v2/pkg/senderzevm.sol/senderzevm.go (1 hunks)
  • v2/scripts/localnet/EvmDepositAndCall.s.sol (2 hunks)
  • v2/scripts/localnet/worker.ts (2 hunks)
  • v2/src/evm/GatewayEVM.sol (1 hunks)
  • v2/src/zevm/GatewayZEVM.sol (2 hunks)
  • v2/src/zevm/interfaces/IGatewayZEVM.sol (2 hunks)
  • v2/test/GatewayEVM.t.sol (1 hunks)
  • v2/test/GatewayZEVM.t.sol (11 hunks)
  • v2/typechain-types/GatewayZEVM.ts (7 hunks)
  • v2/typechain-types/IGatewayZEVM.sol/IGatewayZEVM.ts (7 hunks)
  • v2/typechain-types/factories/GatewayEVMEchidnaTest__factory.ts (1 hunks)
  • v2/typechain-types/factories/GatewayEVM__factory.ts (1 hunks)
  • v2/typechain-types/factories/GatewayZEVM__factory.ts (3 hunks)
  • v2/typechain-types/factories/IGatewayZEVM.sol/IGatewayZEVM__factory.ts (2 hunks)
  • v2/typechain-types/factories/SenderZEVM__factory.ts (1 hunks)
Additional comments not posted (43)
v2/scripts/localnet/EvmDepositAndCall.s.sol (2)

6-6: Import statement is correct.

The addition of the ERC20Custody import is necessary for the new functionality.


14-14: New variable declarations are correct.

The custodyAddress and custody variables are correctly initialized using environment variables.

Verify the whitelist method usage.

Ensure that the whitelist method is implemented correctly in the ERC20Custody contract and that the erc20Address is valid.

Also applies to: 26-28

Verification successful

The whitelist method implementation is appropriate and correct.

The whitelist method in v2/src/evm/ERC20Custody.sol correctly handles the address type and uses role-based access control with WHITELISTER_ROLE. This aligns with the usage in EvmDepositAndCall.s.sol.

  • Location: v2/src/evm/ERC20Custody.sol
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of the `whitelist` method in the `ERC20Custody` contract.

# Test: Search for the `whitelist` method in the `ERC20Custody` contract. Expect: Correct implementation.
ast-grep --lang solidity --pattern $'contract ERC20Custody {
  $$$
  function whitelist(address $_) public {
    $$$
  }
  $$$
}'

Length of output: 247


Script:

#!/bin/bash
# Use rg to search for the `whitelist` method in Solidity files to verify its implementation.
rg 'function whitelist\(' --glob '*.sol' -A 5

Length of output: 767

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

85-87: Function signature change is appropriate.

The addition of the receiver parameter to the withdraw function is a suitable enhancement for cross-chain transactions.


97-101: Function signature change is appropriate.

The addition of the receiver parameter to the withdrawAndCall function enhances its capability for cross-chain interactions.

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

123-123: Verify the updated bytecode.

Ensure that the updated _bytecode constant reflects the intended changes in the contract logic and does not introduce any issues.

v2/typechain-types/factories/IGatewayZEVM.sol/IGatewayZEVM__factory.ts (2)

330-334: Verify ABI consistency for withdraw.

The addition of the receiver parameter to the withdraw function's ABI is consistent with the intended changes. Ensure that all client code interacting with this function is updated to accommodate the new parameter.


381-385: Verify ABI consistency for withdrawAndCall.

The addition of the receiver parameter to the withdrawAndCall function's ABI is consistent with the intended changes. Ensure that all client code interacting with this function is updated to accommodate the new parameter.

v2/scripts/localnet/worker.ts (2)

274-274: Confirm the change of argument in execute call.

The change from 0 to 1 in the execute function call might indicate a shift in operation mode or context. Verify that this change aligns with the intended logic and that all dependent code is adjusted accordingly.


249-249: Confirm the change of argument in execute call.

The change from 0 to 1 in the execute function call might indicate a shift in operation mode or context. Verify that this change aligns with the intended logic and that all dependent code is adjusted accordingly.

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

149-155: Validate receiver parameter in withdraw.

The addition of the receiver parameter enhances the function's capability. Ensure that the receiver is correctly validated and integrated into the withdrawal logic. The event emission now includes the receiver, which aligns with the intended functionality.


Line range hint 159-177:
Validate receiver parameter in withdrawAndCall.

The addition of the receiver parameter and the zero-length check improve the function's robustness. The event emission now includes the receiver, which aligns with the intended functionality. Ensure that all interactions with this function are updated to match the new signature.

v2/typechain-types/IGatewayZEVM.sol/IGatewayZEVM.ts (10)

113-114: LGTM! Encoding method updated for withdraw.

The encodeFunctionData method has been correctly updated to reflect the new function signature with the bytes parameter.


121-122: LGTM! Encoding method updated for withdrawAndCall.

The encodeFunctionData method has been correctly updated to reflect the new function signature with the bytes parameter.


149-149: LGTM! Decoding method updated for withdraw.

The decodeFunctionResult method has been correctly updated to reflect the new function signature with the bytes parameter.


157-157: LGTM! Decoding method updated for withdrawAndCall.

The decodeFunctionResult method has been correctly updated to reflect the new function signature with the bytes parameter.


344-345: LGTM! TypedContractMethod updated for withdraw.

The TypedContractMethod has been correctly updated to reflect the new function signature with the bytes parameter.


361-367: LGTM! TypedContractMethod updated for withdrawAndCall.

The TypedContractMethod has been correctly updated to reflect the new function signature with the bytes parameter.


462-464: LGTM! getFunction method updated for withdraw.

The getFunction method has been correctly updated to reflect the new function signature with the bytes parameter.


481-488: LGTM! getFunction method updated for withdrawAndCall.

The getFunction method has been correctly updated to reflect the new function signature with the bytes parameter.


61-61: Verify the updated withdraw function usage.

The function signature now includes a bytes parameter for the receiver. Ensure that all calls to this function in the codebase are updated to match the new signature.

Verification successful

The withdraw function signature is consistently updated.

The withdraw(bytes,uint256,uint256) function signature is correctly used across the relevant TypeScript files, indicating that the codebase is aligned with the new function signature. No outdated calls were found.

  • Files with updated signature:
    • v2/typechain-types/GatewayZEVM.ts
    • v2/typechain-types/IGatewayZEVM.sol/IGatewayZEVM.ts
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `withdraw` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type typescript -A 5 $'withdraw\\(bytes,uint256,uint256\\)'

Length of output: 98


Script:

#!/bin/bash
# Description: Verify all function calls to `withdraw` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --glob '*.ts' -A 5 $'withdraw\\(bytes,uint256,uint256\\)'

Length of output: 4776


63-63: Verify the updated withdrawAndCall function usage.

The function signature now includes a bytes parameter for the receiver. Ensure that all calls to this function in the codebase are updated to match the new signature.

Verification successful

Function usage updated correctly for withdrawAndCall.

The function signature withdrawAndCall(bytes,uint256,uint256,bytes) is consistently used across relevant TypeScript files, including GatewayZEVM.ts and IGatewayZEVM.ts. This indicates that all necessary updates have been made to accommodate the new parameter.

  • v2/typechain-types/GatewayZEVM.ts
  • v2/typechain-types/IGatewayZEVM.sol/IGatewayZEVM.ts
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `withdrawAndCall` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type typescript -A 5 $'withdrawAndCall\\(bytes,uint256,uint256,bytes\\)'

Length of output: 111


Script:

#!/bin/bash
# Description: Verify all function calls to `withdrawAndCall` match the new signature.

# Test: Search for the function usage in .ts files. Expect: Only occurrences of the new signature.
rg --glob '*.ts' -A 5 $'withdrawAndCall\\(bytes,uint256,uint256,bytes\\)'

Length of output: 4607

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

272-272: LGTM! Added zero address check in call function.

The addition of the check to revert if the receiver is a zero address is a good security measure, preventing unintended interactions.

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

492-497: LGTM! Added test for zero address in call function.

The test function correctly validates that the call method reverts when a zero address is used, ensuring the robustness of the zero address check.

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

35-35: Verify the impact of updated binary data.

The Bin field of SenderZEVMMetaData has been updated. Ensure that this change aligns with the intended contract logic and deployment requirements.

Verification successful

Impact of Updated Binary Data in SenderZEVMMetaData Verified

The SenderZEVMMetaData is only utilized within the v2/pkg/senderzevm.sol/senderzevm.go file. The update to the Bin field is consistent with the intended usage, as the code is structured to use SenderZEVMMetaData.Bin directly. No external dependencies or tests are affected by this change.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the impact of the updated binary data on the contract deployment and functionality.

# Test: Search for the usage of `SenderZEVMMetaData` to ensure it is correctly referenced.
rg --type go 'SenderZEVMMetaData'

Length of output: 719


Script:

#!/bin/bash
# Description: Search for additional references to `SenderZEVMMetaData` in the codebase.

# Test: Find all references to `SenderZEVMMetaData` to ensure its usage is consistent and verify its impact.
rg 'SenderZEVMMetaData'

Length of output: 709

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

77-79: Verify function signature updates.

The withdraw and withdrawAndCall function signatures have been updated to include a bytes parameter. Ensure that these changes are correctly implemented and used throughout the codebase.

Also applies to: 196-197, 204-205, 638-639, 655-661, 830-832, 849-856

Verification successful

Function Signature Updates Verified

The updated function signatures for withdraw and withdrawAndCall have been correctly implemented and are present in the relevant files, including GatewayZEVM.ts and IGatewayZEVM.ts. The changes are reflected in the function fragments and typed contract methods, ensuring consistency across the codebase.

  • Files with updated signatures:
    • v2/typechain-types/GatewayZEVM.ts
    • v2/typechain-types/IGatewayZEVM.sol/IGatewayZEVM.ts

These updates confirm that the function signatures have been correctly modified to include the bytes parameter.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the updated function signatures are correctly implemented and used.

# Test: Search for the usage of `withdraw` and `withdrawAndCall` to ensure they match the new signatures.
rg --type ts 'withdraw\('
rg --type ts 'withdrawAndCall\('

Length of output: 12691

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

172-172: Verify test coverage and correctness.

The test functions have been updated to handle zero addresses and amounts. Ensure that these tests adequately cover the edge cases and that the assertions are correct.

Also applies to: 175-179, 181-184, 195-196, 220-220, 239-239, 251-252, 277-277, 449-449, 520-528, 540-540, 550-557, 755-767

v2/pkg/igatewayzevm.sol/igatewayzevm.go (7)

365-369: Update Withdraw0 function signature.

The Withdraw0 function now includes a receiver parameter. Ensure that all calls to this function are updated to match the new signature.


372-376: Update Withdraw0 function signature in session.

The session method for Withdraw0 now includes a receiver parameter. Ensure that all session calls are updated accordingly.


379-383: Update Withdraw0 function signature in transactor session.

The transactor session method for Withdraw0 now includes a receiver parameter. Ensure that all transactor session calls are updated accordingly.


407-411: Update WithdrawAndCall0 function signature.

The WithdrawAndCall0 function now includes a receiver parameter. Ensure that all calls to this function are updated to match the new signature.


414-418: Update WithdrawAndCall0 function signature in session.

The session method for WithdrawAndCall0 now includes a receiver parameter. Ensure that all session calls are updated accordingly.


421-425: Update WithdrawAndCall0 function signature in transactor session.

The transactor session method for WithdrawAndCall0 now includes a receiver parameter. Ensure that all transactor session calls are updated accordingly.


48-48: Ensure ABI consistency with contract changes.

The ABI string has been updated to include the receiver parameter in the withdraw and withdrawAndCall functions. Verify that these changes are consistent with the contract's Solidity code.

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

1023-1023: Review bytecode changes for GatewayEVM.

The bytecode string has been significantly altered. Ensure that this change reflects the intended updates to the contract's logic and verify that it aligns with the contract's functionality.

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

637-641: Verify usage of the updated withdrawAndCall function.

The withdrawAndCall function now includes a receiver parameter. Ensure all calls to this function are updated to match the new signature and verify that the contract logic correctly handles the receiver parameter.


586-590: Verify usage of the updated withdraw function.

The withdraw function now includes a receiver parameter. Ensure all calls to this function are updated to match the new signature and verify that the contract logic correctly handles the receiver parameter.

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

1075-1076: LGTM! The new type declaration enhances flexibility.

The introduction of GatewayEVMEchidnaTestConstructorParams allows for more versatile constructor configurations, improving usability.


Line range hint 1078-1085:
LGTM! The constructor logic is well-implemented.

The use of isSuperArgs as a type guard ensures type safety and flexibility in handling constructor parameters.

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

847-848: Verify the handling of the receiver parameter.

The receiver parameter has been added to the Withdraw0 function. Ensure that this parameter is correctly utilized and validated in the function logic.


889-890: Verify the handling of the receiver parameter.

The receiver parameter has been added to the WithdrawAndCall0 function. Ensure that this parameter is correctly utilized and validated in the function logic.

Verification successful

The receiver parameter is correctly handled in WithdrawAndCall0.

The receiver parameter is consistently passed to the contract method in the WithdrawAndCall0 function implementations across different contexts. This indicates that the parameter is being utilized as intended.

  • v2/pkg/gatewayzevm.sol/gatewayzevm.go: The parameter is correctly passed in the WithdrawAndCall0 function for both GatewayZEVMTransactor and GatewayZEVMSession.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage and validation of the `receiver` parameter in the `WithdrawAndCall0` function.

# Test: Search for the function implementation. Expect: Correct handling of the `receiver` parameter.
rg --type go -A 5 $'func (_GatewayZEVM *GatewayZEVMTransactor) WithdrawAndCall0(opts *bind.TransactOpts, receiver []byte, amount *big.Int, chainId *big.Int, message []byte) (*types.Transaction, error)'

Length of output: 590


Script:

#!/bin/bash
# Description: Verify the usage and validation of the `receiver` parameter in the `WithdrawAndCall0` function.

# Test: Search for the function implementation. Expect: Correct handling of the `receiver` parameter.
rg --type go -A 5 $'func (_GatewayZEVM *GatewayZEVMTransactor) WithdrawAndCall0(opts *bind.TransactOpts, receiver \\[]byte, amount \\*big.Int, chainId \\*big.Int, message \\[]byte) (\\*types.Transaction, error)'

Length of output: 207


Script:

#!/bin/bash
# Description: Search for the `WithdrawAndCall0` function by name to locate its definition in the codebase.

# Test: Search for the function by name. Expect: Locate the function definition.
rg --type go 'func (_GatewayZEVM \*GatewayZEVMTransactor) WithdrawAndCall0'

Length of output: 77


Script:

#!/bin/bash
# Description: Search for any occurrences of `WithdrawAndCall0` in the entire codebase.

# Test: Search for occurrences of `WithdrawAndCall0`. Expect: Find references or definitions.
rg 'WithdrawAndCall0'

Length of output: 2645

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

35-35: Check event handling and iterator functions.

Ensure that the event handling functions correctly parse logs and manage subscriptions for new events like ExecutedWithERC20 and RoleGranted. Verify that the iterator functions handle errors gracefully and efficiently iterate over events.

Verification successful

Event handling functions are correctly implemented.

The event handling functions FilterExecutedWithERC20 and WatchRoleGranted are present in the codebase and implemented in the file v2/pkg/gatewayevm.sol/gatewayevm.go. This confirms that the necessary functions for handling these events are included.

  • FilterExecutedWithERC20: Line found in v2/pkg/gatewayevm.sol/gatewayevm.go
  • WatchRoleGranted: Line found in v2/pkg/gatewayevm.sol/gatewayevm.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of event handling and iterator functions.

# Test: Check for the implementation of event handling functions in the Go code.
rg --type go $'func (_GatewayEVM *GatewayEVMFilterer) FilterExecutedWithERC20($_, $_, $_) { $$$ }'
rg --type go $'func (_GatewayEVM *GatewayEVMFilterer) WatchRoleGranted($_, $_, $_, $_) { $$$ }'

Length of output: 674


Script:

#!/bin/bash
# Description: Verify the correctness of event handling and iterator functions.

# Corrected Test: Check for the implementation of event handling functions in the Go code.
rg --type go 'func \(_GatewayEVM \*GatewayEVMFilterer\) FilterExecutedWithERC20\('
rg --type go 'func \(_GatewayEVM \*GatewayEVMFilterer\) WatchRoleGranted\('

Length of output: 625


35-35: Verify ABI consistency and correctness.

The ABI changes include new functions and events, such as setCustody, setConnector, and revertWithERC20. Ensure these changes are consistent with the contract's intended functionality and are correctly implemented in the contract code.


35-35: Ensure function bindings match ABI definitions.

The Go bindings for new functions like Deposit0 and DepositAndCall0 should match the ABI definitions. Verify that the parameter handling is correct and consistent with the contract's functionality.

Verification successful

Let's correct the regex pattern and rerun the shell script to verify the function bindings.


Let's search for all functions in the Go file to determine if the functions are present under different names.


Functions Deposit0 and DepositAndCall0 are present and correctly named.

The functions Deposit0 and DepositAndCall0 exist in the Go file with additional parameters as expected. Ensure that their parameter handling aligns with the ABI definitions and contract functionality. Manual verification is recommended to confirm this alignment.

  • Deposit0 and DepositAndCall0 are implemented with additional parameters.
  • Verify parameter handling against ABI specifications.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the Go bindings for new functions match the ABI definitions.

# Test: Check for the function bindings in the Go code.
rg --type go $'func (_GatewayEVM *GatewayEVMTransactor) Deposit0($_, $_, $_, $_) { $$$ }'
rg --type go $'func (_GatewayEVM *GatewayEVMTransactor) DepositAndCall0($_, $_, $_, $_, $_) { $$$ }'

Length of output: 662


Script:

#!/bin/bash
# Description: Verify that the Go bindings for new functions match the ABI definitions.

# Test: Check for the function bindings in the Go code.
rg --type go 'func (_GatewayEVM \*GatewayEVMTransactor) Deposit0\('
rg --type go 'func (_GatewayEVM \*GatewayEVMTransactor) DepositAndCall0\('

Length of output: 146


Script:

#!/bin/bash
# Description: List all functions in the Go file to verify the presence of the expected functions.

# Test: List all function definitions in the Go file.
rg --type go '^func ' v2/pkg/gatewayevm.sol/gatewayevm.go

Length of output: 20446

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 96e69ad and 3768305.

Files selected for processing (4)
  • v2/pkg/gatewayzevm.sol/gatewayzevm.go (3 hunks)
  • v2/src/zevm/GatewayZEVM.sol (2 hunks)
  • v2/test/GatewayZEVM.t.sol (10 hunks)
  • v2/typechain-types/factories/GatewayZEVM__factory.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • v2/typechain-types/factories/GatewayZEVM__factory.ts
Additional comments not posted (14)
v2/src/zevm/GatewayZEVM.sol (2)

149-156: LGTM! But verify the function usage in the codebase.

The addition of the receiver parameter and the zero-length check improve robustness. The event emission update ensures accurate logging.

However, ensure that all function calls to withdraw match the new signature.

Verification successful

Let's correct the file type issue and re-run the verification.


Let's broaden the search to find any usage of the withdraw function in Solidity files, regardless of the specific encoding pattern.


Function usage verified successfully.

The withdraw function calls in the codebase, particularly in v2/test/GatewayZEVM.t.sol, correctly match the new signature with the receiver parameter encoded using abi.encodePacked. No discrepancies found.

  • v2/test/GatewayZEVM.t.sol: Multiple instances confirm correct usage.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `withdraw` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type solidity -A 5 $'withdraw(abi.encodePacked($_), $_, $_)'

Length of output: 99


Script:

#!/bin/bash
# Description: Verify all function calls to `withdraw` match the new signature.

# Test: Search for the function usage in Solidity files. Expect: Only occurrences of the new signature.
rg 'withdraw\(abi.encodePacked\($_\), $_, $_\)' --glob '*.sol' -A 5

Length of output: 69


Script:

#!/bin/bash
# Description: Search for any usage of the `withdraw` function in Solidity files.

# Test: Search for the `withdraw` function calls in Solidity files.
rg 'withdraw\(' --glob '*.sol' -A 5

Length of output: 25505


Line range hint 160-178:
LGTM! But verify the function usage in the codebase.

The addition of the receiver parameter and the zero-length check improve robustness. The event emission update ensures accurate logging.

However, ensure that all function calls to withdrawAndCall match the new signature.

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

172-173: Test for zero amount in withdraw is correct.

The test correctly checks for the InsufficientZetaAmount error when the amount is zero.


175-177: Test for zero address in withdraw is correct.

The test correctly checks for the ZeroAddress error when the receiver is a zero address.


183-184: Test for zero amount in withdrawAndCall is correct.

The test correctly checks for the InsufficientZetaAmount error when the amount is zero.


186-189: Test for zero address in withdrawAndCall is correct.

The test correctly checks for the ZeroAddress error when the receiver is a zero address.


525-533: Test for zero address in executeRevert is correct.

The test correctly checks for the ZeroAddress error when the target is a zero address.


760-771: Test for zero address in depositAndCall is correct.

The test correctly checks for the ZeroAddress error when the target is a zero address.

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

847-848: LGTM: Updated Withdraw0 function signature.

The addition of the receiver parameter in Withdraw0 aligns with the updated ABI and enhances flexibility in specifying the receiver.


854-855: LGTM: Updated Withdraw0 function signature.

The addition of the receiver parameter in Withdraw0 aligns with the updated ABI and enhances flexibility in specifying the receiver.


861-862: LGTM: Updated Withdraw0 function signature.

The addition of the receiver parameter in Withdraw0 aligns with the updated ABI and enhances flexibility in specifying the receiver.


889-890: LGTM: Updated WithdrawAndCall0 function signature.

The addition of the receiver parameter in WithdrawAndCall0 aligns with the updated ABI and enhances flexibility in specifying the receiver.


896-897: LGTM: Updated WithdrawAndCall0 function signature.

The addition of the receiver parameter in WithdrawAndCall0 aligns with the updated ABI and enhances flexibility in specifying the receiver.


903-904: LGTM: Updated WithdrawAndCall0 function signature.

The addition of the receiver parameter in WithdrawAndCall0 aligns with the updated ABI and enhances flexibility in specifying the receiver.

@skosito skosito merged commit 6486dd8 into main Aug 9, 2024
9 checks passed
@skosito skosito deleted the missing-receiver-in-zevm-withdraw branch August 9, 2024 14:36
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.

Add bytes memory receiver to withdrawAndCall
4 participants