-
Notifications
You must be signed in to change notification settings - Fork 1
ops(script): rehearse the timelock before governance moves to it #306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
093ec80
ops(script): rehearse the timelock before governance moves to it
6e89b30
refactor(script): split the rehearsal into per-stage scripts
345818f
fix(script): refuse a privileged executor key BEFORE broadcasting
thedavidmeister 69c9619
test(rehearsal): guard the spent state, pin value and the one-shot salt
thedavidmeister File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| // SPDX-License-Identifier: LicenseRef-DCL-1.0 | ||
| // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd | ||
| pragma solidity =0.8.25; | ||
|
|
||
| import {Script} from "forge-std-1.16.1/src/Script.sol"; | ||
| import {console2} from "forge-std-1.16.1/src/console2.sol"; | ||
| import {IAccessControl} from "@openzeppelin-contracts-5.6.1/access/IAccessControl.sol"; | ||
| import {TimelockController} from "@openzeppelin-contracts-5.6.1/governance/TimelockController.sol"; | ||
|
|
||
| import {LibSafeInvariants} from "../src/lib/LibSafeInvariants.sol"; | ||
| import {LibTimelockInvariants} from "../src/lib/LibTimelockInvariants.sol"; | ||
|
|
||
| /// @notice The active chain's governance-timelock pin is unhydrated. | ||
| /// @param chainId The active chain id. | ||
| error ExecuteTimelockNotPinned(uint256 chainId); | ||
|
|
||
| /// @notice The operation is not in a state this script can execute: it is | ||
| /// unknown, already done, or still waiting out its delay. | ||
| /// @param id The operation id. | ||
| /// @param pending Whether the timelock reports it pending. | ||
| /// @param ready Whether the timelock reports it ready. | ||
| /// @param done Whether the timelock reports it done. | ||
| error OperationNotExecutable(bytes32 id, bool pending, bool ready, bool done); | ||
|
|
||
| /// @notice Execution is not open on this timelock, so the CI deploy key — | ||
| /// which holds no roles — cannot execute. Surfaced by name because the whole | ||
| /// point of this script is that it needs no privilege. | ||
| /// @param timelock The timelock inspected. | ||
| error ExecutionNotPermissionless(address timelock); | ||
|
|
||
| /// @title ExecuteTimelockOperations | ||
| /// @notice **PENDING.** Executes a matured timelock operation from the CI | ||
| /// deploy key. | ||
| /// | ||
| /// This is deliberately NOT a Safe-routed script. The timelock grants | ||
| /// `EXECUTOR_ROLE` to `address(0)`, so once an operation's delay has run | ||
| /// ANYONE may execute it. Driving execution from the CI deploy key — a key | ||
| /// that holds no role on the timelock, the authoriser or any vault — is the | ||
| /// most direct demonstration that the property is real: if this succeeds, | ||
| /// execution is genuinely permissionless and the operator cannot censor a | ||
| /// matured operation. | ||
| /// | ||
| /// Dispatch via `Actions → manual-broadcast` with | ||
| /// `script = 20260813-execute-timelock-operations` and the target `network`. | ||
| /// | ||
| /// ## Which operation | ||
| /// | ||
| /// OZ's `TimelockController` stores only a timestamp per operation id; it | ||
| /// keeps no enumerable list, so "every outstanding proposal" cannot be read | ||
| /// from contract state alone — recovering it would mean indexing | ||
| /// `CallScheduled` logs. Rather than pretend otherwise, this script executes | ||
| /// operations it can RECONSTRUCT, and asserts their state before acting. | ||
| /// Today that is the rehearsal no-op from | ||
| /// `20260813-timelock-rehearsal`, whose parameters are fixed. A future | ||
| /// operation is added by appending its reconstruction here, which also keeps | ||
| /// the executor honest: it can only ever run something whose full calldata is | ||
| /// committed in this repo and therefore reviewable. | ||
| /// | ||
| /// @dev Pre-flight asserts the timelock's pinned configuration AND that | ||
| /// execution is open, so a timelock whose executor role had been closed | ||
| /// fails by name rather than as an opaque `AccessControl` revert. | ||
| contract ExecuteTimelockOperations is Script { | ||
| /// @notice Salt of the rehearsal operation. Must match | ||
| /// `TimelockRehearsal.REHEARSAL_SALT`; the rehearsal test asserts the two | ||
| /// derive the same id so they cannot drift apart silently. | ||
| bytes32 internal constant REHEARSAL_SALT = keccak256("st0x.timelock.rehearsal.20260813"); | ||
|
|
||
| /// @notice The rehearsal's no-op payload: re-set the minimum delay to the | ||
| /// value it already holds. | ||
| /// @return The `updateDelay` calldata. | ||
| function rehearsalPayload() internal pure returns (bytes memory) { | ||
| return abi.encodeCall(TimelockController.updateDelay, (LibTimelockInvariants.TIMELOCK_MIN_DELAY)); | ||
| } | ||
|
|
||
| /// @notice Execute the rehearsal operation on the active chain if it has | ||
| /// matured. Broadcasts from the CI deploy key, which holds no roles. | ||
| function run() external { | ||
| address safe = LibSafeInvariants.assertActiveChainTokenOwnerSafe(block.chainid); | ||
| address timelock = LibTimelockInvariants.timelockForChainId(block.chainid); | ||
| if (timelock == address(0)) revert ExecuteTimelockNotPinned(block.chainid); | ||
| LibTimelockInvariants.assertTimelockState(timelock, safe); | ||
|
|
||
| // The property this script depends on, asserted rather than assumed. | ||
| if (!IAccessControl(timelock).hasRole(LibTimelockInvariants.TIMELOCK_EXECUTOR_ROLE, address(0))) { | ||
| revert ExecutionNotPermissionless(timelock); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| TimelockController controller = TimelockController(payable(timelock)); | ||
| bytes memory payload = rehearsalPayload(); | ||
| bytes32 id = controller.hashOperation(timelock, 0, payload, bytes32(0), REHEARSAL_SALT); | ||
|
|
||
| bool pending = controller.isOperationPending(id); | ||
| bool ready = controller.isOperationReady(id); | ||
| bool done = controller.isOperationDone(id); | ||
| if (!ready) revert OperationNotExecutable(id, pending, ready, done); | ||
|
|
||
| console2.log("Executing matured operation:", vm.toString(id)); | ||
| console2.log("Timelock:", vm.toString(timelock)); | ||
| console2.log("Chain:", block.chainid); | ||
|
|
||
| vm.startBroadcast(); | ||
| address executor = msg.sender; | ||
| controller.execute(timelock, 0, payload, bytes32(0), REHEARSAL_SALT); | ||
| vm.stopBroadcast(); | ||
|
|
||
| require(controller.isOperationDone(id), "ExecuteTimelockOperations: operation did not complete"); | ||
|
|
||
| // The executing key holds no role — that is the point. | ||
| require( | ||
| !IAccessControl(timelock).hasRole(LibTimelockInvariants.TIMELOCK_EXECUTOR_ROLE, executor), | ||
| "ExecuteTimelockOperations: executor unexpectedly holds EXECUTOR_ROLE" | ||
| ); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| console2.log("Executed by:", vm.toString(executor)); | ||
| console2.log("That address holds no EXECUTOR_ROLE - execution is permissionless"); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use
afterwardin American English documentation.Replace
afterwardson Line 157 withafterward.🧰 Tools
🪛 LanguageTool
[locale-violation] ~157-~157: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...eration id becomes schedulable again afterwards. 3.
reschedule()— schedule it again....(AFTERWARDS_US)
🤖 Prompt for AI Agents
Source: Linters/SAST tools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not applying this.
afterwardsis the spelling this repo already uses everywhere it uses the word at all, and three of the four occurrences predate this branch onmain:test/src/concrete/StoxReceiptVaultMigrationIntegrationTest.t.sol:833test/script/20260729-deploy-governance-timelock.t.sol:22test/script/DeployGovernanceTimelockHarness.sol:22afterwardappears zero times, and no American-English convention is stated inCLAUDE.md,README.mdor anything underdocs/. Changing this one line would make the repo inconsistent with itself in order to satisfy a LanguageTool locale default that is not a project rule.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Length of output: 789
@thedavidmeister, repository verification failed because the repository clone was unavailable. I cannot independently confirm the spelling convention or the absence of a language rule from this run.Please retry the repository inspection. The finding remains open until verification succeeds.
You are interacting with an AI system.