diff --git a/example.env b/example.env index 6b49ad20..f8fd24f8 100644 --- a/example.env +++ b/example.env @@ -32,3 +32,17 @@ MULTISIG_OWNER_2_TESTNET= MULTISIG_OWNER_3_TESTNET= MULTISIG_OWNER_4_TESTNET= MULTISIG_OWNER_5_TESTNET= + +# Timelock configuration for legacy LBC ownership +# Delay is in seconds. Temporary default in code is 7 days. +TIMELOCK_MIN_DELAY_MAINNET= +TIMELOCK_PROPOSER_MAINNET= +TIMELOCK_EXECUTOR_MAINNET= + +TIMELOCK_MIN_DELAY_TESTNET= +TIMELOCK_PROPOSER_TESTNET= +TIMELOCK_EXECUTOR_TESTNET= + +TIMELOCK_MIN_DELAY_LOCAL= +TIMELOCK_PROPOSER_LOCAL= +TIMELOCK_EXECUTOR_LOCAL= diff --git a/package-lock.json b/package-lock.json index 7bc2293c..e3a80fd9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "dependencies": { "@openzeppelin/contracts": "^5.3.0", "@openzeppelin/contracts-upgradeable": "^5.3.0", - "@rsksmart/btc-transaction-solidity-helper": "^0.3.0" + "@rsksmart/btc-transaction-solidity-helper": "^0.3.0-beta" }, "devDependencies": { "@eslint/js": "^9.16.0", @@ -468,9 +468,9 @@ } }, "node_modules/@rsksmart/btc-transaction-solidity-helper": { - "version": "0.3.0", - "resolved": "https://npm.pkg.github.com/download/@rsksmart/btc-transaction-solidity-helper/0.3.0/b9218a1772eae4fe06ef266689e0ea84f9370ab9", - "integrity": "sha512-ECmIwTH1ISV0h3HeGiNmM9ebnXelTTDV9/S+/O5Z1PxNIHct2Axk+E6+7Tfq17vbBJUiNb8FsBqgLKW2+9NG0Q==", + "version": "0.3.0-beta", + "resolved": "https://registry.npmjs.org/@rsksmart/btc-transaction-solidity-helper/-/btc-transaction-solidity-helper-0.3.0-beta.tgz", + "integrity": "sha512-m4OvVxKMzxWwN/RDc6HID6oVaRlEJ2Mnd+n/mQ5xjGJB528dvoagics68pBnvcviUuoEChi+FeOs2qnVOU20qQ==", "license": "ISC" }, "node_modules/@rsksmart/pmt-builder": { @@ -4890,9 +4890,9 @@ } }, "@rsksmart/btc-transaction-solidity-helper": { - "version": "0.3.0", - "resolved": "https://npm.pkg.github.com/download/@rsksmart/btc-transaction-solidity-helper/0.3.0/b9218a1772eae4fe06ef266689e0ea84f9370ab9", - "integrity": "sha512-ECmIwTH1ISV0h3HeGiNmM9ebnXelTTDV9/S+/O5Z1PxNIHct2Axk+E6+7Tfq17vbBJUiNb8FsBqgLKW2+9NG0Q==" + "version": "0.3.0-beta", + "resolved": "https://registry.npmjs.org/@rsksmart/btc-transaction-solidity-helper/-/btc-transaction-solidity-helper-0.3.0-beta.tgz", + "integrity": "sha512-m4OvVxKMzxWwN/RDc6HID6oVaRlEJ2Mnd+n/mQ5xjGJB528dvoagics68pBnvcviUuoEChi+FeOs2qnVOU20qQ==" }, "@rsksmart/pmt-builder": { "version": "3.0.0", diff --git a/package.json b/package.json index 3c9393cf..d2752029 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,6 @@ "dependencies": { "@openzeppelin/contracts": "^5.3.0", "@openzeppelin/contracts-upgradeable": "^5.3.0", - "@rsksmart/btc-transaction-solidity-helper": "^0.3.0" + "@rsksmart/btc-transaction-solidity-helper": "^0.3.0-beta" } } diff --git a/script/HelperConfig.s.sol b/script/HelperConfig.s.sol index 2ea6c3ae..1f780277 100644 --- a/script/HelperConfig.s.sol +++ b/script/HelperConfig.s.sol @@ -16,6 +16,10 @@ contract HelperConfig is Script { bool mainnet; address existingProxy; address existingAdmin; + uint256 timelockMinDelay; + address timelockProposer; + address timelockExecutor; + address timelockAdmin; } /// @notice Configuration for the new Flyover contracts @@ -29,6 +33,10 @@ contract HelperConfig is Script { uint256 btcBlockTime; bool mainnet; uint48 adminDelay; + uint256 timelockMinDelay; + address timelockProposer; + address timelockExecutor; + address timelockAdmin; } NetworkConfig private cachedConfig; @@ -94,7 +102,26 @@ contract HelperConfig is Script { btcBlockTime: vm.envOr("BTC_BLOCK_TIME_MAINNET", uint256(600)), mainnet: true, existingProxy: vm.envOr("EXISTING_PROXY_MAINNET", address(0)), - existingAdmin: vm.envOr("EXISTING_ADMIN_MAINNET", address(0)) + existingAdmin: vm.envOr("EXISTING_ADMIN_MAINNET", address(0)), + timelockMinDelay: vm.envOr( + "TIMELOCK_MIN_DELAY_MAINNET", + uint256(7 days) + ), + timelockProposer: vm.envOr( + "TIMELOCK_PROPOSER_MAINNET", + vm.envOr( + "MULTISIG_ADDRESS_MAINNET", + address(0x633D1233eD6251108b61A8365CEEd271BF3e3C9b) + ) + ), + timelockExecutor: vm.envOr( + "TIMELOCK_EXECUTOR_MAINNET", + vm.envOr( + "MULTISIG_ADDRESS_MAINNET", + address(0x633D1233eD6251108b61A8365CEEd271BF3e3C9b) + ) + ), + timelockAdmin: vm.envOr("TIMELOCK_ADMIN_MAINNET", address(0)) }); } @@ -134,7 +161,26 @@ contract HelperConfig is Script { existingAdmin: vm.envOr( "EXISTING_ADMIN_TESTNET", address(0x93891ACe405cC4F7b9974C22e34D6479eE6425e5) - ) + ), + timelockMinDelay: vm.envOr( + "TIMELOCK_MIN_DELAY_TESTNET", + uint256(7 days) + ), + timelockProposer: vm.envOr( + "TIMELOCK_PROPOSER_TESTNET", + vm.envOr( + "MULTISIG_ADDRESS_TESTNET", + address(0x27ad02ABf893F8e01f0089EDE607A76FbB3F1Cd3) + ) + ), + timelockExecutor: vm.envOr( + "TIMELOCK_EXECUTOR_TESTNET", + vm.envOr( + "MULTISIG_ADDRESS_TESTNET", + address(0x27ad02ABf893F8e01f0089EDE607A76FbB3F1Cd3) + ) + ), + timelockAdmin: vm.envOr("TIMELOCK_ADMIN_TESTNET", address(0)) }); } @@ -163,7 +209,20 @@ contract HelperConfig is Script { btcBlockTime: vm.envOr("BTC_BLOCK_TIME_LOCAL", uint256(600)), mainnet: false, existingProxy: vm.envOr("EXISTING_PROXY_LOCAL", address(0)), - existingAdmin: vm.envOr("EXISTING_ADMIN_LOCAL", address(0)) + existingAdmin: vm.envOr("EXISTING_ADMIN_LOCAL", address(0)), + timelockMinDelay: vm.envOr( + "TIMELOCK_MIN_DELAY_LOCAL", + uint256(7 days) + ), + timelockProposer: vm.envOr( + "TIMELOCK_PROPOSER_LOCAL", + address(0x1000000000000000000000000000000000000001) + ), + timelockExecutor: vm.envOr( + "TIMELOCK_EXECUTOR_LOCAL", + address(0x1000000000000000000000000000000000000002) + ), + timelockAdmin: vm.envOr("TIMELOCK_ADMIN_LOCAL", address(0)) }); } @@ -231,6 +290,28 @@ contract HelperConfig is Script { mainnet: isMainnet, adminDelay: uint48( vm.envOr(string.concat("ADMIN_DELAY_", suffix), uint256(0)) + ), + timelockMinDelay: vm.envOr( + string.concat("TIMELOCK_MIN_DELAY_", suffix), + uint256(7 days) + ), + timelockProposer: vm.envOr( + string.concat("TIMELOCK_PROPOSER_", suffix), + vm.envOr( + string.concat("MULTISIG_ADDRESS_", suffix), + address(0) + ) + ), + timelockExecutor: vm.envOr( + string.concat("TIMELOCK_EXECUTOR_", suffix), + vm.envOr( + string.concat("MULTISIG_ADDRESS_", suffix), + address(0) + ) + ), + timelockAdmin: vm.envOr( + string.concat("TIMELOCK_ADMIN_", suffix), + address(0) ) }); } @@ -255,7 +336,20 @@ contract HelperConfig is Script { ), btcBlockTime: vm.envOr("BTC_BLOCK_TIME_LOCAL", uint256(600)), mainnet: false, - adminDelay: uint48(vm.envOr("ADMIN_DELAY_LOCAL", uint256(0))) + adminDelay: uint48(vm.envOr("ADMIN_DELAY_LOCAL", uint256(0))), + timelockMinDelay: vm.envOr( + "TIMELOCK_MIN_DELAY_LOCAL", + uint256(7 days) + ), + timelockProposer: vm.envOr( + "TIMELOCK_PROPOSER_LOCAL", + address(0x1000000000000000000000000000000000000001) + ), + timelockExecutor: vm.envOr( + "TIMELOCK_EXECUTOR_LOCAL", + address(0x1000000000000000000000000000000000000002) + ), + timelockAdmin: vm.envOr("TIMELOCK_ADMIN_LOCAL", address(0)) }); } } diff --git a/script/deployment/ChangeOwnerToTimelock.s.sol b/script/deployment/ChangeOwnerToTimelock.s.sol new file mode 100644 index 00000000..2d8e44ca --- /dev/null +++ b/script/deployment/ChangeOwnerToTimelock.s.sol @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.25; + +import {Script, console} from "lib/forge-std/src/Script.sol"; +import {HelperConfig} from "../HelperConfig.s.sol"; +import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol"; +import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; + +/// @title ChangeOwnerToTimelock (Split Architecture) +/// @notice Deploys a TimelockController and transfers ownership of the shared +/// ProxyAdmin to it, gating contract upgrades behind a time delay. +/// @dev Only the ProxyAdmin is transferred to the timelock. The DEFAULT_ADMIN_ROLE +/// on individual contracts (CollateralManagement, PegIn, PegOut, FlyoverDiscovery) +/// is intentionally left unchanged so that time-sensitive operations like +/// emergency pause can be executed immediately. +/// Proposers and executors are read from timelock-roles.json, falling back +/// to the single-address config values if the file is not available. +contract ChangeOwnerToTimelock is Script { + error NoProposersConfigured(); + error NoExecutorsConfigured(); + error ProxyAdminAddressNotProvided(); + error ProxyAdminOwnerTransferFailed(); + + function run() external { + HelperConfig helper = new HelperConfig(); + HelperConfig.FlyoverConfig memory cfg = helper.getFlyoverConfig(); + uint256 deployerKey = helper.getDeployerPrivateKey(); + vm.rememberKey(deployerKey); + + address proxyAdminAddress = vm.envAddress("PROXY_ADMIN"); + if (proxyAdminAddress == address(0)) { + revert ProxyAdminAddressNotProvided(); + } + + (address[] memory proposers, address[] memory executors) = _readRoles( + cfg + ); + + vm.startBroadcast(deployerKey); + TimelockController timelock = execute( + proxyAdminAddress, + cfg.timelockMinDelay, + proposers, + executors, + cfg.timelockAdmin + ); + vm.stopBroadcast(); + + _logFinalState(proxyAdminAddress, timelock, proposers, executors); + } + + /// @notice Core logic: deploys a TimelockController and transfers ProxyAdmin + /// ownership to it. No console.log calls -- safe for broadcast. + function execute( + address proxyAdminAddress, + uint256 minDelay, + address[] memory proposers, + address[] memory executors, + address admin + ) public returns (TimelockController) { + if (proposers.length == 0) { + revert NoProposersConfigured(); + } + if (executors.length == 0) { + revert NoExecutorsConfigured(); + } + + TimelockController timelock = new TimelockController( + minDelay, + proposers, + executors, + admin + ); + + ProxyAdmin proxyAdmin = ProxyAdmin(proxyAdminAddress); + if (proxyAdmin.owner() != address(timelock)) { + proxyAdmin.transferOwnership(address(timelock)); + if (proxyAdmin.owner() != address(timelock)) { + revert ProxyAdminOwnerTransferFailed(); + } + } + + return timelock; + } + + function _readRoles( + HelperConfig.FlyoverConfig memory cfg + ) + internal + view + returns (address[] memory proposers, address[] memory executors) + { + string memory json = vm.readFile( + "script/deployment/timelock-roles.json" + ); + string memory networkKey = _networkKey(); + + proposers = vm.parseJsonAddressArray( + json, + string.concat(".", networkKey, ".proposers") + ); + executors = vm.parseJsonAddressArray( + json, + string.concat(".", networkKey, ".executors") + ); + + if (proposers.length == 0 && cfg.timelockProposer != address(0)) { + proposers = new address[](1); + proposers[0] = cfg.timelockProposer; + } + if (executors.length == 0 && cfg.timelockExecutor != address(0)) { + executors = new address[](1); + executors[0] = cfg.timelockExecutor; + } + } + + function _networkKey() internal view returns (string memory) { + uint256 chainId = block.chainid; + if (chainId == 30) return "rskMainnet"; + if (chainId == 31) return "rskTestnet"; + return "rskRegtest"; + } + + function _logFinalState( + address proxyAdminAddress, + TimelockController timelock, + address[] memory proposers, + address[] memory executors + ) internal view { + console.log("=== Timelock ownership setup complete ==="); + console.log("Timelock:", address(timelock)); + console.log("Timelock minDelay:", timelock.getMinDelay()); + console.log("ProxyAdmin:", proxyAdminAddress); + console.log("ProxyAdmin owner:", ProxyAdmin(proxyAdminAddress).owner()); + + bytes32 proposerRole = timelock.PROPOSER_ROLE(); + bytes32 executorRole = timelock.EXECUTOR_ROLE(); + for (uint256 i = 0; i < proposers.length; i++) { + console.log( + "Proposer:", + proposers[i], + timelock.hasRole(proposerRole, proposers[i]) + ); + } + for (uint256 i = 0; i < executors.length; i++) { + console.log( + "Executor:", + executors[i], + timelock.hasRole(executorRole, executors[i]) + ); + } + } +} diff --git a/script/deployment/timelock-roles.json b/script/deployment/timelock-roles.json new file mode 100644 index 00000000..df008d8f --- /dev/null +++ b/script/deployment/timelock-roles.json @@ -0,0 +1,14 @@ +{ + "rskMainnet": { + "proposers": ["0x633D1233eD6251108b61A8365CEEd271BF3e3C9b"], + "executors": ["0x633D1233eD6251108b61A8365CEEd271BF3e3C9b"] + }, + "rskTestnet": { + "proposers": ["0x27ad02ABf893F8e01f0089EDE607A76FbB3F1Cd3"], + "executors": ["0x27ad02ABf893F8e01f0089EDE607A76FbB3F1Cd3"] + }, + "rskRegtest": { + "proposers": ["0x1000000000000000000000000000000000000001"], + "executors": ["0x1000000000000000000000000000000000000002"] + } +} diff --git a/script/legacy/deployment/ChangeOwnerToTimelock.s.sol b/script/legacy/deployment/ChangeOwnerToTimelock.s.sol new file mode 100644 index 00000000..eedf5d0e --- /dev/null +++ b/script/legacy/deployment/ChangeOwnerToTimelock.s.sol @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.25; + +import {Script, console} from "lib/forge-std/src/Script.sol"; +import {HelperConfig} from "../../HelperConfig.s.sol"; +import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol"; +import {LiquidityBridgeContractAdmin} from "../../../src/legacy/LiquidityBridgeContractAdmin.sol"; + +/// @title ChangeOwnerToTimelock (Legacy) +/// @notice Deploys a TimelockController and transfers ownership of the LBC's +/// ProxyAdmin to it, gating contract upgrades behind a time delay. +/// @dev Only the ProxyAdmin is transferred to the timelock. The LBC contract +/// ownership is intentionally left unchanged so that time-sensitive operations +/// like emergency actions can be executed immediately. +/// Proposers and executors are read from timelock-roles.json, falling back +/// to the single-address config values if the file is not available. +contract ChangeOwnerToTimelock is Script { + bytes32 internal constant ADMIN_SLOT = + 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103; + + error ProxyAddressNotProvided(); + error NoProposersConfigured(); + error NoExecutorsConfigured(); + error AdminAddressNotFound(); + error ProxyAdminOwnerTransferFailed(); + + function run() external { + HelperConfig helper = new HelperConfig(); + HelperConfig.NetworkConfig memory cfg = helper.getConfig(); + + uint256 deployerKey = helper.getDeployerPrivateKey(); + vm.rememberKey(deployerKey); + + address proxyAddress = cfg.existingProxy; + if (proxyAddress == address(0)) { + revert ProxyAddressNotProvided(); + } + + (address[] memory proposers, address[] memory executors) = _readRoles( + cfg + ); + + vm.startBroadcast(deployerKey); + TimelockController timelock = execute( + proxyAddress, + cfg.timelockMinDelay, + proposers, + executors, + cfg.timelockAdmin + ); + vm.stopBroadcast(); + + _logFinalState(proxyAddress, timelock, proposers, executors); + } + + /// @notice Core logic: deploys a TimelockController and transfers the ProxyAdmin + /// ownership to it. No console.log calls -- safe for broadcast. + function execute( + address proxyAddress, + uint256 minDelay, + address[] memory proposers, + address[] memory executors, + address admin + ) public returns (TimelockController) { + if (proposers.length == 0) { + revert NoProposersConfigured(); + } + if (executors.length == 0) { + revert NoExecutorsConfigured(); + } + + TimelockController timelock = new TimelockController( + minDelay, + proposers, + executors, + admin + ); + + _transferProxyAdminOwnership(proxyAddress, address(timelock)); + + return timelock; + } + + function _transferProxyAdminOwnership( + address proxyAddress, + address timelock + ) internal { + address adminAddress = address( + uint160(uint256(vm.load(proxyAddress, ADMIN_SLOT))) + ); + if (adminAddress == address(0)) { + revert AdminAddressNotFound(); + } + + LiquidityBridgeContractAdmin admin = LiquidityBridgeContractAdmin( + adminAddress + ); + if (admin.owner() == timelock) { + return; + } + + admin.transferOwnership(timelock); + + if (admin.owner() != timelock) { + revert ProxyAdminOwnerTransferFailed(); + } + } + + function _readRoles( + HelperConfig.NetworkConfig memory cfg + ) + internal + view + returns (address[] memory proposers, address[] memory executors) + { + string memory json = vm.readFile( + "script/deployment/timelock-roles.json" + ); + string memory networkKey = _networkKey(); + + proposers = vm.parseJsonAddressArray( + json, + string.concat(".", networkKey, ".proposers") + ); + executors = vm.parseJsonAddressArray( + json, + string.concat(".", networkKey, ".executors") + ); + + if (proposers.length == 0 && cfg.timelockProposer != address(0)) { + proposers = new address[](1); + proposers[0] = cfg.timelockProposer; + } + if (executors.length == 0 && cfg.timelockExecutor != address(0)) { + executors = new address[](1); + executors[0] = cfg.timelockExecutor; + } + } + + function _networkKey() internal view returns (string memory) { + uint256 chainId = block.chainid; + if (chainId == 30) return "rskMainnet"; + if (chainId == 31) return "rskTestnet"; + return "rskRegtest"; + } + + function _logFinalState( + address proxyAddress, + TimelockController timelock, + address[] memory proposers, + address[] memory executors + ) internal view { + address adminAddress = address( + uint160(uint256(vm.load(proxyAddress, ADMIN_SLOT))) + ); + LiquidityBridgeContractAdmin admin = LiquidityBridgeContractAdmin( + adminAddress + ); + + console.log("=== Timelock ownership setup complete ==="); + console.log("Timelock:", address(timelock)); + console.log("Timelock minDelay:", timelock.getMinDelay()); + console.log("ProxyAdmin owner:", admin.owner()); + + bytes32 proposerRole = timelock.PROPOSER_ROLE(); + bytes32 executorRole = timelock.EXECUTOR_ROLE(); + for (uint256 i = 0; i < proposers.length; i++) { + console.log( + "Proposer:", + proposers[i], + timelock.hasRole(proposerRole, proposers[i]) + ); + } + for (uint256 i = 0; i < executors.length; i++) { + console.log( + "Executor:", + executors[i], + timelock.hasRole(executorRole, executors[i]) + ); + } + } +} diff --git a/test/deployment/ChangeOwnerToTimelock.t.sol b/test/deployment/ChangeOwnerToTimelock.t.sol new file mode 100644 index 00000000..f045e279 --- /dev/null +++ b/test/deployment/ChangeOwnerToTimelock.t.sol @@ -0,0 +1,472 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.25; + +import "lib/forge-std/src/Test.sol"; +import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol"; +import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; +import {TransparentUpgradeableProxy, ITransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; +import {HelperConfig} from "../../script/HelperConfig.s.sol"; + +// Legacy imports +import {ChangeOwnerToTimelock as LegacyChangeOwnerToTimelock} from "../../script/legacy/deployment/ChangeOwnerToTimelock.s.sol"; +import {LiquidityBridgeContract} from "../../src/legacy/LiquidityBridgeContract.sol"; +import {LiquidityBridgeContractProxy} from "../../src/legacy/LiquidityBridgeContractProxy.sol"; +import {LiquidityBridgeContractAdmin} from "../../src/legacy/LiquidityBridgeContractAdmin.sol"; + +// Split imports +import {ChangeOwnerToTimelock as SplitChangeOwnerToTimelock} from "../../script/deployment/ChangeOwnerToTimelock.s.sol"; +import {CollateralManagementContract} from "../../src/CollateralManagement.sol"; +import {FlyoverDiscovery} from "../../src/FlyoverDiscovery.sol"; +import {PauseRegistry} from "../../src/PauseRegistry.sol"; +import {PegInContract} from "../../src/PegInContract.sol"; +import {PegOutContract} from "../../src/PegOutContract.sol"; +import {BridgeMock} from "../../src/test-contracts/BridgeMock.sol"; + +// ============================================================ +// Legacy Tests +// ============================================================ + +contract LegacyChangeOwnerToTimelockTest is Test { + bytes32 internal constant ADMIN_SLOT = + 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103; + + HelperConfig public helperConfig; + LegacyChangeOwnerToTimelock public script; + LiquidityBridgeContract public lbcImpl; + LiquidityBridgeContractProxy public proxy; + LiquidityBridgeContractAdmin public admin; + + address public proposer; + address public executor; + uint256 public minDelay; + + function setUp() public { + helperConfig = new HelperConfig(); + script = new LegacyChangeOwnerToTimelock(); + + proposer = makeAddr("proposer"); + executor = makeAddr("executor"); + minDelay = 7 days; + + HelperConfig.NetworkConfig memory cfg = helperConfig.getConfig(); + + lbcImpl = new LiquidityBridgeContract(); + admin = new LiquidityBridgeContractAdmin(); + + bytes memory initData = abi.encodeCall( + LiquidityBridgeContract.initialize, + ( + payable(cfg.bridge), + cfg.minimumCollateral, + cfg.minimumPegIn, + cfg.rewardPercentage, + cfg.resignDelayBlocks, + cfg.dustThreshold, + cfg.btcBlockTime, + cfg.mainnet + ) + ); + // OZ v5 TransparentUpgradeableProxy deploys its own ProxyAdmin and uses + // this argument as the initial owner of that ProxyAdmin. + proxy = new LiquidityBridgeContractProxy( + address(lbcImpl), + address(this), + initData + ); + } + + function test_ScriptTransfersProxyAdminToTimelock() public { + LiquidityBridgeContractAdmin slotAdmin = _getProxyAdmin(); + slotAdmin.transferOwnership(address(script)); + + address[] memory proposers = new address[](1); + proposers[0] = proposer; + address[] memory executors = new address[](1); + executors[0] = executor; + + TimelockController timelock = script.execute( + address(proxy), + minDelay, + proposers, + executors, + address(0) + ); + + assertEq( + slotAdmin.owner(), + address(timelock), + "ProxyAdmin owner should be timelock" + ); + assertEq(timelock.getMinDelay(), minDelay, "minDelay mismatch"); + assertTrue( + timelock.hasRole(timelock.PROPOSER_ROLE(), proposer), + "proposer role missing" + ); + assertTrue( + timelock.hasRole(timelock.EXECUTOR_ROLE(), executor), + "executor role missing" + ); + } + + function test_ProxyAdminStoredInExpectedSlot() public view { + address proxyAdminAddress = address( + uint160(uint256(vm.load(address(proxy), ADMIN_SLOT))) + ); + assertTrue(proxyAdminAddress != address(0), "EIP-1967 admin missing"); + } + + function _getProxyAdmin() + internal + view + returns (LiquidityBridgeContractAdmin) + { + address proxyAdminAddress = address( + uint160(uint256(vm.load(address(proxy), ADMIN_SLOT))) + ); + return LiquidityBridgeContractAdmin(proxyAdminAddress); + } +} + +// ============================================================ +// Split Architecture Tests +// ============================================================ + +contract SplitChangeOwnerToTimelockTest is Test { + HelperConfig public helperConfig; + SplitChangeOwnerToTimelock public script; + BridgeMock public bridgeMock; + + address public pauseRegistryProxy; + CollateralManagementContract public collateralManagement; + FlyoverDiscovery public discovery; + PegInContract public pegInContract; + PegOutContract public pegOutContract; + address public proxyAdmin; + + address public deployer; + address public proposer; + address public executor; + uint256 public minDelay; + + function setUp() public { + deployer = address(this); + helperConfig = new HelperConfig(); + script = new SplitChangeOwnerToTimelock(); + bridgeMock = new BridgeMock(); + + proposer = makeAddr("proposer"); + executor = makeAddr("executor"); + minDelay = 7 days; + + HelperConfig.FlyoverConfig memory cfg = helperConfig.getFlyoverConfig(); + _deployAll(deployer, cfg); + } + + function _buildRoles() + internal + view + returns (address[] memory proposers, address[] memory executors) + { + proposers = new address[](1); + proposers[0] = proposer; + executors = new address[](1); + executors[0] = executor; + } + + function test_ScriptTransfersProxyAdminToTimelock() public { + ( + address[] memory proposers, + address[] memory executors + ) = _buildRoles(); + + ProxyAdmin(proxyAdmin).transferOwnership(address(script)); + + TimelockController timelock = script.execute( + proxyAdmin, + minDelay, + proposers, + executors, + address(0) + ); + + assertEq( + ProxyAdmin(proxyAdmin).owner(), + address(timelock), + "ProxyAdmin owner should be timelock" + ); + assertEq(timelock.getMinDelay(), minDelay, "minDelay mismatch"); + assertTrue( + timelock.hasRole(timelock.PROPOSER_ROLE(), proposer), + "proposer role missing" + ); + assertTrue( + timelock.hasRole(timelock.EXECUTOR_ROLE(), executor), + "executor role missing" + ); + } + + function test_MultipleProposersAndExecutors() public { + address proposer2 = makeAddr("proposer2"); + address executor2 = makeAddr("executor2"); + + address[] memory proposers = new address[](2); + proposers[0] = proposer; + proposers[1] = proposer2; + address[] memory executors = new address[](2); + executors[0] = executor; + executors[1] = executor2; + + ProxyAdmin(proxyAdmin).transferOwnership(address(script)); + + TimelockController timelock = script.execute( + proxyAdmin, + minDelay, + proposers, + executors, + address(0) + ); + + bytes32 proposerRole = timelock.PROPOSER_ROLE(); + bytes32 executorRole = timelock.EXECUTOR_ROLE(); + assertTrue( + timelock.hasRole(proposerRole, proposer), + "proposer1 missing" + ); + assertTrue( + timelock.hasRole(proposerRole, proposer2), + "proposer2 missing" + ); + assertTrue( + timelock.hasRole(executorRole, executor), + "executor1 missing" + ); + assertTrue( + timelock.hasRole(executorRole, executor2), + "executor2 missing" + ); + } + + function test_AdminCanGrantRolesImmediately() public { + ( + address[] memory proposers, + address[] memory executors + ) = _buildRoles(); + address timelockAdmin = makeAddr("timelockAdmin"); + + ProxyAdmin(proxyAdmin).transferOwnership(address(script)); + + TimelockController timelock = script.execute( + proxyAdmin, + minDelay, + proposers, + executors, + timelockAdmin + ); + + assertTrue( + timelock.hasRole(timelock.DEFAULT_ADMIN_ROLE(), timelockAdmin), + "admin should have DEFAULT_ADMIN_ROLE" + ); + + address newProposer = makeAddr("newProposer"); + bytes32 proposerRole = timelock.PROPOSER_ROLE(); + vm.prank(timelockAdmin); + timelock.grantRole(proposerRole, newProposer); + + assertTrue( + timelock.hasRole(proposerRole, newProposer), + "new proposer should have role" + ); + } + + function test_ProxyAdminOwnedByTimelockBlocksDirectUpgrade() public { + ( + address[] memory proposers, + address[] memory executors + ) = _buildRoles(); + + ProxyAdmin(proxyAdmin).transferOwnership(address(script)); + + TimelockController timelock = script.execute( + proxyAdmin, + minDelay, + proposers, + executors, + address(0) + ); + + assertEq( + ProxyAdmin(proxyAdmin).owner(), + address(timelock), + "ProxyAdmin should be owned by timelock" + ); + + address newImpl = address(new CollateralManagementContract()); + vm.expectRevert(); + ProxyAdmin(proxyAdmin).upgradeAndCall( + ITransparentUpgradeableProxy(address(collateralManagement)), + newImpl, + "" + ); + } + + function test_RevertsIfNonOwnerCallsExecute() public { + ( + address[] memory proposers, + address[] memory executors + ) = _buildRoles(); + + vm.expectRevert(); + script.execute(proxyAdmin, minDelay, proposers, executors, address(0)); + } + + function test_RevertsIfNoProposers() public { + address[] memory proposers = new address[](0); + address[] memory executors = new address[](1); + executors[0] = executor; + + ProxyAdmin(proxyAdmin).transferOwnership(address(script)); + + vm.expectRevert( + SplitChangeOwnerToTimelock.NoProposersConfigured.selector + ); + script.execute(proxyAdmin, minDelay, proposers, executors, address(0)); + } + + function test_RevertsIfNoExecutors() public { + address[] memory proposers = new address[](1); + proposers[0] = proposer; + address[] memory executors = new address[](0); + + ProxyAdmin(proxyAdmin).transferOwnership(address(script)); + + vm.expectRevert( + SplitChangeOwnerToTimelock.NoExecutorsConfigured.selector + ); + script.execute(proxyAdmin, minDelay, proposers, executors, address(0)); + } + + // ============================================================ + // Helpers + // ============================================================ + + function _deployAll( + address admin_, + HelperConfig.FlyoverConfig memory cfg + ) internal { + proxyAdmin = address(new ProxyAdmin(admin_)); + _deployCollateralManagement(admin_, cfg); + _deployFlyoverDiscovery(admin_, cfg); + _deployPegIn(admin_, cfg); + _deployPegOut(admin_, cfg); + } + + function _deployCollateralManagement( + address admin_, + HelperConfig.FlyoverConfig memory cfg + ) private { + PauseRegistry prImpl = new PauseRegistry(); + pauseRegistryProxy = address( + new TransparentUpgradeableProxy( + address(prImpl), + proxyAdmin, + abi.encodeCall(prImpl.initialize, (0, admin_)) + ) + ); + + address impl = address(new CollateralManagementContract()); + address proxy = address( + new TransparentUpgradeableProxy( + impl, + proxyAdmin, + abi.encodeCall( + CollateralManagementContract.initialize, + ( + admin_, + cfg.adminDelay, + cfg.minimumCollateral, + cfg.resignDelayBlocks, + cfg.rewardPercentage, + PauseRegistry(pauseRegistryProxy) + ) + ) + ) + ); + collateralManagement = CollateralManagementContract(payable(proxy)); + } + + function _deployFlyoverDiscovery( + address admin_, + HelperConfig.FlyoverConfig memory cfg + ) private { + address impl = address(new FlyoverDiscovery()); + address proxy = address( + new TransparentUpgradeableProxy( + impl, + proxyAdmin, + abi.encodeCall( + FlyoverDiscovery.initialize, + ( + admin_, + cfg.adminDelay, + address(collateralManagement), + PauseRegistry(pauseRegistryProxy) + ) + ) + ) + ); + discovery = FlyoverDiscovery(proxy); + } + + function _deployPegIn( + address admin_, + HelperConfig.FlyoverConfig memory cfg + ) private { + address impl = address(new PegInContract()); + address proxy = address( + new TransparentUpgradeableProxy( + impl, + proxyAdmin, + abi.encodeCall( + PegInContract.initialize, + ( + admin_, + payable(address(bridgeMock)), + cfg.dustThreshold, + cfg.minimumPegIn, + address(collateralManagement), + cfg.mainnet, + PauseRegistry(pauseRegistryProxy) + ) + ) + ) + ); + pegInContract = PegInContract(payable(proxy)); + } + + function _deployPegOut( + address admin_, + HelperConfig.FlyoverConfig memory cfg + ) private { + address impl = address(new PegOutContract()); + address proxy = address( + new TransparentUpgradeableProxy( + impl, + proxyAdmin, + abi.encodeCall( + PegOutContract.initialize, + ( + admin_, + payable(address(bridgeMock)), + cfg.dustThreshold, + address(collateralManagement), + cfg.mainnet, + cfg.btcBlockTime, + PauseRegistry(pauseRegistryProxy) + ) + ) + ) + ); + pegOutContract = PegOutContract(payable(proxy)); + } +} diff --git a/test/deployment/UpgradeLBC.t.sol b/test/deployment/UpgradeLBC.t.sol index 912b0791..435335f5 100644 --- a/test/deployment/UpgradeLBC.t.sol +++ b/test/deployment/UpgradeLBC.t.sol @@ -10,6 +10,7 @@ import {LiquidityBridgeContractV2} from "../../src/legacy/LiquidityBridgeContrac import {LiquidityBridgeContractProxy} from "../../src/legacy/LiquidityBridgeContractProxy.sol"; import {LiquidityBridgeContractAdmin} from "../../src/legacy/LiquidityBridgeContractAdmin.sol"; import {ITransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; +import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol"; /** * @title UpgradeLBCTest @@ -227,4 +228,89 @@ contract UpgradeLBCTest is Test { console.log("\n[PASS] V2 functions callable after upgrade!"); console.log("[PASS] UpgradeLBC.s.sol script pattern validated!"); } + + function test_UpgradeIsDelayedByTimelock() public { + bytes32 adminSlot = bytes32( + uint256(keccak256("eip1967.proxy.admin")) - 1 + ); + address proxyAdminAddress = address( + uint160(uint256(vm.load(address(proxy), adminSlot))) + ); + LiquidityBridgeContractAdmin actualAdmin = LiquidityBridgeContractAdmin( + proxyAdminAddress + ); + + address proposer = makeAddr("proposer"); + address executor = makeAddr("executor"); + uint256 minDelay = 7 days; + address[] memory proposers = new address[](1); + proposers[0] = proposer; + address[] memory executors = new address[](1); + executors[0] = executor; + TimelockController timelock = new TimelockController( + minDelay, + proposers, + executors, + address(0) + ); + + // Timelock must own ProxyAdmin to enforce delayed upgrades. + vm.prank(actualAdmin.owner()); + actualAdmin.transferOwnership(address(timelock)); + assertEq(actualAdmin.owner(), address(timelock), "timelock not owner"); + + lbcV2Impl = new LiquidityBridgeContractV2(); + + bytes32 implSlot = bytes32( + uint256(keccak256("eip1967.proxy.implementation")) - 1 + ); + address oldImpl = address( + uint160(uint256(vm.load(address(proxy), implSlot))) + ); + + bytes memory upgradePayload = abi.encodeWithSignature( + "upgradeAndCall(address,address,bytes)", + address(proxy), + address(lbcV2Impl), + bytes("") + ); + bytes32 predecessor = bytes32(0); + bytes32 salt = keccak256("upgrade-op"); + + vm.prank(proposer); + timelock.schedule( + address(actualAdmin), + 0, + upgradePayload, + predecessor, + salt, + minDelay + ); + + vm.prank(executor); + vm.expectRevert(); + timelock.execute( + address(actualAdmin), + 0, + upgradePayload, + predecessor, + salt + ); + + vm.warp(block.timestamp + minDelay); + vm.prank(executor); + timelock.execute( + address(actualAdmin), + 0, + upgradePayload, + predecessor, + salt + ); + + address newImpl = address( + uint160(uint256(vm.load(address(proxy), implSlot))) + ); + assertEq(newImpl, address(lbcV2Impl), "upgrade did not execute"); + assertTrue(newImpl != oldImpl, "implementation did not change"); + } } diff --git a/test/helpers/FlyoverTestBase.sol b/test/helpers/FlyoverTestBase.sol index a91af33d..16f019f8 100644 --- a/test/helpers/FlyoverTestBase.sol +++ b/test/helpers/FlyoverTestBase.sol @@ -764,7 +764,15 @@ abstract contract FlyoverTestBase is Test { dustThreshold: TEST_DUST_THRESHOLD_PEGIN, btcBlockTime: TEST_BTC_BLOCK_TIME, mainnet: false, - adminDelay: TEST_DEFAULT_ADMIN_DELAY + adminDelay: TEST_DEFAULT_ADMIN_DELAY, + timelockMinDelay: 7 days, + timelockProposer: address( + 0x1000000000000000000000000000000000000001 + ), + timelockExecutor: address( + 0x1000000000000000000000000000000000000002 + ), + timelockAdmin: address(0) }); } }