add port to ICS20Transfer constructor

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

.gas-snapshot

@@ -44,38 +44,42 @@ TestICS04Handshake:testInvalidChanOpenAck() (gas: 2327618)
 TestICS04Handshake:testInvalidChanOpenConfirm() (gas: 2402634)
 TestICS04Handshake:testInvalidChanOpenInit() (gas: 1677184)
 TestICS04Handshake:testInvalidChanOpenTry() (gas: 1692266)
-TestICS04Packet:testAcknowledgementPacket() (gas: 3133839)
-TestICS04Packet:testInvalidSendPacket() (gas: 3317459)
-TestICS04Packet:testRecvPacket() (gas: 9561916)
-TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 3083947)
-TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3107997)
-TestICS04Packet:testSendPacket() (gas: 4425319)
-TestICS04Packet:testTimeoutOnClose() (gas: 3335905)
-TestICS04Upgrade:testCrossingHelloInconsistentVersions() (gas: 9761065)
-TestICS04Upgrade:testUpgradeAuthorityCancel() (gas: 45179742)
-TestICS04Upgrade:testUpgradeCannotCancelWithOldErrorReceipt() (gas: 3309916)
-TestICS04Upgrade:testUpgradeCannotRecvNextUpgradePacket() (gas: 5142516)
-TestICS04Upgrade:testUpgradeCounterpartyAdvanceNextSequenceBeforeOpen() (gas: 5101888)
-TestICS04Upgrade:testUpgradeCrossingHelloIncompatibleProposals() (gas: 4859793)
-TestICS04Upgrade:testUpgradeFull() (gas: 55736320)
-TestICS04Upgrade:testUpgradeInit() (gas: 2937759)
-TestICS04Upgrade:testUpgradeNoChanges() (gas: 2354426)
-TestICS04Upgrade:testUpgradeNotUpgradableModule() (gas: 3470809)
-TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3741133)
-TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5115114)
-TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5487347)
-TestICS04Upgrade:testUpgradeSendPacketFailAtFlushingOrFlushComplete() (gas: 3920258)
-TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17345060)
-TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 20945649)
-TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 69105350)
-TestICS04Upgrade:testUpgradeToOrdered() (gas: 53030759)
-TestICS04Upgrade:testUpgradeToUnordered() (gas: 42323192)
+TestICS04Packet:testAcknowledgementPacket() (gas: 3133817)
+TestICS04Packet:testInvalidSendPacket() (gas: 3317437)
+TestICS04Packet:testRecvPacket() (gas: 9561861)
+TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 3083925)
+TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3107975)
+TestICS04Packet:testSendPacket() (gas: 4425297)
+TestICS04Packet:testTimeoutOnClose() (gas: 3335859)
+TestICS04Upgrade:testCrossingHelloInconsistentVersions() (gas: 9761029)
+TestICS04Upgrade:testUpgradeAuthorityCancel() (gas: 45179694)
+TestICS04Upgrade:testUpgradeCannotCancelWithOldErrorReceipt() (gas: 3309912)
+TestICS04Upgrade:testUpgradeCannotRecvNextUpgradePacket() (gas: 5142512)
+TestICS04Upgrade:testUpgradeCounterpartyAdvanceNextSequenceBeforeOpen() (gas: 5101884)
+TestICS04Upgrade:testUpgradeCrossingHelloIncompatibleProposals() (gas: 4859789)
+TestICS04Upgrade:testUpgradeFull() (gas: 55736252)
+TestICS04Upgrade:testUpgradeInit() (gas: 2937755)
+TestICS04Upgrade:testUpgradeNoChanges() (gas: 2354422)
+TestICS04Upgrade:testUpgradeNotUpgradableModule() (gas: 3470805)
+TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3741129)
+TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5115110)
+TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5487343)
+TestICS04Upgrade:testUpgradeSendPacketFailAtFlushingOrFlushComplete() (gas: 3920254)
+TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17345056)
+TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 20945645)
+TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 69105286)
+TestICS04Upgrade:testUpgradeToOrdered() (gas: 53030719)
+TestICS04Upgrade:testUpgradeToUnordered() (gas: 42323160)
 TestICS04UpgradeApp:testUpgradeAuthorizationChanneNotFound() (gas: 62062)
 TestICS04UpgradeApp:testUpgradeAuthorizationRePropose() (gas: 2376902)
 TestICS04UpgradeApp:testUpgradeAuthorizationRemove() (gas: 2357348)
