feat: initialize a prototypes #162
Conversation
lumtis
requested review from
andresaiello,
brewmaster012,
CharlieMc0,
fadeev and
skosito
as code owners
|
@coderabbitai can you rewrite the Sequence Diagram(s) for all the Solidity smart contracts in the PR? |
|
Tip For best results, initiate chat on the files or code changes. @lumtis: Here are the sequence diagrams for the Solidity smart contracts in the PR: ERC20CustodyNew.sol Sequence DiagramsequenceDiagram
participant User
participant ERC20CustodyNew
User->>ERC20CustodyNew: withdraw(token, to, amount)
ERC20CustodyNew-->>User: Withdraw event
User->>ERC20CustodyNew: withdrawAndCall(token, to, amount, data)
ERC20CustodyNew-->>User: WithdrawAndCall event
GatewayEVM.sol Sequence DiagramsequenceDiagram
participant User
participant GatewayEVM
User->>GatewayEVM: initialize(_tssAddress)
User->>GatewayEVM: execute(destination, data)
GatewayEVM-->>User: Executed event
User->>GatewayEVM: executeWithERC20(token, to, amount, data)
GatewayEVM-->>User: ExecutedWithERC20 event
User->>GatewayEVM: deposit(receiver)
GatewayEVM-->>User: Deposit event
User->>GatewayEVM: depositAndCall(receiver, payload)
GatewayEVM-->>User: Call event
User->>GatewayEVM: setCustody(_custody)
GatewayEVMUpgradeTest.sol Sequence DiagramsequenceDiagram
participant User
participant GatewayEVMUpgradeTest
User->>GatewayEVMUpgradeTest: initialize(_tssAddress)
User->>GatewayEVMUpgradeTest: execute(destination, data)
GatewayEVMUpgradeTest-->>User: ExecutedV2 event
User->>GatewayEVMUpgradeTest: executeWithERC20(token, to, amount, data)
GatewayEVMUpgradeTest-->>User: ExecutedWithERC20 event
User->>GatewayEVMUpgradeTest: sendERC20(recipient, token, amount)
GatewayEVMUpgradeTest-->>User: SendERC20 event
User->>GatewayEVMUpgradeTest: send(recipient, amount)
GatewayEVMUpgradeTest-->>User: Send event
User->>GatewayEVMUpgradeTest: setCustody(_custody)
Receiver.sol Sequence DiagramsequenceDiagram
participant User
participant Receiver
User->>Receiver: receiveA(str, num, flag)
Receiver-->>User: ReceivedA event
User->>Receiver: receiveB(strs, nums, flag)
Receiver-->>User: ReceivedB event
User->>Receiver: receiveC(amount, token, destination)
Receiver-->>User: ReceivedC event
User->>Receiver: receiveD()
Receiver-->>User: ReceivedD event
TestERC20.sol Sequence DiagramsequenceDiagram
participant User
participant TestERC20
User->>TestERC20: mint(to, amount)
GatewayZEVM.sol Sequence DiagramsequenceDiagram
participant User
participant GatewayZEVM
User->>GatewayZEVM: initialize()
User->>GatewayZEVM: withdraw(receiver, amount, zrc20)
GatewayZEVM-->>User: Withdrawal event
User->>GatewayZEVM: withdrawAndCall(receiver, amount, zrc20, message)
GatewayZEVM-->>User: Withdrawal event
User->>GatewayZEVM: call(receiver, message)
GatewayZEVM-->>User: Call event
Sender.sol Sequence DiagramsequenceDiagram
participant User
participant Sender
User->>Sender: callReceiver(receiver, str, num, flag)
User->>Sender: withdrawAndCallReceiver(receiver, amount, zrc20, str, num, flag)
TestZContract.sol Sequence DiagramsequenceDiagram
participant User
participant TestZContract
User->>TestZContract: onCrossChainCall(context, zrc20, amount, message)
TestZContract-->>User: ContextData event
ZRC20New.sol Sequence DiagramsequenceDiagram
participant User
participant ZRC20New
User->>ZRC20New: name()
User->>ZRC20New: symbol()
User->>ZRC20New: decimals()
User->>ZRC20New: totalSupply()
User->>ZRC20New: balanceOf(account)
User->>ZRC20New: transfer(recipient, amount)
User->>ZRC20New: allowance(owner, spender)
User->>ZRC20New: approve(spender, amount)
User->>ZRC20New: transferFrom(sender, recipient, amount)
User->>ZRC20New: burn(amount)
User->>ZRC20New: deposit(to, amount)
User->>ZRC20New: withdraw(to, amount)
User->>ZRC20New: withdrawGasFee()
User->>ZRC20New: updateSystemContractAddress(addr)
User->>ZRC20New: updateGasLimit(gasLimit)
User->>ZRC20New: updateProtocolFlatFee(protocolFlatFee)
Let me know if any adjustments are needed! Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Gathering the function definitions and interactions in the Solidity files
for file in contracts/prototypes/**/*.sol; do
echo "File: $file"
echo "Functions:"
ast-grep --lang solidity --pattern 'function $_($_) $_' $file
echo "Events:"
ast-grep --lang solidity --pattern 'event $_($_)' $file
echo "Errors:"
ast-grep --lang solidity --pattern 'error $_($_)' $file
echo "Imports:"
ast-grep --lang solidity --pattern 'import $_' $file
echo
done
Length of output: 12107 Script: #!/bin/bash
# Gathering the function definitions and interactions in the Solidity files
for file in contracts/prototypes/**/*.sol; do
echo "File: $file"
echo "Functions:"
rg --no-filename 'function ' $file
echo "Events:"
rg --no-filename 'event ' $file
echo "Errors:"
rg --no-filename 'error ' $file
echo "Imports:"
rg --no-filename 'import ' $file
echo
done
Length of output: 15556 |
| event WithdrawAndCall(address indexed token, address indexed to, uint256 amount, bytes data); | ||
|
|
||
| constructor(address _gateway) { | ||
| gateway = IGatewayEVM(_gateway); |
There was a problem hiding this comment.
nit: add a zero address validation
| IERC20(token).transfer(address(gateway), amount); | ||
|
|
||
| // Forward the call to the Gateway contract | ||
| gateway.executeWithERC20(token, to, amount, data); |
There was a problem hiding this comment.
executeWithERC20 will happen always, even if the previous transfer to the gateway failed; is it a valid scenario?
There was a problem hiding this comment.
No it should fail in this case
So if transfer returns a boolean, we need to check the returned value
There was a problem hiding this comment.
should we use safeTransfer instead?
EDIT: will change to safeTransfer because that seems cleaner than always checking response bool value but let me know your thoughts
| } | ||
|
|
||
| // Withdraw ZRC20 tokens to external chain | ||
| function withdraw(bytes memory receiver, uint256 amount, address zrc20) external { |
There was a problem hiding this comment.
I'm probably lacking some context here on how the system contracts works; where do we control that msg.sender has the funds trying to be withdrawn?
There was a problem hiding this comment.
i added more explicit revert errors, but it will revert if that is the case, maybe we can add check at the beginning, but this should be good enough?
Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>
|
@lumtis @andresaiello @fbac I addressed all comments, and opened 2 more issues from not addressed comments: please recheck when you get a chance |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 35
Outside diff range and nitpick comments (6)
pkg/contracts/prototypes/zevm/sender.sol/sender.go (6)
Line range hint
367-369: Ensure proper error handling inFilterWithdraw.The function handles errors well, but consider adding more context to the error messages for better debugging.
- return nil, err + return nil, fmt.Errorf("FilterWithdraw: %w", err)
Line range hint
388-390: Ensure proper error handling inWatchWithdraw.The function handles errors well, but consider adding more context to the error messages for better debugging.
- return nil, err + return nil, fmt.Errorf("WatchWithdraw: %w", err)
Line range hint
425-426: Ensure proper error handling inParseWithdraw.The function handles errors well, but consider adding more context to the error messages for better debugging.
- return nil, err + return nil, fmt.Errorf("ParseWithdraw: %w", err)
Line range hint
522-524: Ensure proper error handling inFilterWithdrawAndCall.The function handles errors well, but consider adding more context to the error messages for better debugging.
- return nil, err + return nil, fmt.Errorf("FilterWithdrawAndCall: %w", err)
Line range hint
543-545: Ensure proper error handling inWatchWithdrawAndCall.The function handles errors well, but consider adding more context to the error messages for better debugging.
- return nil, err + return nil, fmt.Errorf("WatchWithdrawAndCall: %w", err)
Line range hint
580-581: Ensure proper error handling inParseWithdrawAndCall.The function handles errors well, but consider adding more context to the error messages for better debugging.
- return nil, err + return nil, fmt.Errorf("ParseWithdrawAndCall: %w", err)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (11)
contracts/prototypes/evm/ERC20CustodyNew.solis excluded by none and included by nonecontracts/prototypes/evm/GatewayEVM.solis excluded by none and included by nonecontracts/prototypes/evm/GatewayEVMUpgradeTest.solis excluded by none and included by nonecontracts/prototypes/evm/Receiver.solis excluded by none and included by nonecontracts/prototypes/zevm/GatewayZEVM.solis excluded by none and included by nonecontracts/prototypes/zevm/Sender.solis excluded by none and included by nonecontracts/prototypes/zevm/ZRC20New.solis excluded by none and included by nonecontracts/zevm/ZRC20.solis excluded by none and included by nonetest/prototypes/GatewayEVM.spec.tsis excluded by none and included by nonetest/prototypes/GatewayEVMUpgrade.spec.tsis excluded by none and included by nonetest/prototypes/GatewayIntegration.spec.tsis excluded by none and included by none
Files selected for processing (6)
- pkg/contracts/prototypes/evm/erc20custodynew.sol/erc20custodynew.go (1 hunks)
- pkg/contracts/prototypes/evm/receiver.sol/receiver.go (1 hunks)
- pkg/contracts/prototypes/zevm/gatewayzevm.sol/gatewayzevm.go (1 hunks)
- pkg/contracts/prototypes/zevm/sender.sol/sender.go (1 hunks)
- pkg/contracts/prototypes/zevm/zrc20new.sol/zrc20new.go (1 hunks)
- pkg/contracts/zevm/zrc20.sol/zrc20.go (1 hunks)
Files not summarized due to errors (4)
- pkg/contracts/prototypes/evm/receiver.sol/receiver.go: Error: Message exceeds token limit
- pkg/contracts/prototypes/zevm/gatewayzevm.sol/gatewayzevm.go: Error: Message exceeds token limit
- pkg/contracts/prototypes/zevm/zrc20new.sol/zrc20new.go: Error: Message exceeds token limit
- pkg/contracts/zevm/zrc20.sol/zrc20.go: Error: Message exceeds token limit
Additional comments not posted (25)
pkg/contracts/prototypes/zevm/sender.sol/sender.go (1)
261-262: Ensure proper error handling inWithdrawAndCallReceiver.The function handles errors well, but consider adding more context to the error messages for better debugging.
- return _Sender.contract.Transact(opts, "withdrawAndCallReceiver", receiver, amount, zrc20, str, num, flag) + return _Sender.contract.Transact(opts, "withdrawAndCallReceiver", receiver, amount, zrc20, str, num, flag)Likely invalid or redundant comment.
pkg/contracts/prototypes/evm/receiver.sol/receiver.go (10)
1-3: Header Comment: Verify the auto-generation tool.Ensure that the auto-generation tool used to create this file is up-to-date and correctly configured.
19-30: Reference Imports: Verify the usage of referenced imports.Ensure that the referenced imports are correctly used within the file to suppress errors.
46-61: Deployment Function: Ensure error handling and deployment logic.Verify that the
DeployReceiverfunction correctly handles errors and deploys the contract.
63-120: Struct Definitions: Verify the correctness of struct definitions.Ensure that the struct definitions for
Receiver,ReceiverCaller,ReceiverTransactor, andReceiverFiltererare correctly implemented.
85-120: Session Struct Definitions: Verify the correctness of session struct definitions.Ensure that the session struct definitions for
ReceiverSession,ReceiverCallerSession, andReceiverTransactorSessionare correctly implemented.
107-120: Raw Binding Struct Definitions: Verify the correctness of raw binding struct definitions.Ensure that the raw binding struct definitions for
ReceiverRaw,ReceiverCallerRaw, andReceiverTransactorRaware correctly implemented.
122-165: Contract Creation Functions: Ensure error handling and correctness.Verify that the functions for creating instances of the
Receivercontract and its bindings (NewReceiver,NewReceiverCaller,NewReceiverTransactor,NewReceiverFilterer, andbindReceiver) are correctly implemented and handle errors appropriately.
167-833: Method Bindings: Verify the correctness of method bindings.Ensure that the method bindings for contract methods (
Call,Transfer,Transact, etc.) and events (FilterReceivedERC20,WatchReceivedERC20,ParseReceivedERC20, etc.) are correctly implemented.
32-36: Metadata: Verify the contract metadata.Ensure that the metadata for the
Receivercontract is correctly defined and matches the Solidity contract.
6-17: Import Statements: Check for unused imports.Verify that all imported packages are necessary for the functionality of the file.
pkg/contracts/zevm/zrc20.sol/zrc20.go (10)
Line range hint
44-61: Verify the correctness of the DeployZRC20 function.Ensure that the function correctly handles errors and passes parameters for contract deployment.
Line range hint
629-631: Verify the correctness of the FilterWithdrawal function and event filtering logic.Ensure that the function correctly filters the
Withdrawalevent.
Line range hint
539-541: Verify the correctness of the FilterUpdatedProtocolFlatFee function and event filtering logic.Ensure that the function correctly filters the
UpdatedProtocolFlatFeeevent.Verification successful
Verification Successful:
FilterUpdatedProtocolFlatFeefunction and event filtering logic are correct.The
FilterUpdatedProtocolFlatFeefunction correctly filters theUpdatedProtocolFlatFeeevent as expected. The event is correctly referenced and handled in the codebase.
pkg/contracts/zevm/zrc20.sol/zrc20.go: The functionFilterUpdatedProtocolFlatFeeand the eventUpdatedProtocolFlatFeeare implemented correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the FilterUpdatedProtocolFlatFee function and event filtering logic. # Test: Check if the function correctly filters the UpdatedProtocolFlatFee event. go test -run '^TestFilterUpdatedProtocolFlatFee$' pkg/contracts/zevm/zrc20.solLength of output: 150
Script:
#!/bin/bash # Search for the definition of the FilterUpdatedProtocolFlatFee function in the codebase ast-grep --lang go --pattern 'func FilterUpdatedProtocolFlatFee($_) $_' # Search for the UpdatedProtocolFlatFee event in the codebase to see how it is used and filtered rg 'UpdatedProtocolFlatFee'Length of output: 58384
Line range hint
404-406: Verify the correctness of the FilterDeposit function and event filtering logic.Ensure that the function correctly filters the
Depositevent.
Line range hint
494-496: Verify the correctness of the FilterUpdatedGasLimit function and event filtering logic.Ensure that the function correctly filters the
UpdatedGasLimitevent.
Line range hint
584-586: Verify the correctness of the FilterUpdatedSystemContract function and event filtering logic.Ensure that the function correctly filters the
UpdatedSystemContractevent.Verification successful
Verification Successful: The
FilterUpdatedSystemContractfunction correctly filters theUpdatedSystemContractevent and handles errors appropriately.
- The function calls
_ZRC20.contract.FilterLogs(opts, "UpdatedSystemContract")to filter logs for theUpdatedSystemContractevent.- It checks for errors and returns
niland the error if any.- It returns a
ZRC20UpdatedSystemContractIteratorinitialized with the filtered logs and subscription.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the FilterUpdatedSystemContract function and event filtering logic. # Test: Check if the function correctly filters the UpdatedSystemContract event. go test -run '^TestFilterUpdatedSystemContract$' pkg/contracts/zevm/zrc20.solLength of output: 149
Script:
#!/bin/bash # Locate the implementation of the FilterUpdatedSystemContract function in the codebase ast-grep --lang go --pattern 'func FilterUpdatedSystemContract($_) $_' > result.txt cat result.txtLength of output: 89
Script:
#!/bin/bash # Use rg to search for the term FilterUpdatedSystemContract across the entire codebase rg 'FilterUpdatedSystemContract'Length of output: 3282
Script:
#!/bin/bash # Extract the implementation of the FilterUpdatedSystemContract function from pkg/contracts/zevm/zrc20.sol/zrc20.go ast-grep --lang go --pattern 'func (_ZRC20 *ZRC20Filterer) FilterUpdatedSystemContract(opts *bind.FilterOpts) (*ZRC20UpdatedSystemContractIterator, error) { $$$ }' pkg/contracts/zevm/zrc20.sol/zrc20.goLength of output: 924
Line range hint
359-361: Verify the correctness of the FilterApproval function and event filtering logic.Ensure that the function correctly filters the
Approvalevent.
Line range hint
449-451: Verify the correctness of the FilterTransfer function and event filtering logic.Ensure that the function correctly filters the
Transferevent.Verification successful
Verification of the FilterTransfer function and event filtering logic
The
FilterTransferfunction in thepkg/contracts/zevm/zrc20.sol/zrc20.gofile correctly filters theTransferevent by setting up filtering rules for thefromandtoaddresses, retrieving logs that match these rules, and returning an iterator for the logs.
- File:
pkg/contracts/zevm/zrc20.sol/zrc20.go- Lines: 1176-1192
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the FilterTransfer function and event filtering logic. # Test: Check if the function correctly filters the Transfer event. go test -run '^TestFilterTransfer$' pkg/contracts/zevm/zrc20.solLength of output: 136
Script:
#!/bin/bash # Find the implementation of the FilterTransfer function in the codebase ast-grep --lang go --pattern 'func ($_ *$_) FilterTransfer($_) ($_)' # Search for test cases related to the FilterTransfer function rg 'FilterTransfer' --type go --testsLength of output: 540
Script:
#!/bin/bash # Find the implementation of the FilterTransfer function in the codebase ast-grep --lang go --pattern 'func ($_ *$_) FilterTransfer($_) ($_)' # Search for test cases related to the FilterTransfer function rg 'FilterTransfer' --type goLength of output: 14618
Script:
#!/bin/bash # Display the implementation of the FilterTransfer function in the specified file ast-grep --lang go --pattern 'func (_ZRC20 *ZRC20Filterer) FilterTransfer(opts *bind.FilterOpts, from []common.Address, to []common.Address) (*ZRC20TransferIterator, error)' pkg/contracts/zevm/zrc20.sol/zrc20.goLength of output: 1513
35-35: Verify the correctness of the ABI and binary data.Ensure that the ABI and binary data are correctly generated and correspond to the Solidity contract.
Line range hint
64-74: Verify the correctness and completeness of the ZRC20 struct and its methods.Ensure that the struct and its methods are correctly defined and complete.
Verification successful
ZRC20 struct is correctly defined and complete.
The
ZRC20struct inpkg/contracts/zevm/zrc20.sol/zrc20.gois correctly defined with the necessary fields (ZRC20Caller,ZRC20Transactor, andZRC20Filterer). There are no associated methods using the(z *ZRC20)receiver pattern, which is consistent with its intended use as a binding for the Ethereum contract.
pkg/contracts/zevm/zrc20.sol/zrc20.go:
ZRC20struct definition: lines 64-68Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness and completeness of the ZRC20 struct and its methods. # Test: Check if the struct and its methods are correctly defined and complete. go test -run '^TestZRC20Methods$' pkg/contracts/zevm/zrc20.solLength of output: 134
Script:
#!/bin/bash # Description: Verify the correctness and completeness of the ZRC20 struct and its methods by inspecting the code. # Search for the ZRC20 struct definition ast-grep --lang go --pattern $'type ZRC20 struct { $$$ }' # Search for methods associated with the ZRC20 struct rg 'func (\(z \*ZRC20\))'Length of output: 487
Script:
#!/bin/bash # Description: Verify the correctness and completeness of the ZRC20 struct and its methods by inspecting the code. # Search for any methods associated with the ZRC20 struct using different receiver patterns rg 'func .*ZRC20\)' # Search for any usage patterns related to the ZRC20 struct rg 'ZRC20'Length of output: 293369
pkg/contracts/prototypes/zevm/zrc20new.sol/zrc20new.go (4)
845-882: LGTM!The code changes are approved.
124-127: Improve error handling message.The error message for
bindZRC20Newshould be more descriptive to indicate the context of the error.- return nil, err + return nil, fmt.Errorf("failed to bind ZRC20New contract: %w", err)Likely invalid or redundant comment.
48-54: Improve error handling message.The error message for
parsed == nilshould be more descriptive to indicate the context of the error.- return common.Address{}, nil, nil, errors.New("GetABI returned nil") + return common.Address{}, nil, nil, errors.New("ZRC20NewMetaData.GetABI returned nil")Likely invalid or redundant comment.
151-153: Improve error handling message.The error message for
bindZRC20Newshould be more descriptive to indicate the context of the error.- return nil, err + return nil, fmt.Errorf("failed to bind ZRC20NewFilterer contract: %w", err)Likely invalid or redundant comment.
Initialize a prototypes directory in contracts and tests for new prototypes development.
Prototypes is initialize with a very trivial gateway contract.
Later these smart contracts can be moved to a new location depending on the plan moving forward.
Close: #159
Summary by CodeRabbit
New Features
withdrawandwithdrawAndCallcapabilities.GatewayEVMandGatewayZEVMcontracts for cross-chain smart contract interactions and token transfers.ReceiverandTestERC20contracts for handling ERC20 token interactions.TestZContractfor cross-chain call handling.ZRC20Newcontract with enhanced token transfer functionalities.Bug Fixes
Documentation