Merge pull request #296 from hyperledger-labs/upgradeable-contract

Add upgradeable IBCHandler

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

Author: bluele

.gas-snapshot

@@ -1,77 +1,78 @@
+ContractUpgrade:testUpgrade() (gas: 3333)
 IBCBenchmarks:testAcknowledgePacket() (gas: 98040)
 IBCBenchmarks:testCreateMockClient() (gas: 216997)
 IBCBenchmarks:testRecvPacket() (gas: 154889)
 IBCBenchmarks:testSendPacket() (gas: 94843)
 IBCBenchmarks:testUpdateMockClientDirectly() (gas: 64879)
 IBCBenchmarks:testUpdateMockClientViaHandler() (gas: 148239)
-IBCMockAppTest:testHandshake() (gas: 4161353)
-IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 3104214)
-IBCMockAppTest:testPacketRelay() (gas: 11460317)
-IBCMockAppTest:testPacketTimeout() (gas: 4059417)
+IBCMockAppTest:testHandshake() (gas: 4162455)
+IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 3104765)
+IBCMockAppTest:testPacketRelay() (gas: 11461419)
+IBCMockAppTest:testPacketTimeout() (gas: 4059968)
 ICS24HostTest:testValidatePortIdentifier() (gas: 44476)
-TestICS02:testCreateClient() (gas: 30572814)
+TestICS02:testCreateClient() (gas: 30594453)
 TestICS02:testHeightToUint128((uint64,uint64)) (runs: 256, μ: 907, ~: 907)
-TestICS02:testInvalidCreateClient() (gas: 30458130)
-TestICS02:testInvalidUpdateClient() (gas: 30461817)
-TestICS02:testRegisterClient() (gas: 30122086)
-TestICS02:testRegisterClientDuplicatedClientType() (gas: 30108306)
-TestICS02:testRegisterClientInvalidClientType() (gas: 30137268)
-TestICS02:testUpdateClient() (gas: 30628415)
-TestICS03Handshake:testConnOpenAck() (gas: 1810871)
-TestICS03Handshake:testConnOpenConfirm() (gas: 1988723)
-TestICS03Handshake:testConnOpenInit() (gas: 1422940)
-TestICS03Handshake:testConnOpenTry() (gas: 2347689)
-TestICS03Handshake:testInvalidConnOpenAck() (gas: 2322689)
-TestICS03Handshake:testInvalidConnOpenConfirm() (gas: 2421514)
+TestICS02:testInvalidCreateClient() (gas: 30479715)
+TestICS02:testInvalidUpdateClient() (gas: 30483432)
+TestICS02:testRegisterClient() (gas: 30143677)
+TestICS02:testRegisterClientDuplicatedClientType() (gas: 30129897)
+TestICS02:testRegisterClientInvalidClientType() (gas: 30158859)
+TestICS02:testUpdateClient() (gas: 30650006)
+TestICS03Handshake:testConnOpenAck() (gas: 1811232)
+TestICS03Handshake:testConnOpenConfirm() (gas: 1989361)
+TestICS03Handshake:testConnOpenInit() (gas: 1423016)
+TestICS03Handshake:testConnOpenTry() (gas: 2348031)
+TestICS03Handshake:testInvalidConnOpenAck() (gas: 2323525)
+TestICS03Handshake:testInvalidConnOpenConfirm() (gas: 2422540)
 TestICS03Handshake:testInvalidConnOpenInit() (gas: 776893)
-TestICS03Handshake:testInvalidConnOpenTry() (gas: 2283346)
+TestICS03Handshake:testInvalidConnOpenTry() (gas: 2283536)
 TestICS03Version:testCopyVersions() (gas: 570207)
 TestICS03Version:testFindSupportedVersion() (gas: 34452)
 TestICS03Version:testIsSupportedVersion() (gas: 13568)
 TestICS03Version:testPickVersion() (gas: 37836)
 TestICS03Version:testVerifyProposedVersion() (gas: 21308)
 TestICS03Version:testVerifySupportedFeature() (gas: 10229)
 TestICS04Handshake:testBindPort() (gas: 456448)