-TestICS20:testAddressToHex(address) (runs: 256, μ: 26910, ~: 27088)
-TestICS20:testHexToAddress(string) (runs: 256, μ: 4658, ~: 4617)
-TestICS20:testIsEscapedString() (gas: 64753)
-TestICS20:testMarshaling() (gas: 180017)
-TestICS20:testParseAmount(uint256) (runs: 256, μ: 31599, ~: 27438)
-TestICS20:testParseUint256String() (gas: 26745)
+TestICS20Lib:testAddressToHex(address) (runs: 256, μ: 26916, ~: 27088)
+TestICS20Lib:testHexToAddress(string) (runs: 256, μ: 4658, ~: 4617)
+TestICS20Lib:testIsEscapedString() (gas: 64753)
+TestICS20Lib:testMarshaling() (gas: 180017)
+TestICS20Lib:testParseAmount(uint256) (runs: 256, μ: 31619, ~: 27869)
+TestICS20Lib:testParseUint256String() (gas: 26745)
+TestICS20Transfer:testDeposit() (gas: 404927)
+TestICS20Transfer:testDepositTransferWithdraw() (gas: 321969)
+TestICS20Transfer:testRelay() (gas: 7588605)
+TestICS20Transfer:testWithdraw() (gas: 416732)

Makefile

@@ -32,11 +32,11 @@ lint:
 
 .PHONY: test
 test:
-	TEST_UPGRADEABLE=$(TEST_UPGRADEABLE) $(FORGE) test -vvvv --gas-report --isolate --use solc:$(SOLC_VERSION) $(FORGE_SNAPSHOT_OPTION)
+	TEST_UPGRADEABLE=$(TEST_UPGRADEABLE) $(FORGE) test -vvvv --gas-report --isolate --use solc:$(SOLC_VERSION)
 
 .PHONY: snapshot
 snapshot:
-	$(FORGE) snapshot -vvvv --gas-report --isolate --use solc:$(SOLC_VERSION) $(FORGE_SNAPSHOT_OPTION)
+	$(FORGE) snapshot -vvvv --gas-report --isolate --use solc:$(SOLC_VERSION)
 
 .PHONY: coverage
 coverage:

contracts/apps/20-transfer/ICS20Lib.sol

@@ -2,20 +2,10 @@
 pragma solidity ^0.8.20;
 
 import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
