diff --git a/.gitmodules b/.gitmodules index a2df3f1..5bd8d99 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,9 @@ [submodule "lib/dss-test"] path = lib/dss-test url = https://github.com/makerdao/dss-test +[submodule "lib/openzeppelin-foundry-upgrades"] + path = lib/openzeppelin-foundry-upgrades + url = https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades +[submodule "lib/openzeppelin-contracts-upgradeable"] + path = lib/openzeppelin-contracts-upgradeable + url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable diff --git a/README.md b/README.md index 5194e90..d5855b1 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,8 @@ The Arbitrum Token Bridge is a [custom Arbitrum bridge](https://docs.arbitrum.io - `L1TokenGateway.sol` - L1 side of the bridge. Transfers the deposited tokens into an escrow contract. Transfer them back to the user upon receiving a withdrawal message from the `L2TokenGateway`. - `L2TokenGateway.sol` - L2 side of the bridge. Mints new L2 tokens after receiving a deposit message from `L1TokenGateway`. Burns L2 tokens when withdrawing them to L1. +The `L1TokenGateway` and `L2TokenGateway` contracts use the ERC-1822 UUPS pattern for upgradeability and the ERC-1967 proxy storage slots standard. It is important that the `TokenGatewayDeploy` library sequences be used for deploying. + ### External dependencies - The L2 implementations of the bridged tokens are not provided as part of this repository and are assumed to exist in external repositories. It is assumed that only simple, regular ERC20 tokens will be used with this bridge. In particular, the supported tokens are assumed to revert on failure (instead of returning false) and do not execute any hook on transfer. @@ -29,8 +31,14 @@ To withdraw her tokens back to L1, Alice calls `outboundTransfer()` on the `L2To ## Upgrades +### Upgrade the bridge implementation(s) + +`L1TokenGateway` and/or `L2TokenGateway` implementations can be upgraded by calling the `upgradeToAndCall` function of their inherited `UUPSUpgradeable` parent. Special care must be taken to ensure any deposit or withdrawal that is in transit at the time of the upgrade will still be able to get confirmed on the destination side. + ### Upgrade to a new bridge (and deprecate this bridge) +As an alternative upgrade mechanism, a new bridge can be deployed to be used with the escrow. + 1. Deploy the new token bridge and connect it to the same escrow as the one used by this bridge. The old and new bridges can operate in parallel. 2. Optionally, deprecate the old bridge by closing it. This involves calling `close()` on both the `L1TokenGateway` and `L2TokenGateway` so that no new outbound message can be sent to the other side of the bridge. After all cross-chain messages are done processing (can take ~1 week), the bridge is effectively closed and governance can consider revoking the approval to transfer funds from the escrow on L1 and the token minting rights on L2. @@ -45,6 +53,13 @@ To migrate a single token to a new bridge, follow the steps below: Note that step 3 is required because unregistering the token on `L2TokenGateway` not only removes the ability to initiate new L2 to L1 transfers but also causes the finalization of pending L1 to L2 transfers to revert. This is a point of difference with the implementation of the Arbitrum generic-custom gateway, where a missing L2 token triggers a withdrawal of the tokens back to L1 instead of a revert. +## Tests + +### OZ upgradeability validations + +The OZ validations can be run alongside the existing tests: +`VALIDATE=true forge test --ffi --build-info --extra-output storageLayout` + ## Deployment ### Declare env variables diff --git a/deploy/L1TokenGatewayInstance.sol b/deploy/L1TokenGatewayInstance.sol new file mode 100644 index 0000000..6680d38 --- /dev/null +++ b/deploy/L1TokenGatewayInstance.sol @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: © 2024 Dai Foundation +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +pragma solidity >=0.8.0; + +struct L1TokenGatewayInstance { + address gateway; + address gatewayImp; +} diff --git a/deploy/L2TokenGatewayInstance.sol b/deploy/L2TokenGatewayInstance.sol index 8f5edce..4f82a7e 100644 --- a/deploy/L2TokenGatewayInstance.sol +++ b/deploy/L2TokenGatewayInstance.sol @@ -18,5 +18,6 @@ pragma solidity >=0.8.0; struct L2TokenGatewayInstance { address gateway; + address gatewayImp; address spell; } diff --git a/deploy/L2TokenGatewaySpell.sol b/deploy/L2TokenGatewaySpell.sol index 78f89cc..f31506d 100644 --- a/deploy/L2TokenGatewaySpell.sol +++ b/deploy/L2TokenGatewaySpell.sol @@ -20,6 +20,9 @@ interface L2TokenGatewayLike { function isOpen() external view returns (uint256); function counterpartGateway() external view returns (address); function l2Router() external view returns (address); + function version() external view returns (string memory); + function getImplementation() external view returns (address); + function upgradeToAndCall(address, bytes memory) external; function rely(address) external; function deny(address) external; function close() external; @@ -39,6 +42,7 @@ contract L2TokenGatewaySpell { l2Gateway = L2TokenGatewayLike(l2Gateway_); } + function upgradeToAndCall(address newImp, bytes memory data) external { l2Gateway.upgradeToAndCall(newImp, data); } function rely(address usr) external { l2Gateway.rely(usr); } function deny(address usr) external { l2Gateway.deny(usr); } function close() external { l2Gateway.close(); } @@ -60,6 +64,7 @@ contract L2TokenGatewaySpell { function init( address l2Gateway_, + address l2GatewayImp, address counterpartGateway, address l2Router, address[] calldata l1Tokens, @@ -68,6 +73,8 @@ contract L2TokenGatewaySpell { ) external { // sanity checks require(address(l2Gateway) == l2Gateway_, "L2TokenGatewaySpell/l2-gateway-mismatch"); + require(keccak256(bytes(l2Gateway.version())) == keccak256("1"), "L2TokenGatewaySpell/version-does-not-match"); + require(l2Gateway.getImplementation() == l2GatewayImp, "L2TokenGatewaySpell/imp-does-not-match"); require(l2Gateway.isOpen() == 1, "L2TokenGatewaySpell/not-open"); require(l2Gateway.counterpartGateway() == counterpartGateway, "L2TokenGatewaySpell/counterpart-gateway-mismatch"); require(l2Gateway.l2Router() == l2Router, "L2TokenGatewaySpell/l2-router-mismatch"); diff --git a/deploy/TokenGatewayDeploy.sol b/deploy/TokenGatewayDeploy.sol index cff3e3e..85a42eb 100644 --- a/deploy/TokenGatewayDeploy.sol +++ b/deploy/TokenGatewayDeploy.sol @@ -18,6 +18,8 @@ pragma solidity >=0.8.0; import { ScriptTools } from "dss-test/ScriptTools.sol"; +import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import { L1TokenGatewayInstance } from "./L1TokenGatewayInstance.sol"; import { L2TokenGatewayInstance } from "./L2TokenGatewayInstance.sol"; import { L2TokenGatewaySpell } from "./L2TokenGatewaySpell.sol"; import { L1TokenGateway } from "src/L1TokenGateway.sol"; @@ -30,9 +32,10 @@ library TokenGatewayDeploy { address l2Gateway, address l1Router, address inbox - ) internal returns (address l1Gateway) { - l1Gateway = address(new L1TokenGateway(l2Gateway, l1Router, inbox)); - ScriptTools.switchOwner(l1Gateway, deployer, owner); + ) internal returns (L1TokenGatewayInstance memory l1GatewayInstance) { + l1GatewayInstance.gatewayImp = address(new L1TokenGateway(l2Gateway, l1Router, inbox)); + l1GatewayInstance.gateway = address(new ERC1967Proxy(l1GatewayInstance.gatewayImp, abi.encodeCall(L1TokenGateway.initialize, ()))); + ScriptTools.switchOwner(l1GatewayInstance.gateway, deployer, owner); } function deployL2Gateway( @@ -41,7 +44,8 @@ library TokenGatewayDeploy { address l1Gateway, address l2Router ) internal returns (L2TokenGatewayInstance memory l2GatewayInstance) { - l2GatewayInstance.gateway = address(new L2TokenGateway(l1Gateway, l2Router)); + l2GatewayInstance.gatewayImp = address(new L2TokenGateway(l1Gateway, l2Router)); + l2GatewayInstance.gateway = address(new ERC1967Proxy(l2GatewayInstance.gatewayImp, abi.encodeCall(L2TokenGateway.initialize, ()))); l2GatewayInstance.spell = address(new L2TokenGatewaySpell(l2GatewayInstance.gateway)); ScriptTools.switchOwner(l2GatewayInstance.gateway, deployer, owner); } diff --git a/deploy/TokenGatewayInit.sol b/deploy/TokenGatewayInit.sol index fa56039..38107c0 100644 --- a/deploy/TokenGatewayInit.sol +++ b/deploy/TokenGatewayInit.sol @@ -18,6 +18,7 @@ pragma solidity >=0.8.0; import { DssInstance } from "dss-test/MCD.sol"; import { L2TokenGatewayInstance } from "./L2TokenGatewayInstance.sol"; +import { L1TokenGatewayInstance } from "./L1TokenGatewayInstance.sol"; import { L2TokenGatewaySpell } from "./L2TokenGatewaySpell.sol"; interface L1TokenGatewayLike { @@ -26,6 +27,8 @@ interface L1TokenGatewayLike { function counterpartGateway() external view returns (address); function l1Router() external view returns (address); function inbox() external view returns (address); + function version() external view returns (string memory); + function getImplementation() external view returns (address); function file(bytes32, address) external; function registerToken(address, address) external; } @@ -67,16 +70,18 @@ struct GatewaysConfig { library TokenGatewayInit { function initGateways( DssInstance memory dss, - address l1Gateway_, + L1TokenGatewayInstance memory l1GatewayInstance, L2TokenGatewayInstance memory l2GatewayInstance, GatewaysConfig memory cfg ) internal { - L1TokenGatewayLike l1Gateway = L1TokenGatewayLike(l1Gateway_); + L1TokenGatewayLike l1Gateway = L1TokenGatewayLike(l1GatewayInstance.gateway); L1RelayLike l1GovRelay = L1RelayLike(dss.chainlog.getAddress("ARBITRUM_GOV_RELAY")); EscrowLike escrow = EscrowLike(dss.chainlog.getAddress("ARBITRUM_ESCROW")); L1RouterLike l1Router = L1RouterLike(cfg.l1Router); // sanity checks + require(keccak256(bytes(l1Gateway.version())) == keccak256("1"), "TokenGatewayInit/version-does-not-match"); + require(l1Gateway.getImplementation() == l1GatewayInstance.gatewayImp, "TokenGatewayInit/imp-does-not-match"); require(l1Gateway.isOpen() == 1, "TokenGatewayInit/not-open"); require(l1Gateway.counterpartGateway() == l2GatewayInstance.gateway, "TokenGatewayInit/counterpart-gateway-mismatch"); require(l1Gateway.l1Router() == cfg.l1Router, "TokenGatewayInit/l1-router-mismatch"); @@ -100,14 +105,15 @@ library TokenGatewayInit { require(l1Gateway.l1ToL2Token(l1Token) == address(0), "TokenGatewayInit/existing-l1-token"); l1Gateway.registerToken(l1Token, l2Token); - escrow.approve(l1Token, l1Gateway_, type(uint256).max); + escrow.approve(l1Token, l1GatewayInstance.gateway, type(uint256).max); } l1GovRelay.relay({ target: l2GatewayInstance.spell, targetData: abi.encodeCall(L2TokenGatewaySpell.init, ( l2GatewayInstance.gateway, - l1Gateway_, + l2GatewayInstance.gatewayImp, + l1GatewayInstance.gateway, l1Router.counterpartGateway(), cfg.l1Tokens, cfg.l2Tokens, @@ -119,6 +125,7 @@ library TokenGatewayInit { maxSubmissionCost: cfg.xchainMsg.maxSubmissionCost }); - dss.chainlog.setAddress("ARBITRUM_TOKEN_BRIDGE", l1Gateway_); + dss.chainlog.setAddress("ARBITRUM_TOKEN_BRIDGE", l1GatewayInstance.gateway); + dss.chainlog.setAddress("ARBITRUM_TOKEN_BRIDGE_IMP", l1GatewayInstance.gatewayImp); } } diff --git a/lib/openzeppelin-contracts-upgradeable b/lib/openzeppelin-contracts-upgradeable new file mode 160000 index 0000000..723f8ca --- /dev/null +++ b/lib/openzeppelin-contracts-upgradeable @@ -0,0 +1 @@ +Subproject commit 723f8cab09cdae1aca9ec9cc1cfa040c2d4b06c1 diff --git a/lib/openzeppelin-foundry-upgrades b/lib/openzeppelin-foundry-upgrades new file mode 160000 index 0000000..332bd33 --- /dev/null +++ b/lib/openzeppelin-foundry-upgrades @@ -0,0 +1 @@ +Subproject commit 332bd3306242e09520df2685b2edb99ebd7f5d37 diff --git a/remappings.txt b/remappings.txt new file mode 100644 index 0000000..7761e9f --- /dev/null +++ b/remappings.txt @@ -0,0 +1,3 @@ +@openzeppelin/contracts/=lib/openzeppelin-contracts-upgradeable/lib/openzeppelin-contracts/contracts/ +@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/ +forge-std/=lib/dss-test/lib/forge-std/src/ \ No newline at end of file diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol index c87facc..f6986a2 100644 --- a/script/Deploy.s.sol +++ b/script/Deploy.s.sol @@ -21,7 +21,7 @@ import "forge-std/Script.sol"; import { ScriptTools } from "dss-test/ScriptTools.sol"; import { Domain } from "dss-test/domains/Domain.sol"; -import { TokenGatewayDeploy, L2TokenGatewayInstance } from "deploy/TokenGatewayDeploy.sol"; +import { TokenGatewayDeploy, L1TokenGatewayInstance, L2TokenGatewayInstance } from "deploy/TokenGatewayDeploy.sol"; import { ChainLog } from "deploy/mocks/ChainLog.sol"; import { L1Escrow } from "deploy/mocks/L1Escrow.sol"; import { L1GovernanceRelay } from "deploy/mocks/L1GovernanceRelay.sol"; @@ -118,11 +118,11 @@ contract Deploy is Script { // L1 deployment l2Domain.selectFork(); - address l2Gateway = vm.computeCreateAddress(l2Deployer, vm.getNonce(l2Deployer)); + address l2Gateway = vm.computeCreateAddress(l2Deployer, vm.getNonce(l2Deployer) + 1); l1Domain.selectFork(); vm.startBroadcast(l1PrivKey); - address l1Gateway = TokenGatewayDeploy.deployL1Gateway(l1Deployer, owner, l2Gateway, l1Router, inbox); + L1TokenGatewayInstance memory l1GatewayInstance = TokenGatewayDeploy.deployL1Gateway(l1Deployer, owner, l2Gateway, l1Router, inbox); vm.stopBroadcast(); address l2Router = L1RouterLike(l1Router).counterpartGateway(); @@ -130,7 +130,7 @@ contract Deploy is Script { l2Domain.selectFork(); vm.startBroadcast(l2PrivKey); - L2TokenGatewayInstance memory l2GatewayInstance = TokenGatewayDeploy.deployL2Gateway(l2Deployer, l2GovRelay, l1Gateway, l2Router); + L2TokenGatewayInstance memory l2GatewayInstance = TokenGatewayDeploy.deployL2Gateway(l2Deployer, l2GovRelay, l1GatewayInstance.gateway, l2Router); require(l2GatewayInstance.gateway == l2Gateway, "l2Gateway address mismatch"); vm.stopBroadcast(); @@ -144,8 +144,10 @@ contract Deploy is Script { ScriptTools.exportContract("deployed", "escrow", escrow); ScriptTools.exportContract("deployed", "l1GovRelay", l1GovRelay); ScriptTools.exportContract("deployed", "l2GovRelay", l2GovRelay); - ScriptTools.exportContract("deployed", "l1Gateway", l1Gateway); + ScriptTools.exportContract("deployed", "l1Gateway", l1GatewayInstance.gateway); + ScriptTools.exportContract("deployed", "l1GatewayImp", l1GatewayInstance.gatewayImp); ScriptTools.exportContract("deployed", "l2Gateway", l2Gateway); + ScriptTools.exportContract("deployed", "l2GatewayImp", l2GatewayInstance.gatewayImp); ScriptTools.exportContract("deployed", "l2GatewaySpell", l2GatewayInstance.spell); ScriptTools.exportContracts("deployed", "l1Tokens", l1Tokens); ScriptTools.exportContracts("deployed", "l2Tokens", l2Tokens); diff --git a/script/Init.s.sol b/script/Init.s.sol index b8b1e14..ea09440 100644 --- a/script/Init.s.sol +++ b/script/Init.s.sol @@ -23,6 +23,7 @@ import { ScriptTools } from "dss-test/ScriptTools.sol"; import { Domain } from "dss-test/domains/Domain.sol"; import { MCD, DssInstance } from "dss-test/MCD.sol"; import { TokenGatewayInit, GatewaysConfig, MessageParams } from "deploy/TokenGatewayInit.sol"; +import { L1TokenGatewayInstance } from "deploy/L1TokenGatewayInstance.sol"; import { L2TokenGatewayInstance } from "deploy/L2TokenGatewayInstance.sol"; import { L2TokenGatewaySpell } from "deploy/L2TokenGatewaySpell.sol"; import { L2GovernanceRelay } from "deploy/mocks/L2GovernanceRelay.sol"; @@ -64,6 +65,7 @@ contract Init is Script { deps.readAddress(".l2GatewaySpell"), abi.encodeCall(L2TokenGatewaySpell.init, ( deps.readAddress(".l2Gateway"), + deps.readAddress(".l2GatewayImp"), l1Gateway, deps.readAddress(".l2Router"), cfg.l1Tokens, @@ -77,9 +79,14 @@ contract Init is Script { maxSubmissionCost: retryable.getSubmissionFee(initCalldata) * 250 / 100 }); + L1TokenGatewayInstance memory l1GatewayInstance = L1TokenGatewayInstance({ + gateway: deps.readAddress(".l1Gateway"), + gatewayImp: deps.readAddress(".l1GatewayImp") + }); L2TokenGatewayInstance memory l2GatewayInstance = L2TokenGatewayInstance({ - spell: deps.readAddress(".l2GatewaySpell"), - gateway: deps.readAddress(".l2Gateway") + spell: deps.readAddress(".l2GatewaySpell"), + gateway: deps.readAddress(".l2Gateway"), + gatewayImp: deps.readAddress(".l2GatewayImp") }); vm.startBroadcast(l1PrivKey); @@ -89,7 +96,7 @@ contract Init is Script { require(success, "l1GovRelay topup failed"); } - TokenGatewayInit.initGateways(dss, l1Gateway, l2GatewayInstance, cfg); + TokenGatewayInit.initGateways(dss, l1GatewayInstance, l2GatewayInstance, cfg); vm.stopBroadcast(); } } diff --git a/src/L1TokenGateway.sol b/src/L1TokenGateway.sol index 7a990fe..6fe0d44 100644 --- a/src/L1TokenGateway.sol +++ b/src/L1TokenGateway.sol @@ -17,6 +17,7 @@ pragma solidity ^0.8.21; +import { UUPSUpgradeable, ERC1967Utils } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import { ITokenGateway } from "src/arbitrum/ITokenGateway.sol"; import { IL1ArbitrumGateway } from "src/arbitrum/IL1ArbitrumGateway.sol"; import { ICustomGateway } from "src/arbitrum/ICustomGateway.sol"; @@ -27,19 +28,20 @@ interface TokenLike { function transferFrom(address, address, uint256) external; } -contract L1TokenGateway is ITokenGateway, IL1ArbitrumGateway, ICustomGateway, ERC165, L1ArbitrumMessenger { +contract L1TokenGateway is UUPSUpgradeable, ITokenGateway, IL1ArbitrumGateway, ICustomGateway, ERC165, L1ArbitrumMessenger { // --- storage variables --- mapping(address => uint256) public wards; mapping(address => address) public l1ToL2Token; - uint256 public isOpen = 1; + uint256 public isOpen; address public escrow; - // --- immutables --- + // --- immutables and const --- address public immutable counterpartGateway; address public immutable l1Router; address public immutable inbox; + string public constant version = "1"; // --- events --- @@ -87,14 +89,29 @@ contract L1TokenGateway is ITokenGateway, IL1ArbitrumGateway, ICustomGateway, ER address _l1Router, address _inbox ) { + _disableInitializers(); // Avoid initializing in the context of the implementation + counterpartGateway = _counterpartGateway; l1Router = _l1Router; inbox = _inbox; + } + + // --- upgradability --- + + function initialize() initializer external { + __UUPSUpgradeable_init(); + isOpen = 1; wards[msg.sender] = 1; emit Rely(msg.sender); } + function _authorizeUpgrade(address newImplementation) internal override auth {} + + function getImplementation() external view returns (address) { + return ERC1967Utils.getImplementation(); + } + // --- administration --- function rely(address usr) external auth { diff --git a/src/L2TokenGateway.sol b/src/L2TokenGateway.sol index 87c3267..0c9a5c9 100644 --- a/src/L2TokenGateway.sol +++ b/src/L2TokenGateway.sol @@ -17,6 +17,7 @@ pragma solidity ^0.8.21; +import { UUPSUpgradeable, ERC1967Utils } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import { ITokenGateway } from "src/arbitrum/ITokenGateway.sol"; import { ICustomGateway } from "src/arbitrum/ICustomGateway.sol"; import { AddressAliasHelper } from "src/arbitrum/AddressAliasHelper.sol"; @@ -27,18 +28,19 @@ interface TokenLike { function burn(address, uint256) external; } -contract L2TokenGateway is ITokenGateway, ICustomGateway, L2ArbitrumMessenger { +contract L2TokenGateway is UUPSUpgradeable, ITokenGateway, ICustomGateway, L2ArbitrumMessenger { // --- storage variables --- mapping(address => uint256) public wards; mapping(address => address) public l1ToL2Token; mapping(address => uint256) public maxWithdraws; - uint256 public isOpen = 1; + uint256 public isOpen; - // --- immutables --- + // --- immutables and const--- address public immutable l2Router; address public immutable counterpartGateway; + string public constant version = "1"; // --- events --- @@ -82,13 +84,28 @@ contract L2TokenGateway is ITokenGateway, ICustomGateway, L2ArbitrumMessenger { address _counterpartGateway, address _l2Router ) { + _disableInitializers(); // Avoid initializing in the context of the implementation + counterpartGateway = _counterpartGateway; l2Router = _l2Router; + } + + // --- upgradability --- + + function initialize() initializer external { + __UUPSUpgradeable_init(); + isOpen = 1; wards[msg.sender] = 1; emit Rely(msg.sender); } + function _authorizeUpgrade(address newImplementation) internal override auth {} + + function getImplementation() external view returns (address) { + return ERC1967Utils.getImplementation(); + } + // --- administration --- function rely(address usr) external auth { diff --git a/test/Integration.t.sol b/test/Integration.t.sol index bcffe6a..4743da6 100644 --- a/test/Integration.t.sol +++ b/test/Integration.t.sol @@ -22,15 +22,26 @@ import "dss-test/DssTest.sol"; import { Domain } from "dss-test/domains/Domain.sol"; import { ArbitrumDomain } from "dss-test/domains/ArbitrumDomain.sol"; import { TokenGatewayDeploy } from "deploy/TokenGatewayDeploy.sol"; -import { L2TokenGatewaySpell } from "deploy/L2TokenGatewaySpell.sol"; +import { L1TokenGatewayInstance } from "deploy/L1TokenGatewayInstance.sol"; import { L2TokenGatewayInstance } from "deploy/L2TokenGatewayInstance.sol"; +import { L2TokenGatewaySpell } from "deploy/L2TokenGatewaySpell.sol"; import { TokenGatewayInit, GatewaysConfig, MessageParams } from "deploy/TokenGatewayInit.sol"; import { L1TokenGateway } from "src/L1TokenGateway.sol"; import { L2TokenGateway } from "src/L2TokenGateway.sol"; import { GemMock } from "test/mocks/GemMock.sol"; +import { L1TokenGatewayV2Mock } from "test/mocks/L1TokenGatewayV2Mock.sol"; +import { L2TokenGatewayV2Mock } from "test/mocks/L2TokenGatewayV2Mock.sol"; interface L1RelayLike { function l2GovernanceRelay() external view returns (address); + function relay( + address target, + bytes calldata targetData, + uint256 l1CallValue, + uint256 maxGas, + uint256 gasPriceBid, + uint256 maxSubmissionCost + ) external payable; } interface L1RouterLike { @@ -55,6 +66,7 @@ contract IntegrationTest is DssTest { DssInstance dss; address PAUSE_PROXY; address ESCROW; + address L1_GOV_RELAY; GemMock l1Token; L1TokenGateway l1Gateway; address INBOX; @@ -64,6 +76,7 @@ contract IntegrationTest is DssTest { address L2_GOV_RELAY; GemMock l2Token; L2TokenGateway l2Gateway; + address l2Spell; address L2_ROUTER; function setUp() public { @@ -76,9 +89,11 @@ contract IntegrationTest is DssTest { dss = l1Domain.dss(); PAUSE_PROXY = dss.chainlog.getAddress("MCD_PAUSE_PROXY"); ESCROW = dss.chainlog.getAddress("ARBITRUM_ESCROW"); - L2_GOV_RELAY = L1RelayLike(dss.chainlog.getAddress("ARBITRUM_GOV_RELAY")).l2GovernanceRelay(); + L1_GOV_RELAY = dss.chainlog.getAddress("ARBITRUM_GOV_RELAY"); + L2_GOV_RELAY = L1RelayLike(L1_GOV_RELAY).l2GovernanceRelay(); vm.label(address(PAUSE_PROXY), "PAUSE_PROXY"); vm.label(address(ESCROW), "ESCROW"); + vm.label(address(L1_GOV_RELAY), "L1_GOV_RELAY"); vm.label(address(L2_GOV_RELAY), "L2_GOV_RELAY"); l2Domain = new ArbitrumDomain(config, getChain("arbitrum_one"), l1Domain); @@ -89,7 +104,7 @@ contract IntegrationTest is DssTest { L2_ROUTER = L1RouterLike(L1_ROUTER).counterpartGateway(); vm.label(L2_ROUTER, "L2_ROUTER"); - address l1Gateway_ = vm.computeCreateAddress(address(this), vm.getNonce(address(this)) + 2); // foundry increments a global nonce across domains + address l1Gateway_ = vm.computeCreateAddress(address(this), vm.getNonce(address(this)) + 4); // foundry increments a global nonce across domains l2Domain.selectFork(); L2TokenGatewayInstance memory l2GatewayInstance = TokenGatewayDeploy.deployL2Gateway({ deployer: address(this), @@ -98,17 +113,23 @@ contract IntegrationTest is DssTest { l2Router: L2_ROUTER }); l2Gateway = L2TokenGateway(l2GatewayInstance.gateway); - assertEq(address(L2TokenGatewaySpell(l2GatewayInstance.spell).l2Gateway()), address(l2Gateway)); + l2Spell = l2GatewayInstance.spell; + assertEq(address(L2TokenGatewaySpell(l2Spell).l2Gateway()), address(l2Gateway)); + assertEq(l2Gateway.version(), "1"); + assertEq(l2Gateway.getImplementation(), l2GatewayInstance.gatewayImp); l1Domain.selectFork(); - l1Gateway = L1TokenGateway(TokenGatewayDeploy.deployL1Gateway({ + L1TokenGatewayInstance memory l1GatewayInstance = TokenGatewayDeploy.deployL1Gateway({ deployer: address(this), owner: PAUSE_PROXY, l2Gateway: address(l2Gateway), l1Router: L1_ROUTER, inbox: INBOX - })); + }); + l1Gateway = L1TokenGateway(l1GatewayInstance.gateway); assertEq(address(l1Gateway), l1Gateway_); + assertEq(l1Gateway.version(), "1"); + assertEq(l1Gateway.getImplementation(), l1GatewayInstance.gatewayImp); l1Token = new GemMock(100 ether); vm.label(address(l1Token), "l1Token"); @@ -141,13 +162,14 @@ contract IntegrationTest is DssTest { l1Domain.selectFork(); vm.startPrank(PAUSE_PROXY); - TokenGatewayInit.initGateways(dss, address(l1Gateway), l2GatewayInstance, cfg); + TokenGatewayInit.initGateways(dss, l1GatewayInstance, l2GatewayInstance, cfg); vm.stopPrank(); // test L1 side of initGateways assertEq(l1Token.allowance(ESCROW, l1Gateway_), type(uint256).max); assertEq(l1Gateway.l1ToL2Token(address(l1Token)), address(l2Token)); assertEq(dss.chainlog.getAddress("ARBITRUM_TOKEN_BRIDGE"), address(l1Gateway)); + assertEq(dss.chainlog.getAddress("ARBITRUM_TOKEN_BRIDGE_IMP"), l1GatewayInstance.gatewayImp); l2Domain.relayFromHost(true); @@ -251,4 +273,39 @@ contract IntegrationTest is DssTest { function testWithdrawViaRouter() public { _withdraw(L2_ROUTER); } + + function testUpgrade() public { + l2Domain.selectFork(); + address newL2Imp = address(new L2TokenGatewayV2Mock()); + l1Domain.selectFork(); + address newL1Imp = address(new L1TokenGatewayV2Mock()); + + vm.startPrank(PAUSE_PROXY); + l1Gateway.upgradeToAndCall(newL1Imp, abi.encodeCall(L1TokenGatewayV2Mock.reinitialize, ())); + vm.stopPrank(); + + assertEq(l1Gateway.getImplementation(), newL1Imp); + assertEq(l1Gateway.version(), "2"); + assertEq(l1Gateway.wards(PAUSE_PROXY), 1); // still a ward + + vm.startPrank(PAUSE_PROXY); + L1RelayLike(L1_GOV_RELAY).relay({ + target: l2Spell, + targetData: abi.encodeCall(L2TokenGatewaySpell.upgradeToAndCall, ( + newL2Imp, + abi.encodeCall(L2TokenGatewayV2Mock.reinitialize, ()) + )), + l1CallValue: 0.01 ether + 300_000 * 0.1 gwei, + gasPriceBid: 0.1 gwei, + maxGas: 300_000, + maxSubmissionCost: 0.01 ether + }); + vm.stopPrank(); + + l2Domain.relayFromHost(true); + + assertEq(l2Gateway.getImplementation(), newL2Imp); + assertEq(l2Gateway.version(), "2"); + assertEq(l2Gateway.wards(L2_GOV_RELAY), 1); // still a ward + } } diff --git a/test/L1TokenGateway.t.sol b/test/L1TokenGateway.t.sol index 8a5f51d..8d991b5 100644 --- a/test/L1TokenGateway.t.sol +++ b/test/L1TokenGateway.t.sol @@ -18,10 +18,13 @@ pragma solidity ^0.8.21; import "dss-test/DssTest.sol"; - +import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import { Upgrades, Options } from "openzeppelin-foundry-upgrades/Upgrades.sol"; import { L1TokenGateway } from "src/L1TokenGateway.sol"; import { InboxMock, BridgeMock, OutboxMock } from "test/mocks/InboxMock.sol"; import { GemMock } from "test/mocks/GemMock.sol"; +import { L1TokenGatewayV2Mock } from "test/mocks/L1TokenGatewayV2Mock.sol"; contract L1TokenGatewayTest is DssTest { @@ -42,6 +45,7 @@ contract L1TokenGatewayTest is DssTest { uint256 _amount ); event TxToL2(address indexed _from, address indexed _to, uint256 indexed _seqNum, bytes _data); + event UpgradedTo(string version); GemMock l1Token; L1TokenGateway gateway; @@ -49,26 +53,32 @@ contract L1TokenGatewayTest is DssTest { address counterpartGateway = address(0xccc); address l1Router = address(0xbbb); InboxMock inbox; + bool validate; function setUp() public { + validate = vm.envOr("VALIDATE", false); + inbox = new InboxMock(); - gateway = new L1TokenGateway(counterpartGateway, l1Router, address(inbox)); - gateway.file("escrow", escrow); - l1Token = new GemMock(1_000_000 ether); - vm.prank(escrow); l1Token.approve(address(gateway), type(uint256).max); - gateway.registerToken(address(l1Token), address(0xf00)); - } - function testConstructor() public { + L1TokenGateway imp = new L1TokenGateway(counterpartGateway, l1Router, address(inbox)); + assertEq(imp.counterpartGateway(), counterpartGateway); + assertEq(imp.l1Router(), l1Router); + assertEq(imp.inbox(), address(inbox)); + vm.expectEmit(true, true, true, true); emit Rely(address(this)); - L1TokenGateway g = new L1TokenGateway(address(111), address(222), address(333)); + gateway = L1TokenGateway(address(new ERC1967Proxy(address(imp), abi.encodeCall(L1TokenGateway.initialize, ())))); + assertEq(gateway.getImplementation(), address(imp)); + assertEq(gateway.wards(address(this)), 1); + assertEq(gateway.isOpen(), 1); + assertEq(gateway.counterpartGateway(), counterpartGateway); + assertEq(gateway.l1Router(), l1Router); + assertEq(gateway.inbox(), address(inbox)); - assertEq(g.isOpen(), 1); - assertEq(g.counterpartGateway(), address(111)); - assertEq(g.l1Router(), address(222)); - assertEq(g.inbox(), address(333)); - assertEq(g.wards(address(this)), 1); + gateway.file("escrow", escrow); + l1Token = new GemMock(1_000_000 ether); + vm.prank(escrow); l1Token.approve(address(gateway), type(uint256).max); + gateway.registerToken(address(l1Token), address(0xf00)); } function testAuth() public { @@ -80,7 +90,8 @@ contract L1TokenGatewayTest is DssTest { checkModifier(address(gateway), string(abi.encodePacked("L1TokenGateway", "/not-authorized")), [ gateway.close.selector, - gateway.registerToken.selector + gateway.registerToken.selector, + gateway.upgradeToAndCall.selector ]); } @@ -186,4 +197,72 @@ contract L1TokenGatewayTest is DssTest { assertEq(l1Token.balanceOf(escrow), 0); assertEq(l1Token.balanceOf(address(0xced)), 100 ether); } + + function testDeployWithUpgradesLib() public { + Options memory opts; + if (!validate) { + opts.unsafeSkipAllChecks = true; + } else { + opts.unsafeAllow = 'state-variable-immutable,constructor'; + } + opts.constructorData = abi.encode(counterpartGateway, l1Router, address(inbox)); + + vm.expectEmit(true, true, true, true); + emit Rely(address(this)); + address proxy = Upgrades.deployUUPSProxy( + "out/L1TokenGateway.sol/L1TokenGateway.json", + abi.encodeCall(L1TokenGateway.initialize, ()), + opts + ); + assertEq(L1TokenGateway(proxy).version(), "1"); + assertEq(L1TokenGateway(proxy).wards(address(this)), 1); + } + + function testUpgrade() public { + address newImpl = address(new L1TokenGatewayV2Mock()); + vm.expectEmit(true, true, true, true); + emit UpgradedTo("2"); + gateway.upgradeToAndCall(newImpl, abi.encodeCall(L1TokenGatewayV2Mock.reinitialize, ())); + + assertEq(gateway.getImplementation(), newImpl); + assertEq(gateway.version(), "2"); + assertEq(gateway.wards(address(this)), 1); // still a ward + } + + function testUpgradeWithUpgradesLib() public { + address implementation1 = gateway.getImplementation(); + + Options memory opts; + if (!validate) { + opts.unsafeSkipAllChecks = true; + } else { + opts.referenceContract = "out/L1TokenGateway.sol/L1TokenGateway.json"; + opts.unsafeAllow = 'constructor'; + } + + vm.expectEmit(true, true, true, true); + emit UpgradedTo("2"); + Upgrades.upgradeProxy( + address(gateway), + "out/L1TokenGatewayV2Mock.sol/L1TokenGatewayV2Mock.json", + abi.encodeCall(L1TokenGatewayV2Mock.reinitialize, ()), + opts + ); + + address implementation2 = gateway.getImplementation(); + assertTrue(implementation1 != implementation2); + assertEq(gateway.version(), "2"); + assertEq(gateway.wards(address(this)), 1); // still a ward + } + + function testInitializeAgain() public { + vm.expectRevert(Initializable.InvalidInitialization.selector); + gateway.initialize(); + } + + function testInitializeDirectly() public { + address implementation = gateway.getImplementation(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + L1TokenGateway(implementation).initialize(); + } } diff --git a/test/L2TokenGateway.t.sol b/test/L2TokenGateway.t.sol index 1d5162e..6554b3a 100644 --- a/test/L2TokenGateway.t.sol +++ b/test/L2TokenGateway.t.sol @@ -18,11 +18,14 @@ pragma solidity ^0.8.21; import "dss-test/DssTest.sol"; - +import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import { Upgrades, Options } from "openzeppelin-foundry-upgrades/Upgrades.sol"; import { L2TokenGateway } from "src/L2TokenGateway.sol"; import { GemMock } from "test/mocks/GemMock.sol"; import { ArbSysMock } from "test/mocks/ArbSysMock.sol"; import { AddressAliasHelper } from "src/arbitrum/AddressAliasHelper.sol"; +import { L2TokenGatewayV2Mock } from "test/mocks/L2TokenGatewayV2Mock.sol"; contract L2TokenGatewayTest is DssTest { @@ -44,6 +47,7 @@ contract L2TokenGatewayTest is DssTest { uint256 _amount ); event TxToL1(address indexed _from, address indexed _to, uint256 indexed _id, bytes _data); + event UpgradedTo(string version); address ARB_SYS_ADDRESS = address(100); address l1Token = address(0xf00); @@ -51,9 +55,24 @@ contract L2TokenGatewayTest is DssTest { L2TokenGateway gateway; address counterpartGateway = address(0xccc); address l2Router = address(0xbbb); + bool validate; function setUp() public { - gateway = new L2TokenGateway(counterpartGateway, l2Router); + validate = vm.envOr("VALIDATE", false); + + L2TokenGateway imp = new L2TokenGateway(counterpartGateway, l2Router); + assertEq(imp.counterpartGateway(), counterpartGateway); + assertEq(imp.l2Router(), l2Router); + + vm.expectEmit(true, true, true, true); + emit Rely(address(this)); + gateway = L2TokenGateway(address(new ERC1967Proxy(address(imp), abi.encodeCall(L2TokenGateway.initialize, ())))); + assertEq(gateway.getImplementation(), address(imp)); + assertEq(gateway.wards(address(this)), 1); + assertEq(gateway.isOpen(), 1); + assertEq(gateway.counterpartGateway(), counterpartGateway); + assertEq(gateway.l2Router(), l2Router); + l2Token = new GemMock(1_000_000 ether); l2Token.rely(address(gateway)); l2Token.deny(address(this)); @@ -62,17 +81,6 @@ contract L2TokenGatewayTest is DssTest { vm.etch(ARB_SYS_ADDRESS, address(new ArbSysMock()).code); } - function testConstructor() public { - vm.expectEmit(true, true, true, true); - emit Rely(address(this)); - L2TokenGateway g = new L2TokenGateway(address(111), address(222)); - - assertEq(g.isOpen(), 1); - assertEq(g.counterpartGateway(), address(111)); - assertEq(g.l2Router(), address(222)); - assertEq(g.wards(address(this)), 1); - } - function testAuth() public { checkAuth(address(gateway), "L2TokenGateway"); } @@ -83,7 +91,8 @@ contract L2TokenGatewayTest is DssTest { checkModifier(address(gateway), string(abi.encodePacked("L2TokenGateway", "/not-authorized")), [ gateway.close.selector, gateway.registerToken.selector, - gateway.setMaxWithdraw.selector + gateway.setMaxWithdraw.selector, + gateway.upgradeToAndCall.selector ]); } @@ -191,4 +200,72 @@ contract L2TokenGatewayTest is DssTest { assertEq(l2Token.totalSupply(), supplyBefore + 100 ether); } + function testDeployWithUpgradesLib() public { + Options memory opts; + if (!validate) { + opts.unsafeSkipAllChecks = true; + } else { + opts.unsafeAllow = 'state-variable-immutable,constructor'; + } + opts.constructorData = abi.encode(counterpartGateway, l2Router); + + vm.expectEmit(true, true, true, true); + emit Rely(address(this)); + address proxy = Upgrades.deployUUPSProxy( + "out/L2TokenGateway.sol/L2TokenGateway.json", + abi.encodeCall(L2TokenGateway.initialize, ()), + opts + ); + assertEq(L2TokenGateway(proxy).version(), "1"); + assertEq(L2TokenGateway(proxy).wards(address(this)), 1); + } + + function testUpgrade() public { + address newImpl = address(new L2TokenGatewayV2Mock()); + vm.expectEmit(true, true, true, true); + emit UpgradedTo("2"); + gateway.upgradeToAndCall(newImpl, abi.encodeCall(L2TokenGatewayV2Mock.reinitialize, ())); + + assertEq(gateway.getImplementation(), newImpl); + assertEq(gateway.version(), "2"); + assertEq(gateway.wards(address(this)), 1); // still a ward + } + + function testUpgradeWithUpgradesLib() public { + address implementation1 = gateway.getImplementation(); + + Options memory opts; + if (!validate) { + opts.unsafeSkipAllChecks = true; + } else { + opts.referenceContract = "out/L2TokenGateway.sol/L2TokenGateway.json"; + opts.unsafeAllow = 'constructor'; + } + + vm.expectEmit(true, true, true, true); + emit UpgradedTo("2"); + Upgrades.upgradeProxy( + address(gateway), + "out/L2TokenGatewayV2Mock.sol/L2TokenGatewayV2Mock.json", + abi.encodeCall(L2TokenGatewayV2Mock.reinitialize, ()), + opts + ); + + address implementation2 = gateway.getImplementation(); + assertTrue(implementation1 != implementation2); + assertEq(gateway.version(), "2"); + assertEq(gateway.wards(address(this)), 1); // still a ward + } + + function testInitializeAgain() public { + vm.expectRevert(Initializable.InvalidInitialization.selector); + gateway.initialize(); + } + + function testInitializeDirectly() public { + address implementation = gateway.getImplementation(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + L2TokenGateway(implementation).initialize(); + } + } diff --git a/test/mocks/L1TokenGatewayV2Mock.sol b/test/mocks/L1TokenGatewayV2Mock.sol new file mode 100644 index 0000000..dcffc85 --- /dev/null +++ b/test/mocks/L1TokenGatewayV2Mock.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Copyright (C) 2024 Dai Foundation +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +pragma solidity ^0.8.21; + +import { UUPSUpgradeable, ERC1967Utils } from "src/L1TokenGateway.sol"; + +contract L1TokenGatewayV2Mock is UUPSUpgradeable { + mapping(address => uint256) public wards; + mapping(address => address) public l1ToL2Token; + uint256 public isOpen; + address public escrow; + + string public constant version = "2"; + + event UpgradedTo(string version); + + modifier auth { + require(wards[msg.sender] == 1, "L1TokenGateway/not-authorized"); + _; + } + + constructor() { + _disableInitializers(); // Avoid initializing in the context of the implementation + } + + function reinitialize() reinitializer(2) external { + emit UpgradedTo(version); + } + + function _authorizeUpgrade(address newImplementation) internal override auth {} + + function getImplementation() external view returns (address) { + return ERC1967Utils.getImplementation(); + } +} diff --git a/test/mocks/L2TokenGatewayV2Mock.sol b/test/mocks/L2TokenGatewayV2Mock.sol new file mode 100644 index 0000000..8e41e23 --- /dev/null +++ b/test/mocks/L2TokenGatewayV2Mock.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Copyright (C) 2024 Dai Foundation +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +pragma solidity ^0.8.21; + +import { UUPSUpgradeable, ERC1967Utils } from "src/L2TokenGateway.sol"; + +contract L2TokenGatewayV2Mock is UUPSUpgradeable { + mapping(address => uint256) public wards; + mapping(address => address) public l1ToL2Token; + mapping(address => uint256) public maxWithdraws; + uint256 public isOpen; + + string public constant version = "2"; + + event UpgradedTo(string version); + + modifier auth { + require(wards[msg.sender] == 1, "L2TokenGateway/not-authorized"); + _; + } + + constructor() { + _disableInitializers(); // Avoid initializing in the context of the implementation + } + + function reinitialize() reinitializer(2) external { + emit UpgradedTo(version); + } + + function _authorizeUpgrade(address newImplementation) internal override auth {} + + function getImplementation() external view returns (address) { + return ERC1967Utils.getImplementation(); + } +}