Raunak/channel close

.gitignore

@@ -32,3 +32,7 @@ typechain-types
 cache
 artifacts
 
+# Coverage Report
+report/
+lcov.info
+

contracts/core/UniversalChannelHandler.sol

@@ -15,7 +15,6 @@ import {IbcReceiver, IbcReceiverBase} from "../interfaces/IbcReceiver.sol";
 import {ChannelOrder, CounterParty, IbcPacket, AckPacket, UniversalPacket, IbcUtils} from "../libs/Ibc.sol";
 
 contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW {
-    bytes32[] public connectedChannels;
     string public constant VERSION = "1.0";
     uint256 public constant MW_ID = 1;
 
@@ -32,21 +31,12 @@ contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW {
      */
 
     function closeChannel(bytes32 channelId) external onlyOwner {
-        dispatcher.closeIbcChannel(channelId);
+        dispatcher.channelCloseInit(channelId);
     }
 
-    function onCloseIbcChannel(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {
-        // logic to determin if the channel should be closed
-        bool channelFound = false;
-        for (uint256 i = 0; i < connectedChannels.length; i++) {
-            if (connectedChannels[i] == channelId) {
-                delete connectedChannels[i];
-                channelFound = true;
-                break;
-            }
-        }
-        if (!channelFound) revert ChannelNotFound();
-    }
+    function onChanCloseInit(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {}
+
+    function onChanCloseConfirm(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {}
 
     function sendUniversalPacket(
         bytes32 channelId,
@@ -172,7 +162,6 @@ contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW {
         if (keccak256(abi.encodePacked(version)) != keccak256(abi.encodePacked(VERSION))) {
             revert UnsupportedVersion();
         }
-        connectedChannels.push(channelId);
     }
 
     function _openChannel(string calldata version) private pure returns (string memory selectedVersion) {

contracts/examples/Mars.sol

@@ -39,8 +39,14 @@ contract Mars is IbcReceiverBase, IbcReceiver {
         timeoutPackets.push(packet);
     }
 
-    function onCloseIbcChannel(bytes32 channelId, string calldata, bytes32) external virtual onlyIbcDispatcher {
-        // logic to determin if the channel should be closed
+    function onChanCloseInit(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId)
+        external
+        virtual
+        onlyIbcDispatcher
+    {}
+
+    function onChanCloseConfirm(bytes32 channelId, string calldata, bytes32) external virtual onlyIbcDispatcher {
+        // logic to determine if the channel should be closed
         bool channelFound = false;
         for (uint256 i = 0; i < connectedChannels.length; i++) {
             if (connectedChannels[i] == channelId) {
@@ -54,10 +60,10 @@ contract Mars is IbcReceiverBase, IbcReceiver {
 
     /**
      * This func triggers channel closure from the dApp.
-     * Func args can be arbitary, as long as dispatcher.closeIbcChannel is invoked propperly.
+     * Func args can be arbitary, as long as dispatcher.channelCloseInit is invoked propperly.
      */
     function triggerChannelClose(bytes32 channelId) external onlyOwner {
-        dispatcher.closeIbcChannel(channelId);
+        dispatcher.channelCloseInit(channelId);
     }
 
     /**
@@ -147,15 +153,26 @@ contract RevertingStringMars is Mars {
     }
 
     // solhint-disable-next-line
-    function onCloseIbcChannel(bytes32, string calldata, bytes32) external view override onlyIbcDispatcher {
+    function onAcknowledgementPacket(IbcPacket calldata, AckPacket calldata) external view override onlyIbcDispatcher {
         // solhint-disable-next-line
-        require(false, "close ibc channel is reverting");
+        require(false, "acknowledgement packet is reverting");
     }
+}
 
+/// Used to only test reverts for close channel (seperate from RevertingStringMars to avoid reverts on setting up the
+/// channel)
+contract RevertingStringCloseChannelMars is Mars {
+    constructor(IbcDispatcher _dispatcher) Mars(_dispatcher) {}
     // solhint-disable-next-line
-    function onAcknowledgementPacket(IbcPacket calldata, AckPacket calldata) external view override onlyIbcDispatcher {
+
+    function onChanCloseInit(bytes32, string calldata, bytes32) external view override onlyIbcDispatcher {
         // solhint-disable-next-line
-        require(false, "acknowledgement packet is reverting");
+        require(false, "close ibc channel is reverting");
+    }
+
+    function onChanCloseConfirm(bytes32, string calldata, bytes32) external view override onlyIbcDispatcher {
+        // solhint-disable-next-line
+        require(false, "close ibc channel is reverting");
     }
 }
 

contracts/interfaces/IDispatcher.sol

@@ -85,9 +85,8 @@ interface IDispatcher is IbcDispatcher, IbcEventsEmitter {
         Ics23Proof calldata proof
     ) external;
 
-    function closeIbcChannel(bytes32 channelId) external;
-
-    function sendPacket(bytes32 channelId, bytes calldata packet, uint64 timeoutTimestamp) external;
+    function channelCloseConfirm(address portAddress, bytes32 channelId, Ics23Proof calldata proof) external;
+    function channelCloseInit(bytes32 channelId) external;
 
     function acknowledgement(
         IbcPacketReceiver receiver,

contracts/interfaces/IbcDispatcher.sol

@@ -32,7 +32,7 @@ interface IbcDispatcher is IbcPacketSender {
         string calldata counterpartyPortId
     ) external;
 
-    function closeIbcChannel(bytes32 channelId) external;
+    function channelCloseInit(bytes32 channelId) external;
 
     function portPrefix() external view returns (string memory portPrefix);
 }
@@ -72,9 +72,11 @@ interface IbcEventsEmitter {
     event ChannelOpenConfirm(address indexed receiver, bytes32 channelId);
     event ChannelOpenConfirmError(address indexed receiver, bytes error);
 
-    event CloseIbcChannel(address indexed portAddress, bytes32 indexed channelId);
+    event ChannelCloseInit(address indexed portAddress, bytes32 indexed channelId);
+    event ChannelCloseConfirm(address indexed portAddress, bytes32 indexed channelId);
 
-    event CloseIbcChannelError(address indexed receiver, bytes error);
+    event ChannelCloseInitError(address indexed receiver, bytes error);
+    event ChannelCloseConfirmError(address indexed receiver, bytes error);
     event AcknowledgementError(address indexed receiver, bytes error);
     event TimeoutError(address indexed receiver, bytes error);
 

contracts/interfaces/IbcReceiver.sol

@@ -20,7 +20,10 @@ interface IbcChannelReceiver {
 
     function onChanOpenConfirm(bytes32 channelId, string calldata counterpartyVersion) external;
 
-    function onCloseIbcChannel(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId)
+    function onChanCloseInit(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId)
+        external;
+
+    function onChanCloseConfirm(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId)
         external;
 }
 

contracts/libs/Ibc.sol

@@ -5,6 +5,7 @@ pragma solidity ^0.8.9;
 import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
 import {ProtoChannel, ProtoCounterparty} from "proto/channel.sol";
 import {Base64} from "base64/base64.sol";
+import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 
 /**
  * Ibc.sol
@@ -23,7 +24,7 @@ struct IbcPacket {
     bytes data;
     /// block height after which the packet times out
     Height timeoutHeight;
-    /// block timestamp (in nanoseconds) after which the packet times out
+    /// block timestamp (in seconds after the unix epoch) after which the packet times out
     uint64 timeoutTimestamp;
 }
 
@@ -106,6 +107,7 @@ enum ChannelState {
 }
 
 struct Channel {
+    string portId;
     string version;
     ChannelOrder ordering;
     bool feeEnabled;
@@ -311,59 +313,16 @@ library Ibc {
         }
     }
 
-    // For XXXX => vIBC direction, SC needs to verify the proof of membership of TRY_PENDING
-    // For vIBC initiated channel, SC doesn't need to verify any proof, and these should be all empty
-    function _isChannelOpenTry(CounterParty calldata counterparty) external pure returns (bool open) {
-        if (counterparty.channelId == bytes32(0) && bytes(counterparty.version).length == 0) {
-            open = false;
-            // ChanOpenInit with unknow conterparty
-        } else if (counterparty.channelId != bytes32(0) && bytes(counterparty.version).length != 0) {
-            // this is the ChanOpenTry; counterparty must not be zero-value
-            open = true;
-        } else {
-            revert IBCErrors.invalidCounterParty();
-        }
-    }
-
-    function toStr(bytes32 b) public pure returns (string memory outStr) {
-        uint8 i = 0;
-        while (i < 32 && b[i] != 0) {
-            i++;
-        }
-        bytes memory bytesArray = new bytes(i);
-        for (uint8 j = 0; j < i; j++) {
-            bytesArray[j] = b[j];
-        }
-        outStr = string(bytesArray);
-    }
-
-    function toStr(uint256 _number) public pure returns (string memory outStr) {
-        if (_number == 0) {
-            return "0";
-        }
-
-        uint256 length;
-        uint256 number = _number;
-
-        // Determine the length of the string
-        while (number != 0) {
-            length++;
-            number /= 10;
-        }
-
-        bytes memory buffer = new bytes(length);
-
-        // Convert each digit to its ASCII representation
-        for (uint256 i = length; i > 0; i--) {
-            buffer[i - 1] = bytes1(uint8(48 + (_number % 10)));
-            _number /= 10;
-        }
-
-        outStr = string(buffer);
+    // https://github.com/open-ibc/ibcx-go/blob/ef80dd6784fd/modules/core/24-host/keys.go#L135
+    function channelProofKey(string calldata portId, bytes32 channelId) external pure returns (bytes memory proofKey) {
+        proofKey = abi.encodePacked("channelEnds/ports/", portId, "/channels/", toStr(channelId));
     }
 
-    // https://github.com/open-ibc/ibcx-go/blob/ef80dd6784fd/modules/core/24-host/keys.go#L135
-    function channelProofKey(string calldata portId, bytes32 channelId) public pure returns (bytes memory proofKey) {
+    function channelProofKeyMemory(string memory portId, bytes32 channelId)
+        external
+        pure
+        returns (bytes memory proofKey)
+    {
         proofKey = abi.encodePacked("channelEnds/ports/", portId, "/channels/", toStr(channelId));
     }
 
@@ -374,21 +333,41 @@ library Ibc {
         ChannelOrder ordering,
         string calldata version,
         string[] calldata connectionHops,
-        CounterParty calldata counterparty
-    ) public pure returns (bytes memory proofValue) {
+        string calldata counterpartyPortId,
+        bytes32 counterpartyChannelId
+    ) external pure returns (bytes memory proofValue) {
+        proofValue = ProtoChannel.encode(
+            ProtoChannel.Data(
+                int32(uint32(state)),
+                int32(uint32(ordering)),
+                ProtoCounterparty.Data(counterpartyPortId, toStr(counterpartyChannelId)),
+                connectionHops,
+                version
+            )
+        );
+    }
+
+    function channelProofValueMemory(
+        ChannelState state,
+        ChannelOrder ordering,
+        string memory version,
+        string[] memory connectionHops,
+        string memory counterpartyPortId,
+        bytes32 counterpartyChannelId
+    ) external pure returns (bytes memory proofValue) {
         proofValue = ProtoChannel.encode(
             ProtoChannel.Data(
                 int32(uint32(state)),
                 int32(uint32(ordering)),
-                ProtoCounterparty.Data(counterparty.portId, toStr(counterparty.channelId)),
+                ProtoCounterparty.Data(counterpartyPortId, toStr(counterpartyChannelId)),
                 connectionHops,
                 version
             )
         );
     }
 
     // https://github.com/open-ibc/ibcx-go/blob/ef80dd6784fd/modules/core/24-host/keys.go#L185
-    function packetCommitmentProofKey(IbcPacket calldata packet) public pure returns (bytes memory proofKey) {
+    function packetCommitmentProofKey(IbcPacket calldata packet) external pure returns (bytes memory proofKey) {
         proofKey = abi.encodePacked(
             "commitments/ports/",
             packet.src.portId,
@@ -400,7 +379,7 @@ library Ibc {
     }
 
     // https://github.com/open-ibc/ibcx-go/blob/ef80dd6784fd/modules/core/04-channel/types/packet.go#L19
-    function packetCommitmentProofValue(IbcPacket calldata packet) public pure returns (bytes32 proofValue) {
+    function packetCommitmentProofValue(IbcPacket calldata packet) external pure returns (bytes32 proofValue) {
         proofValue = sha256(
             abi.encodePacked(
                 packet.timeoutTimestamp,
@@ -412,7 +391,7 @@ library Ibc {
     }
 
     // https://github.com/open-ibc/ibcx-go/blob/ef80dd6784fd/modules/core/24-host/keys.go#L201
-    function ackProofKey(IbcPacket calldata packet) public pure returns (bytes memory proofKey) {
+    function ackProofKey(IbcPacket calldata packet) external pure returns (bytes memory proofKey) {
         proofKey = abi.encodePacked(
             "acks/ports/",
             packet.dest.portId,
@@ -424,14 +403,51 @@ library Ibc {
     }
 
     // https://github.com/open-ibc/ibcx-go/blob/ef80dd6784fd/modules/core/04-channel/types/packet.go#L38
-    function ackProofValue(bytes calldata ack) public pure returns (bytes32 proofValue) {
+    function ackProofValue(bytes calldata ack) external pure returns (bytes32 proofValue) {
         proofValue = sha256(ack);
     }
 
-    function parseAckData(bytes calldata ack) public pure returns (AckPacket memory ackData) {
+    function parseAckData(bytes calldata ack) external pure returns (AckPacket memory ackData) {
         // this hex value is '"result"'
         ackData = (keccak256(ack[1:9]) == keccak256(hex"22726573756c7422"))
             ? AckPacket(true, Base64.decode(string(ack[11:ack.length - 2]))) // result success
             : AckPacket(false, ack[10:ack.length - 2]); // this is an error
     }
+
+    function toStr(bytes32 b) public pure returns (string memory outStr) {
+        uint8 i = 0;
+        while (i < 32 && b[i] != 0) {
+            i++;
+        }
+        bytes memory bytesArray = new bytes(i);
+        for (uint8 j = 0; j < i; j++) {
+            bytesArray[j] = b[j];
+        }
+        outStr = string(bytesArray);
+    }
+
+    function toStr(uint256 _number) public pure returns (string memory outStr) {
+        if (_number == 0) {
+            return "0";
+        }
+
+        uint256 length;
+        uint256 number = _number;
+
+        // Determine the length of the string
+        while (number != 0) {
+            length++;
+            number /= 10;
+        }
+
+        bytes memory buffer = new bytes(length);
+
+        // Convert each digit to its ASCII representation
+        for (uint256 i = length; i > 0; i--) {
+            buffer[i - 1] = bytes1(uint8(48 + (_number % 10)));
+            _number /= 10;
+        }
+
+        outStr = string(buffer);
+    }
 }

script/Deploy.s.sol

@@ -4,7 +4,7 @@ pragma solidity ^0.8.0;
 import "forge-std/Script.sol";
 import "../contracts/utils/DummyProofVerifier.sol";
 import "../contracts/utils/DummyLightClient.sol";
-import "../contracts/core/Dispatcher.sol";
+import {Dispatcher} from "../contracts/core/Dispatcher.sol";
 import "../contracts/examples/Mars.sol";
 import {IDispatcher} from "../contracts/core/Dispatcher.sol";
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";