+import {Height} from "../../proto/Client.sol";
 import {IICS20Errors} from "./IICS20Errors.sol";
 
 library ICS20Lib {
-    /**
-     * @dev PacketData is defined in [ICS-20](https://github.com/cosmos/ibc/tree/main/spec/app/ics-020-fungible-token-transfer).
-     */
-    struct PacketData {
-        string denom;
-        string sender;
-        string receiver;
-        uint256 amount;
-        string memo;
-    }
-
     bytes internal constant SUCCESSFUL_ACKNOWLEDGEMENT_JSON = bytes('{"result":"AQ=="}');
     bytes internal constant FAILED_ACKNOWLEDGEMENT_JSON = bytes('{"error":"failed"}');
     bytes32 internal constant KECCAK256_SUCCESSFUL_ACKNOWLEDGEMENT_JSON = keccak256(SUCCESSFUL_ACKNOWLEDGEMENT_JSON);
@@ -33,6 +23,25 @@ library ICS20Lib {
 
     bytes16 private constant HEX_DIGITS = "0123456789abcdef";
 
+    /**
+     * @dev PacketData is defined in [ICS-20](https://github.com/cosmos/ibc/tree/main/spec/app/ics-020-fungible-token-transfer).
+     */
+    struct PacketData {
+        string denom;
+        string sender;
+        string receiver;
+        uint256 amount;
+        string memo;
+    }
+
+    /**
+     * @dev Either `height` or `timestampNanos` must be set.
+     */
+    struct Timeout {
+        Height.Data height;
+        uint64 timestampNanos;
+    }
+
     /**
      * @dev marshalUnsafeJSON marshals PacketData into JSON bytes without escaping.
      *      `memo` field is omitted if it is empty.
@@ -147,6 +156,23 @@ library ICS20Lib {
         return pd;
     }
 
+    /**
+     * @dev timeout returns a Timeout struct with the given height.
+     */
+    function timeout(uint64 revisionNumber, uint64 revisionHeight) internal pure returns (Timeout memory) {
+        return Timeout({
+            height: Height.Data({revision_number: revisionNumber, revision_height: revisionHeight}),
+            timestampNanos: 0
+        });
+    }
+
+    /**
+     * @dev timeout returns a Timeout struct with the given timestamp.
+     */
+    function timeout(uint64 timestampNanos) internal pure returns (Timeout memory) {
+        return Timeout({height: Height.Data({revision_number: 0, revision_height: 0}), timestampNanos: timestampNanos});
+    }
+
     /**
      * @dev parseUint256String parses `bz` from a position `pos` to produce a uint256 value.
      * The parse will stop parsing when it encounters a non-digit character.

contracts/apps/20-transfer/ICS20Transfer.sol

@@ -2,51 +2,57 @@
 pragma solidity ^0.8.20;
 
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import {ShortString, ShortStrings} from "@openzeppelin/contracts/utils/ShortStrings.sol";
 import {IBCAppBase} from "../commons/IBCAppBase.sol";
 import {Packet} from "../../core/04-channel/IIBCChannel.sol";
 import {IIBCModule} from "../../core/26-router/IIBCModule.sol";
-import {Height} from "../../proto/Client.sol";
 import {Channel} from "../../proto/Channel.sol";
 import {ICS20Lib} from "./ICS20Lib.sol";
 import {IICS20Errors} from "./IICS20Errors.sol";
 import {IIBCHandler} from "../../core/25-handler/IIBCHandler.sol";
 
 contract ICS20Transfer is IBCAppBase, IICS20Errors {
-    string public constant ICS20_VERSION = "ics20-1";
+    using ShortStrings for string;
+    using ShortStrings for ShortString;
 
-    // mapping from denomination to account balances
-    mapping(string denom => mapping(address account => uint256 balance)) internal _balances;
+    /// @dev ICS20 version
+    string public constant ICS20_VERSION = "ics20-1";
 
     /// @dev IIBCHandler instance
     IIBCHandler internal immutable ibcHandler;
+    /// @dev port identifier
+    ShortString internal immutable port;
+
+    /// @dev balance mapping for the token
+    mapping(string denom => mapping(address account => uint256 balance)) internal _balances;
 
     /// @param ibcHandler_ IIBCHandler instance
-    constructor(IIBCHandler ibcHandler_) {
+    /// @param port_ port identifier
+    constructor(IIBCHandler ibcHandler_, string memory port_) {
         ibcHandler = ibcHandler_;
+        port = port_.toShortString();
     }
 
     // ------------------------------ Public Functions ------------------------------ //
 
     /**
      * @dev sendTransfer sends a transfer packet to the destination chain.
+     * @param sourceChannel source channel of the packet
      * @param denom denomination of the token. It can assume the denom string is escaped or not required to be escaped.
      * @param amount amount of the token
-     * @param receiver receiver address on the destination chain
-     * @param sourcePort source port of the packet
-     * @param sourceChannel source channel of the packet
-     * @param timeoutHeight timeout height of the packet
+     * @param receiver receiver address on the destination chain. This must be a valid address format per destination chain.
      */
     function sendTransfer(
+        string calldata sourceChannel,
         string calldata denom,
         uint256 amount,
         string calldata receiver,
-        string calldata sourcePort,
-        string calldata sourceChannel,
-        uint64 timeoutHeight
+        ICS20Lib.Timeout calldata timeout
     ) external returns (uint64) {
         if (!ICS20Lib.isEscapedJSONString(receiver)) {
             revert ICS20InvalidReceiverAddress(receiver);
         }
+        string memory sourcePort = port.toString();
         bytes memory denomPrefix = ICS20Lib.denomPrefix(sourcePort, sourceChannel);
         bytes memory denomBytes = bytes(denom);
         if (
@@ -62,28 +68,24 @@ contract ICS20Transfer is IBCAppBase, IICS20Errors {
             _burnVoucher(_msgSender(), denom, amount);
         }
         bytes memory packetData = ICS20Lib.marshalJSON(denom, amount, encodeAddress(_msgSender()), receiver);
-        return ibcHandler.sendPacket(
-            sourcePort, sourceChannel, Height.Data({revision_number: 0, revision_height: timeoutHeight}), 0, packetData
-        );
+        return ibcHandler.sendPacket(sourcePort, sourceChannel, timeout.height, timeout.timestampNanos, packetData);
     }
 
     /**
      * @dev depositSendTransfer sends a transfer packet to the destination chain after depositing the token.
+     * @param sourceChannel source channel of the packet
      * @param tokenContract address of the token contract
      * @param amount amount of the token
-     * @param receiver receiver address on the destination chain
-     * @param sourcePort source port of the packet
-     * @param sourceChannel source channel of the packet
-     * @param timeoutHeight timeout height of the packet
+     * @param receiver receiver address on the destination chain. This must be a valid address format per destination chain.
      */
     function depositSendTransfer(
+        string calldata sourceChannel,
         address tokenContract,
         uint256 amount,
         string calldata receiver,
-        string calldata sourcePort,
-        string calldata sourceChannel,
-        uint64 timeoutHeight
+        ICS20Lib.Timeout calldata timeout
     ) external returns (uint64) {
+        string memory sourcePort = port.toString();
         if (!ICS20Lib.isEscapedJSONString(receiver)) {
             revert ICS20InvalidReceiverAddress(receiver);
         }
@@ -97,42 +99,40 @@ contract ICS20Transfer is IBCAppBase, IICS20Errors {
         _mintVoucher(getVoucherEscrow(sourceChannel), tokenContract, amount);
         bytes memory packetData =
             ICS20Lib.marshalJSON(ICS20Lib.addressToHexString(tokenContract), amount, encodeAddress(sender), receiver);
-        return ibcHandler.sendPacket(
-            sourcePort, sourceChannel, Height.Data({revision_number: 0, revision_height: timeoutHeight}), 0, packetData
-        );
+        return ibcHandler.sendPacket(sourcePort, sourceChannel, timeout.height, timeout.timestampNanos, packetData);
     }
 
     /**
      * @dev deposit deposits the ERC20 token to the contract.
+     * @param to address to deposit the token
      * @param tokenContract address of the token contract
      * @param amount amount of the token
-     * @param to address to deposit the token
      */
     function deposit(address to, address tokenContract, uint256 amount) public {
         if (tokenContract == address(0)) {
             revert ICS20InvalidTokenContract(tokenContract);
         }
-        address from = _msgSender();
-        if (!IERC20(tokenContract).transferFrom(from, address(this), amount)) {
-            revert ICS20FailedERC20Transfer(tokenContract, from, address(this), amount);
+        address sender = _msgSender();
+        if (!IERC20(tokenContract).transferFrom(sender, address(this), amount)) {
+            revert ICS20FailedERC20Transfer(tokenContract, sender, address(this), amount);
         }
         _mintVoucher(to, tokenContract, amount);
     }
 
     /**
      * @dev withdraw withdraws the ERC20 token from the contract.
+     * @param to address to withdraw the token
      * @param tokenContract address of the token contract
      * @param amount amount of the token
-     * @param to address to withdraw the token
      */
     function withdraw(address to, address tokenContract, uint256 amount) public {
         if (tokenContract == address(0)) {
             revert ICS20InvalidTokenContract(tokenContract);
         }
-        address from = _msgSender();
-        _burnVoucher(from, ICS20Lib.addressToHexString(tokenContract), amount);
+        address sender = _msgSender();
+        _burnVoucher(sender, ICS20Lib.addressToHexString(tokenContract), amount);
         if (!IERC20(tokenContract).transfer(to, amount)) {
-            revert ICS20FailedERC20TransferFrom(tokenContract, from, address(this), to, amount);
+            revert ICS20FailedERC20TransferFrom(tokenContract, sender, address(this), to, amount);
         }
     }
 
@@ -171,6 +171,7 @@ contract ICS20Transfer is IBCAppBase, IICS20Errors {
     /**
      * @dev getVoucherEscrow returns the voucher escrow address for the given channel.
      * @param channelId channel identifier
+     * @return voucher escrow address
      */
     function getVoucherEscrow(string calldata channelId) public view virtual returns (address) {
         return address(uint160(uint256(keccak256(abi.encode(address(this), channelId)))));
@@ -266,6 +267,9 @@ contract ICS20Transfer is IBCAppBase, IICS20Errors {
         onlyIBC
         returns (address, string memory)
     {
+        if (!_equal(msg_.portId.toShortString(), port)) {
+            revert ICS20UnexpectedPort(msg_.portId, port.toString());
+        }
         if (msg_.order != Channel.Order.ORDER_UNORDERED) {
             revert IBCModuleChannelOrderNotAllowed(msg_.portId, msg_.channelId, msg_.order);
         }
@@ -286,6 +290,9 @@ contract ICS20Transfer is IBCAppBase, IICS20Errors {
         onlyIBC
         returns (address, string memory)
     {
+        if (!_equal(msg_.portId.toShortString(), port)) {
+            revert ICS20UnexpectedPort(msg_.portId, port.toString());
+        }
         if (msg_.order != Channel.Order.ORDER_UNORDERED) {
             revert IBCModuleChannelOrderNotAllowed(msg_.portId, msg_.channelId, msg_.order);
         }
@@ -423,4 +430,11 @@ contract ICS20Transfer is IBCAppBase, IICS20Errors {
     function _decodeReceiver(string memory receiver) internal pure virtual returns (address, bool) {
         return ICS20Lib.hexStringToAddress(receiver);
     }
+
+    /**
+     * @dev _equal compares two ShortString values.
+     */
+    function _equal(ShortString a, ShortString b) internal pure returns (bool) {
+        return ShortString.unwrap(a) == ShortString.unwrap(b);
+    }
 }

contracts/apps/20-transfer/IICS20Errors.sol

@@ -4,6 +4,9 @@ pragma solidity ^0.8.20;
 interface IICS20Errors {
     /// @param version Version string
     error ICS20UnexpectedVersion(string version);
+    /// @param actual port
+    /// @param expected port
+    error ICS20UnexpectedPort(string actual, string expected);
     /// @param tokenContract Address of the token contract
     error ICS20InvalidTokenContract(address tokenContract);
     /// @param tokenContract Address of the token contract