Merge pull request #289 from hyperledger-labs/fix-benchmarks

Improve benchmark accuracy Signed-off-by: Jun Kimura <[email protected]>
hyperledger-labs · Jul 23, 2024 · c1fe05a · c1fe05a
2 parents 138814b + 853ce82
commit c1fe05a
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 110 deletions.
diff --git a/.gas-snapshot b/.gas-snapshot
@@ -1,20 +1,21 @@
+IBCBenchmarks:testAcknowledgePacket() (gas: 100109)
+IBCBenchmarks:testCreateMockClient() (gas: 216843)
+IBCBenchmarks:testRecvPacket() (gas: 158121)
+IBCBenchmarks:testSendPacket() (gas: 96524)
+IBCBenchmarks:testUpdateMockClientDirectly() (gas: 64815)
+IBCBenchmarks:testUpdateMockClientViaHandler() (gas: 150703)
 IBCMockAppTest:testHandshake() (gas: 4420576)
 IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 3339603)
 IBCMockAppTest:testPacketRelay() (gas: 13935831)
 IBCMockAppTest:testPacketTimeout() (gas: 4284259)
-IBCTest:testBenchmarkCreateMockClient() (gas: 233366)
-IBCTest:testBenchmarkLCUpdateMockClient() (gas: 62005)
-IBCTest:testBenchmarkRecvPacket() (gas: 158888)
-IBCTest:testBenchmarkSendPacket() (gas: 128432)
-IBCTest:testBenchmarkUpdateMockClient() (gas: 160229)
-IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 947, ~: 947)
 ICS24HostTest:testValidatePortIdentifier() (gas: 37060)
 TestICS02:testCreateClient() (gas: 36639835)
+TestICS02:testHeightToUint128((uint64,uint64)) (runs: 256, μ: 887, ~: 887)
 TestICS02:testInvalidCreateClient() (gas: 36536946)
 TestICS02:testInvalidUpdateClient() (gas: 36535962)
 TestICS02:testRegisterClient() (gas: 36191628)
 TestICS02:testRegisterClientDuplicatedClientType() (gas: 36176937)
-TestICS02:testRegisterClientInvalidClientType() (gas: 36206398)
+TestICS02:testRegisterClientInvalidClientType() (gas: 36206443)
 TestICS02:testUpdateClient() (gas: 36704162)
 TestICS03Handshake:testConnOpenAck() (gas: 1858230)
 TestICS03Handshake:testConnOpenConfirm() (gas: 2054143)

diff --git a/Makefile b/Makefile
@@ -23,7 +23,7 @@ lint:
 
 .PHONY: test
 test:
-	@forge snapshot -vvvv --gas-report --use solc:$(SOLC_VERSION) $(FORGE_SNAPSHOT_OPTION)
+	@forge snapshot -vvvv --gas-report --isolate --use solc:$(SOLC_VERSION) $(FORGE_SNAPSHOT_OPTION)
 
 .PHONY: coverage
 coverage:

diff --git a/tests/foundry/src/IBC.t.sol → tests/foundry/src/IBCBenchmarks.t.sol b/tests/foundry/src/IBC.t.sol → tests/foundry/src/IBCBenchmarks.t.sol
@@ -2,24 +2,18 @@
 pragma solidity ^0.8.20;
 
 import "forge-std/Test.sol";
-import "../../../contracts/core/02-client/IBCClient.sol";
-import "../../../contracts/core/03-connection/IBCConnectionSelfStateNoValidation.sol";
 import "../../../contracts/core/04-channel/IIBCChannel.sol";
-import "../../../contracts/core/04-channel/IBCChannelHandshake.sol";
-import "../../../contracts/core/04-channel/IBCChannelPacketSendRecv.sol";
-import "../../../contracts/core/04-channel/IBCChannelPacketTimeout.sol";
-import "../../../contracts/core/04-channel/IBCChannelUpgrade.sol";
-import "../../../contracts/core/24-host/IBCCommitment.sol";
 import "../../../contracts/clients/mock/MockClient.sol";
 import "../../../contracts/proto/MockClient.sol";
 import "../../../contracts/proto/Connection.sol";
 import "../../../contracts/proto/Channel.sol";
 import "../../../contracts/apps/mock/IBCMockApp.sol";
