nico/new channel api

[nicopernas]

Split the OpenIbcChannel and ConnectIbcChannel apis into their correspondent Ibc channel handshake
methods:

• ChannelOpenInit
• ChannelOpenTry
• ChannelOpenAck
• ChannelOpenConfirm

Summary by CodeRabbit

• Refactor
  • Updated IBC channel opening and connection logic in multiple contracts by renaming functions and events for clarity and consistency.
• Bug Fixes
  • Streamlined version handling in IBC channel interactions by removing the VersionMismatch error.
• Tests
  • Adjusted and renamed test functions to match the refactored IBC channel opening and connection logic for accurate testing.

[coderabbitai]

Walkthrough

The update focuses on restructuring IBC channel handling in smart contracts, introducing a more detailed and compliant process for managing channel lifecycles. By splitting functions into distinct steps like channelOpenInit, channelOpenTry, channelOpenAck, and channelOpenConfirm, the changes aim to enhance clarity, improve developer experience, and enable easier implementation of state verification logic.

Changes

Files	Change Summary
contracts/core/Dispatcher.sol, contracts/interfaces/IbcDispatcher.sol	Refactored openIbcChannel into channelOpenInit, aligned with new logic flow.
contracts/core/UniversalChannelHandler.sol, contracts/interfaces/IbcReceiver.sol	Split onOpenIbcChannel into onChannelOpenInit and onChannelOpenTry, renamed onConnectIbcChannel to onChannelConnect, removed VersionMismatch error.
contracts/examples/Mars.sol	Updated function names and logic for IBC channel handling to onChannelConnect, onChannelOpenInit, and onChannelOpenTry.
test/... (Multiple test and VirtualChain contracts)	Adjusted for new function names and logic flow in IBC channel opening and connection scenarios.

Related issues

• open-ibc/vibc-core-smart-contracts#22: The changes align with the objectives of clearer IBC handling and improved developer experience by splitting methods into channelOpenInit, channelOpenTry, channelOpenAck, and `channelOpenConfirm, facilitating state verification logic implementation.

Poem

In the blockchain's dance of light,
A rabbit weaves code with all its might.
Init, Try, Ack, Confirm in flight,
Channels clear, in logic's sight.
🌟 Through the bytes, it leaps and weaves,
For a future where clarity relieves.
🚀 "Onward," it cries, a hop in the sky,
With each change, reaching high.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 6

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between b510aef and a66a3f7.

Files selected for processing (10)

• contracts/core/Dispatcher.sol (5 hunks)
• contracts/core/UniversalChannelHandler.sol (2 hunks)
• contracts/examples/Mars.sol (2 hunks)
• contracts/interfaces/IbcDispatcher.sol (3 hunks)
• contracts/interfaces/IbcReceiver.sol (2 hunks)
• test/Dispatcher.base.t.sol (2 hunks)
• test/Dispatcher.proof.t.sol (1 hunks)
• test/Dispatcher.t.sol (8 hunks)
• test/VirtualChain.sol (5 hunks)
• test/universal.channel.t.sol (1 hunks)

Additional comments: 32

contracts/interfaces/IbcReceiver.sol (3)

• 15-15: The addition of onChannelOpenInit function aligns with the PR's objective to decompose the channel opening process into more granular steps. This change enhances clarity and modularity in handling IBC channel handshakes.
• 17-17: The introduction of onChannelOpenTry function is consistent with the IBC channel opening process, providing a more detailed and protocol-aligned handling of channel operations.
• 19-19: Renaming onConnectIbcChannel to onChannelConnect improves clarity and consistency with the newly introduced channel opening functions. This change simplifies understanding of the channel connection process.

contracts/interfaces/IbcDispatcher.sol (2)

• 26-32: Renaming openIbcChannel to channelOpenInit and adjusting its parameters is a crucial change that aligns with the PR's objective. This modification enhances the clarity and modularity of the IBC channel opening process.
• 48-55: Updating event names to ChannelOpenInit, ChannelOpenTry, ChannelOpenAck, and ChannelOpenConfirm significantly improves the descriptiveness and precision of the IBC channel operations. This change is consistent with the PR's goal of aligning with the IBC protocol.

contracts/examples/Mars.sol (3)

