refactor: rename onCrosschainCall and zContext #349
Conversation
skosito
requested review from
andresaiello,
brewmaster012,
lumtis,
CharlieMc0,
fadeev and
fbac
as code owners
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe changes primarily involve the renaming of the Changes
Assessment against linked issues
Possibly related issues
Suggested reviewers
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? 🪧 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 @@
## main #349 +/- ##
=======================================
Coverage 84.95% 84.95%
=======================================
Files 8 8
Lines 412 412
Branches 138 138
=======================================
Hits 350 350
Misses 62 62 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (22)
v2/pkg/gatewayevmzevm.t.sol/gatewayevmzevmtest.gois excluded by!v2/pkg/**v2/pkg/gatewayzevm.sol/gatewayzevm.gois excluded by!v2/pkg/**v2/pkg/gatewayzevm.t.sol/gatewayzevminboundtest.gois excluded by!v2/pkg/**v2/pkg/gatewayzevm.t.sol/gatewayzevmoutboundtest.gois excluded by!v2/pkg/**v2/pkg/igatewayzevm.sol/igatewayzevm.gois excluded by!v2/pkg/**v2/pkg/senderzevm.sol/senderzevm.gois excluded by!v2/pkg/**v2/pkg/systemcontract.sol/systemcontract.gois excluded by!v2/pkg/**v2/pkg/systemcontractmock.sol/systemcontractmock.gois excluded by!v2/pkg/**v2/pkg/testuniversalcontract.sol/testuniversalcontract.gois excluded by!v2/pkg/**v2/pkg/universalcontract.sol/universalcontract.gois excluded by!v2/pkg/**v2/pkg/zrc20.t.sol/zrc20test.gois excluded by!v2/pkg/**v2/types/GatewayZEVM.tsis excluded by!v2/types/**v2/types/IGatewayZEVM.sol/IGatewayZEVM.tsis excluded by!v2/types/**v2/types/TestUniversalContract.tsis excluded by!v2/types/**v2/types/UniversalContract.sol/UniversalContract.tsis excluded by!v2/types/**v2/types/factories/GatewayZEVM__factory.tsis excluded by!v2/types/**v2/types/factories/IGatewayZEVM.sol/IGatewayZEVM__factory.tsis excluded by!v2/types/**v2/types/factories/SenderZEVM__factory.tsis excluded by!v2/types/**v2/types/factories/SystemContract.sol/SystemContract__factory.tsis excluded by!v2/types/**v2/types/factories/SystemContractMock.sol/SystemContractMock__factory.tsis excluded by!v2/types/**v2/types/factories/TestUniversalContract__factory.tsis excluded by!v2/types/**v2/types/factories/UniversalContract.sol/UniversalContract__factory.tsis excluded by!v2/types/**
Files selected for processing (43)
- v2/contracts/zevm/GatewayZEVM.sol (7 hunks)
- v2/contracts/zevm/interfaces/IGatewayZEVM.sol (3 hunks)
- v2/contracts/zevm/interfaces/UniversalContract.sol (2 hunks)
- v2/docs/src/SUMMARY.md (1 hunks)
- v2/docs/src/contracts/Revert.sol/interface.Revertable.md (1 hunks)
- v2/docs/src/contracts/Revert.sol/struct.RevertContext.md (1 hunks)
- v2/docs/src/contracts/Revert.sol/struct.RevertOptions.md (1 hunks)
- v2/docs/src/contracts/evm/ERC20Custody.sol/contract.ERC20Custody.md (1 hunks)
- v2/docs/src/contracts/evm/GatewayEVM.sol/contract.GatewayEVM.md (1 hunks)
- v2/docs/src/contracts/evm/ZetaConnectorBase.sol/abstract.ZetaConnectorBase.md (1 hunks)
- v2/docs/src/contracts/evm/ZetaConnectorNative.sol/contract.ZetaConnectorNative.md (1 hunks)
- v2/docs/src/contracts/evm/ZetaConnectorNonNative.sol/contract.ZetaConnectorNonNative.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20Custody.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyErrors.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyEvents.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVM.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMErrors.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMEvents.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IZetaConnector.sol/interface.IZetaConnectorEvents.md (1 hunks)
- v2/docs/src/contracts/evm/interfaces/IZetaNonEthNew.sol/interface.IZetaNonEthNew.md (1 hunks)
- v2/docs/src/contracts/zevm/GatewayZEVM.sol/contract.GatewayZEVM.md (7 hunks)
- v2/docs/src/contracts/zevm/ZRC20.sol/contract.ZRC20.md (1 hunks)
- v2/docs/src/contracts/zevm/ZRC20.sol/interface.ZRC20Errors.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVM.md (6 hunks)
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMErrors.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMEvents.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/ISystem.sol/interface.ISystem.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IWZETA.sol/interface.IWETH9.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/enum.CoinType.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20Metadata.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.ZRC20Events.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/README.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.UniversalContract.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md (1 hunks)
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/struct.zContext.md (1 hunks)
- v2/docs/src/index.md (1 hunks)
- v2/lib/forge-std (1 hunks)
- v2/lib/openzeppelin-contracts-upgradeable (1 hunks)
- v2/lib/openzeppelin-foundry-upgrades (1 hunks)
- v2/test/GatewayZEVM.t.sol (20 hunks)
- v2/test/utils/TestUniversalContract.sol (1 hunks)
Files skipped from review due to trivial changes (33)
- v2/contracts/zevm/interfaces/IGatewayZEVM.sol
- v2/docs/src/contracts/Revert.sol/interface.Revertable.md
- v2/docs/src/contracts/Revert.sol/struct.RevertContext.md
- v2/docs/src/contracts/Revert.sol/struct.RevertOptions.md
- v2/docs/src/contracts/evm/ERC20Custody.sol/contract.ERC20Custody.md
- v2/docs/src/contracts/evm/GatewayEVM.sol/contract.GatewayEVM.md
- v2/docs/src/contracts/evm/ZetaConnectorBase.sol/abstract.ZetaConnectorBase.md
- v2/docs/src/contracts/evm/ZetaConnectorNative.sol/contract.ZetaConnectorNative.md
- v2/docs/src/contracts/evm/ZetaConnectorNonNative.sol/contract.ZetaConnectorNonNative.md
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20Custody.md
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyErrors.md
- v2/docs/src/contracts/evm/interfaces/IERC20Custody.sol/interface.IERC20CustodyEvents.md
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVM.md
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMErrors.md
- v2/docs/src/contracts/evm/interfaces/IGatewayEVM.sol/interface.IGatewayEVMEvents.md
- v2/docs/src/contracts/evm/interfaces/IZetaConnector.sol/interface.IZetaConnectorEvents.md
- v2/docs/src/contracts/evm/interfaces/IZetaNonEthNew.sol/interface.IZetaNonEthNew.md
- v2/docs/src/contracts/zevm/ZRC20.sol/contract.ZRC20.md
- v2/docs/src/contracts/zevm/ZRC20.sol/interface.ZRC20Errors.md
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMErrors.md
- v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVMEvents.md
- v2/docs/src/contracts/zevm/interfaces/ISystem.sol/interface.ISystem.md
- v2/docs/src/contracts/zevm/interfaces/IWZETA.sol/interface.IWETH9.md
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/enum.CoinType.md
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20.md
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.IZRC20Metadata.md
- v2/docs/src/contracts/zevm/interfaces/IZRC20.sol/interface.ZRC20Events.md
- v2/docs/src/contracts/zevm/interfaces/README.md
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.UniversalContract.md
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.md
- v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/struct.zContext.md
- v2/lib/openzeppelin-contracts-upgradeable
- v2/lib/openzeppelin-foundry-upgrades
Additional context used
Markdownlint
v2/docs/src/SUMMARY.md
32-32: Expected: 4; Actual: 6
Unordered list indentation(MD007, ul-indent)
v2/docs/src/index.md
98-98: Expected: 4; Actual: 6
Unordered list indentation(MD007, ul-indent)
v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVM.md
224-224: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Additional comments not posted (37)
v2/lib/forge-std (1)
1-1: Skipping review.The provided code segment is a subproject commit hash, which does not require a code review.
v2/docs/src/contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md (1)
1-12: LGTM!The
MessageContextstruct definition is clear, concise, and aligns with the PR objectives of improving naming conventions and clarity. The struct encapsulates relevant data for cross-chain message handling, with descriptive field names and appropriate types.The markdown documentation file provides a useful reference to the struct definition, making it easily accessible for developers working with the codebase.
v2/contracts/zevm/interfaces/UniversalContract.sol (3)
6-7: Deprecation notice looks good!The deprecation notice for the
zContextstruct is clear and provides appropriate guidance to useMessageContextinstead. This will help developers transition to the new struct and maintain consistency in the codebase.
26-30: TheMessageContextstruct is well-defined and enhances cross-chain context.The
MessageContextstruct is a suitable replacement for the deprecatedzContextstruct. The field names are descriptive and the addition of thechainIDfield is a valuable improvement. By including the originating blockchain's identifier, theMessageContextstruct provides enhanced context for cross-chain interactions, potentially improving the handling of messages across different blockchain environments.
33-33: The function signature change is appropriate and reflects the newMessageContext.The modification of the function signature from
onCrossChainCalltoonCallaligns with the deprecation ofzContextand the introduction ofMessageContext. This change improves the clarity of the function signature and reflects the shift in handling cross-chain interactions.It's worth noting that the new function name,
onCall, is more generic compared toonCrossChainCall. This may suggest a broader scope for handling various types of calls beyond just cross-chain calls. Consider documenting the intended use cases for theonCallfunction to ensure clarity for developers working with theUniversalContractinterface.v2/test/utils/TestUniversalContract.sol (1)
31-32: LGTM! Verify the impact of the changes.The renaming of
onCrossChainCalltoonCalland the change of thecontextparameter type fromzContexttoMessageContextalign with the PR objectives and improve code clarity. The changes are consistent with the AI-generated summary and do not introduce any logical errors or impact the functionality of the contract.Please ensure that the changes to the function signature are propagated to other parts of the codebase where this function is called or referenced. Run the following script to verify the impact:
v2/docs/src/SUMMARY.md (1)
32-32: LGTM! The addition of theMessageContextstruct to the documentation summary is a valid change.The AI-generated summary provides useful context about the
MessageContextstruct:The diff introduces a new entry in the documentation summary for the
MessageContextstructure, which is located in theUniversalContract.solfile under thecontracts/zevm/interfacesdirectory. This addition enhances the documentation by providing a reference to theMessageContext, which likely plays a role in the context of message handling within the Universal Contract framework.Tools
Markdownlint
32-32: Expected: 4; Actual: 6
Unordered list indentation(MD007, ul-indent)
v2/docs/src/index.md (1)
98-98: LGTM! The addition ofMessageContextstruct to the documentation is valuable.The
MessageContextstruct is part of theUniversalContractinterface, and documenting it will provide users with important information about the context in which messages are processed within the Universal Contract framework.Tools
Markdownlint
98-98: Expected: 4; Actual: 6
Unordered list indentation(MD007, ul-indent)
v2/docs/src/contracts/zevm/interfaces/IGatewayZEVM.sol/interface.IGatewayZEVM.md (3)
Line range hint
165-177: LGTM!The renaming of
zContexttoMessageContextin the function signature enhances clarity and aligns with the PR objectives. The changes are consistent and the function logic remains intact.
Line range hint
191-203: Looks good!The
zContexttype has been appropriately renamed toMessageContextin the function signature, which enhances clarity and maintains consistency with the PR objectives. The changes are well-implemented without affecting the function's behavior.
216-228: Approved!The renaming of
zContexttoMessageContextin the function signature is a positive change that enhances clarity and maintains consistency with the PR objectives. The modifications are well-implemented without introducing any unintended side effects.Tools
Markdownlint
224-224: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
v2/docs/src/contracts/zevm/GatewayZEVM.sol/contract.GatewayZEVM.md (3)
Line range hint
333-347: LGTM!The parameter type change from
zContexttoMessageContextaligns with the PR objective and is a non-functional refactor.
Line range hint
361-375: LGTM!The parameter type change from
zContexttoMessageContextaligns with the PR objective and is a non-functional refactor.
Line range hint
389-402: LGTM!The parameter type change from
zContexttoMessageContextaligns with the PR objective and is a non-functional refactor.v2/contracts/zevm/GatewayZEVM.sol (6)
9-9: LGTM!The import statement update aligns with the PR objective of renaming
zContexttoMessageContext.
289-289: LGTM!The function signature update aligns with the PR objective of renaming
zContexttoMessageContext.
301-301: LGTM!The method call update aligns with the PR objective of renaming
onCrossChainCalltoonCall.
311-311: LGTM!The function signature update aligns with the PR objective of renaming
zContexttoMessageContext.
326-326: LGTM!The method call update aligns with the PR objective of renaming
onCrossChainCalltoonCall.
Line range hint
335-349: LGTM!The function signature and method call updates align with the PR objectives of renaming
zContexttoMessageContextandonCrossChainCalltoonCall.v2/test/GatewayZEVM.t.sol (17)
492-493: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
502-503: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
512-513: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
522-523: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
533-534: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
562-563: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
572-573: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
582-583: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
595-596: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
609-610: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
619-620: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
629-630: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
688-689: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
698-699: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
712-713: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
722-723: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
736-737: LGTM!The changes to rename
zContexttocontextand update its type toMessageContextare consistent with the alterations mentioned in the summary. The test functionality remains unchanged.
| @@ -29,6 +29,7 @@ | |||
| - [CoinType](contracts/zevm/interfaces/IZRC20.sol/enum.CoinType.md) | |||
| - [zContext](contracts/zevm/interfaces/UniversalContract.sol/struct.zContext.md) | |||
| - [zContract](contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.md) | |||
| - [MessageContext](contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md) | |||
There was a problem hiding this comment.
Fix the indentation to match the surrounding entries.
The static analysis tool flagged an indentation issue:
32-32: Expected: 4; Actual: 6
Unordered list indentation
To fix the indentation, apply this diff:
- - [MessageContext](contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md)
+ - [MessageContext](contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [MessageContext](contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md) | |
| - [MessageContext](contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md) |
Tools
Markdownlint
32-32: Expected: 4; Actual: 6
Unordered list indentation(MD007, ul-indent)
| @@ -95,6 +95,7 @@ $ cast --help | |||
| - [CoinType](protocol/contracts/zevm/interfaces/IZRC20.sol/enum.CoinType.md) | |||
| - [zContext](protocol/contracts/zevm/interfaces/UniversalContract.sol/struct.zContext.md) | |||
| - [zContract](protocol/contracts/zevm/interfaces/UniversalContract.sol/interface.zContract.md) | |||
| - [MessageContext](protocol/contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md) | |||
There was a problem hiding this comment.
Fix the indentation of the unordered list item.
To maintain consistent formatting in the documentation, please adjust the indentation of the unordered list item to match the indentation level of the other items in the list.
Apply this diff to fix the indentation:
- - [MessageContext](protocol/contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md)
+ - [MessageContext](protocol/contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [MessageContext](protocol/contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md) | |
| - [MessageContext](protocol/contracts/zevm/interfaces/UniversalContract.sol/struct.MessageContext.md) |
Tools
Markdownlint
98-98: Expected: 4; Actual: 6
Unordered list indentation(MD007, ul-indent)
yea that makes sense, but dont see a way to keep compatibility and rename, adding overloads seems like an overkill for this can we just rename on protocol side as well, it should be simple change? |
Would the following be an overkill?
If we consider ZETA is not supported for now, there are two entrypoint now where |
closes: #345
Summary by CodeRabbit
Release Notes
New Features
MessageContextstruct for enhanced message handling in cross-chain interactions.GatewayZEVMandIGatewayZEVMcontracts to utilizeMessageContext.Bug Fixes
Documentation
MessageContext.Tests
MessageContextstructure, ensuring compatibility with updated function signatures.