refactor: set sender and senderEVM in Message context#3700
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis update introduces several changes to improve the clarity and functionality of cross-chain call message handling. The changelog now includes a refactor section, and multiple smart contracts (Example, TestDAppV2, and others) have been updated to rename parameters (e.g., Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test
participant U as Utils (Validation)
participant C as Smart Contract
T->>U: Call MustHaveCalledExampleContract(..., sender)
U->>C: Invoke contract.LastSender(&bind.CallOpts{})
C-->>U: Return actual sender (bytes)
U->>T: Validate expected sender matches actual sender
Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3700 +/- ##
========================================
Coverage 64.46% 64.46%
========================================
Files 462 462
Lines 32699 32699
========================================
Hits 21080 21080
Misses 10657 10657
Partials 962 962
🚀 New features to boost your workflow:
|
lumtis
added
UPGRADE_TESTS
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
e2e/contracts/testdappv2/TestDAppV2.sol (3)
89-90: Use cautious naming for two sender mappings.
Having two mappings (senderWithMessageZEVMandsenderWithMessage) could introduce confusion. Consider clarifying naming or documenting the distinction to future maintainers.
113-115: Check internal write operations for error conditions.
setSenderWithMessageupdates storage but does not revert on failure. Ensure that potential input issues (e.g., empty messages) are not inadvertently masked.
125-128: Validate payload usage in 'getSenderWithMessage'.
Ensure downstream code interprets the returnedbytescorrectly. If a known format (e.g., compressed addresses) is expected, add clear documentation to avoid confusion.e2e/contracts/testdappv2/TestDAppV2.go (2)
442-458: Validate 'GetSenderWithMessage' output correctness.
Exposing the raw[]bytecan be beneficial for flexible usage. Consider clarifying usage of this data in docstrings if it expects an address, public key, or other format.
535-564: Enhance usage of 'SenderWithMessageZEVM'.
This function provides cross-chain metadata. Consider adding revert checks or data validation if any assumptions about the[]byteformat are crucial.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
e2e/contracts/example/Example.binis excluded by!**/*.bine2e/contracts/testdappv2/TestDAppV2.binis excluded by!**/*.bingo.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
changelog.md(1 hunks)e2e/contracts/example/Example.abi(2 hunks)e2e/contracts/example/Example.go(2 hunks)e2e/contracts/example/Example.json(3 hunks)e2e/contracts/example/Example.sol(2 hunks)e2e/contracts/testdappv2/TestDAppV2.abi(4 hunks)e2e/contracts/testdappv2/TestDAppV2.go(4 hunks)e2e/contracts/testdappv2/TestDAppV2.json(5 hunks)e2e/contracts/testdappv2/TestDAppV2.sol(5 hunks)e2e/e2etests/e2etests.go(8 hunks)e2e/e2etests/legacy/test_eth_deposit_call.go(1 hunks)e2e/e2etests/test_bitcoin_deposit_call.go(1 hunks)e2e/e2etests/test_bitcoin_std_deposit_and_call.go(1 hunks)e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go(2 hunks)e2e/e2etests/test_solana_deposit_call.go(1 hunks)e2e/e2etests/test_spl_deposit_and_call.go(1 hunks)e2e/e2etests/test_sui_deposit_and_call.go(1 hunks)e2e/e2etests/test_ton_deposit_and_call.go(1 hunks)e2e/runner/bitcoin.go(2 hunks)e2e/utils/contracts.go(4 hunks)go.mod(1 hunks)x/fungible/keeper/deposits.go(2 hunks)x/fungible/keeper/evm_gateway.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to ...
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_bitcoin_deposit_call.goe2e/e2etests/legacy/test_eth_deposit_call.goe2e/e2etests/test_sui_deposit_and_call.goe2e/e2etests/test_ton_deposit_and_call.goe2e/e2etests/test_spl_deposit_and_call.gox/fungible/keeper/deposits.goe2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.goe2e/e2etests/test_solana_deposit_call.goe2e/utils/contracts.goe2e/e2etests/test_bitcoin_std_deposit_and_call.goe2e/runner/bitcoin.gox/fungible/keeper/evm_gateway.goe2e/e2etests/e2etests.goe2e/contracts/example/Example.goe2e/contracts/testdappv2/TestDAppV2.go
🔇 Additional comments (46)
go.mod (1)
53-53: Dependency version updated correctlyThe update to the
github.com/zeta-chain/protocol-contractsdependency is correctly implemented, moving from the February version to a newer March release. This update aligns with the refactoring of the Message context to include bothsenderandsenderEVMfields.e2e/e2etests/test_bitcoin_deposit_call.go (1)
44-49: Function signature update correctly implementedThe function call to
MustHaveCalledExampleContracthas been properly updated to include the Bitcoin deployer address as an additional parameter. This change is consistent with the refactoring effort to provide sender information in the Message context for verification.e2e/e2etests/legacy/test_eth_deposit_call.go (1)
58-58: Updated function call with appropriate EVM address parameterThe addition of
r.EVMAddress().Bytes()as the fourth parameter toMustHaveCalledExampleContractis consistent with the refactoring efforts to include sender information in cross-chain call contexts. This ensures that the Ethereum address of the sender is properly passed for verification.x/fungible/keeper/deposits.go (2)
11-12: Import correctly added for the new contextThe addition of the
gatewayzevm.solpackage import is appropriate for the context structure changes being implemented.
105-109: Refactored Message context structureThe context structure has been properly updated from
systemcontract.ZContexttogatewayzevm.MessageContextwith appropriate field modifications:
- Removed the
Originfield- Changed
Senderto use the raw bytes- Added a new
SenderEVMfield that contains the bytes converted to an Ethereum addressThis refactoring provides more clarity about the sender's identity across different blockchain environments, separating the raw bytes representation from the EVM-specific address format.
changelog.md (1)
15-17: Clear and well-formatted changelog entryThe new entry is properly formatted and provides a concise description of the changes made in PR #3700, following the established pattern in the changelog.
e2e/e2etests/test_ton_deposit_and_call.go (1)
49-49: Updated function call to include sender informationThe function call has been properly updated to include the sender's address as a byte array parameter, aligning with the refactoring of sender context in cross-chain calls.
e2e/e2etests/test_bitcoin_std_deposit_and_call.go (1)
49-54: Improved function call with sender informationThe function call has been updated to include the Bitcoin deployer address, and the formatting has been improved by breaking the call into multiple lines. This enhances both functionality and readability.
e2e/e2etests/test_sui_deposit_and_call.go (1)
41-49: Added sender verification for cross-chain callsThis code addition properly verifies that the sender address is correctly propagated through the cross-chain call mechanism. The implementation is clear with appropriate error handling and assertions.
e2e/contracts/example/Example.sol (3)
8-12: Good refactoring of the zContext struct.The renaming from
origin/sendertosender/senderEVMprovides much clearer semantics about what these fields represent. This change makes the code more intuitive by correctly identifying the sender's address in bytes format and its EVM-compatible representation.
16-16: Good addition of lastSender field.Adding this public variable complements the existing
lastMessagefield and allows tracking of the message sender, which is useful for verification purposes and improves the contract's observability.
55-55: Implementation correctly uses the renamed field.The function now assigns
context.sendertolastSender, which is consistent with the struct field renaming. This change properly maintains the contract's functionality while improving field naming clarity.e2e/runner/bitcoin.go (2)
324-324: Enhanced function return signature.Adding the receiver's encoded address to the return values improves function usability by providing critical information about the transaction destination without requiring callers to compute it separately.
365-365: Properly implemented the updated return signature.The implementation now correctly returns the additional information (receiver's encoded address) as promised in the function signature. This maintains the contract with callers of this function.
e2e/contracts/example/Example.abi (2)
66-78: Good ABI update for the new lastSender function.The ABI has been properly updated to include the new
lastSenderfunction, matching the contract implementation. The function signature is correctly defined with no inputs and a bytes output with view mutability.
85-86: Consistent parameter naming across ABI.The parameter names have been updated from
origintosenderand fromsendertosenderEVMin both function definitions, maintaining consistency with the contract implementation. This renaming provides better semantic clarity about the role of each parameter.Also applies to: 90-91, 130-131, 135-136
e2e/e2etests/test_spl_deposit_and_call.go (1)
55-61: Updated test to include sender information.The function call now includes the sender's public key as an additional parameter, which aligns with the contract changes where sender information is explicitly tracked. The multi-line format also improves readability.
e2e/e2etests/test_solana_deposit_call.go (1)
34-41: Updated function call to include sender verification.The function call has been updated to include the Solana public key as a sender parameter, which aligns with the contract's new sender verification mechanism. This ensures that the example contract correctly records and verifies the origin of cross-chain calls.
e2e/contracts/example/Example.json (4)
67-79: New function added for sender verification.The addition of the
lastSenderfunction provides a necessary mechanism to retrieve and verify the sender of cross-chain messages, enhancing security and traceability.
86-86: Renamed context fields for improved clarity.The renaming of
origintosenderandsendertosenderEVMprovides more precise terminology for cross-chain interactions, making the code more self-documenting and consistent with industry standards.Also applies to: 91-91
131-131: Consistent renaming in all struct instances.The consistent application of the field renaming across all struct instances ensures that the contract maintains a uniform interface, which is crucial for preventing confusion during development and maintenance.
Also applies to: 136-136
171-171: Updated binary representation reflects contract changes.The binary has been regenerated to incorporate all the interface changes, which is necessary for proper contract deployment and interaction.
e2e/contracts/testdappv2/TestDAppV2.abi (3)
224-242: Added getSenderWithMessage function.The new function allows retrieval of sender information with a string message parameter, enhancing the contract's ability to track and verify message origins in a user-friendly format.
262-262: Standardized naming convention in zContext struct.The field name changes align with the same pattern applied to the Example contract, maintaining consistency across different contracts in the codebase.
Also applies to: 267-267, 277-277
386-404: Added senderWithMessageZEVM function.This function provides a specialized method for working with sender information in the ZEVM context using bytes32 parameters, offering more flexibility for cross-chain interactions.
e2e/utils/contracts.go (2)
22-22: Added sender verification to MustHaveCalledExampleContract.The function now accepts a sender parameter and verifies that the contract recorded the correct sender. This additional check strengthens the testing framework by ensuring that cross-chain messages are properly attributed to their originators.
Also applies to: 32-34
43-43: Added sender verification to MustHaveCalledExampleContractWithMsg.Similar to the changes in MustHaveCalledExampleContract, this function now also verifies sender information, ensuring consistent verification behavior across different test scenarios.
Also applies to: 57-59
e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go (2)
43-43: Function return value updated correctly.The
InscribeToTSSFromDeployerWithMemonow returns an additionalcommitAddressparameter, which is properly captured here. This aligns with the PR's purpose of including sender information in message contexts.
55-60: Sender verification added appropriately.The
MustHaveCalledExampleContractfunction now receives thecommitAddressas an additional parameter, ensuring proper sender verification in cross-chain calls. This implementation correctly passes the commit address as a byte slice to validate that the example contract was called with the expected sender.e2e/contracts/testdappv2/TestDAppV2.json (3)
225-243: NewgetSenderWithMessagefunction added correctly.This new view function allows retrieving the sender associated with a given message. It returns a bytes value, providing a consistent way to verify sender information in cross-chain calls.
263-264: Improved naming convention for context fields.The renaming of fields improves clarity:
origin→sender: More accurately reflects that this is the original sendersender→senderEVM: Provides clarity that this is the EVM-formatted sender_context→context: Removes unnecessary underscore prefixThese changes make the code more intuitive and align with standard naming conventions.
Also applies to: 268-269, 278-279
387-405: NewsenderWithMessageZEVMfunction added.This view function provides a way to look up sender information by a bytes32 key in the ZEVM context. The byte return type allows for flexibility in encoding different address formats from various chains.
x/fungible/keeper/evm_gateway.go (2)
59-59: Updated context type to match protocol changes.Changed parameter type from
systemcontract.ZContexttogatewayzevm.MessageContextfor theCallDepositAndCallZRC20function. This aligns with the contract changes and maintains consistency throughout the codebase for handling message context.
112-112: Updated context type to match protocol changes.Changed parameter type from
systemcontract.ZContexttogatewayzevm.MessageContextfor theCallExecutefunction. This matches the changes in the gateway contract and ensures consistency across the codebase.e2e/e2etests/e2etests.go (1)
601-601: Version requirements updated for deposit-and-call tests.All deposit-and-call tests now require runner version v30.0.0 or later. This is necessary because these tests now include sender verification functionality, which depends on the updated contract interfaces and test utilities.
The consistent version requirement across all chain implementations (Solana, SPL, TON, Sui, Bitcoin, and Ether) ensures compatibility with the refactored message context handling.
Also applies to: 678-678, 698-698, 741-741, 829-829, 866-866, 899-899, 1444-1444
e2e/contracts/example/Example.go (3)
34-36: Ensure consistent handling of changed sender type.
RenamingSenderto[]byteand addingSenderEVMrequire all consumers and upstream code to handle the new sender representation consistently. Verify that calls to this struct in other files align with these changes.
41-42: Confirm ABI and Binary updates.
These updates to the ABI and Bin reflect the changes in the contract. Double-check that this metadata is correct by verifying with the compiled artifacts to prevent deployment or runtime mismatches.
274-304: Reviewed new 'LastSender' functionality.
The implementation to retrieve the last sender as[]byteseems correct. It follows the standard pattern for a getter method, and the usage ofabi.ConvertTypeis appropriately handled.e2e/contracts/testdappv2/TestDAppV2.sol (4)
63-64: Revisit the struct to confirm cross-chain usage.
Switchingsenderfromaddresstobytesand addingsenderEVMis a sound approach for more flexible representation. Ensure that external calls or libraries expecting anaddresstype do not break.
131-131: Reviewed parameter rename.
Renaming_contexttocontextis straightforward and improves consistency. No concerns here.
140-140: Check fallback for empty messages.
Whenmessage.length == 0, the code usescontext.senderEVM. Confirm thatsenderEVMis always valid in these scenarios to avoid potential default-zero addresses.
144-144: Inspirational approach to unify EVM and cross-chain sender.
Storing the custom sender bytes insenderWithMessageZEVMcan facilitate advanced cross-chain logic. Just ensure no stale data accumulates to avoid storage bloat.e2e/contracts/testdappv2/TestDAppV2.go (4)
47-49: Confirm contract struct alignment.
ChangingSenderto[]byteand addingSenderEVMmust match the on-chain struct formation. Confirm any older code references are updated to avoid potential panics or type mismatches.
54-55: Cross-check updated ABI and Bin.
These updates must reflect the final compiled contract. Verify using a local build to ensure no drift in your build artifacts.
673-674: Ensure consistent usage of 'TestDAppV2zContext'.
When passing the context struct, confirm all fields are populated in a valid manner (basic or advanced cross-chain scenarios).
680-681: Confirm_zrc20references.
As_zrc20is mandatory in some cross-chain workflows, verify that blank or zero addresses do not lead to unexpected behaviors.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
4 participants
Description
Closes #3689
Uses https://github.com/zeta-chain/protocol-contracts/pull/474
Modify tests to check the sender address in the message context
Summary by CodeRabbit