// SPDX-License-Identifier: MIT pragma solidity ^0.8.28; import {ERC1967Factory} from "solady/utils/ERC1967Factory.sol"; /// @title UpgradeTimelock — the bound that makes an upgradeable registry safe to trust. /// /// THE HOLE THIS CLOSES. `AgentRegistry.ACTIVATION_DELAY` is three days, and it /// exists because an adapter is an ERC-7579 EXECUTOR: whitelisting a hostile one /// puts an unbounded spender on every account, so the delay makes it visible /// before it takes effect. /// /// The registry lives behind an ERC1967 proxy, and the proxy's admin was the /// GUARDIAN — the same key the delay bounds. So the guardian never had to /// whitelist anything. It replaced the code behind the registry, and /// `AdapterInstaller` — which trusts the registry precisely because the caller /// cannot choose — installed whatever the new implementation returned. Same /// block, nothing to observe in advance. `test/UpgradeBypassesTheDelay.t.sol` /// proves it against real contracts. /// /// `ADAPTERS_AT_GENESIS.md` §5b called it in advance: "An upgradeable contract /// with an unbounded admin trades one unshippable property for another." /// /// THE RULE. An upgrade is proposed, waits, and only then happens. Two keys are /// involved and neither is the other: the OWNER here is the upgrade key, and it /// is not the guardian that curates day to day. Either can stop the worst thing /// the other can do — the guardian cannot upgrade, and it can CANCEL an upgrade /// it did not expect, which is what makes a compromised upgrade key survivable /// for as long as the window lasts. /// /// ADMIN ROTATION IS TIMELOCKED TOO, and that is not an extra: without it the /// owner changes the admin to an ordinary account and upgrades from there in the /// next transaction. A timelock with an untimelocked escape hatch is decoration. contract UpgradeTimelock { /// The factory this administers. Immutable, and this contract is NOT behind /// a proxy: the thing that bounds upgrades must not itself be upgradeable, /// or the bound is worth exactly as much as the key that can replace it. ERC1967Factory public immutable factory; /// The upgrade key. Distinct from the guardian by construction below. address public owner; /// Veto only. It cannot upgrade and cannot schedule. address public guardian; /// Longer than the registry's own three days, because this power is strictly /// larger: an implementation swap can do everything a whitelist entry can /// and more. A constant rather than an argument, for the same reason /// ACTIVATION_DELAY is one — a configurable delay is one a compromised key /// sets to zero. uint64 public constant DELAY = 7 days; struct Pending { address target; // an implementation, or an admin uint64 readyAt; } mapping(address proxy => Pending) public pendingUpgrade; mapping(address proxy => Pending) public pendingAdmin; Pending public pendingGuardian; /// The guardian's replacement for the UPGRADE key, on the same terms. /// /// M-4 OF THE 2026-09-10 AUDIT: there was no owner-eviction path at all. A /// compromised upgrade key could never be REMOVED, only vetoed — and every /// veto is one cheap transaction the attacker can outlast, announcing again /// and again until the guardian misses one. The delay bounded how fast the /// damage could happen and nothing bounded how long the key stayed. /// /// Symmetric with `pendingGuardian` on purpose: each key can start the /// other's replacement, on the same DELAY, and the other can veto it ONCE. Pending public pendingOwner; /// Whether a key has already spent its one veto of its OWN replacement. /// /// THE OTHER HALF OF M-4. `cancelGuardian` accepted `owner || guardian`, so /// the guardian could veto its own replacement — indefinitely, one cheap /// transaction at a time. A compromised guardian was therefore permanent /// and could block every registry upgrade forever, which is the mirror /// image of the missing owner eviction. /// /// Removing the veto entirely is not the answer either: `scheduleGuardian` /// is precisely how a compromised OWNER would remove the veto that bounds /// it, so the guardian must be able to stop one. So it gets exactly one — /// enough to raise an alarm and for people to intervene, and not enough to /// filibuster. Cleared when the key actually changes, so the next holder /// starts with its own. bool public guardianVetoSpent; bool public ownerVetoSpent; event UpgradeScheduled(address indexed proxy, address indexed impl, uint64 readyAt); event UpgradeExecuted(address indexed proxy, address indexed impl); event UpgradeCancelled(address indexed proxy, address indexed impl, address by); event AdminScheduled(address indexed proxy, address indexed admin, uint64 readyAt); event AdminExecuted(address indexed proxy, address indexed admin); event AdminCancelled(address indexed proxy, address indexed admin, address by); event GuardianScheduled(address indexed guardian, uint64 readyAt); event GuardianExecuted(address indexed from, address indexed to); event GuardianCancelled(address indexed guardian, address by); event OwnerChanged(address indexed from, address indexed to); event OwnerScheduled(address indexed owner, uint64 readyAt); event OwnerCancelled(address indexed owner, address by); error NotOwner(); error NotOwnerOrGuardian(); error NotGuardian(); /// This key has already used its one veto of its own replacement. See /// `guardianVetoSpent`. error VetoAlreadySpent(); error NothingScheduled(address proxy); error NotReady(address proxy, uint64 readyAt); error Wrong(address proxy); error SameKey(); error ZeroAddress(); /// A rotation of the upgrade key while its own eviction is pending. See /// `transferOwnership` — C-1 of the 2026-09-10 round-2 audit. error EvictionPending(); modifier onlyOwner() { if (msg.sender != owner) revert NotOwner(); _; } /// @param factory_ the ERC1967Factory holding the proxies /// @param owner_ the upgrade key /// @param guardian_ the curator, which may veto but never upgrade constructor(ERC1967Factory factory_, address owner_, address guardian_) { if (address(factory_) == address(0) || owner_ == address(0) || guardian_ == address(0)) { revert ZeroAddress(); } // The whole point. One key holding both roles is the arrangement this // exists to end, and it is cheaper to refuse it here than to discover // it in a deploy script a year from now. if (owner_ == guardian_) revert SameKey(); factory = factory_; owner = owner_; guardian = guardian_; } // --- upgrades ---------------------------------------------------------- /// Announce an upgrade. It cannot take effect for DELAY. /// /// Scheduling over an existing proposal replaces it and RESTARTS the clock, /// so the window always covers the implementation that will actually land — /// otherwise a benign proposal could be swapped for a hostile one on its /// last day. function schedule(address proxy, address impl) external onlyOwner { if (impl == address(0)) revert ZeroAddress(); uint64 ready = uint64(block.timestamp) + DELAY; pendingUpgrade[proxy] = Pending({target: impl, readyAt: ready}); emit UpgradeScheduled(proxy, impl, ready); } /// Perform an announced upgrade, once its window has passed. /// /// The implementation is named again by the caller and checked against what /// was scheduled. It is redundant against this contract's own storage and it /// is deliberate: it makes the transaction that lands the upgrade say which /// code it lands, so the act is legible in a block explorer without reading /// storage. function execute(address proxy, address impl) external onlyOwner { Pending memory p = pendingUpgrade[proxy]; if (p.readyAt == 0) revert NothingScheduled(proxy); if (p.target != impl) revert Wrong(proxy); if (block.timestamp < p.readyAt) revert NotReady(proxy, p.readyAt); delete pendingUpgrade[proxy]; factory.upgrade(proxy, impl); emit UpgradeExecuted(proxy, impl); } /// Stop an announced upgrade. EITHER key may. /// /// This is what the window is for. A compromised upgrade key can announce /// anything; it cannot make it happen for a week, and the guardian can end /// it in one transaction — without being able to upgrade anything itself. /// /// UNLIMITED, AND DELIBERATELY ASYMMETRIC WITH `cancelOwner`/`cancelGuardian`. /// Round 2 of the audit asked why the KEY vetoes are one-shot and these are /// not, so: those two bound a rotation of the key that is casting the veto, /// where an unlimited veto is a key that can never be removed. This one /// bounds an upgrade, where a one-shot veto is strictly worse than none — /// a compromised owner would simply `schedule` twice, spend the guardian's /// single cancel on a decoy, and land the real implementation on the second /// pass. So the guardian may cancel here as often as the owner proposes. /// /// THE COST IS A FREEZE, AND IT IS BOUNDED. A compromised GUARDIAN can stall /// every upgrade and every admin rotation for as long as it holds the key — /// but the owner evicts it in `DELAY` plus one absorbed `cancelGuardian`, /// because THAT veto is the one-shot kind. Bounded by the rotation, which is /// why the asymmetry is the right way round. function cancel(address proxy) external { if (msg.sender != owner && msg.sender != guardian) revert NotOwnerOrGuardian(); Pending memory p = pendingUpgrade[proxy]; if (p.readyAt == 0) revert NothingScheduled(proxy); delete pendingUpgrade[proxy]; emit UpgradeCancelled(proxy, p.target, msg.sender); } // --- admin rotation, on the same terms --------------------------------- // // Timelocked for the reason the whole contract exists: an owner who could // hand the proxy to an ordinary account in one transaction would upgrade // from there in the next, and the delay above would bound nothing. function scheduleAdmin(address proxy, address admin) external onlyOwner { if (admin == address(0)) revert ZeroAddress(); uint64 ready = uint64(block.timestamp) + DELAY; pendingAdmin[proxy] = Pending({target: admin, readyAt: ready}); emit AdminScheduled(proxy, admin, ready); } function executeAdmin(address proxy, address admin) external onlyOwner { Pending memory p = pendingAdmin[proxy]; if (p.readyAt == 0) revert NothingScheduled(proxy); if (p.target != admin) revert Wrong(proxy); if (block.timestamp < p.readyAt) revert NotReady(proxy, p.readyAt); delete pendingAdmin[proxy]; factory.changeAdmin(proxy, admin); emit AdminExecuted(proxy, admin); } function cancelAdmin(address proxy) external { if (msg.sender != owner && msg.sender != guardian) revert NotOwnerOrGuardian(); Pending memory p = pendingAdmin[proxy]; if (p.readyAt == 0) revert NothingScheduled(proxy); delete pendingAdmin[proxy]; emit AdminCancelled(proxy, p.target, msg.sender); } // --- keys --------------------------------------------------------------- /// Hand the upgrade key on. NOT timelocked for the key itself. /// /// C-1 OF THE ROUND-2 AUDIT: THIS WAS AN UNLIMITED VETO OF OWNER EVICTION. /// /// It used to clear `pendingOwner` and reset `ownerVetoSpent`, and it is /// `onlyOwner`. So a compromised upgrade key answered the guardian's /// `scheduleOwner` by rotating to another key it also held — or, since /// `to == owner` was accepted, to ITSELF — which zeroed the eviction and /// refunded the veto that was never spent. Thirty thousand gas, every seven /// days, forever. M-4's eviction path existed and could never land, and /// from a permanent owner the rest follows: schedule the guardian out, /// absorb its single veto, hold both seats, and upgrade the registry into /// an implementation that whitelists an executor onto every account with no /// ACTIVATION_DELAY at all. /// /// So the three rules below, and each is load-bearing: /// /// - `pendingOwner` SURVIVES. A rotation the compromised key chooses is /// not a change of holder, and the guardian's eviction is not moot /// merely because the key answered it. /// - `ownerVetoSpent` IS NEVER RESET HERE. It is cleared in /// `executeOwner` only, exactly mirroring `guardianVetoSpent` in /// `executeGuardian`. A veto is refunded when the key actually leaves, /// and this function is not proof that it did. /// - AN EVICTION IN FLIGHT REFUSES THE ROTATION OUTRIGHT. Letting the two /// race would leave `executeOwner` naming a target the guardian chose /// for an owner that is no longer there. An honest owner that wants to /// rotate mid-eviction spends its one veto first, which is the cost the /// design already charges for standing in the way. /// /// WHAT IT STILL CLEARS. `pendingGuardian` goes: a half-rotated guardian /// must not land under an owner that never scheduled it, and it is the /// departing owner's OWN proposal, so dropping it takes nothing from /// anybody else. Per-proxy `pendingUpgrade` and `pendingAdmin` do NOT go: /// they are keyed by proxy, there is no enumeration of them here, and a /// loop over an unbounded set is not something a key rotation may depend /// on. The new owner inherits those and must `cancel` any it did not want — /// which it can, and which the guardian can too. function transferOwnership(address to) external onlyOwner { if (to == address(0)) revert ZeroAddress(); if (to == guardian) revert SameKey(); // A rotation to the incumbent is a no-op that used to buy a veto. if (to == owner) revert SameKey(); if (pendingOwner.readyAt != 0) revert EvictionPending(); pendingGuardian = Pending({target: address(0), readyAt: 0}); emit OwnerChanged(owner, to); owner = to; } /// Propose a new veto key. Same DELAY as admin rotation: an instant /// setGuardian is the escape hatch that makes dual-control decoration. /// The *current* guardian may cancel until it lands. function scheduleGuardian(address to) external onlyOwner { if (to == address(0)) revert ZeroAddress(); if (to == owner) revert SameKey(); uint64 readyAt_ = uint64(block.timestamp) + DELAY; pendingGuardian = Pending({target: to, readyAt: readyAt_}); emit GuardianScheduled(to, readyAt_); } function executeGuardian(address to) external onlyOwner { Pending memory p = pendingGuardian; if (p.readyAt == 0) revert NothingScheduled(address(0)); if (p.target != to) revert Wrong(address(0)); if (block.timestamp < p.readyAt) revert NotReady(address(0), p.readyAt); // RE-CHECKED AT EXECUTION, not only at `scheduleGuardian`. The owner can // change between the two — the guardian's own `executeOwner` is one way // — and its mirror at `executeOwner` already re-checks for exactly that // reason. Without it: guardian schedules X into the owner seat, owner // schedules X into the guardian seat, both land, and one key holds both // roles. The constructor's `SameKey` invariant and the deploy's // `_refuseSharedRoles` are then both defeated, and `cancelGuardian`'s // `msg.sender == guardian && msg.sender != owner` branch stops firing, // so that key regains unlimited self-vetoes. if (to == owner) revert SameKey(); address from = guardian; delete pendingGuardian; // A NEW HOLDER STARTS WITH ITS OWN VETO. The flag records that THAT // key filibustered, and that key is gone. guardianVetoSpent = false; guardian = to; emit GuardianExecuted(from, to); } /// ONE VETO, AND THEN THE OWNER WINS. See `guardianVetoSpent` for why it is /// neither unlimited (a compromised guardian would be permanent) nor absent /// (a compromised owner would remove the veto in one transaction). function cancelGuardian() external { if (msg.sender != owner && msg.sender != guardian) revert NotOwnerOrGuardian(); if (msg.sender == guardian && msg.sender != owner) { if (guardianVetoSpent) revert VetoAlreadySpent(); guardianVetoSpent = true; } Pending memory p = pendingGuardian; if (p.readyAt == 0) revert NothingScheduled(address(0)); delete pendingGuardian; emit GuardianCancelled(p.target, msg.sender); } // --- evicting the upgrade key ------------------------------------------ // // The mirror of the three calls above, driven by the GUARDIAN. Same DELAY, // same shape, and the owner gets the same single veto — because the two // failures are the same failure seen from either end: a key that can only // be vetoed and never removed is a key that stays. function scheduleOwner(address to) external { if (msg.sender != guardian) revert NotGuardian(); if (to == address(0)) revert ZeroAddress(); if (to == guardian) revert SameKey(); uint64 readyAt_ = uint64(block.timestamp) + DELAY; pendingOwner = Pending({target: to, readyAt: readyAt_}); emit OwnerScheduled(to, readyAt_); } function executeOwner(address to) external { if (msg.sender != guardian) revert NotGuardian(); Pending memory p = pendingOwner; if (p.readyAt == 0) revert NothingScheduled(address(0)); if (p.target != to) revert Wrong(address(0)); if (block.timestamp < p.readyAt) revert NotReady(address(0), p.readyAt); if (to == guardian) revert SameKey(); address from = owner; delete pendingOwner; // A NEW HOLDER STARTS WITH ITS OWN VETO. The flag records that THIS // key filibustered, and this key is gone. ownerVetoSpent = false; owner = to; emit OwnerChanged(from, to); } function cancelOwner() external { if (msg.sender != owner && msg.sender != guardian) revert NotOwnerOrGuardian(); if (msg.sender == owner && msg.sender != guardian) { if (ownerVetoSpent) revert VetoAlreadySpent(); ownerVetoSpent = true; } Pending memory p = pendingOwner; if (p.readyAt == 0) revert NothingScheduled(address(0)); delete pendingOwner; emit OwnerCancelled(p.target, msg.sender); } /// When an announced upgrade becomes possible. Zero means nothing is /// announced. function readyAt(address proxy) external view returns (uint64) { return pendingUpgrade[proxy].readyAt; } }