-TestICS04Handshake:testChanClose() (gas: 12159113)
-TestICS04Handshake:testChanOpenAck() (gas: 3242588)
-TestICS04Handshake:testChanOpenConfirm() (gas: 3538396)
-TestICS04Handshake:testChanOpenInit() (gas: 2395187)
-TestICS04Handshake:testChanOpenTry() (gas: 2897666)
-TestICS04Handshake:testInvalidChanOpenAck() (gas: 2317011)
-TestICS04Handshake:testInvalidChanOpenConfirm() (gas: 2391668)
-TestICS04Handshake:testInvalidChanOpenInit() (gas: 1677250)
-TestICS04Handshake:testInvalidChanOpenTry() (gas: 1692203)
-TestICS04Packet:testAcknowledgementPacket() (gas: 3111231)
-TestICS04Packet:testInvalidSendPacket() (gas: 3294851)
-TestICS04Packet:testRecvPacket() (gas: 9485947)
-TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 3053625)
-TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3077674)
-TestICS04Packet:testSendPacket() (gas: 4393823)
-TestICS04Packet:testTimeoutOnClose() (gas: 3304892)
-TestICS04Upgrade:testCrossingHelloInconsistentVersions() (gas: 9730721)
-TestICS04Upgrade:testUpgradeAuthorityCancel() (gas: 44972623)
-TestICS04Upgrade:testUpgradeCannotCancelWithOldErrorReceipt() (gas: 3296637)
-TestICS04Upgrade:testUpgradeCannotRecvNextUpgradePacket() (gas: 5124643)
-TestICS04Upgrade:testUpgradeCounterpartyAdvanceNextSequenceBeforeOpen() (gas: 5087713)
-TestICS04Upgrade:testUpgradeCrossingHelloIncompatibleProposals() (gas: 4843978)
-TestICS04Upgrade:testUpgradeFull() (gas: 55696695)
-TestICS04Upgrade:testUpgradeInit() (gas: 2925072)
-TestICS04Upgrade:testUpgradeNoChanges() (gas: 2342343)
-TestICS04Upgrade:testUpgradeNotUpgradableModule() (gas: 3442666)
-TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3727867)
-TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5097250)
-TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5469407)
-TestICS04Upgrade:testUpgradeSendPacketFailAtFlushingOrFlushComplete() (gas: 3907425)
-TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17323669)
-TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 20920588)
-TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 68891924)
-TestICS04Upgrade:testUpgradeToOrdered() (gas: 52805680)
-TestICS04Upgrade:testUpgradeToUnordered() (gas: 42151776)
+TestICS04Handshake:testChanClose() (gas: 12159177)
+TestICS04Handshake:testChanOpenAck() (gas: 3242652)
+TestICS04Handshake:testChanOpenConfirm() (gas: 3538460)
+TestICS04Handshake:testChanOpenInit() (gas: 2395251)
+TestICS04Handshake:testChanOpenTry() (gas: 2897730)
+TestICS04Handshake:testInvalidChanOpenAck() (gas: 2317075)
+TestICS04Handshake:testInvalidChanOpenConfirm() (gas: 2391732)
+TestICS04Handshake:testInvalidChanOpenInit() (gas: 1677410)
+TestICS04Handshake:testInvalidChanOpenTry() (gas: 1692299)
+TestICS04Packet:testAcknowledgementPacket() (gas: 3111209)
+TestICS04Packet:testInvalidSendPacket() (gas: 3294829)
+TestICS04Packet:testRecvPacket() (gas: 9485892)
+TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 3053603)
+TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3077652)
+TestICS04Packet:testSendPacket() (gas: 4393801)
+TestICS04Packet:testTimeoutOnClose() (gas: 3304846)
+TestICS04Upgrade:testCrossingHelloInconsistentVersions() (gas: 9732925)
+TestICS04Upgrade:testUpgradeAuthorityCancel() (gas: 44979235)
+TestICS04Upgrade:testUpgradeCannotCancelWithOldErrorReceipt() (gas: 3297188)
+TestICS04Upgrade:testUpgradeCannotRecvNextUpgradePacket() (gas: 5125194)
+TestICS04Upgrade:testUpgradeCounterpartyAdvanceNextSequenceBeforeOpen() (gas: 5088264)
+TestICS04Upgrade:testUpgradeCrossingHelloIncompatibleProposals() (gas: 4844529)
+TestICS04Upgrade:testUpgradeFull() (gas: 55706062)
+TestICS04Upgrade:testUpgradeInit() (gas: 2925623)
+TestICS04Upgrade:testUpgradeNoChanges() (gas: 2342894)
+TestICS04Upgrade:testUpgradeNotUpgradableModule() (gas: 3443205)
+TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3728418)
+TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5097801)
+TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5469958)
+TestICS04Upgrade:testUpgradeSendPacketFailAtFlushingOrFlushComplete() (gas: 3907976)
+TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17324220)
+TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 20921139)
+TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 68900740)
+TestICS04Upgrade:testUpgradeToOrdered() (gas: 52811190)
+TestICS04Upgrade:testUpgradeToUnordered() (gas: 42156184)
 TestICS04UpgradeApp:testUpgradeAuthorizationChanneNotFound() (gas: 62062)