• 37-42: Renaming onConnectIbcChannel to onChannelConnect and adjusting its logic to handle channel connections and version checks differently is a positive change. It aligns with the PR's objective of making the system more modular and clear.
• 79-91: Splitting the onOpenIbcChannel function into onChannelOpenInit and adjusting its logic to handle version checks is consistent with the PR's goal. This change enhances the clarity and modularity of the IBC channel opening process.
• 93-104: Introducing onChannelOpenTry and adjusting its logic for version checks further aligns with the PR's objective of decomposing the channel opening process into more granular steps. This enhances the system's clarity and modularity.

test/Dispatcher.proof.t.sol (4)

• 35-38: Updating the test to use the ChannelOpenInit event name and adjusting the function call to channelOpenInit reflects the changes made in the IBC channel opening process. This ensures that the tests remain aligned with the updated protocol.
• 45-47: Using the ChannelOpenTry event name in the test and adjusting the function call accordingly is consistent with the PR's objective of enhancing clarity and modularity in the IBC channel opening process.
• 54-56: Employing the ChannelOpenAck event name in the test and adjusting the function call aligns with the changes made to the IBC channel opening process, ensuring that the tests accurately reflect the updated protocol.
• 63-65: Utilizing the ChannelOpenConfirm event name in the test and adjusting the function call is consistent with the PR's goal of making the IBC channel opening process more descriptive and protocol-aligned.

contracts/core/UniversalChannelHandler.sol (2)

• 36-36: Renaming onConnectIbcChannel to onChannelConnect and adjusting its logic to handle channel connections and version checks differently is a positive change. It aligns with the PR's objective of making the system more modular and clear.
• 148-168: Splitting the onOpenIbcChannel function into onChannelOpenInit and onChannelOpenTry, and adjusting their logic to handle version checks is consistent with the PR's goal. This change enhances the clarity and modularity of the IBC channel opening process.

test/Dispatcher.base.t.sol (4)

• 68-85: Renaming and restructuring the IBC channel opening steps to include channelOpenInit and adjusting the test to reflect these changes is consistent with the PR's objective. This ensures that the tests remain aligned with the updated protocol.
• 95-111: Introducing channelOpenTry in the test and adjusting the function call accordingly is a positive change that aligns with the PR's goal of enhancing clarity and modularity in the IBC channel opening process.
• 130-146: Employing channelOpenAck in the test and adjusting the function call aligns with the changes made to the IBC channel opening process, ensuring that the tests accurately reflect the updated protocol.
• 156-166: Utilizing channelOpenConfirm in the test and adjusting the function call is consistent with the PR's goal of making the IBC channel opening process more descriptive and protocol-aligned.

test/VirtualChain.sol (6)

• 135-138: The implementation of channelOpenConfirm and channelOpenAck as separate functions aligns with the PR objectives to decompose the IBC channel opening process into distinct steps. This change enhances clarity and modularity by closely following the standardized IBC channel handshake protocol. Ensure that the logic within these functions correctly implements the intended steps of the protocol, including proper event emission and state updates.
• 156-162: The emit ChannelOpenInit event correctly reflects the initiation of the channel opening process. It's important to ensure that all parameters passed to the event are accurate and represent the state of the channel at this point in the process. This includes verifying the version, ordering, feeEnabled status, and connectionHops are correctly set according to the ChannelSetting struct provided to the function.
• 165-166: The call to dispatcher.channelOpenInit correctly forwards the channel opening initiation to the Dispatcher contract. It's crucial to ensure that the Dispatcher contract's channelOpenInit method is implemented to handle this call appropriately, including validating the parameters and updating the contract's state to reflect the initiation of a new channel.
• 185-191: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [188-198]

The emit ChannelOpenTry event accurately represents the second step in the IBC channel opening process, where a counterparty chain attempts to open a channel. This event should include all necessary information to identify the channel and its configuration. Ensure that the cpChanId (counterparty channel ID) is correctly determined and that the event parameters accurately reflect the channel's intended configuration.

• 209-242: The implementation of the channelOpenAck function correctly represents the acknowledgment step in the IBC channel opening process. This function should ensure that the channel exists, the counterparty information is valid, and the channel's state is updated accordingly. The event ChannelOpenAck should be emitted with accurate information. It's also important to validate the proof provided in the ChannelSetting struct to ensure the authenticity of the channel opening request from the counterparty.
• 244-268: The channelOpenConfirm function implementation is consistent with the PR objectives, representing the final confirmation step in the IBC channel opening process. This function should verify the existence of the channel, validate counterparty information, and update the channel's state to reflect its open status. The ChannelOpenConfirm event emission is crucial for signaling the successful completion of the channel opening process. Ensure that the proof validation is correctly implemented to confirm the legitimacy of the counterparty's acknowledgment.

