fix some commitment path generator to accept calldata argument

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

.gas-snapshot

@@ -1,20 +1,20 @@
 IBCMockAppTest:testHandshake() (gas: 3435974)
 IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 2542936)
-IBCMockAppTest:testPacketRelay() (gas: 9612397)
-IBCMockAppTest:testPacketTimeout() (gas: 2923745)
-IBCTest:testBenchmarkCreateMockClient() (gas: 209339)
+IBCMockAppTest:testPacketRelay() (gas: 9534826)
+IBCMockAppTest:testPacketTimeout() (gas: 2914422)
+IBCTest:testBenchmarkCreateMockClient() (gas: 209365)
 IBCTest:testBenchmarkLCUpdateMockClient() (gas: 39945)
-IBCTest:testBenchmarkRecvPacket() (gas: 132432)
-IBCTest:testBenchmarkSendPacket() (gas: 85126)
+IBCTest:testBenchmarkRecvPacket() (gas: 130774)
+IBCTest:testBenchmarkSendPacket() (gas: 84557)
 IBCTest:testBenchmarkUpdateMockClient() (gas: 137373)
 IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 947, ~: 947)
-TestICS02:testCreateClient() (gas: 23662000)
-TestICS02:testInvalidCreateClient() (gas: 23511571)
-TestICS02:testInvalidUpdateClient() (gas: 23513490)
-TestICS02:testRegisterClient() (gas: 23256519)
-TestICS02:testRegisterClientDuplicatedClientType() (gas: 23239744)
-TestICS02:testRegisterClientInvalidClientType() (gas: 23223873)
-TestICS02:testUpdateClient() (gas: 23679042)
+TestICS02:testCreateClient() (gas: 23300679)
+TestICS02:testInvalidCreateClient() (gas: 23150346)
+TestICS02:testInvalidUpdateClient() (gas: 23152169)
+TestICS02:testRegisterClient() (gas: 22895198)
+TestICS02:testRegisterClientDuplicatedClientType() (gas: 22878423)
+TestICS02:testRegisterClientInvalidClientType() (gas: 22862552)
+TestICS02:testUpdateClient() (gas: 23317721)
 TestICS03Handshake:testConnOpenAck() (gas: 1631316)
 TestICS03Handshake:testConnOpenConfirm() (gas: 1765734)
 TestICS03Handshake:testConnOpenInit() (gas: 1279055)
@@ -30,22 +30,22 @@ TestICS03Version:testPickVersion() (gas: 25327)
 TestICS03Version:testVerifyProposedVersion() (gas: 11777)
 TestICS03Version:testVerifySupportedFeature() (gas: 4153)
 TestICS04Handshake:testBindPort() (gas: 38248)
-TestICS04Handshake:testChanClose() (gas: 8477010)
-TestICS04Handshake:testChanOpenAck() (gas: 2881785)
-TestICS04Handshake:testChanOpenConfirm() (gas: 3061565)
-TestICS04Handshake:testChanOpenInit() (gas: 2197331)
-TestICS04Handshake:testChanOpenTry() (gas: 2655828)
-TestICS04Handshake:testInvalidChanOpenAck() (gas: 2061616)
-TestICS04Handshake:testInvalidChanOpenConfirm() (gas: 2117572)
-TestICS04Handshake:testInvalidChanOpenInit() (gas: 1275175)
-TestICS04Handshake:testInvalidChanOpenTry() (gas: 1351818)
-TestICS04Packet:testAcknowledgementPacket() (gas: 2283338)
-TestICS04Packet:testInvalidSendPacket() (gas: 2288863)
-TestICS04Packet:testRecvPacket() (gas: 7609145)
-TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 2315927)
-TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 2314896)
-TestICS04Packet:testSendPacket() (gas: 5091415)
-TestICS04Packet:testTimeoutOnClose() (gas: 2538207)
+TestICS04Handshake:testChanClose() (gas: 8477568)
+TestICS04Handshake:testChanOpenAck() (gas: 2882343)
+TestICS04Handshake:testChanOpenConfirm() (gas: 3062123)
+TestICS04Handshake:testChanOpenInit() (gas: 2197889)
+TestICS04Handshake:testChanOpenTry() (gas: 2656386)
+TestICS04Handshake:testInvalidChanOpenAck() (gas: 2062174)
+TestICS04Handshake:testInvalidChanOpenConfirm() (gas: 2118130)
+TestICS04Handshake:testInvalidChanOpenInit() (gas: 1276375)
+TestICS04Handshake:testInvalidChanOpenTry() (gas: 1352538)
+TestICS04Packet:testAcknowledgementPacket() (gas: 2284000)
+TestICS04Packet:testInvalidSendPacket() (gas: 2288945)
+TestICS04Packet:testRecvPacket() (gas: 7669339)
+TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 2327617)
+TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 2326594)
+TestICS04Packet:testSendPacket() (gas: 5082327)
+TestICS04Packet:testTimeoutOnClose() (gas: 2546246)
 TestICS20:testAddressToHex(address) (runs: 256, μ: 22676, ~: 22804)
 TestICS20:testHexToAddress(string) (runs: 256, μ: 4776, ~: 4734)
 TestICS20:testIsEscapedString() (gas: 48979)