-TestICS04UpgradeApp:testUpgradeAuthorizationRePropose() (gas: 2365121)
-TestICS04UpgradeApp:testUpgradeAuthorizationRemove() (gas: 2345567)
-TestICS20:testAddressToHex(address) (runs: 256, μ: 26850, ~: 27044)
+TestICS04UpgradeApp:testUpgradeAuthorizationRePropose() (gas: 2365672)
+TestICS04UpgradeApp:testUpgradeAuthorizationRemove() (gas: 2346118)
+TestICS20:testAddressToHex(address) (runs: 256, μ: 26869, ~: 27044)
 TestICS20:testHexToAddress(string) (runs: 256, μ: 4636, ~: 4595)
 TestICS20:testIsEscapedString() (gas: 62745)
 TestICS20:testMarshaling() (gas: 180081)
-TestICS20:testParseAmount(uint256) (runs: 256, μ: 32408, ~: 28568)
+TestICS20:testParseAmount(uint256) (runs: 256, μ: 31973, ~: 28039)

.github/workflows/test.yml

@@ -38,6 +38,9 @@ jobs:
     - name: Run tests
       run: make test
 
+    - name: Run upgradeable tests
+      run: make TEST_UPGRADEABLE=true clean test
+
     - name: Run coverage
       run: make coverage
 
@@ -104,3 +107,6 @@ jobs:
 
     - name: E2E test (QBFT)
       run: make e2e-test network-down
+
+    - name: E2E test with upgredable IBCHandler (QBFT)
+      run: make TEST_UPGRADEABLE=true clean network-qbft e2e-test network-down

.gitmodules

@@ -1,3 +1,6 @@
 [submodule "lib/forge-std"]
 	path = tests/foundry/lib/forge-std
 	url = https://github.com/foundry-rs/forge-std
+[submodule "lib/openzeppelin-foundry-upgrades"]
+	path = tests/foundry/lib/openzeppelin-foundry-upgrades
+	url = https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades

Makefile

@@ -9,13 +9,18 @@ DOCKER_COMPOSE=$(DOCKER) compose
 E2E_TEST_COMPOSE_FILE=./chains/compose.yml
 TEST_BROADCAST_LOG_DIR=./broadcast/Deploy.s.sol
 TEST_MNEMONIC="math razor capable expose worth grape metal sunset metal sudden usage scheme"
+TEST_UPGRADEABLE=false
 
 ######## Development ########
 
 .PHONY: build
 build:
 	$(FORGE) build --sizes --skip test --use solc:$(SOLC_VERSION)
 
+.PHONY: clean
+clean:
+	$(FORGE) clean
+
 .PHONY: fmt
 fmt:
 	$(FORGE) fmt $(FORGE_FMT_OPTS)
@@ -27,7 +32,7 @@ lint:
 
 .PHONY: test
 test:
-	$(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) $(FORGE_SNAPSHOT_OPTION)
 
 .PHONY: snapshot
 snapshot:
@@ -88,9 +93,9 @@ network-qbft:
 
 .PHONY: deploy
 deploy:
-	TEST_MNEMONIC=$(TEST_MNEMONIC) $(FORGE) script --legacy --batch-size 5 --use solc:${SOLC_VERSION} --fork-url http://127.0.0.1:8645 --broadcast \
+	TEST_UPGRADEABLE=$(TEST_UPGRADEABLE) TEST_MNEMONIC=$(TEST_MNEMONIC) $(FORGE) script --legacy --batch-size 5 --use solc:${SOLC_VERSION} --fork-url http://127.0.0.1:8645 --broadcast \
 		./tests/foundry/src/Deploy.s.sol