test/universal.channel.t.sol (1)

• 80-83: The modifications to use channelOpenAck and channelOpenConfirm in the test_channel_ok_by_anyone function align with the PR objectives to decompose the channel opening process into distinct steps. This change improves the test's clarity and ensures it accurately reflects the updated channel opening protocol. It's important to verify that these function calls are correctly implemented in the VirtualChain contract and that the test adequately covers the scenarios for both successful and unsuccessful channel openings.

test/Dispatcher.t.sol (7)

• 35-35: The function channelOpenInit is correctly used to initiate the channel opening process. This aligns with the PR's objectives to decompose the channel opening into granular steps.
• 52-56: The use of channelOpenInit and channelOpenTry for the receiver's side of the channel opening process is appropriate. It's good to see both explicit version selection and auto version selection being tested.
• 72-75: Testing the complete channel handshake process (channelOpenInit, channelOpenTry, channelOpenAck, channelOpenConfirm) in sequence is crucial for ensuring the robustness of the implementation. This test case effectively validates the entire handshake process.
• 69-83: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [80-90]

The test test_channelOpenInit_fail_unsupportedVersion correctly simulates the scenario where an unsupported version is provided, expecting a revert with IbcReceiverBase.UnsupportedVersion.selector. This is an important test for validating version compatibility during the channel opening process.

• 121-126: The test test_connectChannel_fail_unsupportedVersion effectively simulates a failure scenario due to an unsupported version during the channel handshake process. It's good to see the test covering the scenario where the counterparty version is only available in later stages of the handshake.
• 140-146: The test test_connectChannel_fail_invalidProof correctly simulates a failure scenario due to invalid proof during the channel handshake process. This test is important for ensuring the robustness of proof verification mechanisms.
• 204-207: The setup method in ChannelOpenTestBase correctly executes the channel handshake process (channelOpenInit, channelOpenTry, channelOpenAck, channelOpenConfirm) to prepare the test environment. This base setup is essential for ensuring that subsequent tests start with a correctly opened channel.

[coderabbitai]

Review Status

Actionable comments generated: 5

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between b510aef and 83c3c7d.

Files selected for processing (10)

• contracts/core/Dispatcher.sol (5 hunks)
• contracts/core/UniversalChannelHandler.sol (2 hunks)
• contracts/examples/Mars.sol (2 hunks)
• contracts/interfaces/IbcDispatcher.sol (3 hunks)
• contracts/interfaces/IbcReceiver.sol (2 hunks)
• test/Dispatcher.base.t.sol (2 hunks)
• test/Dispatcher.proof.t.sol (1 hunks)
• test/Dispatcher.t.sol (6 hunks)
• test/VirtualChain.sol (5 hunks)
• test/universal.channel.t.sol (1 hunks)

Files skipped from review as they are similar to previous changes (7)

• contracts/core/UniversalChannelHandler.sol
• contracts/examples/Mars.sol
• test/Dispatcher.base.t.sol
• test/Dispatcher.proof.t.sol
• test/Dispatcher.t.sol
• test/VirtualChain.sol
• test/universal.channel.t.sol

Additional comments: 4

contracts/interfaces/IbcReceiver.sol (2)

• 15-17: The addition of onChannelOpenInit and onChannelOpenTry functions in the IbcChannelReceiver interface aligns with the goal of decomposing the channel opening process into more granular steps. This change enhances clarity and modularity in handling IBC channel handshakes.
• 19-19: The introduction of the onChannelConnect function as a replacement for onConnectIbcChannel is a positive change, providing a clearer and more standardized method name that aligns with the other channel handshake methods. This consistency improves readability and maintainability.

contracts/interfaces/IbcDispatcher.sol (2)

• 26-32: Renaming the openIbcChannel function to channelOpenInit and adjusting its parameters is a strategic move that aligns with the goal of making the IBC channel opening process more structured and clear. This change, along with the updated parameter list, enhances the interface's alignment with the IBC protocol's standards.
• 48-55: Updating event names to ChannelOpenInit, ChannelOpenTry, ChannelOpenAck, and ChannelOpenConfirm is a commendable change that brings consistency and clarity to the events related to the IBC channel opening process. This modification aids in making the event names more descriptive and aligned with the actions they represent.

[RnkSngh]

