From 49d69c6d0c16f582ab4835cdaa88da4c431cb92e Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 2 Jul 2024 13:41:50 +0200 Subject: [PATCH] Add transferUSDCRoles function --- .../arbitrum/gateway/L2USDCGateway.sol | 32 +++- .../ethereum/gateway/L1USDCGateway.sol | 2 +- .../tokenbridge/libraries/IFiatToken.sol | 15 ++ test-foundry/L2USDCGateway.t.sol | 150 +++++++++++++++--- 4 files changed, 174 insertions(+), 25 deletions(-) diff --git a/contracts/tokenbridge/arbitrum/gateway/L2USDCGateway.sol b/contracts/tokenbridge/arbitrum/gateway/L2USDCGateway.sol index fc54fc9d3..8934808f5 100644 --- a/contracts/tokenbridge/arbitrum/gateway/L2USDCGateway.sol +++ b/contracts/tokenbridge/arbitrum/gateway/L2USDCGateway.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.4; import "./L2ArbitrumGateway.sol"; -import {L1USDCGateway, IFiatToken} from "../../ethereum/gateway/L1USDCGateway.sol"; +import {IFiatToken, IFiatTokenProxy} from "../../ethereum/gateway/L1USDCGateway.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; /** @@ -28,10 +28,14 @@ contract L2USDCGateway is L2ArbitrumGateway { address public l1USDC; address public l2USDC; address public owner; + address public usdcOwnershipTransferrer; bool public withdrawalsPaused; event WithdrawalsPaused(); event WithdrawalsUnpaused(); + event OwnerSet(address indexed owner); + event USDCOwnershipTransferrerSet(address indexed usdcOwnershipTransferrer); + event USDCOwnershipTransferred(address indexed newOwner, address indexed newProxyAdmin); error L2USDCGateway_WithdrawalsAlreadyPaused(); error L2USDCGateway_WithdrawalsAlreadyUnpaused(); @@ -40,6 +44,7 @@ contract L2USDCGateway is L2ArbitrumGateway { error L2USDCGateway_InvalidL2USDC(); error L2USDCGateway_NotOwner(); error L2USDCGateway_InvalidOwner(); + error L2USDCGateway_NotUSDCOwnershipTransferrer(); modifier onlyOwner() { if (msg.sender != owner) { @@ -101,6 +106,31 @@ contract L2USDCGateway is L2ArbitrumGateway { revert L2USDCGateway_InvalidOwner(); } owner = newOwner; + emit OwnerSet(newOwner); + } + + /** + * @notice Sets the account which is able to transfer USDC role away from the gateway to some other account. + */ + function setUsdcOwnershipTransferrer(address _usdcOwnershipTransferrer) external onlyOwner { + usdcOwnershipTransferrer = _usdcOwnershipTransferrer; + emit USDCOwnershipTransferrerSet(_usdcOwnershipTransferrer); + } + + /** + * @notice In accordance with bridged USDC standard, the ownership of the USDC token contract is transferred + * to the new owner, and the proxy admin is transferred to the caller (usdcOwnershipTransferrer). + * @dev For transfer to be successful, this gaetway should be both the owner and the proxy admin of L2 USDC token. + */ + function transferUSDCRoles(address _owner) external { + if (msg.sender != usdcOwnershipTransferrer) { + revert L2USDCGateway_NotUSDCOwnershipTransferrer(); + } + + IFiatTokenProxy(l2USDC).changeAdmin(msg.sender); + IFiatToken(l2USDC).transferOwnership(_owner); + + emit USDCOwnershipTransferred(_owner, msg.sender); } /** diff --git a/contracts/tokenbridge/ethereum/gateway/L1USDCGateway.sol b/contracts/tokenbridge/ethereum/gateway/L1USDCGateway.sol index 605f6b9f2..4283ab0b3 100644 --- a/contracts/tokenbridge/ethereum/gateway/L1USDCGateway.sol +++ b/contracts/tokenbridge/ethereum/gateway/L1USDCGateway.sol @@ -7,7 +7,7 @@ import { ITokenGateway, TokenGateway } from "./L1ArbitrumExtendedGateway.sol"; -import {IFiatToken} from "../../libraries/IFiatToken.sol"; +import {IFiatToken, IFiatTokenProxy} from "../../libraries/IFiatToken.sol"; /** * @title Custom gateway for USDC implementing Bridged USDC Standard. diff --git a/contracts/tokenbridge/libraries/IFiatToken.sol b/contracts/tokenbridge/libraries/IFiatToken.sol index 78be6e869..acca9da4f 100644 --- a/contracts/tokenbridge/libraries/IFiatToken.sol +++ b/contracts/tokenbridge/libraries/IFiatToken.sol @@ -123,3 +123,18 @@ interface IFiatToken { function updateRescuer(address newRescuer) external; function version() external pure returns (string memory); } + +/** + * @title IFiatTokenProxy + * @dev Interface contains functions from Circle's referent implementation of the USDC proxy + * Ref: https://github.com/circlefin/stablecoin-evm + * + * This interface is used in the L1USDCGateway, L1OrbitUSDCGateway and L2USDCGateway contracts. + */ +interface IFiatTokenProxy { + function admin() external view returns (address); + function changeAdmin(address newAdmin) external; + function implementation() external view returns (address); + function upgradeTo(address newImplementation) external; + function upgradeToAndCall(address newImplementation, bytes memory data) external payable; +} diff --git a/test-foundry/L2USDCGateway.t.sol b/test-foundry/L2USDCGateway.t.sol index c37888847..09102a247 100644 --- a/test-foundry/L2USDCGateway.t.sol +++ b/test-foundry/L2USDCGateway.t.sol @@ -10,23 +10,26 @@ import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import {AddressAliasHelper} from "contracts/tokenbridge/libraries/AddressAliasHelper.sol"; import {L2GatewayToken} from "contracts/tokenbridge/libraries/L2GatewayToken.sol"; import {IFiatToken} from "contracts/tokenbridge/libraries/IFiatToken.sol"; -import {TestUtil} from "./util/TestUtil.sol"; contract L2USDCGatewayTest is L2ArbitrumGatewayTest { L2USDCGateway public l2USDCGateway; address public l1USDC = makeAddr("l1USDC"); address public l2USDC; address public user = makeAddr("usdc_user"); - address public owner = makeAddr("l2gw-owner"); - address masterMinter = makeAddr("masterMinter"); + address public gatewayOwner = makeAddr("l2gw-owner"); + address public usdcOwner = makeAddr("usdc-owner"); + address public usdcProxyAdmin = makeAddr("usdcProxyAdmin"); + address public masterMinter = makeAddr("masterMinter"); function setUp() public virtual { l2USDCGateway = new L2USDCGateway(); l2Gateway = L2ArbitrumGateway(address(l2USDCGateway)); - // address bridgedUsdcLogic = TestUtil.deployBridgedUsdcToken(); address bridgedUsdcLogic = _deployBridgedUsdcToken(); - l2USDC = TestUtil.deployProxy(bridgedUsdcLogic); + vm.prank(usdcProxyAdmin); + l2USDC = _deployUsdcProxy(bridgedUsdcLogic); + + vm.startPrank(usdcOwner); IFiatToken(l2USDC).initialize( "USDC token", "USDC.e", @@ -35,18 +38,22 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { masterMinter, makeAddr("newPauser"), makeAddr("newBlacklister"), - owner + usdcOwner ); IFiatToken(l2USDC).initializeV2("USDC"); IFiatToken(l2USDC).initializeV2_1(makeAddr("lostAndFound")); IFiatToken(l2USDC).initializeV2_2(new address[](0), "USDC.e"); + vm.stopPrank(); vm.prank(masterMinter); IFiatToken(l2USDC).configureMinter(address(l2USDCGateway), type(uint256).max); - vm.prank(owner); - l2USDCGateway.initialize(l1Counterpart, router, l1USDC, l2USDC, owner); + vm.prank(gatewayOwner); + l2USDCGateway.initialize(l1Counterpart, router, l1USDC, l2USDC, gatewayOwner); vm.etch(0x0000000000000000000000000000000000000064, address(arbSysMock).code); + + assertEq(IFiatToken(l2USDC).owner(), usdcOwner, "Invalid owner"); + assertEq(IFiatTokenProxy(l2USDC).admin(), usdcProxyAdmin, "Invalid proxyAdmin"); } /* solhint-disable func-name-mixedcase */ @@ -113,32 +120,32 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { function test_initialize() public { L2USDCGateway gateway = new L2USDCGateway(); - L2USDCGateway(gateway).initialize(l1Counterpart, router, l1USDC, l2USDC, owner); + L2USDCGateway(gateway).initialize(l1Counterpart, router, l1USDC, l2USDC, gatewayOwner); assertEq(gateway.counterpartGateway(), l1Counterpart, "Invalid counterpartGateway"); assertEq(gateway.router(), router, "Invalid router"); assertEq(gateway.l1USDC(), l1USDC, "Invalid l1USDC"); assertEq(gateway.l2USDC(), l2USDC, "Invalid l2USDC"); - assertEq(gateway.owner(), owner, "Invalid owner"); + assertEq(gateway.owner(), gatewayOwner, "Invalid owner"); } function test_initialize_revert_InvalidL1USDC() public { L2USDCGateway gateway = new L2USDCGateway(); vm.expectRevert(abi.encodeWithSelector(L2USDCGateway.L2USDCGateway_InvalidL1USDC.selector)); - L2USDCGateway(gateway).initialize(l1Counterpart, router, address(0), l2USDC, owner); + L2USDCGateway(gateway).initialize(l1Counterpart, router, address(0), l2USDC, gatewayOwner); } function test_initialize_revert_InvalidL2USDC() public { L2USDCGateway gateway = new L2USDCGateway(); vm.expectRevert(abi.encodeWithSelector(L2USDCGateway.L2USDCGateway_InvalidL2USDC.selector)); - L2USDCGateway(gateway).initialize(l1Counterpart, router, l1USDC, address(0), owner); + L2USDCGateway(gateway).initialize(l1Counterpart, router, l1USDC, address(0), gatewayOwner); } function test_initialize_revert_AlreadyInit() public { L2USDCGateway gateway = new L2USDCGateway(); - L2USDCGateway(gateway).initialize(l1Counterpart, router, l1USDC, l2USDC, owner); + L2USDCGateway(gateway).initialize(l1Counterpart, router, l1USDC, l2USDC, gatewayOwner); vm.expectRevert("ALREADY_INIT"); - L2USDCGateway(gateway).initialize(l1Counterpart, router, l1USDC, l2USDC, owner); + L2USDCGateway(gateway).initialize(l1Counterpart, router, l1USDC, l2USDC, gatewayOwner); } function test_initialize_revert_InvalidOwner() public { @@ -207,7 +214,7 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { function test_outboundTransfer_revert_WithdrawalsPaused() public { // pause withdrawals - vm.prank(owner); + vm.prank(gatewayOwner); l2USDCGateway.pauseWithdrawals(); vm.expectRevert( @@ -233,7 +240,7 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { emit WithdrawalsPaused(); // pause withdrawals - vm.prank(owner); + vm.prank(gatewayOwner); l2USDCGateway.pauseWithdrawals(); // checks @@ -241,7 +248,7 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { } function test_pauseWithdrawals_revert_WithdrawalsAlreadyPaused() public { - vm.startPrank(owner); + vm.startPrank(gatewayOwner); l2USDCGateway.pauseWithdrawals(); vm.expectRevert( @@ -257,7 +264,11 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { function test_setOwner() public { address newOwner = makeAddr("new-owner"); - vm.prank(owner); + + vm.expectEmit(true, true, true, true); + emit OwnerSet(newOwner); + + vm.prank(gatewayOwner); l2USDCGateway.setOwner(newOwner); assertEq(l2USDCGateway.owner(), newOwner, "Invalid owner"); @@ -265,18 +276,82 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { function test_setOwner_revert_InvalidOwner() public { vm.expectRevert(abi.encodeWithSelector(L2USDCGateway.L2USDCGateway_InvalidOwner.selector)); - vm.prank(owner); + vm.prank(gatewayOwner); l2USDCGateway.setOwner(address(0)); } function test_setOwner_revert_NotOwner() public { vm.expectRevert(abi.encodeWithSelector(L2USDCGateway.L2USDCGateway_NotOwner.selector)); - l2USDCGateway.setOwner(owner); + l2USDCGateway.setOwner(gatewayOwner); + } + + function test_setUsdcOwnershipTransferrer() public { + assertEq( + l2USDCGateway.usdcOwnershipTransferrer(), address(0), "Invalid usdcOwnershipTransferrer" + ); + + address transferrer = makeAddr("transferrer"); + + vm.expectEmit(true, true, true, true); + emit USDCOwnershipTransferrerSet(transferrer); + + vm.prank(gatewayOwner); + l2USDCGateway.setUsdcOwnershipTransferrer(transferrer); + + assertEq( + l2USDCGateway.usdcOwnershipTransferrer(), + transferrer, + "Invalid usdcOwnershipTransferrer" + ); + } + + function test_setUsdcOwnershipTransferrer_revert_NotOwner() public { + vm.expectRevert(abi.encodeWithSelector(L2USDCGateway.L2USDCGateway_NotOwner.selector)); + l2USDCGateway.setUsdcOwnershipTransferrer(makeAddr("rand")); + } + + function test_transferUSDCRoles() public { + // set the transferrer + address transferrer = makeAddr("transferrer"); + vm.prank(gatewayOwner); + l2USDCGateway.setUsdcOwnershipTransferrer(transferrer); + + // transfer ownership to gateway + vm.prank(usdcOwner); + IFiatToken(l2USDC).transferOwnership(address(l2USDCGateway)); + + // transfer proxy admin to gateway + vm.prank(usdcProxyAdmin); + IFiatTokenProxy(l2USDC).changeAdmin(address(l2USDCGateway)); + + assertEq(IFiatToken(l2USDC).owner(), address(l2USDCGateway), "Invalid owner"); + assertEq(IFiatTokenProxy(l2USDC).admin(), address(l2USDCGateway), "Invalid proxyAdmin"); + + address newTokenOwner = makeAddr("new-token-owner"); + + // events + vm.expectEmit(true, true, true, true); + emit USDCOwnershipTransferred(newTokenOwner, transferrer); + + // transferUSDCRoles + vm.prank(transferrer); + l2USDCGateway.transferUSDCRoles(newTokenOwner); + + // checks + assertEq(IFiatToken(l2USDC).owner(), newTokenOwner, "Invalid owner"); + assertEq(IFiatTokenProxy(l2USDC).admin(), transferrer, "Invalid proxyAdmin"); + } + + function test_transferUSDCRoles_revert_NotUSDCOwnershipTransferrer() public { + vm.expectRevert( + abi.encodeWithSelector(L2USDCGateway.L2USDCGateway_NotUSDCOwnershipTransferrer.selector) + ); + l2USDCGateway.transferUSDCRoles(makeAddr("rand")); } function test_unpauseWithdrawals() public { // pause withdrawals - vm.prank(owner); + vm.prank(gatewayOwner); l2USDCGateway.pauseWithdrawals(); assertEq(l2USDCGateway.withdrawalsPaused(), true, "Invalid withdrawalsPaused"); @@ -285,7 +360,7 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { emit WithdrawalsUnpaused(); // unpause withdrawals - vm.prank(owner); + vm.prank(gatewayOwner); l2USDCGateway.unpauseWithdrawals(); // checks @@ -298,7 +373,7 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { } function test_unpauseWithdrawals_revert_AlreadyUnpaused() public { - vm.prank(owner); + vm.prank(gatewayOwner); vm.expectRevert( abi.encodeWithSelector(L2USDCGateway.L2USDCGateway_WithdrawalsAlreadyUnpaused.selector) ); @@ -310,6 +385,9 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { //// event WithdrawalsPaused(); event WithdrawalsUnpaused(); + event OwnerSet(address indexed owner); + event USDCOwnershipTransferrerSet(address indexed usdcOwnershipTransferrer); + event USDCOwnershipTransferred(address indexed newOwner, address indexed newProxyAdmin); function _deployBridgedUsdcToken() public returns (address) { /// deploy library @@ -344,6 +422,16 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { return bridgedUsdcToken; } + function _deployUsdcProxy(address logic) public returns (address) { + address proxyAddress = _deployBytecodeWithConstructorFromJSON( + "/node_modules/@offchainlabs/stablecoin-evm/artifacts/hardhat/contracts/v1/FiatTokenProxy.sol/FiatTokenProxy.json", + abi.encode(logic) + ); + require(proxyAddress != address(0), "Failed to deploy contract"); + + return proxyAddress; + } + function _deployBytecode(bytes memory bytecode) public returns (address) { address addr; assembly { @@ -353,11 +441,27 @@ contract L2USDCGatewayTest is L2ArbitrumGatewayTest { return addr; } + function _deployBytecodeWithConstructor(bytes memory bytecode, bytes memory abiencodedargs) + internal + returns (address) + { + bytes memory bytecodeWithConstructor = bytes.concat(bytecode, abiencodedargs); + return _deployBytecode(bytecodeWithConstructor); + } + function _deployBytecodeFromJSON(bytes memory path) public returns (address) { bytes memory bytecode = _getBytecode(path); return _deployBytecode(bytecode); } + function _deployBytecodeWithConstructorFromJSON(bytes memory path, bytes memory abiencodedargs) + internal + returns (address) + { + bytes memory bytecode = _getBytecode(path); + return _deployBytecodeWithConstructor(bytecode, abiencodedargs); + } + function _getBytecode(bytes memory path) public returns (bytes memory) { string memory readerBytecodeFilePath = string(abi.encodePacked(vm.projectRoot(), path)); string memory json = vm.readFile(readerBytecodeFilePath);