feat: implement message context v2 #590
Conversation
📝 WalkthroughWalkthroughIntroduces a richer MessageContext (sender, asset, amount), adds LegacyMessageContext for backward compatibility, updates interfaces and contracts to branch between legacy and V2 onCall paths, extends spoofing checks, and adds a versioned withdrawAndCall V2 flow and event on ZEVM. Tests are updated to cover both legacy and V2 behaviors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant GatewayEVM as GatewayEVM
participant Dest as Destination Contract
note over GatewayEVM: Authenticated call execution
User->>GatewayEVM: execute(messageContext{sender, asset, amount}, destination, message)
alt amount == 0 (legacy path)
note right of GatewayEVM: Wrap sender in LegacyMessageContext
GatewayEVM->>Dest: Callable.onCall(LegacyMessageContext{sender}, message)
else amount > 0 (V2 path)
GatewayEVM->>Dest: CallableV2.onCall(MessageContext{sender, asset, amount}, message)
end
rect rgba(240,240,255,0.6)
note over GatewayEVM: Spoof guard
GatewayEVM-->>GatewayEVM: reject if calldata matches onCall selector<br/>(Callable or CallableV2)
end
sequenceDiagram
autonumber
actor User
participant GatewayZEVM as GatewayZEVM
participant ZRC20 as ZRC20 Token
note over GatewayZEVM: withdrawAndCall V2
User->>GatewayZEVM: withdrawAndCall(receiver, amount, zrc20, message, version, callOptions, revertOptions)
GatewayZEVM-->>GatewayZEVM: validate receiver, amount, gasLimit, message size
GatewayZEVM->>ZRC20: _withdrawZRC20WithGasLimit(zrc20, amount, callOptions.gasLimit)
ZRC20-->>GatewayZEVM: gasFee, protocolFlatFee
GatewayZEVM-->>User: emit WithdrawnAndCalledV2(..., version, callOptions, revertOptions)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Seems there is an issue for the bytecode generation |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
contracts/evm/GatewayEVM.sol (1)
8-8: Consider selective imports for clarity.The change from selective imports to a general import increases the compilation scope. While functionally correct, selective imports (e.g.,
import { Callable, CallableV2, IGatewayEVM, LegacyMessageContext, MessageContext } from "./interfaces/IGatewayEVM.sol";) improve clarity about what's used.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
contracts/evm/ERC20Custody.sol(1 hunks)contracts/evm/GatewayEVM.sol(3 hunks)contracts/evm/Registry.sol(1 hunks)contracts/evm/ZetaConnectorBase.sol(1 hunks)contracts/evm/interfaces/IERC20Custody.sol(1 hunks)contracts/evm/interfaces/IGatewayEVM.sol(1 hunks)contracts/zevm/GatewayZEVM.sol(1 hunks)contracts/zevm/interfaces/IGatewayZEVM.sol(2 hunks)test/ERC20Custody.t.sol(2 hunks)test/GatewayEVM.t.sol(5 hunks)test/GatewayEVMZEVM.t.sol(1 hunks)test/GatewayZEVM.t.sol(1 hunks)test/Registry.t.sol(19 hunks)test/ZetaConnectorNative.t.sol(2 hunks)test/ZetaConnectorNonNative.t.sol(2 hunks)test/utils/IReceiverEVM.sol(1 hunks)test/utils/ReceiverEVM.sol(3 hunks)test/utils/upgrades/ERC20CustodyUpgradeTest.sol(1 hunks)test/utils/upgrades/GatewayEVMUpgradeTest.sol(2 hunks)test/utils/upgrades/GatewayZEVMUpgradeTest.sol(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
test/**
⚙️ CodeRabbit configuration file
Review the test files for proper coverage, edge cases, and best practices.
Files:
test/GatewayEVMZEVM.t.soltest/Registry.t.soltest/GatewayEVM.t.soltest/GatewayZEVM.t.soltest/ZetaConnectorNative.t.soltest/utils/upgrades/ERC20CustodyUpgradeTest.soltest/utils/ReceiverEVM.soltest/utils/upgrades/GatewayEVMUpgradeTest.soltest/ERC20Custody.t.soltest/ZetaConnectorNonNative.t.soltest/utils/IReceiverEVM.soltest/utils/upgrades/GatewayZEVMUpgradeTest.sol
contracts/**
⚙️ CodeRabbit configuration file
Review the Solidity contracts for security vulnerabilities and best practices.
Files:
contracts/evm/interfaces/IGatewayEVM.solcontracts/zevm/interfaces/IGatewayZEVM.solcontracts/evm/ERC20Custody.solcontracts/evm/Registry.solcontracts/evm/interfaces/IERC20Custody.solcontracts/evm/ZetaConnectorBase.solcontracts/evm/GatewayEVM.solcontracts/zevm/GatewayZEVM.sol
🪛 GitHub Actions: Lint TS/JS/Sol
test/GatewayEVMZEVM.t.sol
[error] 53-54: Formatting mismatch detected: MessageContext arbitraryCallMessageContext initialization formatting.
test/GatewayEVM.t.sol
[error] 42-43: Formatting mismatch detected: MessageContext arbitraryCallMessageContext initialization formatting.
contracts/evm/interfaces/IGatewayEVM.sol
[error] 230-236: Formatting mismatch detected: onCall signature formatting (multi-line vs single-line).
Expected consistent formatting for function onCall.
test/GatewayZEVM.t.sol
[error] 358-360: Formatting mismatch detected: withdrawAndCall call formatting (single-line vs multi-line).
contracts/evm/ERC20Custody.sol
[error] 5-5: Formatting mismatch detected: spacing in import statement for IGatewayEVM.sol.
test/ZetaConnectorNative.t.sol
[error] 44-45: Formatting mismatch detected: MessageContext arbitraryCallMessageContext initialization formatting.
test/utils/upgrades/ERC20CustodyUpgradeTest.sol
[error] 5-5: Formatting mismatch detected: import line spacing around IGatewayEVM and MessageContext.
test/utils/ReceiverEVM.sol
[error] 5-5: Formatting mismatch detected: spacing in import of MessageContext from IGatewayEVM.sol.
contracts/evm/interfaces/IERC20Custody.sol
[error] 6-6: Formatting mismatch detected: spacing in import of MessageContext from IGatewayEVM.sol.
test/ERC20Custody.t.sol
[error] 38-39: Formatting mismatch detected: MessageContext arbitraryCallMessageContext initialization formatting.
test/ZetaConnectorNonNative.t.sol
[error] 44-45: Formatting mismatch detected: MessageContext arbitraryCallMessageContext initialization formatting.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: slither
- GitHub Check: generate
- GitHub Check: test
🔇 Additional comments (24)
contracts/evm/Registry.sol (1)
70-73: LGTM!The parameter type update from
MessageContexttoLegacyMessageContextcorrectly aligns with the versioning strategy introduced in this PR. The function continues to validate the sender and execute the self-call pattern as expected.test/ZetaConnectorNonNative.t.sol (1)
204-214: LGTM! Updated to V2 authenticated call flow.The test correctly:
- Expects the new
ReceivedOnCallV2event with asset and amount parameters- Passes an expanded
MessageContextwith sender, asset (zetaToken), and amount towithdrawAndCallThis aligns with the V2 authenticated call implementation where non-zero amounts trigger the CallableV2 path.
test/utils/upgrades/GatewayEVMUpgradeTest.sol (2)
449-456: LGTM! Correct dual-path routing implementation.The function correctly routes authenticated calls based on
messageContext.amount:
- When
amount == 0: Calls legacyCallable.onCallwithLegacyMessageContext(sender only)- When
amount > 0: CallsCallableV2.onCallwith fullMessageContext(sender, asset, amount)This versioning strategy maintains backward compatibility while enabling V2 receivers to access asset and amount information.
466-468: LGTM! Expanded anti-spoofing protection.The revert guard now correctly prevents spoofing of both
Callable.onCall.selectorandCallableV2.onCall.selector, ensuring that only the gateway can invoke authenticated call handlers.contracts/zevm/interfaces/IGatewayZEVM.sol (2)
74-98: LGTM! Versioned event for V2 withdrawAndCall.The new
WithdrawnAndCalledV2event includes all fields from the original event plus aversionparameter, enabling the protocol to signal which MessageContext version to use on the destination chain. This aligns with the discussed approach per past review comments.
190-207: LGTM! Versioned withdrawAndCall overload.The new overload adds a
versionparameter, allowing callers to explicitly specify which MessageContext version should be used on the destination chain. This provides the flexibility needed for the dual-path routing introduced in this PR.test/utils/IReceiverEVM.sol (1)
45-48: LGTM! V2 event for extended MessageContext.The new
ReceivedOnCallV2event addsassetandamountparameters, supporting the V2 authenticated call flow while maintaining backward compatibility by retaining the originalReceivedOnCallevent.test/ERC20Custody.t.sol (1)
353-363: LGTM! Updated to V2 authenticated call flow.The test correctly:
- Expects the new
ReceivedOnCallV2event with sender, asset (token), and amount- Passes an expanded
MessageContextwith all three fields tocustody.withdrawAndCallThis validates the V2 authenticated call implementation in the custody contract.
test/utils/upgrades/GatewayZEVMUpgradeTest.sol (1)
256-295: LGTM! Versioned withdrawAndCall for upgrade testing.The new overload correctly implements the versioned withdrawAndCall flow:
- Validates inputs (receiver, amount, gasLimit, message size)
- Computes gas fees via
_withdrawZRC20WithGasLimit- Emits
WithdrawnAndCalledV2with the version parameterThis mirrors the pattern of other
withdrawAndCalloverloads and enables testing of the V2 cross-chain call context.test/GatewayZEVM.t.sol (4)
341-347: LGTM!The test correctly validates that the V2 withdrawAndCall rejects zero-address receivers.
361-367: LGTM!Properly validates the minimum gas limit constraint for V2 calls.
369-374: LGTM!Correctly validates that zero amounts are rejected for V2 withdrawAndCall.
376-402: LGTM!The test comprehensively validates the V2 withdrawAndCall flow including event emission and balance changes.
test/GatewayEVM.t.sol (4)
190-201: LGTM!The test correctly validates the legacy path using MessageContext with zero amount, which routes to LegacyMessageContext internally.
204-217: LGTM!The new test properly validates the V2 authenticated call flow with asset and amount, emitting the ReceivedOnCallV2 event.
287-295: LGTM!The test correctly validates that the V2 onCall selector is blocked to prevent spoofing, which is an important security measure.
242-244: LGTM!The MessageContext usage is correctly updated for arbitrary calls with zero asset and amount.
Also applies to: 318-318
test/ZetaConnectorNative.t.sol (1)
243-243: LGTM!The test correctly migrates to the V2 flow, passing asset and amount in MessageContext and expecting the ReceivedOnCallV2 event.
Also applies to: 248-253
test/Registry.t.sol (2)
14-19: LGTM!The MockGatewayEVM correctly uses LegacyMessageContext for the registry's legacy onCall path.
109-109: LGTM!All test methods correctly migrated to use LegacyMessageContext, maintaining consistency across the registry test suite.
Also applies to: 120-120, 128-128, 139-139, 177-177, 199-199, 220-220, 237-237, 254-254, 282-282, 302-302, 341-341, 372-372, 413-413, 437-437, 461-461, 486-486, 519-519, 531-531
contracts/evm/interfaces/IGatewayEVM.sol (1)
237-252: LGTM!The new MessageContext and CallableV2 interface cleanly extend the API to support asset-aware authenticated calls while maintaining backward compatibility with the legacy path.
contracts/evm/GatewayEVM.sol (2)
472-472: LGTM!The anti-spoofing check correctly extends to cover both Callable.onCall and CallableV2.onCall selectors, preventing spoofing across both call paths.
455-461: Verify implicit amount-based routing
Theamount == 0check distinguishes legacy vs V2 onCall but could misroute calls whereasset != address(0)andamount == 0. Confirm TSS never sends that combination or introduce an explicitversionflag (or boolean) inMessageContextto control routing.test/utils/ReceiverEVM.sol (1)
88-98: LGTM!The new onCall overload correctly implements the CallableV2 interface, emitting ReceivedOnCallV2 with asset and amount information.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
test/utils/upgrades/ERC20CustodyUpgradeTest.sol (3)
128-143: Signature mismatch with IERC20Custody.withdraw — will not compile.Your contract claims to implement IERC20Custody but parameter order differs.
-function withdraw( - address to, - address token, - uint256 amount -) +function withdraw( + address token, + address to, + uint256 amount +) external nonReentrant onlyRole(WITHDRAWER_ROLE) whenNotPaused { - if (!whitelisted[token]) revert NotWhitelisted(); - IERC20(token).safeTransfer(to, amount); - emit WithdrawnV2(to, token, amount); + if (!whitelisted[token]) revert NotWhitelisted(); + IERC20(token).safeTransfer(to, amount); + emit WithdrawnV2(to, token, amount); }
153-173: Signature mismatch with IERC20Custody.withdrawAndCall — will not compile.Order must be (messageContext, token, to, ...).
-function withdrawAndCall( - MessageContext calldata messageContext, - address to, - address token, - uint256 amount, - bytes calldata data -) +function withdrawAndCall( + MessageContext calldata messageContext, + address token, + address to, + uint256 amount, + bytes calldata data +) public nonReentrant onlyRole(WITHDRAWER_ROLE) whenNotPaused { if (!whitelisted[token]) revert NotWhitelisted(); IERC20(token).safeTransfer(address(gateway), amount); gateway.executeWithERC20(messageContext, token, to, amount, data); emit WithdrawnAndCalled(to, token, amount, data); }
183-205: Signature mismatch with IERC20Custody.withdrawAndRevert — will not compile.Order must be (token, to, ...).
-function withdrawAndRevert( - address to, - address token, - uint256 amount, - bytes calldata data, - RevertContext calldata revertContext -) +function withdrawAndRevert( + address token, + address to, + uint256 amount, + bytes calldata data, + RevertContext calldata revertContext +) public nonReentrant onlyRole(WITHDRAWER_ROLE) whenNotPaused { if (!whitelisted[token]) revert NotWhitelisted(); IERC20(token).safeTransfer(address(gateway), amount); gateway.revertWithERC20(token, to, amount, data, revertContext); emit WithdrawnAndReverted(to, token, amount, data, revertContext); }test/GatewayEVMZEVM.t.sol (1)
1-1: Runforge fmtto fix formatting.The pipeline reports formatting differences in this file.
test/utils/ReceiverEVM.sol (1)
1-1: Runforge fmtto fix formatting.The pipeline reports formatting differences in this file.
test/ZetaConnectorNonNative.t.sol (1)
1-1: Runforge fmtto fix formatting.The pipeline reports formatting differences in this file.
test/GatewayEVM.t.sol (1)
1-1: Runforge fmtto fix formatting.The pipeline reports formatting differences in this file.
🧹 Nitpick comments (5)
test/utils/IReceiverEVM.sol (1)
45-48: Complete NatSpec for new event (missing asset/amount docs).Document all parameters to avoid ambiguity and tools warnings. Consider indexing sender/asset for log filtering.
- /// @notice Emitted when onCall function is called with new MessageContext call. - /// @param sender Message context sender. - /// @param message Message received. - event ReceivedOnCallV2(address sender, address asset, uint256 amount, bytes message); + /// @notice Emitted when onCall is called with the new MessageContext (sender, asset, amount). + /// @param sender Message context sender. + /// @param asset Asset address (zero address if native). + /// @param amount Asset amount delivered. + /// @param message Message received. + event ReceivedOnCallV2(address sender, address asset, uint256 amount, bytes message);contracts/evm/interfaces/IGatewayEVM.sol (1)
119-131: Update NatSpec for MessageContext fields (sender/asset/amount).Docs still mention “arbitrary call flag.” Align comments with new struct fields.
-/// @param messageContext Message context containing sender and arbitrary call flag. +/// @param messageContext Message context (sender, asset, amount).Also applies to: 146-160
test/utils/upgrades/ERC20CustodyUpgradeTest.sol (1)
5-5: Fix formatting (forge fmt).Space before closing brace is non-standard. Run
forge fmtto satisfy CI.contracts/evm/interfaces/IERC20Custody.sol (1)
76-83: Update NatSpec for MessageContext (now includes asset and amount).Align docs with struct fields.
- /// @param messageContext Message context containing sender. + /// @param messageContext Message context (sender, asset, amount).contracts/evm/ZetaConnectorBase.sol (1)
114-126: Refresh NatSpec for MessageContext to reflect asset/amount.Keep comments consistent with new struct.
- /// @param messageContext Message context containing sender. + /// @param messageContext Message context (sender, asset, amount).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
contracts/evm/ERC20Custody.sol(1 hunks)contracts/evm/GatewayEVM.sol(3 hunks)contracts/evm/Registry.sol(1 hunks)contracts/evm/ZetaConnectorBase.sol(1 hunks)contracts/evm/interfaces/IERC20Custody.sol(1 hunks)contracts/evm/interfaces/IGatewayEVM.sol(1 hunks)contracts/zevm/GatewayZEVM.sol(1 hunks)contracts/zevm/interfaces/IGatewayZEVM.sol(2 hunks)test/ERC20Custody.t.sol(2 hunks)test/GatewayEVM.t.sol(5 hunks)test/GatewayEVMZEVM.t.sol(1 hunks)test/GatewayZEVM.t.sol(1 hunks)test/Registry.t.sol(19 hunks)test/ZetaConnectorNative.t.sol(2 hunks)test/ZetaConnectorNonNative.t.sol(2 hunks)test/utils/IReceiverEVM.sol(1 hunks)test/utils/ReceiverEVM.sol(3 hunks)test/utils/upgrades/ERC20CustodyUpgradeTest.sol(1 hunks)test/utils/upgrades/GatewayEVMUpgradeTest.sol(2 hunks)test/utils/upgrades/GatewayZEVMUpgradeTest.sol(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
contracts/**
⚙️ CodeRabbit configuration file
Review the Solidity contracts for security vulnerabilities and best practices.
Files:
contracts/evm/Registry.solcontracts/evm/interfaces/IERC20Custody.solcontracts/evm/interfaces/IGatewayEVM.solcontracts/evm/ERC20Custody.solcontracts/zevm/interfaces/IGatewayZEVM.solcontracts/evm/ZetaConnectorBase.solcontracts/evm/GatewayEVM.solcontracts/zevm/GatewayZEVM.sol
test/**
⚙️ CodeRabbit configuration file
Review the test files for proper coverage, edge cases, and best practices.
Files:
test/GatewayZEVM.t.soltest/utils/IReceiverEVM.soltest/utils/upgrades/GatewayEVMUpgradeTest.soltest/ZetaConnectorNonNative.t.soltest/utils/upgrades/GatewayZEVMUpgradeTest.soltest/ERC20Custody.t.soltest/Registry.t.soltest/ZetaConnectorNative.t.soltest/utils/upgrades/ERC20CustodyUpgradeTest.soltest/GatewayEVM.t.soltest/GatewayEVMZEVM.t.soltest/utils/ReceiverEVM.sol
🪛 GitHub Actions: Lint TS/JS/Sol
test/GatewayZEVM.t.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
test/ZetaConnectorNonNative.t.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
test/ERC20Custody.t.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
contracts/evm/interfaces/IERC20Custody.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
contracts/evm/interfaces/IGatewayEVM.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
test/ZetaConnectorNative.t.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
test/utils/upgrades/ERC20CustodyUpgradeTest.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
contracts/evm/ERC20Custody.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
test/GatewayEVM.t.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
test/GatewayEVMZEVM.t.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
test/utils/ReceiverEVM.sol
[error] 1-1: forge fmt --check reported formatting differences. Run 'forge fmt' to fix code style issues in this file.
🔇 Additional comments (28)
contracts/evm/interfaces/IGatewayEVM.sol (1)
1-1: Run forge fmt to fix lint failure.CI reported formatting diffs. Run
forge fmtto fix.contracts/evm/interfaces/IERC20Custody.sol (1)
6-6: Run forge fmt to resolve formatting diffs.CI flagged formatting differences.
test/GatewayEVMZEVM.t.sol (1)
53-54: LGTM! MessageContext expanded correctly.The MessageContext initialization now includes
assetandamountfields, properly set to zero values to indicate arbitrary calls. This aligns with the V2 message context structure.test/utils/ReceiverEVM.sol (3)
5-5: LGTM! Import updated for dual context support.The import correctly includes both
LegacyMessageContextandMessageContextto support the legacy and V2 onCall paths.
76-86: LGTM! Legacy onCall path preserved correctly.The signature update to
LegacyMessageContextcorrectly maintains backward compatibility for the legacy message context containing only sender information.
88-98: LGTM! V2 onCall path implemented correctly.The new
onCalloverload correctly accepts the enrichedMessageContext(including asset and amount) and emits theReceivedOnCallV2event with all context fields. Function overloading is properly used to support both legacy and V2 paths.contracts/evm/GatewayEVM.sol (2)
455-461: Verify that zero-amount V2 calls are not a valid use case.The conditional logic uses
amount == 0to discriminate between legacy and V2 paths. This creates an implicit constraint: V2 calls withamount=0will be routed to the legacy path, preventing contracts from accessing theassetfield even when the asset context is meaningful (e.g., gas token on source chain, or simply knowing which asset was involved even if the amount is zero).Consider whether:
- Zero-amount V2 calls are a valid use case that should be supported
- A more explicit discriminator (e.g., a boolean flag in MessageContext) would be clearer and more flexible
- The current design adequately covers all intended use cases
This relates to the past review comment at line 455 questioning whether a boolean flag might be preferable.
472-472: LGTM! Spoofing guard correctly extended.The guard now prevents spoofing of both
Callable.onCallandCallableV2.onCallselectors, ensuring arbitrary calls cannot impersonate authenticated calls on either path.test/utils/upgrades/GatewayEVMUpgradeTest.sol (2)
449-455: Same concern as GatewayEVM.sol: verify zero-amount V2 calls.This upgrade test contract mirrors the production logic that discriminates between legacy and V2 paths based on
amount == 0. The same verification concern applies here: ensure that zero-amount V2 calls are not a valid use case, or consider a more explicit discriminator.
466-466: LGTM! Spoofing guard correctly extended in upgrade test.The guard correctly prevents spoofing of both
Callable.onCallandCallableV2.onCallselectors, consistent with the production contract.test/ZetaConnectorNonNative.t.sol (3)
44-45: LGTM! MessageContext expanded correctly.The MessageContext initialization includes
assetandamountfields with zero values, correctly indicating arbitrary calls.
204-204: LGTM! V2 event expectation correct.The test correctly expects
ReceivedOnCallV2event with all MessageContext fields (sender, asset, amount, message).
209-214: LGTM! V2 MessageContext passed correctly.The test correctly constructs and passes the enriched
MessageContextwith sender, asset (zetaToken), and amount to exercise the V2 onCall path.test/Registry.t.sol (2)
14-19: LGTM! MockGatewayEVM correctly uses LegacyMessageContext.The mock gateway and its event signature correctly use
LegacyMessageContext, aligning with the Registry's use of the legacy context path (sender-only).
109-113: LGTM! Test correctly uses LegacyMessageContext.All test functions consistently use
LegacyMessageContextfor Registry interactions, correctly reflecting that Registry operates on the legacy context path.test/GatewayEVM.t.sol (5)
42-43: LGTM! MessageContext expanded correctly.The MessageContext initialization includes
assetandamountfields with zero values, correctly indicating arbitrary calls.
191-201: LGTM! Legacy onCall path tested correctly.The test correctly exercises the legacy path by passing
MessageContextwithamount: 0, which triggers theLegacyMessageContextcode path and expects theReceivedOnCallevent.
204-217: LGTM! V2 onCall path tested correctly.The test correctly exercises the V2 path by passing
MessageContextwith non-zeroamount: 100andasset, which triggers theCallableV2code path and expects theReceivedOnCallV2event with all context fields.
287-295: LGTM! V2 spoofing protection tested correctly.The test correctly verifies that arbitrary calls cannot directly invoke the
CallableV2.onCallselector, ensuring the spoofing guard works for both legacy and V2 paths.
242-244: LGTM! MessageContext usage consistent.The MessageContext is correctly constructed with
amount: 0for legacy path testing.test/ERC20Custody.t.sol (3)
38-39: LGTM! MessageContext initialization aligns with V2 structure.The arbitraryCallMessageContext is correctly initialized with the new MessageContext shape including sender, asset, and amount fields. This setup provides a neutral context for arbitrary calls throughout the test suite.
357-363: LGTM! MessageContext construction is complete and consistent.The withdrawAndCall invocation correctly constructs a MessageContext with all required fields (sender, asset, amount), matching the V2 event expectations and enabling the receiver to access token metadata without parsing the message payload.
353-353: All test assertions already align with the ReceivedOnCallV2 signature The interface’s(address sender, address asset, uint256 amount, bytes message)matches the test’semit ReceivedOnCallV2(sender, address(token), amount, message).test/GatewayZEVM.t.sol (2)
341-374: LGTM! Comprehensive V2 failure scenario coverage.The new V2 test functions thoroughly validate failure paths:
- Zero receiver address
- Message size exceeded
- Gas limit below minimum
- Zero amount
These tests mirror the existing test patterns and ensure the V2 path maintains the same validation logic as the original implementation.
376-402: LGTM! V2 success path properly validated.The test correctly:
- Validates WithdrawnAndCalledV2 event emission with all fields including version
- Verifies balance changes (owner balance decreased by amount + gas fee)
- Uses the new MessageContext shape throughout
The test provides complete end-to-end validation of the V2 withdrawAndCall flow.
contracts/evm/Registry.sol (1)
71-71: LGTM! Signature change maintains backward compatibility.The change from
MessageContexttoLegacyMessageContextcorrectly aligns the Registry with the legacy path while allowing new contracts to use the richer MessageContext with asset and amount fields. This preserves compatibility with existing deployed Registry instances while the gateway handles routing between legacy and V2 paths.contracts/zevm/interfaces/IGatewayZEVM.sol (1)
74-98: LGTM! Interface additions are well-structured and documented.The new WithdrawnAndCalledV2 event and withdrawAndCall overload are properly defined with:
- Clear documentation explaining the version parameter
- Consistent field naming with the original event/function
- Backward compatibility maintained by keeping the original definitions
The additions enable versioned message context support while preserving the existing API surface.
Also applies to: 190-207
test/ZetaConnectorNative.t.sol (1)
44-45: LGTM! V2 MessageContext integration is complete and correct.The test file has been properly updated to work with the V2 MessageContext:
- arbitraryCallMessageContext initialization includes asset and amount (lines 44-45)
- Event expectation changed to ReceivedOnCallV2 with token address and amount (line 243)
- withdrawAndCall invocation passes complete MessageContext (lines 247-253)
These changes align with the V2 event shape and enable receivers to access token metadata.
Also applies to: 243-243, 247-253
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/GatewayZEVM.t.sol (1)
143-163: Fix pre-balance token in Solana test.Using zrc20 for ownerBalanceBefore while withdrawing solanaZRC20 is brittle; use solanaZRC20 for both.
Apply:
- uint256 ownerBalanceBefore = zrc20.balanceOf(owner); + uint256 ownerBalanceBefore = solanaZRC20.balanceOf(owner);
♻️ Duplicate comments (1)
test/GatewayZEVM.t.sol (1)
358-360: Formatting: withdrawAndCall V2 invocation.The linter note about formatting this call still applies; re-run the formatter.
🧹 Nitpick comments (2)
test/ZetaConnectorNative.t.sol (1)
44-45: V2 MessageContext and ReceivedOnCallV2 usage looks good.Minor nit: rename testWithdrawAndCallReceiveOnCallTNotAllowed... to remove stray “T”.
Also applies to: 242-252
test/utils/ReceiverEVM.sol (1)
5-5: Overloaded onCall (legacy + v2) is well-formed; tiny clarity tweak.Return bytes("") for explicit type.
- return ""; + return bytes("");And apply similarly in the V2 overload.
Also applies to: 76-87, 88-98
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
contracts/evm/interfaces/IGatewayEVM.sol(1 hunks)test/ERC20Custody.t.sol(2 hunks)test/GatewayEVM.t.sol(5 hunks)test/GatewayEVMZEVM.t.sol(1 hunks)test/GatewayZEVM.t.sol(1 hunks)test/Registry.t.sol(19 hunks)test/ZetaConnectorNative.t.sol(2 hunks)test/ZetaConnectorNonNative.t.sol(2 hunks)test/utils/ReceiverEVM.sol(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- test/GatewayEVM.t.sol
- test/GatewayEVMZEVM.t.sol
- test/ZetaConnectorNonNative.t.sol
🧰 Additional context used
📓 Path-based instructions (2)
test/**
⚙️ CodeRabbit configuration file
Review the test files for proper coverage, edge cases, and best practices.
Files:
test/GatewayZEVM.t.soltest/ERC20Custody.t.soltest/utils/ReceiverEVM.soltest/Registry.t.soltest/ZetaConnectorNative.t.sol
contracts/**
⚙️ CodeRabbit configuration file
Review the Solidity contracts for security vulnerabilities and best practices.
Files:
contracts/evm/interfaces/IGatewayEVM.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: slither
- GitHub Check: test
- GitHub Check: generate
🔇 Additional comments (3)
test/Registry.t.sol (1)
14-22: LegacyMessageContext migration in tests looks consistent.Event and onCall signature updated to LegacyMessageContext; context construction/usage across tests aligns with the legacy path.
Also applies to: 115-121, 126-131, 134-139, 145-151
test/ERC20Custody.t.sol (1)
38-39: MessageContext V2 usage and events are correct.Struct init formatting fixed; V2 onCall flow and ReceivedOnCallV2 assertion look good.
Also applies to: 352-362
contracts/evm/interfaces/IGatewayEVM.sol (1)
115-132: Move struct and Callable interfaces before IGatewayEVM; update NatSpec
ReorderLegacyMessageContext,Callable,MessageContext, andCallableV2to precedeinterface IGatewayEVM(affects lines 115–132, 152–160, 222–252) and adjust NatSpec to reflect(sender, asset, amount)in V2. Re-run build/lint to confirm ABI stability.
Sherlock AI FindingsThe automated tool completed its analysis of the codebase and found no potential security issues. Next Steps: No immediate actions are required. Continue monitoring the codebase with future scans. Full report available at: https://ai.sherlock.xyz/runs/c6c0a95b-c9aa-47fa-ab2e-30a6ee08b940 |
3 participants
Add asset and amount information to the onCall hook on connected chains.
Closes: #426
Summary by CodeRabbit
New Features
Bug Fixes
Tests