Fix port identifier validation and add tests

.gas-snapshot

@@ -1,5 +1,5 @@
 IBCMockAppTest:testHandshake() (gas: 4420576)
-IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 3339589)
+IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 3339603)
 IBCMockAppTest:testPacketRelay() (gas: 13935831)
 IBCMockAppTest:testPacketTimeout() (gas: 4284259)
 IBCTest:testBenchmarkCreateMockClient() (gas: 233366)
@@ -8,13 +8,14 @@ IBCTest:testBenchmarkRecvPacket() (gas: 158888)
 IBCTest:testBenchmarkSendPacket() (gas: 128432)
 IBCTest:testBenchmarkUpdateMockClient() (gas: 160229)
 IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 947, ~: 947)
-TestICS02:testCreateClient() (gas: 36633805)
-TestICS02:testInvalidCreateClient() (gas: 36530922)
-TestICS02:testInvalidUpdateClient() (gas: 36529932)
-TestICS02:testRegisterClient() (gas: 36185598)
-TestICS02:testRegisterClientDuplicatedClientType() (gas: 36170907)
-TestICS02:testRegisterClientInvalidClientType() (gas: 36200368)
-TestICS02:testUpdateClient() (gas: 36698132)
+ICS24HostTest:testValidatePortIdentifier() (gas: 37060)
+TestICS02:testCreateClient() (gas: 36639835)
+TestICS02:testInvalidCreateClient() (gas: 36536946)
+TestICS02:testInvalidUpdateClient() (gas: 36535962)
+TestICS02:testRegisterClient() (gas: 36191628)
+TestICS02:testRegisterClientDuplicatedClientType() (gas: 36176937)
+TestICS02:testRegisterClientInvalidClientType() (gas: 36206398)
+TestICS02:testUpdateClient() (gas: 36704162)
 TestICS03Handshake:testConnOpenAck() (gas: 1858230)
 TestICS03Handshake:testConnOpenConfirm() (gas: 2054143)
 TestICS03Handshake:testConnOpenInit() (gas: 1429838)
@@ -29,7 +30,7 @@ TestICS03Version:testIsSupportedVersion() (gas: 7864)
 TestICS03Version:testPickVersion() (gas: 25327)
 TestICS03Version:testVerifyProposedVersion() (gas: 11777)
 TestICS03Version:testVerifySupportedFeature() (gas: 4153)
-TestICS04Handshake:testBindPort() (gas: 458549)
+TestICS04Handshake:testBindPort() (gas: 458623)
 TestICS04Handshake:testChanClose() (gas: 12973942)
 TestICS04Handshake:testChanOpenAck() (gas: 3459492)
 TestICS04Handshake:testChanOpenConfirm() (gas: 3770761)
@@ -54,7 +55,7 @@ TestICS04Upgrade:testUpgradeCrossingHelloIncompatibleProposals() (gas: 5068281)
 TestICS04Upgrade:testUpgradeFull() (gas: 56559261)
 TestICS04Upgrade:testUpgradeInit() (gas: 3074224)
 TestICS04Upgrade:testUpgradeNoChanges() (gas: 2473090)
-TestICS04Upgrade:testUpgradeNotUpgradableModule() (gas: 3648610)
+TestICS04Upgrade:testUpgradeNotUpgradableModule() (gas: 3648638)
 TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3905908)
 TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5230956)
 TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5604165)

contracts/core/24-host/IBCHostConfigurator.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.20;
 import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import {IBCClientLib} from "../02-client/IBCClientLib.sol";
 import {ILightClient} from "../02-client/ILightClient.sol";
+import {IBCHostLib} from "./IBCHostLib.sol";
 import {IBCModuleManager} from "../26-router/IBCModuleManager.sol";
 import {IIBCModule} from "../26-router/IIBCModule.sol";
 import {IIBCHostConfigurator} from "./IIBCHostConfigurator.sol";