contracts/core/04-channel/IBCChannelPacketSendRecv.sol

@@ -69,7 +69,7 @@ contract IBCChannelPacketSendRecv is
 
         uint64 packetSequence = nextSequenceSends[sourcePort][sourceChannel];
         nextSequenceSends[sourcePort][sourceChannel] = packetSequence + 1;
-        commitments[IBCCommitment.packetCommitmentKey(sourcePort, sourceChannel, packetSequence)] = keccak256(
+        commitments[IBCCommitment.packetCommitmentKeyCalldata(sourcePort, sourceChannel, packetSequence)] = keccak256(
             abi.encodePacked(
                 sha256(
                     abi.encodePacked(
@@ -110,7 +110,7 @@ contract IBCChannelPacketSendRecv is
         bytes memory acknowledgement
     ) internal {
         bytes32 ackCommitmentKey =
-            IBCCommitment.packetAcknowledgementCommitmentKey(destinationPortId, destinationChannel, sequence);
+            IBCCommitment.packetAcknowledgementCommitmentKeyCalldata(destinationPortId, destinationChannel, sequence);
         if (commitments[ackCommitmentKey] != bytes32(0)) {
             revert IBCChannelAcknowledgementAlreadyWritten(destinationPortId, destinationChannel, sequence);
         }
@@ -146,7 +146,9 @@ contract IBCChannelPacketSendRecv is
             connections[channel.connection_hops[0]],
             msg_.proofHeight,
             msg_.proof,
-            IBCCommitment.packetCommitmentPath(msg_.packet.sourcePort, msg_.packet.sourceChannel, msg_.packet.sequence),
+            IBCCommitment.packetCommitmentPathCalldata(
+                msg_.packet.sourcePort, msg_.packet.sourceChannel, msg_.packet.sequence
+            ),
             sha256(
                 abi.encodePacked(
                     msg_.packet.timeoutTimestamp,
@@ -158,7 +160,7 @@ contract IBCChannelPacketSendRecv is
         );
 
         if (channel.ordering == Channel.Order.ORDER_UNORDERED) {
-            bytes32 commitmentKey = IBCCommitment.packetReceiptCommitmentKey(
+            bytes32 commitmentKey = IBCCommitment.packetReceiptCommitmentKeyCalldata(
                 msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
             );
             if (commitments[commitmentKey] != bytes32(0)) {
@@ -175,7 +177,7 @@ contract IBCChannelPacketSendRecv is
                 );
             }
             nextSequenceRecvs[msg_.packet.destinationPort][msg_.packet.destinationChannel]++;
-            commitments[IBCCommitment.nextSequenceRecvCommitmentKey(
+            commitments[IBCCommitment.nextSequenceRecvCommitmentKeyCalldata(
                 msg_.packet.destinationPort, msg_.packet.destinationChannel
             )] = keccak256(
                 uint64ToBigEndianBytes(nextSequenceRecvs[msg_.packet.destinationPort][msg_.packet.destinationChannel])
@@ -219,8 +221,9 @@ contract IBCChannelPacketSendRecv is
         // NOTE: We can assume here that the connection state is OPEN because the channel state is OPEN
         ConnectionEnd.Data storage connection = connections[channel.connection_hops[0]];
 
-        bytes32 packetCommitmentKey =
-            IBCCommitment.packetCommitmentKey(msg_.packet.sourcePort, msg_.packet.sourceChannel, msg_.packet.sequence);
+        bytes32 packetCommitmentKey = IBCCommitment.packetCommitmentKeyCalldata(
+            msg_.packet.sourcePort, msg_.packet.sourceChannel, msg_.packet.sequence
+        );
         bytes32 packetCommitment = commitments[packetCommitmentKey];
         if (packetCommitment == bytes32(0)) {
             revert IBCChannelPacketCommitmentNotFound(
@@ -247,7 +250,7 @@ contract IBCChannelPacketSendRecv is
             connection,
             msg_.proofHeight,
             msg_.proof,
-            IBCCommitment.packetAcknowledgementCommitmentPath(
+            IBCCommitment.packetAcknowledgementCommitmentPathCalldata(
                 msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
             ),
             sha256(msg_.acknowledgement)

contracts/core/04-channel/IBCChannelPacketTimeout.sol

@@ -43,7 +43,7 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
         }
 
         {
-            bytes32 commitment = commitments[IBCCommitment.packetCommitmentKey(
+            bytes32 commitment = commitments[IBCCommitment.packetCommitmentKeyCalldata(
                 msg_.packet.sourcePort, msg_.packet.sourceChannel, msg_.packet.sequence
             )];
             // NOTE: if false, this indicates that the timeoutPacket already been executed
@@ -100,6 +100,9 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
             }
             channel.state = Channel.State.STATE_CLOSED;
         } else if (channel.ordering == Channel.Order.ORDER_UNORDERED) {
+            bytes memory path = IBCCommitment.packetReceiptCommitmentPathCalldata(
+                msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
+            );
             if (
                 !client.verifyNonMembership(
                     connection.client_id,
@@ -108,25 +111,18 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
                     calcBlockDelay(connection.delay_period),
                     msg_.proof,
                     connection.counterparty.prefix.key_prefix,
-                    IBCCommitment.packetReceiptCommitmentPath(
-                        msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
-                    )
+                    path
                 )
             ) {
                 revert IBCChannelFailedVerifyPacketReceiptAbsence(
-                    connection.client_id,
-                    IBCCommitment.packetReceiptCommitmentPath(
-                        msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
-                    ),
-                    msg_.proof,
-                    msg_.proofHeight
+                    connection.client_id, path, msg_.proof, msg_.proofHeight
                 );
             }
         } else {
             revert IBCChannelUnknownChannelOrder(channel.ordering);
         }
 
-        delete commitments[IBCCommitment.packetCommitmentKey(
+        delete commitments[IBCCommitment.packetCommitmentKeyCalldata(
             msg_.packet.sourcePort, msg_.packet.sourceChannel, msg_.packet.sequence
         )];
 
@@ -153,7 +149,7 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
         ConnectionEnd.Data storage connection = connections[channel.connection_hops[0]];
         ILightClient client = ILightClient(clientImpls[connection.client_id]);
         {
-            bytes32 commitment = commitments[IBCCommitment.packetCommitmentKey(
+            bytes32 commitment = commitments[IBCCommitment.packetCommitmentKeyCalldata(
                 msg_.packet.sourcePort, msg_.packet.sourceChannel, msg_.packet.sequence
             )];
             // NOTE: if false, this indicates that the timeoutPacket already been executed
@@ -242,6 +238,9 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
                 );
             }
         } else if (channel.ordering == Channel.Order.ORDER_UNORDERED) {
+            bytes memory path = IBCCommitment.packetReceiptCommitmentPathCalldata(
+                msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
+            );
             if (
                 !client.verifyNonMembership(
                     connection.client_id,
@@ -250,18 +249,11 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
                     calcBlockDelay(connection.delay_period),
                     msg_.proofUnreceived,
                     connection.counterparty.prefix.key_prefix,
-                    IBCCommitment.packetReceiptCommitmentPath(
-                        msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
-                    )
+                    path
                 )
             ) {
                 revert IBCChannelFailedVerifyPacketReceiptAbsence(
-                    connection.client_id,
-                    IBCCommitment.packetReceiptCommitmentPath(
-                        msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
-                    ),
-                    msg_.proofUnreceived,
-                    msg_.proofHeight
+                    connection.client_id, path, msg_.proofUnreceived, msg_.proofHeight
                 );
             }
         } else {

contracts/core/24-host/IBCCommitment.sol

@@ -33,7 +33,7 @@ library IBCCommitment {
         return abi.encodePacked("channelEnds/ports/", portId, "/channels/", channelId);
     }
 
-    function packetCommitmentPath(string memory portId, string memory channelId, uint64 sequence)
+    function packetCommitmentPathCalldata(string calldata portId, string calldata channelId, uint64 sequence)
         internal
         pure
         returns (bytes memory)
@@ -43,16 +43,16 @@ library IBCCommitment {
         );
     }
 
-    function packetAcknowledgementCommitmentPath(string memory portId, string memory channelId, uint64 sequence)
-        internal
-        pure
-        returns (bytes memory)
-    {
+    function packetAcknowledgementCommitmentPathCalldata(
+        string calldata portId,
+        string calldata channelId,
+        uint64 sequence
+    ) internal pure returns (bytes memory) {
         return
             abi.encodePacked("acks/ports/", portId, "/channels/", channelId, "/sequences/", Strings.toString(sequence));
     }
 
-    function packetReceiptCommitmentPath(string memory portId, string memory channelId, uint64 sequence)
+    function packetReceiptCommitmentPathCalldata(string calldata portId, string calldata channelId, uint64 sequence)
         internal
         pure
         returns (bytes memory)
@@ -70,6 +70,14 @@ library IBCCommitment {
         return abi.encodePacked("nextSequenceRecv/ports/", portId, "/channels/", channelId);
     }
 
+    function nextSequenceRecvCommitmentPathCalldata(string calldata portId, string calldata channelId)
+        internal
+        pure
+        returns (bytes memory)
+    {
+        return abi.encodePacked("nextSequenceRecv/ports/", portId, "/channels/", channelId);
+    }
+
     // Key generators for Commitment mapping
 
     function clientStateCommitmentKey(string memory clientId) internal pure returns (bytes32) {
@@ -92,35 +100,43 @@ library IBCCommitment {
         return keccak256(channelPath(portId, channelId));
     }
 
-    function packetCommitmentKey(string memory portId, string memory channelId, uint64 sequence)
+    function nextSequenceRecvCommitmentKey(string memory portId, string memory channelId)
         internal
         pure
         returns (bytes32)
     {
-        return keccak256(packetCommitmentPath(portId, channelId, sequence));
+        return keccak256(nextSequenceRecvCommitmentPath(portId, channelId));
     }
 
-    function packetAcknowledgementCommitmentKey(string memory portId, string memory channelId, uint64 sequence)
+    function packetCommitmentKeyCalldata(string calldata portId, string calldata channelId, uint64 sequence)
         internal
         pure
         returns (bytes32)
     {
-        return keccak256(packetAcknowledgementCommitmentPath(portId, channelId, sequence));
+        return keccak256(packetCommitmentPathCalldata(portId, channelId, sequence));
     }
 
-    function packetReceiptCommitmentKey(string memory portId, string memory channelId, uint64 sequence)
+    function packetAcknowledgementCommitmentKeyCalldata(
+        string calldata portId,
+        string calldata channelId,
+        uint64 sequence
+    ) internal pure returns (bytes32) {
+        return keccak256(packetAcknowledgementCommitmentPathCalldata(portId, channelId, sequence));
+    }
+
+    function packetReceiptCommitmentKeyCalldata(string calldata portId, string calldata channelId, uint64 sequence)
         internal
         pure
         returns (bytes32)
     {
-        return keccak256(packetReceiptCommitmentPath(portId, channelId, sequence));
+        return keccak256(packetReceiptCommitmentPathCalldata(portId, channelId, sequence));
     }
 
-    function nextSequenceRecvCommitmentKey(string memory portId, string memory channelId)
+    function nextSequenceRecvCommitmentKeyCalldata(string calldata portId, string calldata channelId)
         internal
         pure
         returns (bytes32)
     {
-        return keccak256(nextSequenceRecvCommitmentPath(portId, channelId));
+        return keccak256(nextSequenceRecvCommitmentPathCalldata(portId, channelId));
     }
 }

contracts/core/25-handler/IBCQuerier.sol

@@ -66,7 +66,7 @@ contract IBCQuerier is IBCHost, IIBCQuerier {
         returns (IBCChannelLib.PacketReceipt)
     {
         return IBCChannelLib.receiptCommitmentToReceipt(
-            commitments[IBCCommitment.packetReceiptCommitmentKey(portId, channelId, sequence)]
+            commitments[IBCCommitment.packetReceiptCommitmentKeyCalldata(portId, channelId, sequence)]
         );
     }
 

tests/foundry/src/IBC.t.sol

@@ -15,6 +15,7 @@ import "../../../contracts/proto/Connection.sol";
 import "../../../contracts/proto/Channel.sol";
 import "../../../contracts/apps/mock/IBCMockApp.sol";
 import "./helpers/TestableIBCHandler.t.sol";
+import "./helpers/IBCCommitmentTestHelper.sol";
 
 // TODO split setup code into other contracts
 contract IBCTest is Test {
@@ -243,7 +244,11 @@ contract IBCTest is Test {
             abi.encodePacked(
                 proofHeight.toUint128(),
                 sha256("ibc"),
-                sha256(IBCCommitment.packetCommitmentPath(packet.sourcePort, packet.sourceChannel, packet.sequence)),
+                sha256(
+                    IBCCommitmentTestHelper.packetCommitmentPath(
+                        packet.sourcePort, packet.sourceChannel, packet.sequence
+                    )
+                ),
                 sha256(abi.encodePacked(value))
             )
         );