IBC-8: fix missing checks on revision_number

Signed-off-by: Jun Kimura <[email protected]>
hyperledger-labs · Nov 26, 2024 · 9fac703 · 9fac703
1 parent be0f2e2
commit 9fac703
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 15 deletions.
diff --git a/contracts/core/04-channel/IBCChannelPacketSendRecv.sol b/contracts/core/04-channel/IBCChannelPacketSendRecv.sol
@@ -132,9 +132,8 @@ contract IBCChannelPacketSendRecv is
             revert IBCChannelUnexpectedPacketSource(msg_.packet.sourcePort, msg_.packet.sourceChannel);
         }
 
-        if (msg_.packet.timeoutHeight.revision_height != 0 && block.number >= msg_.packet.timeoutHeight.revision_height)
-        {
-            revert IBCChannelTimeoutPacketHeight(block.number, msg_.packet.timeoutHeight.revision_height);
+        if (!msg_.packet.timeoutHeight.isZero() && hostHeight().gte(msg_.packet.timeoutHeight)) {
+            revert IBCChannelTimeoutPacketHeight(hostHeight(), msg_.packet.timeoutHeight);
         }
         if (msg_.packet.timeoutTimestamp != 0 && hostTimestamp() >= msg_.packet.timeoutTimestamp) {
             revert IBCChannelTimeoutPacketTimestamp(hostTimestamp(), msg_.packet.timeoutTimestamp);

diff --git a/contracts/core/04-channel/IBCChannelUpgrade.sol b/contracts/core/04-channel/IBCChannelUpgrade.sol
@@ -5,6 +5,7 @@ import {Height} from "../../proto/Client.sol";
 import {ConnectionEnd} from "../../proto/Connection.sol";
 import {Channel, ChannelCounterparty, Upgrade, UpgradeFields, ErrorReceipt, Timeout} from "../../proto/Channel.sol";
 import {ILightClient} from "../02-client/ILightClient.sol";
+import {IBCHeight} from "../02-client/IBCHeight.sol";
 import {IBCConnectionLib} from "../03-connection/IBCConnectionLib.sol";
 import {IIBCConnectionErrors} from "../03-connection/IIBCConnectionErrors.sol";
 import {IIBCChannelErrors} from "./IIBCChannelErrors.sol";
@@ -220,6 +221,8 @@ contract IBCChannelUpgradeInitTryAck is
     IIBCConnectionErrors,
     IIBCChannelErrors
 {
+    using IBCHeight for Height.Data;
+
     /**
      * @dev See {IIBCChannelUpgrade-channelUpgradeInit}
      */
@@ -422,9 +425,9 @@ contract IBCChannelUpgradeInitTryAck is
 
         // counterparty-specified timeout must not have exceeded
         // if it has, then restore the channel and abort upgrade handshake
-        Timeout.Data calldata timeout = msg_.counterpartyUpgrade.timeout;
+        Timeout.Data memory timeout = msg_.counterpartyUpgrade.timeout;
         if (
-            (timeout.height.revision_height != 0 && block.number >= timeout.height.revision_height)
+            (!timeout.height.isZero() && hostHeight().gte(timeout.height))
                 || (timeout.timestamp != 0 && hostTimestamp() >= timeout.timestamp)
         ) {
             restoreChannel(msg_.portId, msg_.channelId, UpgradeHandshakeError.Timeout);
@@ -548,6 +551,8 @@ contract IBCChannelUpgradeConfirmOpenTimeoutCancel is
     IBCChannelUpgradeCommon,
     IIBCChannelUpgradeConfirmOpenTimeoutCancel
 {
+    using IBCHeight for Height.Data;
+
     /**
      * @dev See {IIBCChannelUpgrade-channelUpgradeConfirm}
      */
@@ -587,9 +592,9 @@ contract IBCChannelUpgradeConfirmOpenTimeoutCancel is
 
         // counterparty-specified timeout must not have exceeded
         // if it has, then restore the channel and abort upgrade handshake
-        Timeout.Data calldata timeout = msg_.counterpartyUpgrade.timeout;
+        Timeout.Data memory timeout = msg_.counterpartyUpgrade.timeout;
         if (
-            (timeout.height.revision_height != 0 && block.number >= timeout.height.revision_height)
+            (!timeout.height.isZero() && hostHeight().gte(timeout.height))
                 || (timeout.timestamp != 0 && hostTimestamp() >= timeout.timestamp)
         ) {
             restoreChannel(msg_.portId, msg_.channelId, UpgradeHandshakeError.Timeout);
@@ -765,10 +770,7 @@ contract IBCChannelUpgradeConfirmOpenTimeoutCancel is
         // Either timeoutHeight or timeoutTimestamp must be defined.
         // if timeoutHeight is defined and proof is from before
         // timeout height then abort transaction
-        if (
-            upgrade.timeout.height.revision_height != 0
-                && msg_.proofHeight.revision_height < upgrade.timeout.height.revision_height
-        ) {
+        if (!upgrade.timeout.height.isZero() && msg_.proofHeight.lt(upgrade.timeout.height)) {
             revert IBCChannelUpgradeTimeoutHeightNotReached();
         }
         // if timeoutTimestamp is defined then the consensus time

diff --git a/contracts/core/04-channel/IIBCChannelErrors.sol b/contracts/core/04-channel/IIBCChannelErrors.sol
@@ -71,9 +71,9 @@ interface IIBCChannelErrors {
 
     error IBCChannelAckAlreadyProcessedInPreviousUpgrade(uint64 sequence, uint64 ackStartSequence);
 
-    /// @param currentBlockNumber current block number
+    /// @param currentHeight current height
     /// @param timeoutHeight packet timeout height
-    error IBCChannelTimeoutPacketHeight(uint256 currentBlockNumber, uint64 timeoutHeight);
+    error IBCChannelTimeoutPacketHeight(Height.Data currentHeight, Height.Data timeoutHeight);
 
     /// @param currentTimestamp current timestamp
     /// @param timeoutTimestamp packet timeout timestamp

diff --git a/tests/foundry/src/ICS04Packet.t.sol b/tests/foundry/src/ICS04Packet.t.sol
@@ -432,8 +432,8 @@ contract TestICS04Packet is
             vm.expectRevert(
                 abi.encodeWithSelector(
                     IIBCChannelErrors.IBCChannelTimeoutPacketHeight.selector,
-                    getBlockNumber(),
-                    p0.timeoutHeight.revision_height
+                    H(getBlockNumber()),
+                    p0.timeoutHeight
                 )
             );
             counterpartyHandler.recvPacket(msg_);