From b2209cff627191be95163346123dd50e4d2acc4e Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Tue, 10 Dec 2024 11:29:30 +0100 Subject: [PATCH] Timelock for the Bridge The goal is to deploy a Timelock between the Bridge ProxyAdmin and the Threshold Council multisig to enforce 24h delay between upgrades. This changeset adds the deployment script and a basic integration test simulation Bridge proxy upgrade. The goal is not to test OpenZeppelin implementation but to prove the integration works and present how the upgrade transaction should be assembled. --- solidity/contracts/Timelock.sol | 12 ++++ solidity/deploy/42_deploy_timelock.ts | 50 +++++++++++++++ solidity/test/Timelock.test.ts | 89 +++++++++++++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 solidity/contracts/Timelock.sol create mode 100644 solidity/deploy/42_deploy_timelock.ts create mode 100644 solidity/test/Timelock.test.ts diff --git a/solidity/contracts/Timelock.sol b/solidity/contracts/Timelock.sol new file mode 100644 index 000000000..76c2f5937 --- /dev/null +++ b/solidity/contracts/Timelock.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity 0.8.17; + +import "@openzeppelin/contracts/governance/TimelockController.sol"; + +contract Timelock is TimelockController { + constructor( + uint256 minDelay, + address[] memory proposers, + address[] memory executors + ) TimelockController(minDelay, proposers, executors, address(0)) {} +} diff --git a/solidity/deploy/42_deploy_timelock.ts b/solidity/deploy/42_deploy_timelock.ts new file mode 100644 index 000000000..53630c96a --- /dev/null +++ b/solidity/deploy/42_deploy_timelock.ts @@ -0,0 +1,50 @@ +import { HardhatRuntimeEnvironment } from "hardhat/types" +import { DeployFunction } from "hardhat-deploy/types" + +const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { + const { deployments, helpers, getNamedAccounts } = hre + const { deploy } = deployments + const { deployer, governance } = await getNamedAccounts() + + const timelock = await deploy("Timelock", { + from: deployer, + args: [ + 86400, // 24h governance delay + [governance], // Threshold Council multisig as a proposer + // All current signers from the Threshold Council multisig as executors + // plus the Threshold Council multisig itself. The last one is here in + // case Threshold Council multisig rotates the owners but forgets to + // update the Timelock contract. + // See https://app.safe.global/settings/setup?safe=eth:0x9F6e831c8F8939DC0C830C6e492e7cEf4f9C2F5f + [ + "0x2844a0d6442034D3027A05635F4224d966C54fD7", + "0xf35dEE924F483Bc234F09cbfbc8B4488fD06be20", + "0x739730cCb2a34cc83D3e30645002C52bA4B06167", + "0xe989805835093e37E6b12dCddF718e0481024573", + "0x1Ba899530A89fAb245De9ff6cc23534F4a8A4e58", + "0x75ed7b219a737134f00255e331a36a706BD2ae2C", + "0xcE3778528fC73D46685069D455bbCcE16A6e22Af", + "0x35B46702C5d1CD36194217Fb92F72B563eFf851A", + "0xf791EfdF778a3Ca9cc193fFbe57Da33d1596E854", + governance, + ], + ], + log: true, + waitConfirmations: 1, + }) + + if (hre.network.tags.etherscan) { + await helpers.etherscan.verify(timelock) + } + + if (hre.network.tags.tenderly) { + await hre.tenderly.verify({ + name: "Timelock", + address: timelock.address, + }) + } +} + +export default func + +func.tags = ["Timelock"] diff --git a/solidity/test/Timelock.test.ts b/solidity/test/Timelock.test.ts new file mode 100644 index 000000000..98b146327 --- /dev/null +++ b/solidity/test/Timelock.test.ts @@ -0,0 +1,89 @@ +import { helpers, waffle, upgrades } from "hardhat" +import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers" +import { expect } from "chai" +import type { Bridge, TBTCVault, Timelock, ProxyAdmin } from "../typechain" + +import bridgeFixture from "./fixtures/bridge" + +const { createSnapshot, restoreSnapshot } = helpers.snapshot + +describe("Timelock", () => { + let governance: SignerWithAddress + let governanceSigner: SignerWithAddress + + let bridge: Bridge + let tbtcVault: TBTCVault + let timelock: Timelock + let proxyAdmin: ProxyAdmin + + const zeroBytes32 = + "0x0000000000000000000000000000000000000000000000000000000000000000" + const timelockDelay = 86400 // 24h governance delay + + before(async () => { + const { esdm } = await helpers.signers.getNamedSigners() + // eslint-disable-next-line @typescript-eslint/no-extra-semi + ;({ governance, bridge, tbtcVault } = await waffle.loadFixture( + bridgeFixture + )) + + // One of the Threshold Council signers + governanceSigner = await helpers.account.impersonateAccount( + "0x2844a0d6442034D3027A05635F4224d966C54fD7", + { + from: governance, + value: 10, + } + ) + + timelock = (await helpers.contracts.getContract("Timelock")) as Timelock + proxyAdmin = (await upgrades.admin.getInstance()) as ProxyAdmin + + await proxyAdmin.connect(esdm).transferOwnership(timelock.address) + }) + + context("when upgrading Bridge implementation via Timelock", async () => { + let expectedNewImplementation: string + + before(async () => { + await createSnapshot() + + // We need an existing contract. Otherwise, ProxyAdmin.upgrade will + // revert. Obviously, in a real world, it does not make sense to upgrade + // Bridge implementation address to point to the vault contract but we + // just want to confirm switching the implementation address works. + expectedNewImplementation = tbtcVault.address + + const upgradeTxData = await proxyAdmin.interface.encodeFunctionData( + "upgrade", + [bridge.address, expectedNewImplementation] + ) + + await timelock + .connect(governance) + .schedule( + proxyAdmin.address, + 0, + upgradeTxData, + zeroBytes32, + zeroBytes32, + timelockDelay + ) + await helpers.time.increaseTime(timelockDelay) + await timelock + .connect(governanceSigner) + .execute(proxyAdmin.address, 0, upgradeTxData, zeroBytes32, zeroBytes32) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should switch the implementation address", async () => { + const newImplementation = await upgrades.erc1967.getImplementationAddress( + bridge.address + ) + expect(newImplementation).to.equal(expectedNewImplementation) + }) + }) +})