@@ -30,7 +31,7 @@ abstract contract IBCHostConfigurator is IIBCHostConfigurator, IBCModuleManager
 
     function _bindPort(string calldata portId, IIBCModule module) internal virtual {
         address moduleAddress = address(module);
-        if (!validatePortIdentifier(bytes(portId))) {
+        if (!IBCHostLib.validatePortIdentifier(bytes(portId))) {
             revert IBCHostInvalidPortIdentifier(portId);
         }
         if (moduleAddress == address(0) || moduleAddress == address(this)) {

contracts/core/24-host/IBCHostLib.sol

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.20;
+
+library IBCHostLib {
+    /**
+     * @dev validatePortIdentifier validates a port identifier string
+     * check if the string consist of characters in one of the following categories only:
+     * - Alphanumeric
+     * - `.`, `_`, `+`, `-`, `#`
+     * - `[`, `]`, `<`, `>`
+     *
+     * The validation is based on the ibc-go implementation:
+     * https://github.com/cosmos/ibc-go/blob/b0ed0b412ea75e66091cc02746c95f9e6cf4445d/modules/core/24-host/validate.go#L86
+     */
+    function validatePortIdentifier(bytes memory portId) internal pure returns (bool) {
+        uint256 portIdLength = portId.length;
+        if (portIdLength < 2 || portIdLength > 128) {
+            return false;
+        }
+        unchecked {
+            for (uint256 i = 0; i < portIdLength; i++) {
+                uint256 c = uint256(uint8(portId[i]));
+                if (
+                    // a-z
+                    !(c >= 0x61 && c <= 0x7A)
+                    // 0-9
+                    && !(c >= 0x30 && c <= 0x39)
+                    // A-Z
+                    && !(c >= 0x41 && c <= 0x5A)
+                    // ".", "_", "+", "-"
+                    && !(c == 0x2E || c == 0x5F || c == 0x2B || c == 0x2D)
+                    // "#", "[", "]", "<", ">"
+                    && !(c == 0x23 || c == 0x5B || c == 0x5D || c == 0x3C || c == 0x3E)
+                ) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+}

contracts/core/26-router/IBCModuleManager.sol

@@ -112,37 +112,4 @@ contract IBCModuleManager is IBCHost, Context {
             portId, channelId, upgradeSequence, _msgSender()
         );
     }
-
-    /**
-     * @dev validatePortIdentifier validates a port identifier string
-     *     check if the string consist of characters in one of the following categories only:
-     *     - Alphanumeric
-     *     - `.`, `_`, `+`, `-`, `#`
-     *     - `[`, `]`, `<`, `>`
-     */
-    function validatePortIdentifier(bytes memory portId) internal pure returns (bool) {
-        if (portId.length < 2 || portId.length > 128) {
-            return false;
-        }
-        unchecked {
-            for (uint256 i = 0; i < portId.length; i++) {
-                uint256 c = uint256(uint8(portId[i]));
-                if (
-                    // a-z
-                    (c >= 0x61 && c <= 0x7A)
-                    // 0-9
-                    || (c >= 0x30 && c <= 0x39)
-                    // A-Z
-                    || (c >= 0x41 && c <= 0x5A)
-                    // ".", "_", "+", "-"
-                    || (c == 0x2E || c == 0x5F || c == 0x2B || c == 0x2D)
-                    // "#", "[", "]", "<", ">"
-                    || (c == 0x23 || c == 0x5B || c == 0x5D || c == 0x3C || c == 0x3E)
-                ) {
-                    continue;
-                }
-            }
-        }
-        return true;
-    }
 }

tests/foundry/src/ICS24Host.t.sol

@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.20;
+
+import "./helpers/IBCTestHelper.t.sol";
+import {IBCHostLib} from "../../../contracts/core/24-host/IBCHostLib.sol";
+
+contract ICS24HostTest is IBCTestHelper {
+    struct ValidatePortIdentifierTestCase {
+        string m;
+        string id;
+        bool expPass;
+    }
+
+    function testValidatePortIdentifier() public {
+        // The following test cases are based on the test cases of ibc-go:
+        // https://github.com/cosmos/ibc-go/blob/e443a88e0f2c84c131c5a1de47945a5733ff9c91/modules/core/24-host/validate_test.go#L57
+        ValidatePortIdentifierTestCase[] memory testCases = new ValidatePortIdentifierTestCase[](12);
+        testCases[0] = ValidatePortIdentifierTestCase({
+            m: "valid lowercase",
+            id: "transfer",
+            expPass: true
+        });
+        testCases[1] = ValidatePortIdentifierTestCase({
+            m: "valid id special chars",
+            id: "._+-#[]<>._+-#[]<>",
+            expPass: true
+        });
+        testCases[2] = ValidatePortIdentifierTestCase({
+            m: "valid id lower and special chars",
+            id: "lower._+-#[]<>",
+            expPass: true
+        });
+        testCases[3] = ValidatePortIdentifierTestCase({
+            m: "numeric id",
+            id: "1234567890",
+            expPass: true
+        });
+        testCases[4] = ValidatePortIdentifierTestCase({
+            m: "uppercase id",
+            id: "NOTLOWERCASE",
+            expPass: true
+        });
+        testCases[5] = ValidatePortIdentifierTestCase({
+            m: "numeric id",
+            id: "1234567890",
+            expPass: true
+        });
+        testCases[6] = ValidatePortIdentifierTestCase({
+            m: "blank id",
+            id: "               ",
+            expPass: false
+        });
+        testCases[7] = ValidatePortIdentifierTestCase({
+            m: "id length out of range",
+            id: "1",
+            expPass: false
+        });
+        testCases[8] = ValidatePortIdentifierTestCase({
+            m: "id is too long",
+            id: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis eros neque, ultricies vel ligula ac, convallis porttitor elit. Maecenas tincidunt turpis elit, vel faucibus nisl pellentesque sodales",
+            expPass: false
+        });
+        testCases[9] = ValidatePortIdentifierTestCase({
+            m: "path-like id",
+            id: "lower/case/id",
+            expPass: false
+        });
+        testCases[10] = ValidatePortIdentifierTestCase({
+            m: "invalid id",
+            id: "(clientid)",
+            expPass: false
+        });
+        testCases[11] = ValidatePortIdentifierTestCase({
+            m: "empty string",
+            id: "",
+            expPass: false
+        });
+
+        for (uint i = 0; i < testCases.length; i++) {
+            ValidatePortIdentifierTestCase memory tc = testCases[i];
+            bool res = IBCHostLib.validatePortIdentifier(bytes(tc.id));
+            if (tc.expPass) {
+                require(res, tc.m);
+            } else {
+                require(!res, tc.m);
+            }
+        }
+    }
+}