-	TEST_MNEMONIC=$(TEST_MNEMONIC) $(FORGE) script --legacy --batch-size 5 --use solc:${SOLC_VERSION} --fork-url http://127.0.0.1:8745 --broadcast \
+	TEST_UPGRADEABLE=$(TEST_UPGRADEABLE) TEST_MNEMONIC=$(TEST_MNEMONIC) $(FORGE) script --legacy --batch-size 5 --use solc:${SOLC_VERSION} --fork-url http://127.0.0.1:8745 --broadcast \
 		./tests/foundry/src/Deploy.s.sol
 
 .PHONY: network-down
@@ -99,4 +104,4 @@ network-down:
 
 .PHONY: e2e-test
 e2e-test:
-	TEST_MNEMONIC=$(TEST_MNEMONIC) TEST_BROADCAST_LOG_DIR=$(CURDIR)/$(TEST_BROADCAST_LOG_DIR) go test -v ./tests/e2e/... -count=1
+	TEST_UPGRADEABLE=$(TEST_UPGRADEABLE) TEST_MNEMONIC=$(TEST_MNEMONIC) TEST_BROADCAST_LOG_DIR=$(CURDIR)/$(TEST_BROADCAST_LOG_DIR) go test -v ./tests/e2e/... -count=1

contracts/core/25-handler/IBCClientConnectionChannelHandler.sol

