Merge pull request #210 from hyperledger-labs/fix-timeout-validation

04-channel: fix timeout validation in `sendPacket`

Signed-off-by: Jun Kimura <[email protected]>

.gas-snapshot

@@ -1,6 +1,7 @@
-IBCTest:testBenchmarkCreateMockClient() (gas: 214256)
-IBCTest:testBenchmarkRecvPacket() (gas: 157427)
-IBCTest:testBenchmarkSendPacket() (gas: 85405)
-IBCTest:testBenchmarkUpdateMockClient() (gas: 129811)
-IBCTest:testConnectionOpenInit() (gas: 496552)
-IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 1051, ~: 1051)
+IBCTest:testBenchmarkCreateMockClient() (gas: 214252)
+IBCTest:testBenchmarkRecvPacket() (gas: 157441)
+IBCTest:testBenchmarkSendPacket() (gas: 85940)
+IBCTest:testBenchmarkUpdateMockClient() (gas: 129807)
+IBCTest:testConnectionOpenInit() (gas: 496548)
+IBCTest:testSendPacketInvalidTimestamp() (gas: 39167)
+IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 1047, ~: 1047)

contracts/core/04-channel/IBCPacket.sol

@@ -42,6 +42,7 @@ contract IBCPacket is IBCStore, IIBCPacket {
             ILightClient client = ILightClient(clientImpls[connection.client_id]);
             require(address(client) != address(0), "client not found");
 
+            require(!timeoutHeight.isZero() || timeoutTimestamp != 0, "timeout height and timestamp cannot both be 0");
             (Height.Data memory latestHeight, bool found) = client.getLatestHeight(connection.client_id);
             require(found, "clientState not found");
             require(

tests/foundry/src/IBC.t.sol

@@ -79,7 +79,7 @@ contract IBCTest is Test {
         handler.setNextSequenceRecv(MOCK_PORT_ID, "channel-0", 1);
         handler.setNextSequenceAck(MOCK_PORT_ID, "channel-0", 1);
 
-        testPacketCommitment = makePacketCommitment(getPacket());
+        testPacketCommitment = makePacketCommitment(createPacket(0, 100));
     }
 
     function setUpMockApp() internal {
@@ -111,6 +111,14 @@ contract IBCTest is Test {
         assertEq(connectionId, "connection-1");
     }
 
+    function testSendPacketInvalidTimestamp() public {
+        Packet.Data memory packet = createPacket(0, 0);
+        vm.expectRevert("timeout height and timestamp cannot both be 0");
+        handler.sendPacket(
+            packet.source_port, packet.source_channel, packet.timeout_height, packet.timeout_timestamp, packet.data
+        );
+    }
+
     /* gas benchmarks */
 
     function testBenchmarkCreateMockClient() public {
@@ -122,7 +130,7 @@ contract IBCTest is Test {
     }
 
     function testBenchmarkSendPacket() public {
-        Packet.Data memory packet = getPacket();
+        Packet.Data memory packet = createPacket(0, 100);
         handler.sendPacket(
             packet.source_port, packet.source_channel, packet.timeout_height, packet.timeout_timestamp, packet.data
         );
@@ -131,7 +139,7 @@ contract IBCTest is Test {
     event MockRecv(bool ok);
 
     function testBenchmarkRecvPacket() public {
-        Packet.Data memory packet = getPacket();
+        Packet.Data memory packet = createPacket(0, 100);
         vm.expectEmit(false, false, false, true);
         emit MockRecv(true);
         handler.recvPacket(
@@ -213,15 +221,19 @@ contract IBCTest is Test {
         return versions;
     }
 
-    function getPacket() internal pure returns (Packet.Data memory packet) {
+    function createPacket(uint64 revisionNumber, uint64 revisionHeight)
+        internal
+        pure
+        returns (Packet.Data memory packet)
+    {
         return Packet.Data({
             sequence: 1,
             source_port: MOCK_PORT_ID,
             source_channel: "channel-0",
             destination_port: MOCK_PORT_ID,
             destination_channel: "channel-0",
             data: bytes("{\"amount\": \"100\"}"),
-            timeout_height: Height.Data({revision_number: 0, revision_height: 100}),
+            timeout_height: Height.Data({revision_number: revisionNumber, revision_height: revisionHeight}),
             timeout_timestamp: 0
         });
     }