+import "../../../contracts/apps/mock/IBCMockLib.sol";
+import "./helpers/IBCTestHelper.t.sol";
 import "./helpers/TestableIBCHandler.t.sol";
 import "./helpers/IBCCommitmentTestHelper.sol";
 
-// TODO split setup code into other contracts
-contract IBCTest is Test {
+contract IBCBenchmarks is IBCTestHelper {
     using IBCHeight for Height.Data;
 
     TestableIBCHandler handler;
@@ -28,18 +22,9 @@ contract IBCTest is Test {
 
     string private constant MOCK_CLIENT_TYPE = "mock-client";
     string private constant MOCK_PORT_ID = "mock";
-    bytes32 private testPacketCommitmentProof;
 
     function setUp() public {
-        handler = new TestableIBCHandler(
-            new IBCClient(),
-            new IBCConnectionSelfStateNoValidation(),
-            new IBCChannelHandshake(),
-            new IBCChannelPacketSendRecv(),
-            new IBCChannelPacketTimeout(),
-            new IBCChannelUpgradeInitTryAck(),
-            new IBCChannelUpgradeConfirmTimeoutCancel()
-        );
+        handler = defaultIBCHandler();
 
         mockClient = new MockClient(address(handler));
         handler.registerClient(MOCK_CLIENT_TYPE, mockClient);
@@ -50,6 +35,66 @@ contract IBCTest is Test {
         setUpMockApp();
     }
 
+    // --------------- Benchmarks --------------- //
+
+    function testCreateMockClient() public {
+        createMockClient(1);
+    }
+
+    function testUpdateMockClientViaHandler() public {
+        updateMockClient(2);
+    }
+
+    function testUpdateMockClientDirectly() public {
+        updateLCMockClient(2);
+    }
+
+    function testSendPacket() public {
+        vm.pauseGasMetering();
+        handler.setChannelCapability(MOCK_PORT_ID, "channel-0", address(this));
+        Packet memory packet = createPacket(0, 100);
+        vm.resumeGasMetering();
+        handler.sendPacket(
+            packet.sourcePort, packet.sourceChannel, packet.timeoutHeight, packet.timeoutTimestamp, packet.data
+        );
+    }
+
+    function testRecvPacket() public {
+        vm.pauseGasMetering();
+        Packet memory packet = createPacket(0, 100);
+        IIBCChannelRecvPacket.MsgPacketRecv memory msg_ = IIBCChannelRecvPacket.MsgPacketRecv({
+                packet: packet,
+                proof: abi.encodePacked(makeMockClientPacketCommitmentProof(
+                    createPacket(0, 100), Height.Data({revision_number: 0, revision_height: 1})
+                )),
+                proofHeight: Height.Data({revision_number: 0, revision_height: 1})
+            });
+        vm.resumeGasMetering();
+        handler.recvPacket(msg_);
+    }
+
+    function testAcknowledgePacket() public {
+        vm.pauseGasMetering();
+        Packet memory packet = createPacket(0, 100);
+        handler.setChannelCapability(MOCK_PORT_ID, "channel-0", address(this));
+        handler.sendPacket(
+            packet.sourcePort, packet.sourceChannel, packet.timeoutHeight, packet.timeoutTimestamp, packet.data
+        );
+        handler.setChannelCapability(MOCK_PORT_ID, "channel-0", address(mockApp));
+        IIBCChannelAcknowledgePacket.MsgPacketAcknowledgement memory msg_ = IIBCChannelAcknowledgePacket.MsgPacketAcknowledgement({
+                packet: packet,
+                acknowledgement: IBCMockLib.SUCCESSFUL_ACKNOWLEDGEMENT_JSON,
+                proof: abi.encodePacked(makeMockClientAcknowledgePacketCommitmentProof(
+                    packet, IBCMockLib.SUCCESSFUL_ACKNOWLEDGEMENT_JSON, Height.Data({revision_number: 0, revision_height: 1})
+                )),
+                proofHeight: Height.Data({revision_number: 0, revision_height: 1})
+            });
+        vm.resumeGasMetering();
+        handler.acknowledgePacket(msg_);
+    }
+
+    // --------------- Internal Functions --------------- //
+
     function setUpMockClient() internal {
         createMockClient(1);
     }
@@ -86,10 +131,6 @@ contract IBCTest is Test {
         handler.setNextSequenceSend(MOCK_PORT_ID, "channel-0", 1);
         handler.setNextSequenceRecv(MOCK_PORT_ID, "channel-0", 1);
         handler.setNextSequenceAck(MOCK_PORT_ID, "channel-0", 1);
-
-        testPacketCommitmentProof = makeMockClientPacketCommitmentProof(
-            createPacket(0, 100), Height.Data({revision_number: 0, revision_height: 1})
-        );
     }
 
     function setUpMockApp() internal {
@@ -98,85 +139,48 @@ contract IBCTest is Test {
         handler.setChannelCapability(MOCK_PORT_ID, "channel-0", address(mockApp));
     }
 
-    /* test cases */
-
-    function testToUint128(Height.Data memory height) public pure {
-        Height.Data memory actual = IBCHeight.fromUint128(IBCHeight.toUint128(height));
-        assert(height.eq(actual));
-    }
-
-    /* gas benchmarks */
-
-    function testBenchmarkCreateMockClient() public {
-        createMockClient(1);
-    }
-
-    function testBenchmarkUpdateMockClient() public {
-        updateMockClient(2);
-    }
-
-    function testBenchmarkLCUpdateMockClient() public {
-        updateLCMockClient(2);
-    }
-
-    function testBenchmarkSendPacket() public {
-        handler.setChannelCapability(MOCK_PORT_ID, "channel-0", address(this));
-        Packet memory packet = createPacket(0, 100);
-        handler.sendPacket(
-            packet.sourcePort, packet.sourceChannel, packet.timeoutHeight, packet.timeoutTimestamp, packet.data
-        );
-    }
-
-    function testBenchmarkRecvPacket() public {
-        Packet memory packet = createPacket(0, 100);
-        handler.recvPacket(
-            IIBCChannelRecvPacket.MsgPacketRecv({
-                packet: packet,
-                proof: abi.encodePacked(testPacketCommitmentProof),
-                proofHeight: Height.Data({revision_number: 0, revision_height: 1})
-            })
-        );
-    }
-
-    /* internal functions */
-
     function createMockClient(uint64 revisionHeight) internal {
-        handler.createClient(
-            IIBCClient.MsgCreateClient({
-                clientType: MOCK_CLIENT_TYPE,
-                protoClientState: wrapAnyMockClientState(
-                    IbcLightclientsMockV1ClientState.Data({
-                        latest_height: Height.Data({revision_number: 0, revision_height: revisionHeight})
-                    })
-                ),
-                protoConsensusState: wrapAnyMockConsensusState(
-                    IbcLightclientsMockV1ConsensusState.Data({timestamp: uint64(block.timestamp * 1e9)})
-                )
-            })
-        );
+        vm.pauseGasMetering();
+        IIBCClient.MsgCreateClient memory msg_ = IIBCClient.MsgCreateClient({
+            clientType: MOCK_CLIENT_TYPE,
+            protoClientState: wrapAnyMockClientState(
+                IbcLightclientsMockV1ClientState.Data({
+                    latest_height: Height.Data({revision_number: 0, revision_height: revisionHeight})
+                })
+            ),
+            protoConsensusState: wrapAnyMockConsensusState(
+                IbcLightclientsMockV1ConsensusState.Data({timestamp: uint64(block.timestamp * 1e9)})
+            )
+        });
+        vm.resumeGasMetering();
+        handler.createClient(msg_);
     }
 
     function updateMockClient(uint64 nextRevisionHeight) internal {
-        handler.updateClient(
-            IIBCClient.MsgUpdateClient({
-                clientId: "mock-client-0",
-                protoClientMessage: wrapAnyMockHeader(
-                    IbcLightclientsMockV1Header.Data({
-                        height: Height.Data({revision_number: 0, revision_height: nextRevisionHeight}),
-                        timestamp: uint64(block.timestamp * 1e9)
-                    })
-                )
-            })
-        );
+        vm.pauseGasMetering();
+        IIBCClient.MsgUpdateClient memory msg_ = IIBCClient.MsgUpdateClient({
+            clientId: "mock-client-0",
+            protoClientMessage: wrapAnyMockHeader(
+                IbcLightclientsMockV1Header.Data({
+                    height: Height.Data({revision_number: 0, revision_height: nextRevisionHeight}),
+                    timestamp: uint64(block.timestamp * 1e9)
+                })
+            )
+        });
+        vm.resumeGasMetering();
+        handler.updateClient(msg_);
     }
 
     function updateLCMockClient(uint64 nextRevisionHeight) internal {
+        vm.pauseGasMetering();
+        IbcLightclientsMockV1Header.Data memory header = IbcLightclientsMockV1Header.Data({
+            height: Height.Data({revision_number: 0, revision_height: nextRevisionHeight}),
+            timestamp: uint64(block.timestamp * 1e9)
+        });
+        vm.resumeGasMetering();
         mockClient.updateClient(
             "mock-client-0",
-            IbcLightclientsMockV1Header.Data({
-                height: Height.Data({revision_number: 0, revision_height: nextRevisionHeight}),
-                timestamp: uint64(block.timestamp * 1e9)
-            })
+            header
         );
     }
 
@@ -225,7 +229,7 @@ contract IBCTest is Test {
             sourceChannel: "channel-0",
             destinationPort: MOCK_PORT_ID,
             destinationChannel: "channel-0",
-            data: bytes("{\"amount\": \"100\"}"),
+            data: IBCMockLib.MOCK_PACKET_DATA,
             timeoutHeight: Height.Data({revision_number: revisionNumber, revision_height: revisionHeight}),
             timeoutTimestamp: 0
         });
@@ -257,4 +261,24 @@ contract IBCTest is Test {
             )
         );
     }
+
+    function makeMockClientAcknowledgePacketCommitmentProof(Packet memory packet, bytes memory acknowledgement, Height.Data memory proofHeight)
+        internal
+        pure
+        returns (bytes32)
+    {
+        bytes32 value = sha256(acknowledgement);
+        return sha256(
+            abi.encodePacked(
+                proofHeight.toUint128(),
+                sha256("ibc"),
+                sha256(
+                    IBCCommitmentTestHelper.packetAcknowledgementCommitmentPath(
+                        packet.destinationPort, packet.destinationChannel, packet.sequence
+                    )
+                ),
+                sha256(abi.encodePacked(value))
+            )
+        );
+    }
 }
diff --git a/tests/foundry/src/ICS02.t.sol b/tests/foundry/src/ICS02.t.sol
@@ -2,24 +2,18 @@
 pragma solidity ^0.8.20;
 
 import "forge-std/Test.sol";
-import "../../../contracts/core/02-client/IBCClient.sol";
 import "../../../contracts/core/02-client/IIBCClientErrors.sol";
-import "../../../contracts/core/03-connection/IBCConnectionSelfStateNoValidation.sol";
-import "../../../contracts/core/04-channel/IBCChannelHandshake.sol";
-import "../../../contracts/core/04-channel/IBCChannelPacketSendRecv.sol";
-import "../../../contracts/core/04-channel/IBCChannelPacketTimeout.sol";
 import "../../../contracts/core/24-host/IBCCommitment.sol";
 import "../../../contracts/proto/MockClient.sol";
-import "../../../contracts/proto/Connection.sol";
-import "../../../contracts/proto/Channel.sol";
-import "../../../contracts/apps/mock/IBCMockApp.sol";
 import "../../../contracts/clients/mock/MockClient.sol";
 import "./helpers/TestableIBCHandler.t.sol";
 import "./helpers/IBCTestHelper.t.sol";
 import "./helpers/MockClientTestHelper.t.sol";
 import {IIBCHostErrors} from "../../../contracts/core/24-host/IIBCHostErrors.sol";
 
 contract TestICS02 is Test, MockClientTestHelper {
+    using IBCHeight for Height.Data;
+
     function testRegisterClient() public {
         TestableIBCHandler handler = defaultIBCHandler();
         MockClient mockClient = new MockClient(address(handler));
@@ -53,6 +47,11 @@ contract TestICS02 is Test, MockClientTestHelper {
         handler.registerClient("mock-", mockClient);
     }
 
+    function testHeightToUint128(Height.Data memory height) public pure {
+        Height.Data memory actual = IBCHeight.fromUint128(IBCHeight.toUint128(height));
+        assert(height.eq(actual));
+    }
+
     function testCreateClient() public {
         (TestableIBCHandler handler, MockClient mockClient) = ibcHandlerMockClient();
         {