@@ -31,12 +31,19 @@ abstract contract IBCClientConnectionChannelHandler is
     IIBCChannelPacketTimeout,
     IIBCChannelUpgrade
 {
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address internal immutable ibcClient;
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address internal immutable ibcConnection;
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address internal immutable ibcChannelHandshake;
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address internal immutable ibcChannelPacketSendRecv;
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address internal immutable ibcChannelPacketTimeout;
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address internal immutable ibcChannelUpgradeInitTryAck;
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address internal immutable ibcChannelUpgradeConfirmOpenTimeoutCancel;
 
     /**
@@ -48,6 +55,7 @@ abstract contract IBCClientConnectionChannelHandler is
      * @param ibcChannelPacketTimeout_ is the address of a contract that implements `IIBCChannelPacketTimeout`.
      * @param ibcChannelUpgradeInitTryAck_ is the address of a contract that implements `IIBCChannelUpgradeInitTryAck`.
      * @param ibcChannelUpgradeConfirmOpenTimeoutCancel_ is the address of a contract that implements `IIBCChannelUpgradeConfirmOpenTimeoutCancel`.
+     * @custom:oz-upgrades-unsafe-allow constructor
      */
     constructor(
         IIBCClient ibcClient_,
@@ -84,6 +92,7 @@ abstract contract IBCClientConnectionChannelHandler is
     }
 
     function wrappedRouteUpdateClient(MsgUpdateClient calldata msg_) public returns (address, bytes4, bytes memory) {
+        /// @custom:oz-upgrades-unsafe-allow delegatecall
         (bool success, bytes memory returndata) =
             address(ibcClient).delegatecall(abi.encodeWithSelector(IIBCClient.routeUpdateClient.selector, msg_));
         if (!success) {

contracts/core/25-handler/IBCHandler.sol

@@ -25,6 +25,7 @@ abstract contract IBCHandler is IBCHostConfigurator, IBCClientConnectionChannelH
      * @param ibcChannelPacketTimeout_ is the address of a contract that implements `IIBCChannelPacketTimeout`.
      * @param ibcChannelUpgradeInitTryAck_ is the address of a contract that implements `IIBCChannelUpgrade`.
      * @param ibcChannelUpgradeConfirmOpenTimeoutCancel_ is the address of a contract that implements `IIBCChannelUpgrade`.
+     * @custom:oz-upgrades-unsafe-allow constructor
      */
     constructor(
         IIBCClient ibcClient_,

contracts/core/25-handler/OwnableUpgradeableIBCHandler.sol

@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.20;
+
+import {ILightClient} from "../02-client/ILightClient.sol";
+import {IIBCClient} from "../02-client/IIBCClient.sol";
+import {IIBCConnection} from "../03-connection/IIBCConnection.sol";
+import {
+    IIBCChannelHandshake, IIBCChannelPacketSendRecv, IIBCChannelPacketTimeout
+} from "../04-channel/IIBCChannel.sol";
+import {
+    IIBCChannelUpgradeInitTryAck,
+    IIBCChannelUpgradeConfirmOpenTimeoutCancel
+} from "../04-channel/IIBCChannelUpgrade.sol";
+import {IIBCModule} from "../26-router/IIBCModule.sol";
+import {IBCHandler} from "./IBCHandler.sol";
+import {Context} from "@openzeppelin/contracts/utils/Context.sol";
+import {ContextUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";
+import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
+import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+
+contract OwnableUpgradeableIBCHandler is IBCHandler, UUPSUpgradeable, OwnableUpgradeable {
+    /**
+     * @dev The arguments of constructor must satisfy the followings:
+     * @param ibcClient_ is the address of a contract that implements `IIBCClient`.
+     * @param ibcConnection_ is the address of a contract that implements `IIBCConnection`.
+     * @param ibcChannelHandshake_ is the address of a contract that implements `IIBCChannelHandshake`.
+     * @param ibcChannelPacketSendRecv_ is the address of a contract that implements `IIBCChannelPacketSendRecv`.
+     * @param ibcChannelPacketTimeout_ is the address of a contract that implements `IIBCChannelPacketTimeout`.
+     * @param ibcChannelUpgradeInitTryAck_ is the address of a contract that implements `IIBCChannelUpgrade`.
+     * @param ibcChannelUpgradeConfirmOpenTimeoutCancel_ is the address of a contract that implements `IIBCChannelUpgrade`.
+     * @custom:oz-upgrades-unsafe-allow constructor
+     */
+    constructor(
+        IIBCClient ibcClient_,
+        IIBCConnection ibcConnection_,
+        IIBCChannelHandshake ibcChannelHandshake_,
+        IIBCChannelPacketSendRecv ibcChannelPacketSendRecv_,
+        IIBCChannelPacketTimeout ibcChannelPacketTimeout_,
+        IIBCChannelUpgradeInitTryAck ibcChannelUpgradeInitTryAck_,
+        IIBCChannelUpgradeConfirmOpenTimeoutCancel ibcChannelUpgradeConfirmOpenTimeoutCancel_
+    )
+        IBCHandler(
+            ibcClient_,
+            ibcConnection_,
+            ibcChannelHandshake_,
+            ibcChannelPacketSendRecv_,
+            ibcChannelPacketTimeout_,
+            ibcChannelUpgradeInitTryAck_,
+            ibcChannelUpgradeConfirmOpenTimeoutCancel_
+        )
+    {}
+
+    function initialize() public virtual initializer {
+        __UUPSUpgradeable_init();
+        __Ownable_init(msg.sender);
+    }
+
+    function registerClient(string calldata clientType, ILightClient client) public virtual onlyOwner {
+        super._registerClient(clientType, client);
+    }
+
+    function bindPort(string calldata portId, IIBCModule moduleAddress) public virtual onlyOwner {
+        super._bindPort(portId, moduleAddress);
+    }
+
+    function setExpectedTimePerBlock(uint64 expectedTimePerBlock_) public virtual onlyOwner {
+        super._setExpectedTimePerBlock(expectedTimePerBlock_);
+    }
+
+    function _msgSender() internal view virtual override(Context, ContextUpgradeable) returns (address) {
+        return ContextUpgradeable._msgSender();
+    }
+
+    function _msgData() internal view virtual override(Context, ContextUpgradeable) returns (bytes calldata) {
+        return ContextUpgradeable._msgData();
+    }
+
+    function _contextSuffixLength() internal view virtual override(Context, ContextUpgradeable) returns (uint256) {
+        return ContextUpgradeable._contextSuffixLength();
+    }
+
+    function _authorizeUpgrade(address newImplementation) internal virtual override onlyOwner {}
+}

foundry.toml

@@ -7,6 +7,10 @@ gas_reports = ["*"]
 optimizer = true
 optimizer_runs = 9_999_999
 via-ir = false
+ffi = true
+ast = true
+build_info = true
+extra_output = ["storageLayout"]
 
 [fmt]
 line_length = 120