From 64bbd41d30e125f55dbe35363f1f67fd1ff1c75d Mon Sep 17 00:00:00 2001 From: Ramarti Date: Sun, 20 Oct 2024 01:59:02 -0300 Subject: [PATCH] feat(genesis): create3 and timelock (#242) Adds `CREATE3` predeploy, which deploys a `TimelockController` that will control all the admin methods and upgrades. issue: none --- contracts/README.md | 12 +- contracts/script/GenerateAlloc.s.sol | 138 +++++++++++++++--- contracts/script/TestPrecompileUpgrades.s.sol | 82 ----------- contracts/src/libraries/Predeploys.sol | 4 + contracts/test/deploy/Create3.t.sol | 8 +- contracts/test/script/DeployCore.t.sol | 27 ---- contracts/test/stake/IPTokenStaking.t.sol | 58 +++++--- contracts/test/timelock/Timelock.t.sol | 105 +++++++++++++ contracts/test/ubipool/UBIPool.t.sol | 80 ++++++---- .../UpgradeEntryPoint.t.sol | 11 +- .../test/upgrades/PredeployUpgrades.t.sol | 111 ++++++++++++++ contracts/test/utils/Test.sol | 138 +++++++++++++++++- 12 files changed, 584 insertions(+), 190 deletions(-) delete mode 100644 contracts/script/TestPrecompileUpgrades.s.sol delete mode 100644 contracts/test/script/DeployCore.t.sol create mode 100644 contracts/test/timelock/Timelock.t.sol rename contracts/test/{upgrade => upgrade-entrypoint}/UpgradeEntryPoint.t.sol (75%) create mode 100644 contracts/test/upgrades/PredeployUpgrades.t.sol diff --git a/contracts/README.md b/contracts/README.md index 34b53aee..4928ca22 100644 --- a/contracts/README.md +++ b/contracts/README.md @@ -49,13 +49,15 @@ To generate this first state: 1. Add a .env file in `contracts/.env` ``` -ADMIN_ADDRESS=0x123... -UPGRADE_ADMIN_ADDRESS=0x234... +ADMIN_ADDRESS=0x... +TIMELOCK_EXECUTOR_ADDRESS=0x... +TIMELOCK_GUARDIAN_ADDRESS=0x... ``` -- `ADMIN_ADDRESS` will be the owner of `IPTokenStaking` and `UpgradeEntryPoint`, able to execute admin methods. -- `UPGRADE_ADMIN_ADDRESS` will be the owner of the [ProxyAdmin](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/proxy/transparent/ProxyAdmin.sol) for each upgradeable predeploy. +- `ADMIN_ADDRESS` will be the owner of the `TimelockController` contract. Will be able to propose transactions to the timelock, and cancel them. +- `TIMELOCK_EXECUTOR_ADDRESS` address allowed to execute the scheduled actions once the timelock matures. +- `TIMELOCK_GUARDIAN_ADDRESS` address allowed to cancel proposals -2. Run +1. Run ``` forge script script/GenerateAlloc.s.sol -vvvv --chain-id ``` diff --git a/contracts/script/GenerateAlloc.s.sol b/contracts/script/GenerateAlloc.s.sol index 13750de7..e47ee261 100644 --- a/contracts/script/GenerateAlloc.s.sol +++ b/contracts/script/GenerateAlloc.s.sol @@ -6,7 +6,7 @@ pragma solidity ^0.8.23; import { Script } from "forge-std/Script.sol"; import { console2 } from "forge-std/console2.sol"; import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; - +import { TimelockController } from "@openzeppelin/contracts/governance/TimelockController.sol"; import { IIPTokenStaking } from "../src/interfaces/IIPTokenStaking.sol"; import { IPTokenStaking } from "../src/protocol/IPTokenStaking.sol"; import { UpgradeEntrypoint } from "../src/protocol/UpgradeEntrypoint.sol"; @@ -15,7 +15,7 @@ import { UBIPool } from "../src/protocol/UBIPool.sol"; import { EIP1967Helper } from "./utils/EIP1967Helper.sol"; import { InitializableHelper } from "./utils/InitializableHelper.sol"; import { Predeploys } from "../src/libraries/Predeploys.sol"; - +import { Create3 } from "../src/deploy/Create3.sol"; /** * @title GenerateAlloc * @dev A script to generate the alloc section of EL genesis @@ -33,11 +33,15 @@ contract GenerateAlloc is Script { */ address internal deployer = 0xDDdDddDdDdddDDddDDddDDDDdDdDDdDDdDDDDDDd; - // Upgrade admin controls upgradeability (by being Owner of each ProxyAdmin), - // protocol admin is Owner of precompiles (admin/governance methods). - // To disable upgradeability, we transfer ProxyAdmin ownership to a dead address - address internal upgradeAdmin; + // TimelockController + address internal timelock; + // Governance multi-sig address internal protocolAdmin; + // Executor of scheduled operations + address internal timelockExecutor; + // Guardian of timelock + address internal timelockGuardian; + string internal dumpPath = getDumpPath(); bool public saveState = true; uint256 public constant MAINNET_CHAIN_ID = 0; // TBD @@ -49,10 +53,11 @@ contract GenerateAlloc is Script { } /// @notice call from Test.sol only - function setAdminAddresses(address upgrade, address protocol) external { + function setAdminAddresses(address protocol, address executor, address guardian) external { require(block.chainid == 31337, "Only for local tests"); - upgradeAdmin = upgrade; protocolAdmin = protocol; + timelockExecutor = executor; + timelockGuardian = guardian; } /// @notice path where alloc file will be stored @@ -74,18 +79,70 @@ contract GenerateAlloc is Script { } } - /// @notice main script method - function run() public { - if (upgradeAdmin == address(0)) { - upgradeAdmin = vm.envAddress("UPGRADE_ADMIN_ADDRESS"); + /// @notice Get the minimum delay for the timelock + function getTimelockMinDelay() internal view returns (uint256) { + if (block.chainid == 1513) { + // Iliad + return 1 days; + } else if (block.chainid == 1512) { + // Mininet + return 10 seconds; + } else if (block.chainid == 1315) { + // Odyssey devnet + return 10 seconds; + } else if (block.chainid == 1516) { + // Odyssey testnet + return 1 days; + } else if (block.chainid == 31337) { + // Local + return 10 seconds; + } else { + revert("Unsupported chain id"); } - require(upgradeAdmin != address(0), "upgradeAdmin not set"); + } + function getTimelockControllers() + internal + view + returns (address[] memory proposers, address[] memory executors, address canceller) + { + proposers = new address[](1); + executors = new address[](1); + if (block.chainid == 1513) { + // Iliad + proposers[0] = protocolAdmin; + executors[0] = protocolAdmin; + canceller = protocolAdmin; + return (proposers, executors, protocolAdmin); + } else if (block.chainid == 1512 || block.chainid == 1315 || block.chainid == 31337 || block.chainid == 1516) { + // Mininet, Odyssey devnet, Local, Odyssey testnet + proposers[0] = protocolAdmin; + executors[0] = timelockExecutor; + canceller = timelockGuardian; + return (proposers, executors, protocolAdmin); + } else { + revert("Unsupported chain id"); + } + } + + /// @notice main script method + function run() public { + // Tests should set these addresses first if (protocolAdmin == address(0)) { protocolAdmin = vm.envAddress("ADMIN_ADDRESS"); } require(protocolAdmin != address(0), "protocolAdmin not set"); + if (timelockExecutor == address(0)) { + timelockExecutor = vm.envAddress("TIMELOCK_EXECUTOR_ADDRESS"); + } + require(timelockExecutor != address(0), "executor not set"); + + if (timelockGuardian == address(0)) { + timelockGuardian = vm.envAddress("TIMELOCK_GUARDIAN_ADDRESS"); + } + require(timelockGuardian != address(0), "canceller not set"); + vm.startPrank(deployer); setPredeploys(); @@ -111,6 +168,11 @@ contract GenerateAlloc is Script { } function setPredeploys() internal { + // predeploys that are not upgradable + setCreate3(); + deployTimelock(); + + // predeploys that are upgradable setProxy(Predeploys.Staking); setProxy(Predeploys.Upgrades); setProxy(Predeploys.UBIPool); @@ -120,6 +182,32 @@ contract GenerateAlloc is Script { setUBIPool(); } + /// @notice Deploy TimelockController + function deployTimelock() internal { + // We deploy this with Create3 because we can't set storage variables in constructor with vm.etch + + uint256 minDelay = getTimelockMinDelay(); + (address[] memory proposers, address[] memory executors, address canceller) = getTimelockControllers(); + + bytes memory creationCode = abi.encodePacked( + type(TimelockController).creationCode, + abi.encode(minDelay, proposers, executors, deployer) + ); + bytes32 salt = keccak256("STORY_TIMELOCK_CONTROLLER"); + timelock = Create3(Predeploys.Create3).deploy(salt, creationCode); + + TimelockController(payable(timelock)).grantRole( + TimelockController(payable(timelock)).CANCELLER_ROLE(), + canceller + ); + TimelockController(payable(timelock)).renounceRole( + TimelockController(payable(timelock)).DEFAULT_ADMIN_ROLE(), + deployer + ); + + console2.log("TimelockController deployed at:", timelock); + } + function setProxy(address proxyAddr) internal { address impl = Predeploys.getImplAddress(proxyAddr); @@ -129,7 +217,7 @@ contract GenerateAlloc is Script { vm.etch(impl, "00"); // use new, so that the immutable variable the holds the ProxyAdmin proxyAddr is set in properly in bytecode - address tmp = address(new TransparentUpgradeableProxy(impl, upgradeAdmin, "")); + address tmp = address(new TransparentUpgradeableProxy(impl, timelock, "")); vm.etch(proxyAddr, tmp.code); // set implempentation storage manually @@ -170,7 +258,7 @@ contract GenerateAlloc is Script { InitializableHelper.disableInitializers(impl); IIPTokenStaking.InitializerArgs memory args = IIPTokenStaking.InitializerArgs({ - owner: protocolAdmin, + owner: timelock, minStakeAmount: 1024 ether, minUnstakeAmount: 1024 ether, minCommissionRate: 5_00, // 5% in basis points @@ -201,7 +289,7 @@ contract GenerateAlloc is Script { vm.resetNonce(tmp); InitializableHelper.disableInitializers(impl); - UpgradeEntrypoint(Predeploys.Upgrades).initialize(protocolAdmin); + UpgradeEntrypoint(Predeploys.Upgrades).initialize(timelock); console2.log("UpgradeEntrypoint proxy deployed at:", Predeploys.Upgrades); console2.log("UpgradeEntrypoint ProxyAdmin deployed at:", EIP1967Helper.getAdmin(Predeploys.Upgrades)); @@ -220,7 +308,7 @@ contract GenerateAlloc is Script { vm.resetNonce(tmp); InitializableHelper.disableInitializers(impl); - UBIPool(Predeploys.UBIPool).initialize(protocolAdmin); + UBIPool(Predeploys.UBIPool).initialize(timelock); console2.log("UBIPool proxy deployed at:", Predeploys.UBIPool); console2.log("UBIPool ProxyAdmin deployed at:", EIP1967Helper.getAdmin(Predeploys.UBIPool)); @@ -228,9 +316,22 @@ contract GenerateAlloc is Script { console2.log("UBIPool owner:", UBIPool(Predeploys.UBIPool).owner()); } + function setCreate3() internal { + address tmp = address(new Create3()); + vm.etch(Predeploys.Create3, tmp.code); + + // reset tmp + vm.etch(tmp, ""); + vm.store(tmp, 0, "0x"); + vm.resetNonce(tmp); + + vm.deal(Predeploys.Create3, 1); + console2.log("Create3 deployed at:", Predeploys.Create3); + } + function setAllocations() internal { // EL Predeploys - vm.deal(0x0000000000000000000000000000000000000001, 1); + // Geth precompiles vm.deal(0x0000000000000000000000000000000000000001, 1); vm.deal(0x0000000000000000000000000000000000000002, 1); vm.deal(0x0000000000000000000000000000000000000003, 1); @@ -240,6 +341,7 @@ contract GenerateAlloc is Script { vm.deal(0x0000000000000000000000000000000000000007, 1); vm.deal(0x0000000000000000000000000000000000000008, 1); vm.deal(0x0000000000000000000000000000000000000009, 1); + // Story's IPGraph precompile vm.deal(0x000000000000000000000000000000000000001a, 1); // Allocation if (block.chainid == MAINNET_CHAIN_ID) { diff --git a/contracts/script/TestPrecompileUpgrades.s.sol b/contracts/script/TestPrecompileUpgrades.s.sol deleted file mode 100644 index de4ca9c4..00000000 --- a/contracts/script/TestPrecompileUpgrades.s.sol +++ /dev/null @@ -1,82 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-only -pragma solidity ^0.8.23; -/* solhint-disable no-console */ -/* solhint-disable max-line-length */ - -import { Script } from "forge-std/Script.sol"; -import { console2 } from "forge-std/console2.sol"; -import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; - -import { IPTokenStaking } from "../src/protocol/IPTokenStaking.sol"; -import { UpgradeEntrypoint } from "../src/protocol/UpgradeEntrypoint.sol"; -import { ITransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; - -import { EIP1967Helper } from "./utils/EIP1967Helper.sol"; -import { Predeploys } from "../src/libraries/Predeploys.sol"; - -abstract contract MockNewFeatures { - function foo() external pure returns (string memory) { - return "bar"; - } -} - -contract IPTokenStakingV2 is IPTokenStaking, MockNewFeatures { - constructor(uint256 stakingRounding, uint256 defaultMinFee) IPTokenStaking(stakingRounding, defaultMinFee) {} -} - -contract UpgradeEntrypointV2 is UpgradeEntrypoint, MockNewFeatures {} - -/** - * @title TestPrecompileUpgrades - * @dev A script to test upgrading the precompile contracts - */ -contract TestPrecompileUpgrades is Script { - // To run the script: - // - Dry run - // forge script script/DeployIPTokenSlashing.s.sol --fork-url - // - // - Deploy (OK for devnet) - // forge script script/DeployIPTokenSlashing.s.sol --fork-url --broadcast - // - // - Deploy and Verify (for testnet) - function run() public { - // Read env for admin address - uint256 upgradeKey = vm.envUint("UPGRADE_ADMIN_KEY"); - address upgrader = vm.addr(upgradeKey); - console2.log("upgrader", upgrader); - vm.startBroadcast(upgradeKey); - - // ---- Staking - address newImpl = address( - new IPTokenStakingV2( - 1 gwei, // stakingRounding - 1 ether - ) - ); - ProxyAdmin proxyAdmin = ProxyAdmin(EIP1967Helper.getAdmin(Predeploys.Staking)); - console2.log("staking proxy admin", address(proxyAdmin)); - console2.log("staking proxy admin owner", proxyAdmin.owner()); - proxyAdmin.upgradeAndCall(ITransparentUpgradeableProxy(Predeploys.Staking), newImpl, ""); - if (EIP1967Helper.getImplementation(Predeploys.Staking) != newImpl) { - revert("Staking not upgraded"); - } - if (keccak256(abi.encode(IPTokenStakingV2(Predeploys.Staking).foo())) != keccak256(abi.encode("bar"))) { - revert("Upgraded to wrong iface"); - } - - // ---- Upgrades - newImpl = address(new UpgradeEntrypointV2()); - proxyAdmin = ProxyAdmin(EIP1967Helper.getAdmin(Predeploys.Upgrades)); - console2.log("upgrades proxy admin", address(proxyAdmin)); - console2.log("upgrades proxy admin owner", proxyAdmin.owner()); - - proxyAdmin.upgradeAndCall(ITransparentUpgradeableProxy(Predeploys.Upgrades), newImpl, ""); - if (keccak256(abi.encode(UpgradeEntrypointV2(Predeploys.Upgrades).foo())) != keccak256(abi.encode("bar"))) { - revert("Upgraded to wrong iface"); - } - if (EIP1967Helper.getImplementation(Predeploys.Upgrades) != newImpl) { - revert("UpgradeEntrypoint not upgraded"); - } - vm.stopBroadcast(); - } -} diff --git a/contracts/src/libraries/Predeploys.sol b/contracts/src/libraries/Predeploys.sol index 62e8cc89..824d013b 100644 --- a/contracts/src/libraries/Predeploys.sol +++ b/contracts/src/libraries/Predeploys.sol @@ -15,6 +15,10 @@ library Predeploys { address internal constant UBIPool = 0xCccCCC0000000000000000000000000000000002; address internal constant Upgrades = 0xccCCcc0000000000000000000000000000000003; + /// @notice Create3 factory address + /// @dev We maximize compatibility with the contracts deployed by ZeframLou + address internal constant Create3 = 0x9fBB3DF7C40Da2e5A0dE984fFE2CCB7C47cd0ABf; + /// @notice Return true if `addr` is not proxied function notProxied(address addr) internal pure returns (bool) { return addr == WIP; diff --git a/contracts/test/deploy/Create3.t.sol b/contracts/test/deploy/Create3.t.sol index 5d5119e8..cb8d1ed4 100644 --- a/contracts/test/deploy/Create3.t.sol +++ b/contracts/test/deploy/Create3.t.sol @@ -5,17 +5,11 @@ pragma solidity ^0.8.23; /// NOTE: pragma allowlist-secret must be inline (same line as the pubkey hex string) to avoid false positive /// flag "Hex High Entropy String" in CI run detect-secrets -import { Test } from "forge-std/Test.sol"; +import { Test } from "../utils/Test.sol"; import { Create3 } from "../../src/deploy/Create3.sol"; contract Create3Test is Test { - Create3 private create3; - - function setUp() public { - create3 = new Create3(); - } - function testCreate3_deploy() public { // deploy and getDeployed should return same address when deployed by the same deployer and with same salt. bytes32 salt = 0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef; diff --git a/contracts/test/script/DeployCore.t.sol b/contracts/test/script/DeployCore.t.sol deleted file mode 100644 index 84cbe225..00000000 --- a/contracts/test/script/DeployCore.t.sol +++ /dev/null @@ -1,27 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-only -pragma solidity ^0.8.23; -// /* solhint-disable no-console */ -// /* solhint-disable max-line-length */ -/// NOTE: pragma allowlist-secret must be inline (same line as the pubkey hex string) to avoid false positive -/// flag "Hex High Entropy String" in CI run detect-secrets - -import { Test } from "forge-std/Test.sol"; - -// import { DeployCore } from "../../script/DeployCore.s.sol"; - -contract DeployCoreTest is Test { - // DeployCore private deployCore; - // function setUp() public { - // deployCore = new DeployCore(); - // } - // function testDeployDeployCore_run() public { - // // Network shall not deploy the IPTokenStaking contract if IPTOKENSTAKING_DEPLOYER_KEY not set. - // vm.chainId(1513); - // // solhint-disable - // vm.expectRevert('vm.envUint: environment variable "IPTOKENSTAKING_DEPLOYER_KEY" not found'); - // deployCore.run(); - // // Network shall deploy the IPTokenStaking contract. - // vm.setEnv("IPTOKENSTAKING_DEPLOYER_KEY", "0x123456789abcdef"); - // deployCore.run(); - // } -} diff --git a/contracts/test/stake/IPTokenStaking.t.sol b/contracts/test/stake/IPTokenStaking.t.sol index 449bb46a..7fbb044b 100644 --- a/contracts/test/stake/IPTokenStaking.t.sol +++ b/contracts/test/stake/IPTokenStaking.t.sol @@ -674,45 +674,63 @@ contract IPTokenStakingTest is Test { function testIPTokenStaking_setMinStakeAmount() public { // Set amount that will be rounded down to 0 - vm.prank(admin); - ipTokenStaking.setMinStakeAmount(5 wei); + performTimelocked( + address(ipTokenStaking), + abi.encodeWithSelector(IPTokenStaking.setMinStakeAmount.selector, 5 wei) + ); assertEq(ipTokenStaking.minStakeAmount(), 0); // Set amount that will not be rounded - vm.prank(admin); + schedule(address(ipTokenStaking), abi.encodeWithSelector(IPTokenStaking.setMinStakeAmount.selector, 1 ether)); + waitForTimelock(); vm.expectEmit(address(ipTokenStaking)); emit IIPTokenStaking.MinStakeAmountSet(1 ether); - ipTokenStaking.setMinStakeAmount(1 ether); + executeTimelocked( + address(ipTokenStaking), + abi.encodeWithSelector(IPTokenStaking.setMinStakeAmount.selector, 1 ether) + ); assertEq(ipTokenStaking.minStakeAmount(), 1 ether); // Set 0 - vm.prank(admin); - vm.expectRevert("IPTokenStaking: Zero min stake amount"); - ipTokenStaking.setMinStakeAmount(0 ether); + expectRevertTimelocked( + address(ipTokenStaking), + abi.encodeWithSelector(IPTokenStaking.setMinStakeAmount.selector, 0 ether), + "IPTokenStaking: Zero min stake amount" + ); // Set using a non-owner address vm.prank(delegatorAddr); - vm.expectRevert(); + vm.expectRevert(); // TODO: encode OwnableUnauthorizedAccount ipTokenStaking.setMinStakeAmount(1 ether); } function testIPTokenStaking_setMinUnstakeAmount() public { // Set amount that will be rounded down to 0 - vm.prank(admin); - ipTokenStaking.setMinUnstakeAmount(5 wei); + // Set amount that will be rounded down to 0 + performTimelocked( + address(ipTokenStaking), + abi.encodeWithSelector(IPTokenStaking.setMinUnstakeAmount.selector, 5 wei) + ); assertEq(ipTokenStaking.minUnstakeAmount(), 0); // Set amount that will not be rounded - vm.prank(admin); + schedule(address(ipTokenStaking), abi.encodeWithSelector(IPTokenStaking.setMinUnstakeAmount.selector, 1 ether)); + waitForTimelock(); vm.expectEmit(address(ipTokenStaking)); emit IIPTokenStaking.MinUnstakeAmountSet(1 ether); - ipTokenStaking.setMinUnstakeAmount(1 ether); + executeTimelocked( + address(ipTokenStaking), + abi.encodeWithSelector(IPTokenStaking.setMinUnstakeAmount.selector, 1 ether) + ); assertEq(ipTokenStaking.minUnstakeAmount(), 1 ether); // Set 0 vm.prank(admin); - vm.expectRevert("IPTokenStaking: Zero min unstake amount"); - ipTokenStaking.setMinUnstakeAmount(0 ether); + expectRevertTimelocked( + address(ipTokenStaking), + abi.encodeWithSelector(IPTokenStaking.setMinUnstakeAmount.selector, 0 ether), + "IPTokenStaking: Zero min unstake amount" + ); // Set using a non-owner address vm.prank(delegatorAddr); @@ -813,10 +831,11 @@ contract IPTokenStakingTest is Test { function testIPTokenStaking_SetFee() public { // Network shall allow the owner to set the fee charged for adding to CL storage. uint256 newFee = 2 ether; + schedule(address(ipTokenStaking), abi.encodeWithSelector(IPTokenStaking.setFee.selector, newFee)); + waitForTimelock(); vm.expectEmit(address(ipTokenStaking)); emit IIPTokenStaking.FeeSet(newFee); - vm.prank(admin); - ipTokenStaking.setFee(newFee); + executeTimelocked(address(ipTokenStaking), abi.encodeWithSelector(IPTokenStaking.setFee.selector, newFee)); assertEq(ipTokenStaking.fee(), newFee); // Network shall not allow non-owner to set the fee charged for adding to CL storage. @@ -826,7 +845,10 @@ contract IPTokenStakingTest is Test { assertEq(ipTokenStaking.fee(), newFee); // Network shall not allow fees < default - vm.expectRevert(); - ipTokenStaking.setFee(1); + expectRevertTimelocked( + address(ipTokenStaking), + abi.encodeWithSelector(IPTokenStaking.setFee.selector, 1), + "IPTokenStaking: Invalid min fee" + ); } } diff --git a/contracts/test/timelock/Timelock.t.sol b/contracts/test/timelock/Timelock.t.sol new file mode 100644 index 00000000..153f8e54 --- /dev/null +++ b/contracts/test/timelock/Timelock.t.sol @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity ^0.8.23; + +import { Test } from "../utils/Test.sol"; +import { IPTokenStaking } from "../../src/protocol/IPTokenStaking.sol"; +import { TimelockController } from "@openzeppelin/contracts/governance/TimelockController.sol"; + +contract TimelockTest is Test { + function setUp() public override { + super.setUp(); + } + + function testCancelBeforeExecute() public { + // Prepare a sample operation + address target = address(ipTokenStaking); + uint256 value = 0; + bytes memory data = abi.encodeWithSelector(IPTokenStaking.setFee.selector, 2 ether); + bytes32 predecessor = bytes32(0); + bytes32 salt = keccak256("TEST_SALT"); + uint256 delay = timelock.getMinDelay(); + + // Schedule the operation + vm.prank(admin); + timelock.schedule(target, value, data, predecessor, salt, delay); + + // Ensure the operation is pending + bytes32 operationId = timelock.hashOperation(target, value, data, predecessor, salt); + assertTrue(timelock.isOperationPending(operationId)); + + // Cancel the operation + vm.prank(admin); + timelock.cancel(operationId); + + // Ensure the operation is no longer pending + assertFalse(timelock.isOperationPending(operationId)); + + // Wait for the delay to pass + vm.warp(block.timestamp + delay + 1); + + // Try to execute the cancelled operation + vm.prank(executor); + vm.expectRevert( + abi.encodeWithSelector( + TimelockController.TimelockUnexpectedOperationState.selector, + operationId, + bytes32(1 << uint8(TimelockController.OperationState.Ready)) + ) + ); + + timelock.execute(target, value, data, predecessor, salt); + + // Verify that the fee wasn't changed + assertEq(ipTokenStaking.fee(), 1 ether); + } + + function testExecuteSequenceWithPredecessors() public { + // Prepare sample operations + address target = address(ipTokenStaking); + uint256 value = 0; + bytes memory data1 = abi.encodeWithSelector(IPTokenStaking.setFee.selector, 2 ether); + bytes memory data2 = abi.encodeWithSelector(IPTokenStaking.setFee.selector, 3 ether); + bytes memory data3 = abi.encodeWithSelector(IPTokenStaking.setFee.selector, 4 ether); + bytes32 salt1 = keccak256("SALT_1"); + bytes32 salt2 = keccak256("SALT_2"); + bytes32 salt3 = keccak256("SALT_3"); + uint256 delay = timelock.getMinDelay(); + + // Schedule the first operation + vm.prank(admin); + timelock.schedule(target, value, data1, bytes32(0), salt1, delay); + bytes32 id1 = timelock.hashOperation(target, value, data1, bytes32(0), salt1); + + // Schedule the second operation with the first as predecessor + vm.prank(admin); + timelock.schedule(target, value, data2, id1, salt2, delay); + bytes32 id2 = timelock.hashOperation(target, value, data2, id1, salt2); + + // Schedule the third operation with the second as predecessor + vm.prank(admin); + timelock.schedule(target, value, data3, id2, salt3, delay); + + // Wait for the delay to pass + vm.warp(block.timestamp + delay + 1); + + // Execute the first operation + vm.prank(executor); + timelock.execute(target, value, data1, bytes32(0), salt1); + assertEq(ipTokenStaking.fee(), 2 ether); + + // Try to execute the third operation (should fail due to unexecuted predecessor) + vm.prank(executor); + vm.expectRevert(abi.encodeWithSelector(TimelockController.TimelockUnexecutedPredecessor.selector, id2)); + timelock.execute(target, value, data3, id2, salt3); + + // Execute the second operation + vm.prank(executor); + timelock.execute(target, value, data2, id1, salt2); + assertEq(ipTokenStaking.fee(), 3 ether); + + // Finally, execute the third operation + vm.prank(executor); + timelock.execute(target, value, data3, id2, salt3); + assertEq(ipTokenStaking.fee(), 4 ether); + } +} diff --git a/contracts/test/ubipool/UBIPool.t.sol b/contracts/test/ubipool/UBIPool.t.sol index 1acda8f3..7378a138 100644 --- a/contracts/test/ubipool/UBIPool.t.sol +++ b/contracts/test/ubipool/UBIPool.t.sol @@ -22,15 +22,18 @@ contract UBIPoolTest is Test, ValidatorData { // Fail if percentage too high uint32 tooMuch = ubiPool.MAX_UBI_PERCENTAGE() + 1; - vm.expectRevert("UBIPool: percentage too high"); - vm.prank(admin); - ubiPool.setUBIPercentage(tooMuch); + expectRevertTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIPercentage.selector, tooMuch), + "UBIPool: percentage too high" + ); // Set percentage + schedule(address(ubiPool), abi.encodeWithSelector(IUBIPool.setUBIPercentage.selector, 12_00)); + waitForTimelock(); vm.expectEmit(true, true, true, true); emit IUBIPool.UBIPercentageSet(12_00); - vm.prank(admin); - ubiPool.setUBIPercentage(12_00); + executeTimelocked(address(ubiPool), abi.encodeWithSelector(IUBIPool.setUBIPercentage.selector, 12_00)); } function test_setUBIDistribution_claimUBI() public { @@ -46,9 +49,12 @@ contract UBIPoolTest is Test, ValidatorData { ubiPool.claimUBI(1, validatorUncmpPubKeys[i]); } vm.deal(address(ubiPool), totalAmount); - vm.prank(admin); - uint256 distributionId = ubiPool.setUBIDistribution(totalAmount, validatorUncmpPubKeys, amounts); - assertEq(distributionId, 1); + performTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIDistribution.selector, totalAmount, validatorUncmpPubKeys, amounts) + ); + assertEq(ubiPool.currentDistributionId(), 1); + for (uint256 i = 0; i < validators.length; i++) { uint256 amount = amounts[i]; assertEq(ubiPool.validatorUBIAmounts(1, validatorUncmpPubKeys[i]), amount); @@ -61,8 +67,13 @@ contract UBIPoolTest is Test, ValidatorData { assertEq(ubiPool.validatorUBIAmounts(1, validatorUncmpPubKeys[i]), 0); } vm.deal(address(ubiPool), totalAmount); - vm.prank(admin); - assertEq(ubiPool.setUBIDistribution(totalAmount, validatorUncmpPubKeys, amounts), 2); + performTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIDistribution.selector, totalAmount, validatorUncmpPubKeys, amounts), + bytes32(keccak256("setUBIDistribution 2nd time")) + ); + assertEq(ubiPool.currentDistributionId(), 2); + for (uint256 i = 0; i < validators.length; i++) { uint256 amount = amounts[i]; assertEq(ubiPool.validatorUBIAmounts(2, validatorUncmpPubKeys[i]), amount); @@ -82,29 +93,38 @@ contract UBIPoolTest is Test, ValidatorData { ubiPool.setUBIDistribution(100 ether, new bytes[](0), new uint256[](0)); // Fail if validatorUncmpPubKeys is empty - vm.expectRevert("UBIPool: validatorUncmpPubKeys cannot be empty"); - vm.prank(admin); - ubiPool.setUBIDistribution(100 ether, new bytes[](0), new uint256[](0)); + expectRevertTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIDistribution.selector, 100 ether, new bytes[](0), new uint256[](0)), + "UBIPool: validatorUncmpPubKeys cannot be empty" + ); // Fail if validatorUncmpPubKeys and percentages do not match - vm.expectRevert("UBIPool: length mismatch"); - vm.prank(admin); - ubiPool.setUBIDistribution(100 ether, new bytes[](1), new uint256[](0)); + expectRevertTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIDistribution.selector, 100 ether, new bytes[](1), new uint256[](0)), + "UBIPool: length mismatch" + ); // Fail if UBIPool: not enough balance - vm.expectRevert("UBIPool: not enough balance"); - vm.prank(admin); - ubiPool.setUBIDistribution(100 ether, new bytes[](1), new uint256[](1)); + expectRevertTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIDistribution.selector, 100 ether, new bytes[](1), new uint256[](1)), + "UBIPool: not enough balance" + ); // Fail if amounts do not sum to totalUBI - vm.expectRevert("UBIPool: total amount mismatch"); + uint256[] memory amounts = new uint256[](1); bytes[] memory validatorUncmpPubKeys = new bytes[](1); validatorUncmpPubKeys[0] = validators[0].uncompressedHex; amounts[0] = 1 ether; vm.deal(address(ubiPool), 100 ether); - vm.prank(admin); - ubiPool.setUBIDistribution(100 ether, validatorUncmpPubKeys, amounts); + expectRevertTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIDistribution.selector, 100 ether, validatorUncmpPubKeys, amounts), + "UBIPool: total amount mismatch" + ); // Fail if one amount is zero vm.deal(address(ubiPool), 100 ether); @@ -112,9 +132,11 @@ contract UBIPoolTest is Test, ValidatorData { amounts[0] = 0; validatorUncmpPubKeys = new bytes[](1); validatorUncmpPubKeys[0] = validators[0].uncompressedHex; - vm.expectRevert("UBIPool: amounts cannot be zero"); - vm.prank(admin); - ubiPool.setUBIDistribution(100 ether, validatorUncmpPubKeys, amounts); + expectRevertTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIDistribution.selector, 100 ether, validatorUncmpPubKeys, amounts), + "UBIPool: amounts cannot be zero" + ); // Fail if pubkey is not valid vm.deal(address(ubiPool), 100 ether); @@ -125,8 +147,10 @@ contract UBIPoolTest is Test, ValidatorData { validatorUncmpPubKeys[ 0 ] = hex"0482782124bc9cd03c38aa4cac234dc4e4e3cecf04d57914371baf7fa78ffb975f6d58e245bea952dd039f0fec4e9db418c3b00000"; // pragma: allowlist secret - vm.expectRevert("PubKeyVerifier: Invalid pubkey length"); - vm.prank(admin); - ubiPool.setUBIDistribution(100 ether, validatorUncmpPubKeys, amounts); + expectRevertTimelocked( + address(ubiPool), + abi.encodeWithSelector(IUBIPool.setUBIDistribution.selector, 100 ether, validatorUncmpPubKeys, amounts), + "PubKeyVerifier: Invalid pubkey length" + ); } } diff --git a/contracts/test/upgrade/UpgradeEntryPoint.t.sol b/contracts/test/upgrade-entrypoint/UpgradeEntryPoint.t.sol similarity index 75% rename from contracts/test/upgrade/UpgradeEntryPoint.t.sol rename to contracts/test/upgrade-entrypoint/UpgradeEntryPoint.t.sol index ae492cb9..cf289864 100644 --- a/contracts/test/upgrade/UpgradeEntryPoint.t.sol +++ b/contracts/test/upgrade-entrypoint/UpgradeEntryPoint.t.sol @@ -16,10 +16,17 @@ contract UpgradeEntrypointTest is Test { int64 height = 1; string memory info = "info"; + schedule( + address(upgradeEntrypoint), + abi.encodeWithSelector(IUpgradeEntrypoint.planUpgrade.selector, name, height, info) + ); + waitForTimelock(); vm.expectEmit(address(upgradeEntrypoint)); emit IUpgradeEntrypoint.SoftwareUpgrade(name, height, info); - vm.prank(admin); - upgradeEntrypoint.planUpgrade(name, height, info); + executeTimelocked( + address(upgradeEntrypoint), + abi.encodeWithSelector(IUpgradeEntrypoint.planUpgrade.selector, name, height, info) + ); // Network shall not allow non-protocol owner to submit an upgrade plan. address otherAddr = address(0xf398C12A45Bc409b6C652E25bb0a3e702492A4ab); diff --git a/contracts/test/upgrades/PredeployUpgrades.t.sol b/contracts/test/upgrades/PredeployUpgrades.t.sol new file mode 100644 index 00000000..30b79d8d --- /dev/null +++ b/contracts/test/upgrades/PredeployUpgrades.t.sol @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity ^0.8.23; + +import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; +// solhint-disable-next-line max-line-length +import { ITransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; + +import { IPTokenStaking } from "../../src/protocol/IPTokenStaking.sol"; +import { UpgradeEntrypoint } from "../../src/protocol/UpgradeEntrypoint.sol"; +import { UBIPool } from "../../src/protocol/UBIPool.sol"; + +import { EIP1967Helper } from "../../script/utils/EIP1967Helper.sol"; +import { Predeploys } from "../../src/libraries/Predeploys.sol"; +import { Test } from "../utils/Test.sol"; + +abstract contract MockNewFeatures { + function foo() external pure returns (string memory) { + return "bar"; + } +} + +contract IPTokenStakingV2 is IPTokenStaking, MockNewFeatures { + constructor(uint256 stakingRounding, uint256 defaultMinFee) IPTokenStaking(stakingRounding, defaultMinFee) {} +} + +contract UpgradeEntrypointV2 is UpgradeEntrypoint, MockNewFeatures {} + +contract UBIPoolV2 is UBIPool, MockNewFeatures { + constructor(uint32 maxUBIPercentage) UBIPool(maxUBIPercentage) {} +} + +/** + * @title PrecompileUpgrades + * @dev A script to test upgrading the precompile contracts + */ +contract PrecompileUpgrades is Test { + function testUpgradeStaking() public { + // ---- Staking + address newImpl = address( + new IPTokenStakingV2( + 1 gwei, // stakingRounding + 1 ether + ) + ); + ProxyAdmin proxyAdmin = ProxyAdmin(EIP1967Helper.getAdmin(Predeploys.Staking)); + assertEq(proxyAdmin.owner(), address(timelock)); + + performTimelocked( + address(proxyAdmin), + abi.encodeWithSelector( + ProxyAdmin.upgradeAndCall.selector, + ITransparentUpgradeableProxy(Predeploys.Staking), + newImpl, + "" + ) + ); + + assertEq(EIP1967Helper.getImplementation(Predeploys.Staking), newImpl, "Staking not upgraded"); + assertEq( + keccak256(abi.encode(IPTokenStakingV2(Predeploys.Staking).foo())), + keccak256(abi.encode("bar")), + "Upgraded to wrong iface" + ); + } + + function testUpgradeUpgradeEntrypoint() public { + // ---- Upgrades + address newImpl = address(new UpgradeEntrypointV2()); + ProxyAdmin proxyAdmin = ProxyAdmin(EIP1967Helper.getAdmin(Predeploys.Upgrades)); + assertEq(proxyAdmin.owner(), address(timelock)); + + performTimelocked( + address(proxyAdmin), + abi.encodeWithSelector( + ProxyAdmin.upgradeAndCall.selector, + ITransparentUpgradeableProxy(Predeploys.Upgrades), + newImpl, + "" + ) + ); + assertEq(EIP1967Helper.getImplementation(Predeploys.Upgrades), newImpl, "Upgrades not upgraded"); + assertEq( + keccak256(abi.encode(IPTokenStakingV2(Predeploys.Upgrades).foo())), + keccak256(abi.encode("bar")), + "Upgraded to wrong iface" + ); + } + + function testUpgradeUBIPool() public { + // ---- UBIPool + address newImpl = address(new UBIPoolV2(10_00)); + ProxyAdmin proxyAdmin = ProxyAdmin(EIP1967Helper.getAdmin(Predeploys.UBIPool)); + assertEq(proxyAdmin.owner(), address(timelock)); + + performTimelocked( + address(proxyAdmin), + abi.encodeWithSelector( + ProxyAdmin.upgradeAndCall.selector, + ITransparentUpgradeableProxy(Predeploys.UBIPool), + newImpl, + "" + ) + ); + assertEq(EIP1967Helper.getImplementation(Predeploys.UBIPool), newImpl, "Upgrades not upgraded"); + assertEq( + keccak256(abi.encode(IPTokenStakingV2(Predeploys.UBIPool).foo())), + keccak256(abi.encode("bar")), + "Upgraded to wrong iface" + ); + } +} diff --git a/contracts/test/utils/Test.sol b/contracts/test/utils/Test.sol index ee61cf6c..c8da21e7 100644 --- a/contracts/test/utils/Test.sol +++ b/contracts/test/utils/Test.sol @@ -9,24 +9,156 @@ import { IPTokenStaking } from "../../src/protocol/IPTokenStaking.sol"; import { UpgradeEntrypoint } from "../../src/protocol/UpgradeEntrypoint.sol"; import { UBIPool } from "../../src/protocol/UBIPool.sol"; import { Predeploys } from "../../src/libraries/Predeploys.sol"; - +import { Create3 } from "../../src/deploy/Create3.sol"; import { GenerateAlloc } from "../../script/GenerateAlloc.s.sol"; +import { TimelockController } from "@openzeppelin/contracts/governance/TimelockController.sol"; contract Test is ForgeTest { address internal admin = address(0x123); - address internal upgradeAdmin = address(0x456); + address internal executor = address(0x456); + address internal guardian = address(0x789); + + address internal deployer = address(0xDDdDddDdDdddDDddDDddDDDDdDdDDdDDdDDDDDDd); IPTokenStaking internal ipTokenStaking; UpgradeEntrypoint internal upgradeEntrypoint; UBIPool internal ubiPool; + Create3 internal create3; + TimelockController internal timelock; function setUp() public virtual { GenerateAlloc initializer = new GenerateAlloc(); initializer.disableStateDump(); // Faster tests. Don't call to verify JSON output - initializer.setAdminAddresses(upgradeAdmin, admin); + initializer.setAdminAddresses(admin, executor, guardian); initializer.run(); ipTokenStaking = IPTokenStaking(Predeploys.Staking); upgradeEntrypoint = UpgradeEntrypoint(Predeploys.Upgrades); ubiPool = UBIPool(Predeploys.UBIPool); + create3 = Create3(Predeploys.Create3); + address timelockAddress = create3.getDeployed(deployer, keccak256("STORY_TIMELOCK_CONTROLLER")); + timelock = TimelockController(payable(timelockAddress)); + require(timelockAddress.code.length > 0, "Timelock not deployed"); + } + + /// @notice schedules, waits for timelock and executes a timelocked call + /// @param target The address to call + /// @param data The data to call with + function performTimelocked(address target, bytes memory data) internal { + uint256 minDelay = timelock.getMinDelay(); + vm.prank(admin); + timelock.schedule( + target, + 0, // value + data, + bytes32(0), // predecessor: Non Zero if order must be respected + bytes32(keccak256("SALT")), // salt + minDelay + ); + vm.warp(block.timestamp + minDelay + 1); + vm.prank(executor); + timelock.execute( + target, + 0, // value + data, + bytes32(0), // predecessor: Non Zero if order must be respected + bytes32(keccak256("SALT")) // salt + ); + } + + /// @notice schedules, waits for timelock and executes a timelocked call, with a custom salt + /// @dev This is to be used if we want to call the same target with the same data multiple times + /// @param target The address to call + /// @param data The data to call with + /// @param salt The salt to use for the timelock + function performTimelocked(address target, bytes memory data, bytes32 salt) internal { + uint256 minDelay = timelock.getMinDelay(); + vm.prank(admin); + timelock.schedule( + target, + 0, // value + data, + bytes32(0), // predecessor: Non Zero if order must be respected + bytes32(salt), // salt + minDelay + ); + vm.warp(block.timestamp + minDelay + 1); + vm.prank(executor); + timelock.execute( + target, + 0, // value + data, + bytes32(0), // predecessor: Non Zero if order must be respected + bytes32(salt) // salt + ); + } + + /// @notice schedules a timelocked call + /// @param target The address to call + /// @param data The data to call with + function schedule(address target, bytes memory data) internal { + uint256 minDelay = timelock.getMinDelay(); + vm.prank(admin); + timelock.schedule( + target, + 0, // value + data, + bytes32(0), // predecessor: Non Zero if order must be respected + bytes32(keccak256("SALT")), // salt + minDelay + ); + } + + /// @notice waits for timelock (minDelay) + /// @dev This is to be called after schedule() + /// If the scheduled time > minDelay, this wait time won't be enough + /// and the test will revert + function waitForTimelock() internal { + uint256 minDelay = timelock.getMinDelay(); + vm.warp(block.timestamp + minDelay + 1); + } + + /// @notice executes a timelocked call + /// @param target The address to call + /// @param data The data to call with + function executeTimelocked(address target, bytes memory data) internal { + vm.prank(executor); + timelock.execute( + target, + 0, // value + data, + bytes32(0), // predecessor: Non Zero if order must be respected + bytes32(keccak256("SALT")) // salt + ); + } + + /// @notice schedules, waits for timelock and executes a timelocked call that is expected to revert + /// @param target The address to call + /// @param data The data to call with + /// @param reason The expected revert reason + function expectRevertTimelocked(address target, bytes memory data, string memory reason) internal { + uint256 minDelay = timelock.getMinDelay(); + vm.prank(admin); + timelock.schedule( + target, + 0, // value + data, + bytes32(0), // predecessor: Non Zero if order must be respected + bytes32(keccak256("SALT")), // salt + minDelay + ); + vm.warp(block.timestamp + minDelay + 1); + vm.prank(executor); + vm.expectRevert(bytes(reason)); + timelock.execute( + target, + 0, // value + data, + bytes32(0), // predecessor: Non Zero if order must be respected + bytes32(keccak256("SALT")) // salt + ); + // Cancel the scheduled call to clean the hash in the timelock + bytes32 id = timelock.hashOperation(target, 0, data, bytes32(0), bytes32(keccak256("SALT"))); + vm.prank(admin); + timelock.cancel(id); } }