These changes look good! This already seems simpler on the smart contract level; and I'm sure this will further simplify the rollup code as well!

[coderabbitai]

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between b510aef and b88a797.

Files selected for processing (10)

• contracts/core/Dispatcher.sol (5 hunks)
• contracts/core/UniversalChannelHandler.sol (2 hunks)
• contracts/examples/Mars.sol (2 hunks)
• contracts/interfaces/IbcDispatcher.sol (3 hunks)
• contracts/interfaces/IbcReceiver.sol (2 hunks)
• test/Dispatcher.base.t.sol (2 hunks)
• test/Dispatcher.proof.t.sol (1 hunks)
• test/Dispatcher.t.sol (6 hunks)
• test/VirtualChain.sol (5 hunks)
• test/universal.channel.t.sol (1 hunks)

Files skipped from review as they are similar to previous changes (6)

• contracts/examples/Mars.sol
• test/Dispatcher.base.t.sol
• test/Dispatcher.proof.t.sol
• test/Dispatcher.t.sol
• test/VirtualChain.sol
• test/universal.channel.t.sol

Additional comments: 13

contracts/interfaces/IbcReceiver.sol (1)

• 15-21: The functions onChanOpenInit, onChanOpenTry, onChanOpenAck, and onChanOpenConfirm have been correctly added to align with the new IBC channel opening process. This change enhances clarity and modularity by providing specific methods for each step of the channel handshake.

contracts/interfaces/IbcDispatcher.sol (2)

• 26-32: The channelOpenInit function has been correctly renamed and updated to reflect the new IBC channel opening process. This change aligns with the PR objectives to enhance clarity and modularity.
• 45-60: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [48-69]

The event names ChannelOpenInit, ChannelOpenTry, ChannelOpenAck, and ChannelOpenConfirm have been correctly updated to align with the new IBC channel opening process. This ensures consistency and clarity in event naming.

contracts/core/UniversalChannelHandler.sol (1)

• 141-173: The implementation of onChanOpenAck, onChanOpenConfirm, onChanOpenInit, and onChanOpenTry correctly aligns with the new IBC channel opening process. The use of _connectChannel and _openChannel private functions ensures modularity and reusability. However, it's important to ensure that connectedChannels is properly managed to avoid potential issues with channel tracking.

contracts/core/Dispatcher.sol (9)

• 83-84: The validation for counterpartyPortId being non-empty is a good practice to ensure that the function is called with valid arguments. This check prevents potential issues with invalid or missing counterparty information.
• 87-91: The callback invocation onChanOpenInit and the subsequent event emission ChannelOpenInit are correctly implemented. This pattern follows the IBC channel opening process, ensuring that the dApp can participate in the channel opening and that the event is correctly relayed to the IBC/VIBC hub chain.
• 112-115: The call to consensusStateManager.verifyMembership with the proof, local portId, and channelId, along with the channel state and other parameters, is crucial for ensuring the integrity of the channel opening process. This verification step ensures that the channel's state is consistent with the consensus state on the blockchain.
• 118-120: The callback invocation onChanOpenTry and the subsequent event emission ChannelOpenTry are correctly implemented. This ensures that the dApp can respond to the channel opening attempt and that the event is correctly relayed to the IBC/VIBC hub chain.
• 144-147: The call to consensusStateManager.verifyMembership within channelOpenAck is correctly implemented, ensuring that the channel's state is consistent with the consensus state on the blockchain. This verification is crucial for the integrity of the channel opening process.
• 150-154: The internal call to _connectChannel followed by the callback invocation onChanOpenAck and the event emission ChannelOpenAck are correctly implemented. This sequence ensures that the channel is correctly connected and that the dApp is notified of the acknowledgment, with the event being relayed to the IBC/VIBC hub chain.
• 171-174: The call to consensusStateManager.verifyMembership within channelOpenConfirm is correctly implemented, ensuring that the channel's state is consistent with the consensus state on the blockchain. This verification is crucial for the integrity of the channel opening process.
• 177-181: The internal call to _connectChannel followed by the callback invocation onChanOpenConfirm and the event emission ChannelOpenConfirm are correctly implemented. This sequence ensures that the channel is correctly connected and that the dApp is notified of the confirmation, with the event being relayed to the IBC/VIBC hub chain.
• 509-512: The initialization of sequence numbers for sending, receiving, and acknowledging packets is correctly implemented. This setup is essential for tracking the state of packets within the channel.