From 9df53250dea7149ced222b50326faaa212ab8b6d Mon Sep 17 00:00:00 2001 From: Josh Hardy Date: Thu, 28 May 2026 15:20:45 +0000 Subject: [PATCH 1/2] feat(safe): migrate prod beacon ownership from EOA to token-owner Safe Transfers ownership of the three live-token V1 beacons (receipt, receipt vault, wrapped token vault) from the rainlang.eth EOA to the ST0x token-owner Safe so beacon upgrades route through the multisig. Adds LibSafeInvariants.assertBeaconInvariants for generic beacon pre/post state checks (deployed, pinned OZ UpgradeableBeacon codehash, owner, impl) and generalises the n+1 reversibility helper: LibSafeOps.simulateNPlus1 runs the approveHash -> execTransaction mechanics against arbitrary calldata, with simulateNPlus1Reversal delegating to it (behaviour unchanged) and simulateBeaconNPlus1 proving the Safe can act on each beacon via an idempotent upgradeTo. Co-Authored-By: Claude Opus 4.7 --- script/MigrateBeaconOwners.s.sol | 131 +++++++++++++++++++ src/lib/LibSafeInvariants.sol | 114 ++++++++++++++++ src/lib/LibSafeOps.sol | 126 +++++++++++++++--- test/script/MigrateBeaconOwnersHarness.sol | 27 ++++ test/script/MigrateBeaconOwnersTest.t.sol | 145 +++++++++++++++++++++ test/src/lib/LibSafeInvariants.t.sol | 73 ++++++++++- test/src/lib/LibSafeInvariantsHarness.sol | 4 + 7 files changed, 598 insertions(+), 22 deletions(-) create mode 100644 script/MigrateBeaconOwners.s.sol create mode 100644 test/script/MigrateBeaconOwnersHarness.sol create mode 100644 test/script/MigrateBeaconOwnersTest.t.sol diff --git a/script/MigrateBeaconOwners.s.sol b/script/MigrateBeaconOwners.s.sol new file mode 100644 index 00000000..a95f195f --- /dev/null +++ b/script/MigrateBeaconOwners.s.sol @@ -0,0 +1,131 @@ +// 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 {Ownable} from "@openzeppelin-contracts-5.6.1/access/Ownable.sol"; +import {IGnosisSafe} from "../src/interface/IGnosisSafe.sol"; +import {LibProdDeployV1} from "../src/lib/LibProdDeployV1.sol"; +import {LibSafeInvariants} from "../src/lib/LibSafeInvariants.sol"; +import {LibSafeInvariants} from "../src/lib/LibSafeInvariants.sol"; +import {LibSafeOps} from "../src/lib/LibSafeOps.sol"; + +/// @title MigrateBeaconOwners +/// @notice Forge script that transfers ownership of the three production V1 +/// beacons that live ST0x tokens actually use — the receipt beacon, the +/// receipt vault beacon, and the wrapped token vault beacon — from the +/// externally-owned account at `LibProdDeployV1.BEACON_INITIAL_OWNER` +/// (rainlang.eth) to the ST0x token-owner Safe at +/// `LibSafeInvariants.STOX_TOKEN_OWNER_SAFE`. +/// +/// Unlike the threshold migration, this is a direct EOA-broadcast operation, +/// not a Safe-routed transaction: ownership of an `Ownable` beacon transfers +/// by the current owner calling `transferOwnership`, and the current owner is +/// the EOA. The script therefore emits no Tx Builder JSON artifact — the +/// output is the on-chain `transferOwnership` transaction(s) themselves. +/// +/// @dev The flow is the operational-script standard shape adapted for a +/// direct-broadcast op: +/// +/// 1. **Pre-flight** — every beacon is asserted to be in the expected +/// EOA-owned state via `LibSafeInvariants.assertBeaconInvariants` (deployed +/// contract, pinned OZ `UpgradeableBeacon` codehash, EOA owner, pinned +/// current implementation). If any beacon has drifted, the script aborts +/// before broadcasting anything. +/// 2. **Broadcast** — `transferOwnership(STOX_TOKEN_OWNER_SAFE)` is called on +/// each beacon under `vm.startBroadcast()`. Three separate transactions +/// (one per beacon) rather than a batch: simpler, and abortable partway if +/// the first goes sideways. +/// 3. **Post-state** — every beacon is re-asserted, now expecting the Safe as +/// owner and the same (unchanged) implementation. +/// 4. **n+1 reversibility** — for each beacon, `simulateBeaconNPlus1` proves +/// the Safe can act on the beacon post-migration by running an idempotent +/// `upgradeTo(currentImpl)` through the Safe's `execTransaction` (exercising +/// the threshold gate both ways). The n+1 runs as a fork-local simulation; +/// it is not broadcast. +/// +/// Execution mode: +/// ```shell +/// forge script script/MigrateBeaconOwners.s.sol \ +/// --rpc-url base --broadcast --private-key +/// ``` +contract MigrateBeaconOwners is Script { + /// @notice The three V1 beacons whose ownership is migrated. Order is + /// fixed: receipt beacon, receipt vault beacon, wrapped token vault + /// beacon. The parallel `currentImpls()` helper returns each beacon's + /// pinned current implementation in the same order. + /// @return The three beacon addresses to migrate. + function beacons() internal pure returns (address[3] memory) { + return [ + LibProdDeployV1.STOX_RECEIPT_BEACON_V1, + LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1, + LibProdDeployV1.STOX_WRAPPED_TOKEN_VAULT_BEACON_V1 + ]; + } + + /// @notice The pinned current implementation for each beacon, in the same + /// order as `beacons()`. Asserted unchanged across the ownership transfer + /// (the migration changes the owner, never the implementation) and used as + /// the idempotent `upgradeTo` argument in the n+1 check. + /// @return The three implementation addresses, index-aligned with + /// `beacons()`. + function currentImpls() internal pure returns (address[3] memory) { + return [ + LibProdDeployV1.STOX_RECEIPT_IMPLEMENTATION, + LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION, + LibProdDeployV1.STOX_WRAPPED_TOKEN_VAULT_IMPLEMENTATION + ]; + } + + /// @notice Run the beacon-ownership migration: pre-flight every beacon + /// against the EOA-owned state, broadcast the three `transferOwnership` + /// calls, re-assert every beacon against the Safe-owned state, then prove + /// each beacon's n+1 reversibility through the Safe. Broadcasts the + /// transfers; the n+1 checks are fork-local simulations only. + function run() external { + IGnosisSafe safe = IGnosisSafe(LibSafeInvariants.STOX_TOKEN_OWNER_SAFE); + address[3] memory beaconList = beacons(); + address[3] memory implList = currentImpls(); + + // Pre-flight: every beacon is in the expected EOA-owned state. Reverts + // with the relevant typed error from `LibSafeInvariants` on the first + // drift, before any broadcast happens. + for (uint256 i = 0; i < beaconList.length; i++) { + LibSafeInvariants.assertBeaconInvariants( + beaconList[i], LibProdDeployV1.BEACON_INITIAL_OWNER, implList[i] + ); + } + + // Broadcast the ownership transfers from the EOA. Three separate + // transactions — Foundry submits each `transferOwnership` as its own + // tx from the single script run. + vm.startBroadcast(); + for (uint256 i = 0; i < beaconList.length; i++) { + Ownable(beaconList[i]).transferOwnership(LibSafeInvariants.STOX_TOKEN_OWNER_SAFE); + } + vm.stopBroadcast(); + + // Post-state: every beacon is now Safe-owned, implementations + // unchanged. + for (uint256 i = 0; i < beaconList.length; i++) { + LibSafeInvariants.assertBeaconInvariants(beaconList[i], LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, implList[i]); + } + + // n+1 reversibility: prove the Safe can act on each beacon by running + // an idempotent `upgradeTo(currentImpl)` through the Safe's + // `execTransaction`. The threshold gate is exercised both ways + // (undersigned reverts with GS020, full threshold succeeds). Reads the + // live threshold from `LibSafeInvariants` so the check tracks whatever the + // Safe's current threshold is. + for (uint256 i = 0; i < beaconList.length; i++) { + LibSafeOps.simulateBeaconNPlus1( + safe, beaconList[i], implList[i], LibSafeInvariants.STOX_TOKEN_OWNER_SAFE_THRESHOLD + ); + } + + console2.log("Beacon ownership migration pre-flight + post-state + n+1 checks passed."); + console2.log("Transferred ownership of 3 beacons to:", vm.toString(LibSafeInvariants.STOX_TOKEN_OWNER_SAFE)); + } +} diff --git a/src/lib/LibSafeInvariants.sol b/src/lib/LibSafeInvariants.sol index c909fe5a..afefaa02 100644 --- a/src/lib/LibSafeInvariants.sol +++ b/src/lib/LibSafeInvariants.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.25; import {IGnosisSafe} from "../interface/IGnosisSafe.sol"; +import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; +import {IOwnable} from "./LibTokenInvariants.sol"; /// @notice The runtime codehash at the Safe's address does not match the /// pinned Safe v1.4.1 L2 proxy codehash. Signals either that the address has @@ -97,6 +99,52 @@ error SafeOwnerMismatch(address safe, uint256 index, address expectedOwner, addr /// @param actual The threshold returned by `getThreshold()`. error SafeThresholdMismatch(address safe, uint256 expected, uint256 actual); +/// @notice The address supplied as a beacon has no runtime code. Either the +/// beacon was never deployed at this address or it has been +/// `SELFDESTRUCT`-ed. Caught first so later reads against the address are +/// only attempted once it is known to be a contract. +/// @param beacon The address that was expected to be a deployed beacon. +error BeaconNotDeployed(address beacon); + +/// @notice The beacon's runtime codehash does not match the pinned OZ +/// `UpgradeableBeacon` bytecode (`UPGRADEABLE_BEACON_CODEHASH`). +/// Signals either an address swap or a look-alike contract shadowing the +/// `implementation()` / `owner()` selectors. The codehash pin is what lets +/// the access-control behaviour be trusted as OZ's audited bytecode rather +/// than re-tested here. +/// @param beacon The beacon address whose codehash was checked. +/// @param expected The pinned `UpgradeableBeacon` codehash. +/// @param actual The codehash returned by `extcodehash(beacon)`. +error BeaconCodehashMismatch(address beacon, bytes32 expected, bytes32 actual); + +/// @notice The beacon's `owner()` does not match the expected owner. Used +/// both to assert the pre-migration EOA owner and the post-migration Safe +/// owner; the caller supplies which one it expects because the owner is the +/// property the migration deliberately changes. +/// @param beacon The beacon address whose owner was read. +/// @param expected The owner address the caller expected. +/// @param actual The owner address returned by `Ownable(beacon).owner()`. +error BeaconOwnerMismatch(address beacon, address expected, address actual); + +/// @notice The beacon's `implementation()` does not match the expected +/// implementation. The ownership migration must not change any beacon's +/// implementation, so this is asserted equal pre- and post-migration; the +/// upgrade script asserts it against the new implementation after the +/// upgrade. +/// @param beacon The beacon address whose implementation pointer was read. +/// @param expected The implementation address the caller expected. +/// @param actual The implementation address returned by +/// `IBeacon(beacon).implementation()`. +error BeaconImplementationMismatch(address beacon, address expected, address actual); + +/// @notice The beacon's implementation pointer resolves to an address with +/// no runtime code. A beacon pointing at a code-less implementation would +/// brick every proxy that delegates through it, so this is surfaced as an +/// invariant break rather than discovered at the first proxy call. +/// @param beacon The beacon address whose implementation was inspected. +/// @param implementation The implementation address that has no code. +error BeaconImplNotDeployed(address beacon, address implementation); + /// @title LibSafeInvariants /// @notice Reusable invariant assertions for a Safe v1.4.1 L2 multisig /// pinned to the ST0x token-owner deployment. Each public assertion either @@ -449,4 +497,70 @@ library LibSafeInvariants { owners[5] = STOX_TOKEN_OWNER_SAFE_OWNER_6; return owners; } + + /// @notice Assert the invariants of an OpenZeppelin `UpgradeableBeacon` + /// at `beacon`: it is a deployed contract, its runtime codehash matches + /// the pinned OZ `UpgradeableBeacon` bytecode, its `owner()` matches + /// `expectedOwner`, its `implementation()` matches `expectedImpl`, and + /// that implementation is itself deployed. Reverts with a typed error on + /// first failure; returns silently otherwise. + /// @dev Generic over any beacon (V1, V2, or future) so the same helper + /// serves the beacon-ownership migration pre/post-flight and the receipt + /// vault upgrade pre/post-flight. The owner and implementation are + /// caller-supplied because both are properties an operational script + /// deliberately mutates: the ownership migration changes the owner from + /// the EOA to the Safe, and the V3 upgrade changes the implementation. + /// + /// The codehash pin (check #2) is the load-bearing invariant. OZ's + /// `UpgradeableBeacon` ships the access control (`onlyOwner` on + /// `upgradeTo`, `Ownable` transfer/renounce semantics) that this + /// deployment relies on; pinning the bytecode means that behaviour is + /// guaranteed by OZ's audit rather than re-tested in this repo. A + /// beacon whose codehash matches by definition behaves like the OZ + /// beacon, so no behavioural access-control assertions are duplicated + /// here. + /// + /// Check ordering mirrors `assertImmutableInvariants`: code presence + /// first (cheapest, and catches an EOA or empty address), codehash + /// second (catches a look-alike), then the storage-backed reads + /// (`owner()`, `implementation()`) once the bytecode is proven to be the + /// OZ beacon, and the implementation code-presence check last because it + /// depends on the implementation read having succeeded. + /// @notice OpenZeppelin `UpgradeableBeacon` runtime codehash. Pinned + /// here so the beacon-side codehash check has a concrete invariant + /// target; matches the bytecode at every prod beacon deployment. + bytes32 internal constant UPGRADEABLE_BEACON_CODEHASH = + 0x8e95867e52db417944afd90f3b6c3c980962831e8a944e7f6958ba8f8cc10630; + + /// @param beacon The beacon to assert invariants on. + /// @param expectedOwner The owner the beacon is expected to report. + /// @param expectedImpl The implementation the beacon is expected to + /// point at. + function assertBeaconInvariants(address beacon, address expectedOwner, address expectedImpl) internal view { + if (beacon.code.length == 0) { + revert BeaconNotDeployed(beacon); + } + + bytes32 actualCodehash; + assembly ("memory-safe") { + actualCodehash := extcodehash(beacon) + } + if (actualCodehash != UPGRADEABLE_BEACON_CODEHASH) { + revert BeaconCodehashMismatch(beacon, UPGRADEABLE_BEACON_CODEHASH, actualCodehash); + } + + address actualOwner = IOwnable(beacon).owner(); + if (actualOwner != expectedOwner) { + revert BeaconOwnerMismatch(beacon, expectedOwner, actualOwner); + } + + address actualImpl = IBeacon(beacon).implementation(); + if (actualImpl != expectedImpl) { + revert BeaconImplementationMismatch(beacon, expectedImpl, actualImpl); + } + + if (actualImpl.code.length == 0) { + revert BeaconImplNotDeployed(beacon, actualImpl); + } + } } diff --git a/src/lib/LibSafeOps.sol b/src/lib/LibSafeOps.sol index 482552fb..f5a35190 100644 --- a/src/lib/LibSafeOps.sol +++ b/src/lib/LibSafeOps.sol @@ -4,6 +4,19 @@ pragma solidity ^0.8.25; import {Vm} from "forge-std-1.16.1/src/Vm.sol"; import {IGnosisSafe} from "../interface/IGnosisSafe.sol"; +import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; + +/// @notice Minimal `UpgradeableBeacon` surface used by the beacon n+1 +/// helper: the privileged `upgradeTo` mutator. Declared inline so +/// `LibSafeOps` owns the only beacon selector it encodes rather than +/// importing the full OZ `UpgradeableBeacon` type for a single `abi.encodeCall`. +interface IUpgradeableBeacon { + /// @notice Point the beacon at a new implementation. `onlyOwner` on the + /// OZ beacon; reachable here only because the n+1 helper routes the call + /// through the owning Safe's `execTransaction`. + /// @param newImplementation The implementation address to set. + function upgradeTo(address newImplementation) external; +} /// @notice The parsed Tx Builder JSON has zero transactions. Empty bundles /// are never produced by `emitTxBuilderJson` and so a zero-length @@ -320,26 +333,71 @@ library LibSafeOps { /// the number of approvals collected and the number passed in the /// successful `execTransaction` call. function simulateNPlus1Reversal(IGnosisSafe safe, uint256 oldThreshold, uint256 newThreshold) internal { - // Construct the inverse calldata: `changeThreshold(oldThreshold)`. - // The follow-up is by definition a self-call; the inverse threshold - // change is the simplest, most reversible "n+1" transaction we can - // model and it carries no side effects other than the threshold - // mutation itself. - bytes memory revertData = abi.encodeCall(IGnosisSafe.changeThreshold, (oldThreshold)); - SafeTx memory followup = SafeTx({to: address(safe), value: 0, data: revertData, operation: 0}); + // The inverse op is a self-call to `changeThreshold(oldThreshold)`: + // the simplest, most reversible follow-up, with no side effect other + // than the threshold mutation itself. Delegated to the generic + // `simulateNPlus1` so the signature mechanics live in one place; this + // wrapper keeps its original signature and behaviour so the + // threshold-migration tests continue to pass unchanged. + bytes memory inverseCalldata = abi.encodeCall(IGnosisSafe.changeThreshold, (oldThreshold)); + simulateNPlus1(safe, address(safe), inverseCalldata, newThreshold); + require(safe.getThreshold() == oldThreshold, "LibSafeOps: n+1 did not restore the prior threshold"); + } + + /// @notice Generic n+1 reversibility / "not stuck" check. Given a Safe in + /// some post-mutation state, an arbitrary follow-up call (`target` + + /// `inverseCalldata`), and the Safe's current `threshold`, this proves on + /// the active fork that: + /// + /// 1. The Safe accepts a valid `execTransaction` for the follow-up under + /// the current threshold (the positive case), and + /// 2. The threshold gate rejects an undersigned attempt with `GS020` + /// (the negative case). + /// + /// Together these prove the post-mutation state is genuinely exitable: + /// the owning Safe can still author and execute a transaction against + /// `target`, and the signature gate is doing its job. The follow-up call + /// is supplied as raw calldata so the same mechanics serve a Safe + /// self-call (threshold migration: `target == safe`), a beacon upgrade + /// (`target == beacon`, `upgradeTo(...)`), or any other critical state + /// change. + /// + /// As with `simulateNPlus1Reversal`, approvals are sourced via + /// `approveHash` under `vm.prank` rather than ECDSA signatures, so no + /// test private keys are needed; the real `checkSignatures` path is + /// exercised end-to-end. The Safe's nonce IS advanced by the successful + /// `execTransaction` (unlike `simulateSelfCall`), because this models the + /// full wrapping exec. + /// + /// @dev This helper asserts the follow-up executes and the gate rejects + /// undersigned attempts, but does NOT assert anything about the inner + /// call's effect — callers that need a specific post-condition (e.g. the + /// threshold rolled back to a prior value) assert it themselves after + /// this returns. `simulateNPlus1Reversal` is exactly such a caller. + /// @param safe The Safe whose post-mutation state to exercise. + /// @param target The destination of the follow-up call. Pass + /// `address(safe)` for a Safe self-call. + /// @param inverseCalldata The calldata for the follow-up call. + /// @param threshold The Safe's current threshold. Both the number of + /// approvals collected and the number passed in the successful + /// `execTransaction` call. + function simulateNPlus1(IGnosisSafe safe, address target, bytes memory inverseCalldata, uint256 threshold) + internal + { + SafeTx memory followup = SafeTx({to: target, value: 0, data: inverseCalldata, operation: 0}); uint256 followupNonce = safe.nonce(); bytes32 followupHash = computeSafeTxHashViaSafe(safe, followup, followupNonce); - // Collect approvals from `newThreshold` owners. We always take the - // first `newThreshold` entries from `getOwners()` — the linked-list + // Collect approvals from `threshold` owners. We always take the + // first `threshold` entries from `getOwners()` — the linked-list // order is deterministic per-Safe so the choice is stable, and // sorting the resulting array by address normalises against the // arbitrary Safe-internal ordering before passing to // `checkSignatures`. address[] memory owners = safe.getOwners(); - require(owners.length >= newThreshold, "LibSafeOps: not enough owners for n+1"); - address[] memory approvers = new address[](newThreshold); - for (uint256 i = 0; i < newThreshold; i++) { + require(owners.length >= threshold, "LibSafeOps: not enough owners for n+1"); + address[] memory approvers = new address[](threshold); + for (uint256 i = 0; i < threshold; i++) { approvers[i] = owners[i]; VM.prank(owners[i]); safe.approveHash(followupHash); @@ -355,7 +413,7 @@ library LibSafeOps { // gate is doing its job — a follow-up tx is not magically waveable // through just because the approvals exist; the packed blob has to // contain at least `threshold` entries. - bytes memory tooFewSigs = packApprovedHashSignatures(sortedSigners, newThreshold - 1); + bytes memory tooFewSigs = packApprovedHashSignatures(sortedSigners, threshold - 1); VM.expectRevert(bytes("GS020")); // The return value is meaningless under `expectRevert` — the call // must revert with the literal `GS020` reason, and the cheatcode @@ -375,13 +433,11 @@ library LibSafeOps { tooFewSigs ); - // Positive case: full `newThreshold`-many signatures must succeed - // and roll the threshold back to `oldThreshold`. The success - // assertion plus the threshold check together prove that the new - // state is genuinely exitable: the signature path verifies, the - // inner call executes, and the safe is back in a state another - // forward migration could run against. - bytes memory enoughSigs = packApprovedHashSignatures(sortedSigners, newThreshold); + // Positive case: full `threshold`-many signatures must succeed. The + // success assertion proves the new state is genuinely exitable: the + // signature path verifies, the inner call executes, and the owning + // Safe could run another forward migration against `target`. + bytes memory enoughSigs = packApprovedHashSignatures(sortedSigners, threshold); bool ok = safe.execTransaction( followup.to, followup.value, @@ -395,7 +451,35 @@ library LibSafeOps { enoughSigs ); require(ok, "LibSafeOps: n+1 execTransaction reverted unexpectedly"); - require(safe.getThreshold() == oldThreshold, "LibSafeOps: n+1 did not restore the prior threshold"); + } + + /// @notice Beacon-specific n+1 reversibility convenience. Proves the + /// owning Safe can act on `beacon` post-ownership-migration by running an + /// idempotent `upgradeTo(currentImpl)` as the follow-up op: the call + /// routes through the Safe's `execTransaction` (exercising the threshold + /// gate both ways) and re-sets the beacon to the implementation it + /// already points at, so there is no net state change. + /// @dev The idempotent `upgradeTo` is the inverse op recommended in the + /// design plan: it touches no real state (the beacon ends pointing at the + /// same implementation) yet proves the Safe -> beacon call path works + /// end-to-end through the signature-verified exec. Delegates to the + /// generic `simulateNPlus1` with the `upgradeTo(currentImpl)` calldata. + /// @param safe The Safe that owns the beacon after the migration. + /// @param beacon The beacon to exercise. + /// @param currentImpl The beacon's current implementation. Passed as the + /// `upgradeTo` argument so the op is idempotent. + /// @param threshold The Safe's current threshold. + function simulateBeaconNPlus1(IGnosisSafe safe, address beacon, address currentImpl, uint256 threshold) internal { + bytes memory inverseCalldata = abi.encodeCall(IUpgradeableBeacon.upgradeTo, (currentImpl)); + simulateNPlus1(safe, beacon, inverseCalldata, threshold); + // Post-condition: the idempotent upgrade left the beacon pointing at + // the same implementation it started on, confirming the routed call + // actually executed against the beacon (not just that the Safe + // accepted the signatures). + require( + IBeacon(beacon).implementation() == currentImpl, + "LibSafeOps: beacon n+1 did not preserve the implementation" + ); } /// @notice Insertion-sort an in-memory address array ascending. Used to diff --git a/test/script/MigrateBeaconOwnersHarness.sol b/test/script/MigrateBeaconOwnersHarness.sol new file mode 100644 index 00000000..1ddf3f3c --- /dev/null +++ b/test/script/MigrateBeaconOwnersHarness.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {IGnosisSafe} from "../../src/interface/IGnosisSafe.sol"; +import {LibSafeInvariants} from "../../src/lib/LibSafeInvariants.sol"; +import {LibSafeOps} from "../../src/lib/LibSafeOps.sol"; + +/// @title MigrateBeaconOwnersHarness +/// @notice External-call shim around the migration steps so `vm.expectRevert` +/// can intercept the typed errors raised by `LibSafeInvariants`. Library +/// `internal` functions inline into the test and would fail the +/// `expectRevert` depth check otherwise. The harness mirrors the exact +/// sequence `MigrateBeaconOwners.run()` performs, minus the `vm.broadcast` +/// wrapper (the test drives the ownership transfer via `vm.prank(EOA)` to +/// simulate the on-chain broadcast's effect). +contract MigrateBeaconOwnersHarness { + function callAssertBeaconInvariants(address beacon, address expectedOwner, address expectedImpl) external view { + LibSafeInvariants.assertBeaconInvariants(beacon, expectedOwner, expectedImpl); + } + + function callSimulateBeaconNPlus1(IGnosisSafe safe, address beacon, address currentImpl, uint256 threshold) + external + { + LibSafeOps.simulateBeaconNPlus1(safe, beacon, currentImpl, threshold); + } +} diff --git a/test/script/MigrateBeaconOwnersTest.t.sol b/test/script/MigrateBeaconOwnersTest.t.sol new file mode 100644 index 00000000..a4786b5b --- /dev/null +++ b/test/script/MigrateBeaconOwnersTest.t.sol @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.1/src/Test.sol"; +import {Ownable} from "@openzeppelin-contracts-5.6.1/access/Ownable.sol"; +import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; + +import {IGnosisSafe} from "../../src/interface/IGnosisSafe.sol"; +import {LibProdDeployV1} from "../../src/lib/LibProdDeployV1.sol"; +import {LibSafeInvariants, BeaconOwnerMismatch} from "../../src/lib/LibSafeInvariants.sol"; +import {MigrateBeaconOwnersHarness} from "./MigrateBeaconOwnersHarness.sol"; +import {LibRainDeploy} from "rain-deploy-0.1.3/src/lib/LibRainDeploy.sol"; + +/// @title MigrateBeaconOwnersTest +/// @notice End-to-end fork tests for the beacon-ownership migration. Because +/// the migration broadcasts from the rainlang.eth EOA (which owns the +/// beacons), the test simulates the broadcast's effect by pranking the EOA +/// to perform the `transferOwnership` calls, exercising the same pre-flight, +/// post-state, and n+1 reversibility steps the script runs. +/// @dev Uses an unpinned Base head fork (same precedent as +/// `MigrateMultisigThresholdTest`): any drift in the live beacon state +/// surfaces on the next CI run rather than being frozen against a stale +/// snapshot. +contract MigrateBeaconOwnersTest is Test { + /// @notice Live Safe handle, reset per fork. + IGnosisSafe internal safe; + + /// @notice The harness deployed fresh per fork. + MigrateBeaconOwnersHarness internal harness; + + /// @notice The three beacons under migration, in the script's order. + address[3] internal beaconList = [ + LibProdDeployV1.STOX_RECEIPT_BEACON_V1, + LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1, + LibProdDeployV1.STOX_WRAPPED_TOKEN_VAULT_BEACON_V1 + ]; + + /// @notice Each beacon's pinned current implementation, index-aligned with + /// `beaconList`. + address[3] internal implList = [ + LibProdDeployV1.STOX_RECEIPT_IMPLEMENTATION, + LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION, + LibProdDeployV1.STOX_WRAPPED_TOKEN_VAULT_IMPLEMENTATION + ]; + + function selectBaseFork() internal { + vm.createSelectFork(LibRainDeploy.BASE); + safe = IGnosisSafe(LibSafeInvariants.STOX_TOKEN_OWNER_SAFE); + harness = new MigrateBeaconOwnersHarness(); + } + + /// @notice Simulate the migration's on-chain effect: prank the EOA owner + /// and transfer each beacon to the Safe. Models exactly what + /// `MigrateBeaconOwners.run()`'s broadcast block does. + function simulateTransfers() internal { + for (uint256 i = 0; i < beaconList.length; i++) { + vm.prank(LibProdDeployV1.BEACON_INITIAL_OWNER); + Ownable(beaconList[i]).transferOwnership(LibSafeInvariants.STOX_TOKEN_OWNER_SAFE); + } + } + + /// @notice Pre-flight passes against the live EOA-owned state for all + /// three beacons. This is the gate `run()` runs before broadcasting. + function testPreflightPassesAgainstEoaOwnedState() external { + selectBaseFork(); + for (uint256 i = 0; i < beaconList.length; i++) { + // No revert == invariant holds. + harness.callAssertBeaconInvariants(beaconList[i], LibProdDeployV1.BEACON_INITIAL_OWNER, implList[i]); + } + } + + /// @notice Full migration walk: pre-flight (EOA) passes, transfers + /// applied, post-state (Safe) passes, n+1 reversibility passes for every + /// beacon. This is the happy-path mirror of `MigrateBeaconOwners.run()`. + function testFullMigrationWalk() external { + selectBaseFork(); + + // Pre-flight: every beacon EOA-owned. + for (uint256 i = 0; i < beaconList.length; i++) { + harness.callAssertBeaconInvariants(beaconList[i], LibProdDeployV1.BEACON_INITIAL_OWNER, implList[i]); + } + + // Simulate the broadcast effect. + simulateTransfers(); + + // Post-state: every beacon now Safe-owned, implementations unchanged. + for (uint256 i = 0; i < beaconList.length; i++) { + harness.callAssertBeaconInvariants(beaconList[i], LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, implList[i]); + assertEq(Ownable(beaconList[i]).owner(), LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, "beacon now Safe-owned"); + assertEq(IBeacon(beaconList[i]).implementation(), implList[i], "implementation unchanged by transfer"); + } + + // n+1 reversibility: the Safe can act on each beacon via an idempotent + // upgradeTo routed through execTransaction. The post-condition inside + // the helper asserts the implementation is preserved. + for (uint256 i = 0; i < beaconList.length; i++) { + harness.callSimulateBeaconNPlus1( + safe, beaconList[i], implList[i], LibSafeInvariants.STOX_TOKEN_OWNER_SAFE_THRESHOLD + ); + // After the idempotent n+1, the beacon still points at the same + // implementation and is still Safe-owned. + assertEq(IBeacon(beaconList[i]).implementation(), implList[i], "implementation preserved through n+1"); + assertEq(Ownable(beaconList[i]).owner(), LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, "still Safe-owned after n+1"); + } + } + + /// @notice Inverted: the pre-flight rejects a wrong expected owner. The + /// live beacon is EOA-owned; asserting it should be Safe-owned (before the + /// transfer) trips `BeaconOwnerMismatch` with the actual EOA owner. This + /// is the property that makes the post-state assertion meaningful — it + /// would catch a transfer that silently failed. + function testInvertedWrongExpectedOwnerReverts() external { + selectBaseFork(); + address beacon = beaconList[0]; + vm.expectRevert( + abi.encodeWithSelector( + BeaconOwnerMismatch.selector, + beacon, + LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, + LibProdDeployV1.BEACON_INITIAL_OWNER + ) + ); + harness.callAssertBeaconInvariants(beacon, LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, implList[0]); + } + + /// @notice Inverted: after the transfers land, asserting the OLD EOA owner + /// trips `BeaconOwnerMismatch` reporting the Safe as the actual owner. + /// Confirms the post-state assertion is sensitive to the ownership flip in + /// both directions. + function testInvertedStaleEoaOwnerRevertsPostTransfer() external { + selectBaseFork(); + simulateTransfers(); + address beacon = beaconList[0]; + vm.expectRevert( + abi.encodeWithSelector( + BeaconOwnerMismatch.selector, + beacon, + LibProdDeployV1.BEACON_INITIAL_OWNER, + LibSafeInvariants.STOX_TOKEN_OWNER_SAFE + ) + ); + harness.callAssertBeaconInvariants(beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, implList[0]); + } +} diff --git a/test/src/lib/LibSafeInvariants.t.sol b/test/src/lib/LibSafeInvariants.t.sol index cf232dcc..c13a2abc 100644 --- a/test/src/lib/LibSafeInvariants.t.sol +++ b/test/src/lib/LibSafeInvariants.t.sol @@ -3,6 +3,7 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; +import {IOwnable} from "../../../src/lib/LibTokenInvariants.sol"; import {LibSafeInvariants} from "../../../src/lib/LibSafeInvariants.sol"; import {LibSafeInvariantsHarness} from "./LibSafeInvariantsHarness.sol"; import {IGnosisSafe} from "../../../src/interface/IGnosisSafe.sol"; @@ -17,8 +18,13 @@ import { SafeFallbackHandlerMismatch, SafeOwnerCountMismatch, SafeOwnerMismatch, - SafeThresholdMismatch + SafeThresholdMismatch, + BeaconCodehashMismatch, + BeaconOwnerMismatch, + BeaconImplementationMismatch } from "../../../src/lib/LibSafeInvariants.sol"; +import {LibProdDeployV1} from "../../../src/lib/LibProdDeployV1.sol"; +import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; /// @title LibSafeInvariantsTest /// @notice Inverted fork tests that exercise each invariant in @@ -275,4 +281,69 @@ contract LibSafeInvariantsTest is Test { vm.expectRevert(abi.encodeWithSelector(SafeThresholdMismatch.selector, address(safe), uint256(4), uint256(1))); harness.callAssertAll(safe, 4, LibSafeInvariants.expectedOwners()); } + + /// @notice `assertBeaconInvariants` trips `BeaconCodehashMismatch` when + /// the beacon's runtime codehash drifts from the pinned OZ + /// `UpgradeableBeacon` bytecode. Simulated by `vm.etch`-ing a single + /// `INVALID` opcode at the beacon address; `extcodehash` then returns the + /// hash of `0xFE`, which differs from the pinned codehash. Uses the live + /// receipt vault beacon as the victim. + function testInvertedBeaconCodehashMismatch() external { + selectBaseFork(); + address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; + bytes memory mutatedCode = hex"FE"; + vm.etch(beacon, mutatedCode); + bytes32 mutatedCodehash; + assembly ("memory-safe") { + mutatedCodehash := extcodehash(beacon) + } + vm.expectRevert( + abi.encodeWithSelector( + BeaconCodehashMismatch.selector, beacon, LibSafeInvariants.UPGRADEABLE_BEACON_CODEHASH, mutatedCodehash + ) + ); + harness.callAssertBeaconInvariants( + beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION + ); + } + + /// @notice `assertBeaconInvariants` trips `BeaconOwnerMismatch` when the + /// beacon's `owner()` differs from the expected owner. Simulated by + /// mocking `owner()` on the live receipt vault beacon to a rogue address. + function testInvertedBeaconOwnerMismatch() external { + selectBaseFork(); + address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; + address rogueOwner = address(0xBADC0DE); + vm.mockCall(beacon, abi.encodeWithSelector(IOwnable.owner.selector), abi.encode(rogueOwner)); + vm.expectRevert( + abi.encodeWithSelector( + BeaconOwnerMismatch.selector, beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, rogueOwner + ) + ); + harness.callAssertBeaconInvariants( + beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION + ); + } + + /// @notice `assertBeaconInvariants` trips `BeaconImplementationMismatch` + /// when the beacon's `implementation()` differs from the expected + /// implementation. Simulated by mocking `implementation()` on the live + /// receipt vault beacon to a rogue address. + function testInvertedBeaconImplementationMismatch() external { + selectBaseFork(); + address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; + address rogueImpl = address(0xBADBEEF); + vm.mockCall(beacon, abi.encodeWithSelector(IBeacon.implementation.selector), abi.encode(rogueImpl)); + vm.expectRevert( + abi.encodeWithSelector( + BeaconImplementationMismatch.selector, + beacon, + LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION, + rogueImpl + ) + ); + harness.callAssertBeaconInvariants( + beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION + ); + } } diff --git a/test/src/lib/LibSafeInvariantsHarness.sol b/test/src/lib/LibSafeInvariantsHarness.sol index f4251553..40e225ee 100644 --- a/test/src/lib/LibSafeInvariantsHarness.sol +++ b/test/src/lib/LibSafeInvariantsHarness.sol @@ -30,4 +30,8 @@ contract LibSafeInvariantsHarness { function callAssertAllDefaults(IGnosisSafe safe) external view { LibSafeInvariants.assertAll(safe); } + + function callAssertBeaconInvariants(address beacon, address expectedOwner, address expectedImpl) external view { + LibSafeInvariants.assertBeaconInvariants(beacon, expectedOwner, expectedImpl); + } } From d516673061095d1093caba863be23bd4cf7dcda4 Mon Sep 17 00:00:00 2001 From: Josh Hardy Date: Fri, 29 May 2026 16:03:05 +0000 Subject: [PATCH 2/2] refactor(safe): extract LibBeaconInvariants from LibSafeInvariants A beacon is an OpenZeppelin UpgradeableBeacon, not a Safe, so its invariants do not belong in a Safe-named library. Move the five beacon errors, assertBeaconInvariants, the IBeacon import, and the UPGRADEABLE_BEACON_CODEHASH constant out of LibSafeInvariants / LibProdSafes and into a self-contained LibBeaconInvariants with its own minimal IOwnable. Move the beacon inverted tests into a dedicated file and point the beacon-ownership migration script and its test at the new library. LibSafeInvariants keeps its Safe and authoriser/ownership legs untouched. Co-Authored-By: Claude Opus 4.7 --- script/MigrateBeaconOwners.s.sol | 16 +- src/lib/LibBeaconInvariants.sol | 155 +++++++++++++++ src/lib/LibSafeInvariants.sol | 199 +------------------- test/script/MigrateBeaconOwnersHarness.sol | 4 +- test/script/MigrateBeaconOwnersTest.t.sol | 7 +- test/src/lib/LibBeaconInvariants.t.sol | 109 +++++++++++ test/src/lib/LibBeaconInvariantsHarness.sol | 16 ++ test/src/lib/LibSafeInvariants.t.sol | 75 +------- test/src/lib/LibSafeInvariantsHarness.sol | 3 +- 9 files changed, 304 insertions(+), 280 deletions(-) create mode 100644 src/lib/LibBeaconInvariants.sol create mode 100644 test/src/lib/LibBeaconInvariants.t.sol create mode 100644 test/src/lib/LibBeaconInvariantsHarness.sol diff --git a/script/MigrateBeaconOwners.s.sol b/script/MigrateBeaconOwners.s.sol index a95f195f..3478ad05 100644 --- a/script/MigrateBeaconOwners.s.sol +++ b/script/MigrateBeaconOwners.s.sol @@ -9,7 +9,7 @@ import {Ownable} from "@openzeppelin-contracts-5.6.1/access/Ownable.sol"; import {IGnosisSafe} from "../src/interface/IGnosisSafe.sol"; import {LibProdDeployV1} from "../src/lib/LibProdDeployV1.sol"; import {LibSafeInvariants} from "../src/lib/LibSafeInvariants.sol"; -import {LibSafeInvariants} from "../src/lib/LibSafeInvariants.sol"; +import {LibBeaconInvariants} from "../src/lib/LibBeaconInvariants.sol"; import {LibSafeOps} from "../src/lib/LibSafeOps.sol"; /// @title MigrateBeaconOwners @@ -30,7 +30,7 @@ import {LibSafeOps} from "../src/lib/LibSafeOps.sol"; /// direct-broadcast op: /// /// 1. **Pre-flight** — every beacon is asserted to be in the expected -/// EOA-owned state via `LibSafeInvariants.assertBeaconInvariants` (deployed +/// EOA-owned state via `LibBeaconInvariants.assertBeaconInvariants` (deployed /// contract, pinned OZ `UpgradeableBeacon` codehash, EOA owner, pinned /// current implementation). If any beacon has drifted, the script aborts /// before broadcasting anything. @@ -90,12 +90,10 @@ contract MigrateBeaconOwners is Script { address[3] memory implList = currentImpls(); // Pre-flight: every beacon is in the expected EOA-owned state. Reverts - // with the relevant typed error from `LibSafeInvariants` on the first - // drift, before any broadcast happens. + // with the relevant typed error from `LibBeaconInvariants` on the + // first drift, before any broadcast happens. for (uint256 i = 0; i < beaconList.length; i++) { - LibSafeInvariants.assertBeaconInvariants( - beaconList[i], LibProdDeployV1.BEACON_INITIAL_OWNER, implList[i] - ); + LibBeaconInvariants.assertBeaconInvariants(beaconList[i], LibProdDeployV1.BEACON_INITIAL_OWNER, implList[i]); } // Broadcast the ownership transfers from the EOA. Three separate @@ -110,7 +108,9 @@ contract MigrateBeaconOwners is Script { // Post-state: every beacon is now Safe-owned, implementations // unchanged. for (uint256 i = 0; i < beaconList.length; i++) { - LibSafeInvariants.assertBeaconInvariants(beaconList[i], LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, implList[i]); + LibBeaconInvariants.assertBeaconInvariants( + beaconList[i], LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, implList[i] + ); } // n+1 reversibility: prove the Safe can act on each beacon by running diff --git a/src/lib/LibBeaconInvariants.sol b/src/lib/LibBeaconInvariants.sol new file mode 100644 index 00000000..64439209 --- /dev/null +++ b/src/lib/LibBeaconInvariants.sol @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity ^0.8.25; + +import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; + +/// @notice Minimal `Ownable`-like surface used to read a beacon's owner. +/// Every OpenZeppelin `UpgradeableBeacon` exposes `owner()`; this library +/// only needs the getter, not the transfer/renounce mutators. Declared +/// inline here so the beacon invariant bundle owns its only ownership-read +/// surface rather than re-coupling to a Safe-side or token-side interface +/// that could drift. +interface IOwnable { + /// @notice The current owner of the contract. + /// @return The owner address. + function owner() external view returns (address); +} + +/// @notice The address supplied as a beacon has no runtime code. Either the +/// beacon was never deployed at this address or it has been +/// `SELFDESTRUCT`-ed. Caught first so later reads against the address are +/// only attempted once it is known to be a contract. +/// @param beacon The address that was expected to be a deployed beacon. +error BeaconNotDeployed(address beacon); + +/// @notice The beacon's runtime codehash does not match the pinned OZ +/// `UpgradeableBeacon` bytecode +/// (`LibBeaconInvariants.UPGRADEABLE_BEACON_CODEHASH`). Signals either an +/// address swap or a look-alike contract shadowing the `implementation()` / +/// `owner()` selectors. The codehash pin is what lets the access-control +/// behaviour be trusted as OZ's audited bytecode rather than re-tested here. +/// @param beacon The beacon address whose codehash was checked. +/// @param expected The pinned `UpgradeableBeacon` codehash. +/// @param actual The codehash returned by `extcodehash(beacon)`. +error BeaconCodehashMismatch(address beacon, bytes32 expected, bytes32 actual); + +/// @notice The beacon's `owner()` does not match the expected owner. Used +/// both to assert the pre-migration EOA owner and the post-migration Safe +/// owner; the caller supplies which one it expects because the owner is the +/// property the migration deliberately changes. +/// @param beacon The beacon address whose owner was read. +/// @param expected The owner address the caller expected. +/// @param actual The owner address returned by `Ownable(beacon).owner()`. +error BeaconOwnerMismatch(address beacon, address expected, address actual); + +/// @notice The beacon's `implementation()` does not match the expected +/// implementation. The ownership migration must not change any beacon's +/// implementation, so this is asserted equal pre- and post-migration; the +/// upgrade script asserts it against the new implementation after the +/// upgrade. +/// @param beacon The beacon address whose implementation pointer was read. +/// @param expected The implementation address the caller expected. +/// @param actual The implementation address returned by +/// `IBeacon(beacon).implementation()`. +error BeaconImplementationMismatch(address beacon, address expected, address actual); + +/// @notice The beacon's implementation pointer resolves to an address with +/// no runtime code. A beacon pointing at a code-less implementation would +/// brick every proxy that delegates through it, so this is surfaced as an +/// invariant break rather than discovered at the first proxy call. +/// @param beacon The beacon address whose implementation was inspected. +/// @param implementation The implementation address that has no code. +error BeaconImplNotDeployed(address beacon, address implementation); + +/// @title LibBeaconInvariants +/// @notice Reusable invariant assertions for an OpenZeppelin +/// `UpgradeableBeacon`. The single public assertion either returns silently +/// when the invariant holds against the live chain state or reverts with a +/// typed error that pinpoints the drift. +/// @dev Extracted from `LibSafeInvariants` because a beacon is an OZ +/// `UpgradeableBeacon`, not a Safe: the beacon checks share none of the +/// Safe v1.4.1 storage-slot pins and have no business living in a +/// Safe-named library. Keeping them here means the beacon-ownership +/// migration and the receipt vault upgrade reach into a library named for +/// what it actually validates. +library LibBeaconInvariants { + /// @notice Runtime codehash shared by every OpenZeppelin + /// `UpgradeableBeacon` instance on Base. An `UpgradeableBeacon` keeps its + /// implementation pointer and owner in storage rather than in code, so the + /// runtime bytecode is identical across every beacon constructed from the + /// same `UpgradeableBeacon` source. Pinning this codehash lets a beacon + /// invariant assert that `implementation()` / `owner()` are serviced by + /// the canonical OZ beacon bytecode — whose access-control behaviour is + /// then guaranteed by OZ's own audit — rather than a look-alike contract + /// shadowing those selectors. + /// @dev Equal to + /// `LibProdDeployV1.PROD_BEACON_BASE_RUNTIME_CODEHASH_V1`; re-declared + /// here so the beacon invariant does not have to reach into the V1 deploy + /// library for a value that is a property of the OZ bytecode rather than + /// of any one deployment generation. Verified on Base on 2026-05-22 + /// against the three live V1 beacons (receipt, receipt vault, wrapped + /// token vault), all of which share this codehash. + bytes32 internal constant UPGRADEABLE_BEACON_CODEHASH = + 0x8e95867e52db417944afd90f3b6c3c980962831e8a944e7f6958ba8f8cc10630; + + /// @notice Assert the invariants of an OpenZeppelin `UpgradeableBeacon` + /// at `beacon`: it is a deployed contract, its runtime codehash matches + /// the pinned OZ `UpgradeableBeacon` bytecode, its `owner()` matches + /// `expectedOwner`, its `implementation()` matches `expectedImpl`, and + /// that implementation is itself deployed. Reverts with a typed error on + /// first failure; returns silently otherwise. + /// @dev Generic over any beacon (V1, V2, or future) so the same helper + /// serves the beacon-ownership migration pre/post-flight and the receipt + /// vault upgrade pre/post-flight. The owner and implementation are + /// caller-supplied because both are properties an operational script + /// deliberately mutates: the ownership migration changes the owner from + /// the EOA to the Safe, and the V3 upgrade changes the implementation. + /// + /// The codehash pin (check #2) is the load-bearing invariant. OZ's + /// `UpgradeableBeacon` ships the access control (`onlyOwner` on + /// `upgradeTo`, `Ownable` transfer/renounce semantics) that this + /// deployment relies on; pinning the bytecode means that behaviour is + /// guaranteed by OZ's audit rather than re-tested in this repo. A + /// beacon whose codehash matches by definition behaves like the OZ + /// beacon, so no behavioural access-control assertions are duplicated + /// here. + /// + /// Check ordering mirrors `LibSafeInvariants.assertImmutableInvariants`: + /// code presence first (cheapest, and catches an EOA or empty address), + /// codehash second (catches a look-alike), then the storage-backed reads + /// (`owner()`, `implementation()`) once the bytecode is proven to be the + /// OZ beacon, and the implementation code-presence check last because it + /// depends on the implementation read having succeeded. + /// @param beacon The beacon to assert invariants on. + /// @param expectedOwner The owner the beacon is expected to report. + /// @param expectedImpl The implementation the beacon is expected to + /// point at. + function assertBeaconInvariants(address beacon, address expectedOwner, address expectedImpl) internal view { + if (beacon.code.length == 0) { + revert BeaconNotDeployed(beacon); + } + + bytes32 actualCodehash; + assembly ("memory-safe") { + actualCodehash := extcodehash(beacon) + } + if (actualCodehash != UPGRADEABLE_BEACON_CODEHASH) { + revert BeaconCodehashMismatch(beacon, UPGRADEABLE_BEACON_CODEHASH, actualCodehash); + } + + address actualOwner = IOwnable(beacon).owner(); + if (actualOwner != expectedOwner) { + revert BeaconOwnerMismatch(beacon, expectedOwner, actualOwner); + } + + address actualImpl = IBeacon(beacon).implementation(); + if (actualImpl != expectedImpl) { + revert BeaconImplementationMismatch(beacon, expectedImpl, actualImpl); + } + + if (actualImpl.code.length == 0) { + revert BeaconImplNotDeployed(beacon, actualImpl); + } + } +} diff --git a/src/lib/LibSafeInvariants.sol b/src/lib/LibSafeInvariants.sol index afefaa02..7816a241 100644 --- a/src/lib/LibSafeInvariants.sol +++ b/src/lib/LibSafeInvariants.sol @@ -3,8 +3,6 @@ pragma solidity ^0.8.25; import {IGnosisSafe} from "../interface/IGnosisSafe.sol"; -import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; -import {IOwnable} from "./LibTokenInvariants.sol"; /// @notice The runtime codehash at the Safe's address does not match the /// pinned Safe v1.4.1 L2 proxy codehash. Signals either that the address has @@ -99,52 +97,6 @@ error SafeOwnerMismatch(address safe, uint256 index, address expectedOwner, addr /// @param actual The threshold returned by `getThreshold()`. error SafeThresholdMismatch(address safe, uint256 expected, uint256 actual); -/// @notice The address supplied as a beacon has no runtime code. Either the -/// beacon was never deployed at this address or it has been -/// `SELFDESTRUCT`-ed. Caught first so later reads against the address are -/// only attempted once it is known to be a contract. -/// @param beacon The address that was expected to be a deployed beacon. -error BeaconNotDeployed(address beacon); - -/// @notice The beacon's runtime codehash does not match the pinned OZ -/// `UpgradeableBeacon` bytecode (`UPGRADEABLE_BEACON_CODEHASH`). -/// Signals either an address swap or a look-alike contract shadowing the -/// `implementation()` / `owner()` selectors. The codehash pin is what lets -/// the access-control behaviour be trusted as OZ's audited bytecode rather -/// than re-tested here. -/// @param beacon The beacon address whose codehash was checked. -/// @param expected The pinned `UpgradeableBeacon` codehash. -/// @param actual The codehash returned by `extcodehash(beacon)`. -error BeaconCodehashMismatch(address beacon, bytes32 expected, bytes32 actual); - -/// @notice The beacon's `owner()` does not match the expected owner. Used -/// both to assert the pre-migration EOA owner and the post-migration Safe -/// owner; the caller supplies which one it expects because the owner is the -/// property the migration deliberately changes. -/// @param beacon The beacon address whose owner was read. -/// @param expected The owner address the caller expected. -/// @param actual The owner address returned by `Ownable(beacon).owner()`. -error BeaconOwnerMismatch(address beacon, address expected, address actual); - -/// @notice The beacon's `implementation()` does not match the expected -/// implementation. The ownership migration must not change any beacon's -/// implementation, so this is asserted equal pre- and post-migration; the -/// upgrade script asserts it against the new implementation after the -/// upgrade. -/// @param beacon The beacon address whose implementation pointer was read. -/// @param expected The implementation address the caller expected. -/// @param actual The implementation address returned by -/// `IBeacon(beacon).implementation()`. -error BeaconImplementationMismatch(address beacon, address expected, address actual); - -/// @notice The beacon's implementation pointer resolves to an address with -/// no runtime code. A beacon pointing at a code-less implementation would -/// brick every proxy that delegates through it, so this is surfaced as an -/// invariant break rather than discovered at the first proxy call. -/// @param beacon The beacon address whose implementation was inspected. -/// @param implementation The implementation address that has no code. -error BeaconImplNotDeployed(address beacon, address implementation); - /// @title LibSafeInvariants /// @notice Reusable invariant assertions for a Safe v1.4.1 L2 multisig /// pinned to the ST0x token-owner deployment. Each public assertion either @@ -187,96 +139,28 @@ error BeaconImplNotDeployed(address beacon, address implementation); /// they cannot collide with the owner/module/threshold linked-list slots. library LibSafeInvariants { // ========================================================================= - // Safe v1.4.1 deployment manifest constants. Universal to every v1.4.1 L2 - // Safe; sourced from `safe-deployments` for chainId 8453 and cross-checked - // against the live ST0x production Safe. + // Safe v1.4.1 deployment manifest constants. // ========================================================================= - - /// @notice Safe v1.4.1 L2 singleton (master copy) address on Base. - /// Verified by reading proxy storage slot `0x0` of - /// `STOX_TOKEN_OWNER_SAFE` and matching against the - /// `safe-deployments` manifest. address internal constant SAFE_V1_4_1_L2_SINGLETON = 0x29fcB43b46531BcA003ddC8FCB67FFE91900C762; - - /// @notice Runtime codehash of a Safe v1.4.1 proxy on Base. Equal to - /// `extcodehash(STOX_TOKEN_OWNER_SAFE)` and to every other v1.4.1 L2 - /// proxy pointing at `SAFE_V1_4_1_L2_SINGLETON`. Pinning this codehash - /// guards against the Safe address being replaced by an EOA-controlled - /// contract or a fake proxy pointing at a malicious singleton. bytes32 internal constant SAFE_V1_4_1_L2_PROXY_CODEHASH = 0xb89c1b3bdf2cf8827818646bce9a8f6e372885f8c55e5c07acbd307cb133b000; - - /// @notice Expected `VERSION()` string from a Safe v1.4.1 singleton. string internal constant SAFE_V1_4_1_VERSION = "1.4.1"; - - /// @notice Runtime codehash of the Safe v1.4.1 L2 singleton bytecode at - /// `SAFE_V1_4_1_L2_SINGLETON`. Pinning this guards against an attacker - /// who replaces the bytecode at the singleton address (e.g. via - /// `SELFDESTRUCT` + re-create) while preserving the proxy codehash. - /// Without this pin, every implementation-backed accessor on the Safe - /// (`VERSION()`, `getOwners()`, `getThreshold()`, etc.) is mediated by - /// untrusted code at the singleton address. Asserting this codehash - /// before any of those reads closes that gap. - /// @dev Computed via `keccak256(eth_getCode(SAFE_V1_4_1_L2_SINGLETON))` - /// on Base on 2026-05-20. bytes32 internal constant SAFE_V1_4_1_L2_SINGLETON_CODEHASH = 0xb1f926978a0f44a2c0ec8fe822418ae969bd8c3f18d61e5103100339894f81ff; - - /// @notice CompatibilityFallbackHandler v1.4.1 address on Base. Verified - /// against the live Safe's fallback handler storage slot. Pinned so a - /// swapped-in malicious handler that shadows view selectors via - /// fallback can be detected by `assertImmutableInvariants`. - /// @dev Source: github.com/safe-global/safe-deployments - /// `src/assets/v1.4.1/compatibility_fallback_handler.json` (chainId - /// 8453 entry). Cross-checked on Base on 2026-05-20. address internal constant SAFE_V1_4_1_COMPATIBILITY_FALLBACK_HANDLER = 0xfd0732Dc9E303f09fCEf3a7388Ad10A83459Ec99; // ========================================================================= - // ST0x token-owner Safe pins. Current-state invariants for the specific - // Safe at `STOX_TOKEN_OWNER_SAFE`; updated when the live state changes - // (e.g. the threshold migration bumps `STOX_TOKEN_OWNER_SAFE_THRESHOLD` - // from `1` to `3` in the same PR that records the post-execution state). + // ST0x token-owner Safe current-state pins. // ========================================================================= - - /// @notice The Safe that owns every ST0x receipt vault on Base. Subject - /// of the threshold migration (1 -> 3, against the post-rotation - /// 6-owner roster). - /// https://basescan.org/address/0xe70d821f3462A074E63b42D0aac6523faAe1D611 address internal constant STOX_TOKEN_OWNER_SAFE = 0xe70d821f3462a074e63b42d0AaC6523faAe1d611; - - /// @notice The current expected threshold for `STOX_TOKEN_OWNER_SAFE`. - /// Updated by the threshold-migration PR family once live execution - /// lands: scripts and the post-migration pin both treat this constant - /// as the canonical current truth, so the value bumps from `1` to `3` - /// in the same PR that records the live post-execution state. uint256 internal constant STOX_TOKEN_OWNER_SAFE_THRESHOLD = 1; - - /// @notice Owner #1 of `STOX_TOKEN_OWNER_SAFE`. Order matches - /// `getOwners()` (Safe-internal linked-list order) against the - /// post-rotation roster: `getOwners()` returns owners newest-first, - /// so the last signer to be added via `addOwnerWithThreshold` appears - /// at slot 0. address internal constant STOX_TOKEN_OWNER_SAFE_OWNER_1 = 0x4746095B1Ea1A84446d34448f44e74D3d51f92F2; - - /// @notice Owner #2 of `STOX_TOKEN_OWNER_SAFE`. address internal constant STOX_TOKEN_OWNER_SAFE_OWNER_2 = 0xceC2cb8B8EE4000FFA3F8a7f8E0Fa0A3E3DAb72d; - - /// @notice Owner #3 of `STOX_TOKEN_OWNER_SAFE`. address internal constant STOX_TOKEN_OWNER_SAFE_OWNER_3 = 0x8D5901d8aE48101B59400235ad8614A2e0510466; - - /// @notice Owner #4 of `STOX_TOKEN_OWNER_SAFE`. address internal constant STOX_TOKEN_OWNER_SAFE_OWNER_4 = 0xC1C89b7f5448F447d59f920456A9610f6b2544bC; - - /// @notice Owner #5 of `STOX_TOKEN_OWNER_SAFE`. address internal constant STOX_TOKEN_OWNER_SAFE_OWNER_5 = 0xAB92b327c97A6E7461cBd76E2a789E5e106FF87e; - - /// @notice Owner #6 of `STOX_TOKEN_OWNER_SAFE`. address internal constant STOX_TOKEN_OWNER_SAFE_OWNER_6 = 0x5CCd3cE683b66ff271DDB8915fF528b8fcFa23c2; - // ========================================================================= - // Storage layout constants for paginated / direct slot reads. - // ========================================================================= - /// @notice Storage slot at which Safe v1.4.1 stores the transaction /// guard address. Equal to /// `keccak256("guard_manager.guard.address")`. A non-zero value here @@ -477,16 +361,9 @@ library LibSafeInvariants { assertAll(safe, STOX_TOKEN_OWNER_SAFE_THRESHOLD, expectedOwners()); } - /// @notice Returns the expected owner set for `STOX_TOKEN_OWNER_SAFE` in - /// the exact order returned by `getOwners()` against an unpinned Base - /// head fork (the live-state pin lives in - /// `StoxProdV2.t.sol::testProdDeployBaseV2`, which selects head rather - /// than pinning to a historical block so the next CI run catches any - /// further drift). Provided as a helper because Solidity 0.8 cannot - /// express a file-scope `constant address[]` and declaring the array - /// as `immutable` is contract-scoped only. - /// @return The six owners of the ST0x token-owner Safe in - /// `getOwners()` order. + /// @notice Expected owner set for `STOX_TOKEN_OWNER_SAFE` in + /// `getOwners()` order. Helper because Solidity 0.8 cannot express a + /// file-scope `constant address[]`. function expectedOwners() internal pure returns (address[] memory) { address[] memory owners = new address[](6); owners[0] = STOX_TOKEN_OWNER_SAFE_OWNER_1; @@ -497,70 +374,4 @@ library LibSafeInvariants { owners[5] = STOX_TOKEN_OWNER_SAFE_OWNER_6; return owners; } - - /// @notice Assert the invariants of an OpenZeppelin `UpgradeableBeacon` - /// at `beacon`: it is a deployed contract, its runtime codehash matches - /// the pinned OZ `UpgradeableBeacon` bytecode, its `owner()` matches - /// `expectedOwner`, its `implementation()` matches `expectedImpl`, and - /// that implementation is itself deployed. Reverts with a typed error on - /// first failure; returns silently otherwise. - /// @dev Generic over any beacon (V1, V2, or future) so the same helper - /// serves the beacon-ownership migration pre/post-flight and the receipt - /// vault upgrade pre/post-flight. The owner and implementation are - /// caller-supplied because both are properties an operational script - /// deliberately mutates: the ownership migration changes the owner from - /// the EOA to the Safe, and the V3 upgrade changes the implementation. - /// - /// The codehash pin (check #2) is the load-bearing invariant. OZ's - /// `UpgradeableBeacon` ships the access control (`onlyOwner` on - /// `upgradeTo`, `Ownable` transfer/renounce semantics) that this - /// deployment relies on; pinning the bytecode means that behaviour is - /// guaranteed by OZ's audit rather than re-tested in this repo. A - /// beacon whose codehash matches by definition behaves like the OZ - /// beacon, so no behavioural access-control assertions are duplicated - /// here. - /// - /// Check ordering mirrors `assertImmutableInvariants`: code presence - /// first (cheapest, and catches an EOA or empty address), codehash - /// second (catches a look-alike), then the storage-backed reads - /// (`owner()`, `implementation()`) once the bytecode is proven to be the - /// OZ beacon, and the implementation code-presence check last because it - /// depends on the implementation read having succeeded. - /// @notice OpenZeppelin `UpgradeableBeacon` runtime codehash. Pinned - /// here so the beacon-side codehash check has a concrete invariant - /// target; matches the bytecode at every prod beacon deployment. - bytes32 internal constant UPGRADEABLE_BEACON_CODEHASH = - 0x8e95867e52db417944afd90f3b6c3c980962831e8a944e7f6958ba8f8cc10630; - - /// @param beacon The beacon to assert invariants on. - /// @param expectedOwner The owner the beacon is expected to report. - /// @param expectedImpl The implementation the beacon is expected to - /// point at. - function assertBeaconInvariants(address beacon, address expectedOwner, address expectedImpl) internal view { - if (beacon.code.length == 0) { - revert BeaconNotDeployed(beacon); - } - - bytes32 actualCodehash; - assembly ("memory-safe") { - actualCodehash := extcodehash(beacon) - } - if (actualCodehash != UPGRADEABLE_BEACON_CODEHASH) { - revert BeaconCodehashMismatch(beacon, UPGRADEABLE_BEACON_CODEHASH, actualCodehash); - } - - address actualOwner = IOwnable(beacon).owner(); - if (actualOwner != expectedOwner) { - revert BeaconOwnerMismatch(beacon, expectedOwner, actualOwner); - } - - address actualImpl = IBeacon(beacon).implementation(); - if (actualImpl != expectedImpl) { - revert BeaconImplementationMismatch(beacon, expectedImpl, actualImpl); - } - - if (actualImpl.code.length == 0) { - revert BeaconImplNotDeployed(beacon, actualImpl); - } - } } diff --git a/test/script/MigrateBeaconOwnersHarness.sol b/test/script/MigrateBeaconOwnersHarness.sol index 1ddf3f3c..2a37e20f 100644 --- a/test/script/MigrateBeaconOwnersHarness.sol +++ b/test/script/MigrateBeaconOwnersHarness.sol @@ -3,7 +3,7 @@ pragma solidity =0.8.25; import {IGnosisSafe} from "../../src/interface/IGnosisSafe.sol"; -import {LibSafeInvariants} from "../../src/lib/LibSafeInvariants.sol"; +import {LibBeaconInvariants} from "../../src/lib/LibBeaconInvariants.sol"; import {LibSafeOps} from "../../src/lib/LibSafeOps.sol"; /// @title MigrateBeaconOwnersHarness @@ -16,7 +16,7 @@ import {LibSafeOps} from "../../src/lib/LibSafeOps.sol"; /// simulate the on-chain broadcast's effect). contract MigrateBeaconOwnersHarness { function callAssertBeaconInvariants(address beacon, address expectedOwner, address expectedImpl) external view { - LibSafeInvariants.assertBeaconInvariants(beacon, expectedOwner, expectedImpl); + LibBeaconInvariants.assertBeaconInvariants(beacon, expectedOwner, expectedImpl); } function callSimulateBeaconNPlus1(IGnosisSafe safe, address beacon, address currentImpl, uint256 threshold) diff --git a/test/script/MigrateBeaconOwnersTest.t.sol b/test/script/MigrateBeaconOwnersTest.t.sol index a4786b5b..b5b8a931 100644 --- a/test/script/MigrateBeaconOwnersTest.t.sol +++ b/test/script/MigrateBeaconOwnersTest.t.sol @@ -8,7 +8,8 @@ import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; import {IGnosisSafe} from "../../src/interface/IGnosisSafe.sol"; import {LibProdDeployV1} from "../../src/lib/LibProdDeployV1.sol"; -import {LibSafeInvariants, BeaconOwnerMismatch} from "../../src/lib/LibSafeInvariants.sol"; +import {LibSafeInvariants} from "../../src/lib/LibSafeInvariants.sol"; +import {BeaconOwnerMismatch} from "../../src/lib/LibBeaconInvariants.sol"; import {MigrateBeaconOwnersHarness} from "./MigrateBeaconOwnersHarness.sol"; import {LibRainDeploy} from "rain-deploy-0.1.3/src/lib/LibRainDeploy.sol"; @@ -101,7 +102,9 @@ contract MigrateBeaconOwnersTest is Test { // After the idempotent n+1, the beacon still points at the same // implementation and is still Safe-owned. assertEq(IBeacon(beaconList[i]).implementation(), implList[i], "implementation preserved through n+1"); - assertEq(Ownable(beaconList[i]).owner(), LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, "still Safe-owned after n+1"); + assertEq( + Ownable(beaconList[i]).owner(), LibSafeInvariants.STOX_TOKEN_OWNER_SAFE, "still Safe-owned after n+1" + ); } } diff --git a/test/src/lib/LibBeaconInvariants.t.sol b/test/src/lib/LibBeaconInvariants.t.sol new file mode 100644 index 00000000..f445239e --- /dev/null +++ b/test/src/lib/LibBeaconInvariants.t.sol @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.1/src/Test.sol"; +import { + LibBeaconInvariants, + IOwnable, + BeaconCodehashMismatch, + BeaconOwnerMismatch, + BeaconImplementationMismatch +} from "../../../src/lib/LibBeaconInvariants.sol"; +import {LibSafeInvariants} from "../../../src/lib/LibSafeInvariants.sol"; +import {LibProdDeployV1} from "../../../src/lib/LibProdDeployV1.sol"; +import {LibBeaconInvariantsHarness} from "./LibBeaconInvariantsHarness.sol"; +import {LibRainDeploy} from "rain-deploy-0.1.3/src/lib/LibRainDeploy.sol"; +import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; + +/// @title LibBeaconInvariantsTest +/// @notice Inverted fork tests that exercise each invariant in +/// `LibBeaconInvariants` by injecting drift via `vm.etch` / `vm.mockCall` +/// and asserting the matching typed error is raised. The positive +/// ("live state passes") cases live in the beacon-ownership migration's +/// happy-path test, so this file focuses on coverage of every error path. +/// @dev Uses an unpinned Base head fork (same precedent as +/// `LibSafeInvariants.t.sol`). Pinning would freeze the invariant +/// assertions against a stale snapshot and let new drift slip through +/// unnoticed. +contract LibBeaconInvariantsTest is Test { + /// @notice External-call harness deployed fresh per test (via fork + /// rebuild). Each test calls `selectBaseFork` before deploying the + /// harness; the harness is recreated against the active fork. + LibBeaconInvariantsHarness internal harness; + + /// @notice Selects the Base fork at chain head — deliberately + /// unpinned. Live drift detector; see contract-level rationale. + function selectBaseFork() internal { + vm.createSelectFork(LibRainDeploy.BASE); + harness = new LibBeaconInvariantsHarness(); + } + + /// @notice `assertBeaconInvariants` trips `BeaconCodehashMismatch` when + /// the beacon's runtime codehash drifts from the pinned OZ + /// `UpgradeableBeacon` bytecode. Simulated by `vm.etch`-ing a single + /// `INVALID` opcode at the beacon address; `extcodehash` then returns the + /// hash of `0xFE`, which differs from the pinned codehash. Uses the live + /// receipt vault beacon as the victim. + function testInvertedBeaconCodehashMismatch() external { + selectBaseFork(); + address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; + bytes memory mutatedCode = hex"FE"; + vm.etch(beacon, mutatedCode); + bytes32 mutatedCodehash; + assembly ("memory-safe") { + mutatedCodehash := extcodehash(beacon) + } + vm.expectRevert( + abi.encodeWithSelector( + BeaconCodehashMismatch.selector, + beacon, + LibBeaconInvariants.UPGRADEABLE_BEACON_CODEHASH, + mutatedCodehash + ) + ); + harness.callAssertBeaconInvariants( + beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION + ); + } + + /// @notice `assertBeaconInvariants` trips `BeaconOwnerMismatch` when the + /// beacon's `owner()` differs from the expected owner. Simulated by + /// mocking `owner()` on the live receipt vault beacon to a rogue address. + function testInvertedBeaconOwnerMismatch() external { + selectBaseFork(); + address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; + address rogueOwner = address(0xBADC0DE); + vm.mockCall(beacon, abi.encodeWithSelector(IOwnable.owner.selector), abi.encode(rogueOwner)); + vm.expectRevert( + abi.encodeWithSelector( + BeaconOwnerMismatch.selector, beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, rogueOwner + ) + ); + harness.callAssertBeaconInvariants( + beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION + ); + } + + /// @notice `assertBeaconInvariants` trips `BeaconImplementationMismatch` + /// when the beacon's `implementation()` differs from the expected + /// implementation. Simulated by mocking `implementation()` on the live + /// receipt vault beacon to a rogue address. + function testInvertedBeaconImplementationMismatch() external { + selectBaseFork(); + address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; + address rogueImpl = address(0xBADBEEF); + vm.mockCall(beacon, abi.encodeWithSelector(IBeacon.implementation.selector), abi.encode(rogueImpl)); + vm.expectRevert( + abi.encodeWithSelector( + BeaconImplementationMismatch.selector, + beacon, + LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION, + rogueImpl + ) + ); + harness.callAssertBeaconInvariants( + beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION + ); + } +} diff --git a/test/src/lib/LibBeaconInvariantsHarness.sol b/test/src/lib/LibBeaconInvariantsHarness.sol new file mode 100644 index 00000000..427e6517 --- /dev/null +++ b/test/src/lib/LibBeaconInvariantsHarness.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {LibBeaconInvariants} from "../../../src/lib/LibBeaconInvariants.sol"; + +/// @title LibBeaconInvariantsHarness +/// @notice External-call shim around the internal library so +/// `vm.expectRevert` can intercept the typed errors. `vm.expectRevert` only +/// catches reverts from external calls; library `internal` functions inline +/// and would fail the depth check otherwise. +contract LibBeaconInvariantsHarness { + function callAssertBeaconInvariants(address beacon, address expectedOwner, address expectedImpl) external view { + LibBeaconInvariants.assertBeaconInvariants(beacon, expectedOwner, expectedImpl); + } +} diff --git a/test/src/lib/LibSafeInvariants.t.sol b/test/src/lib/LibSafeInvariants.t.sol index c13a2abc..dfeb1148 100644 --- a/test/src/lib/LibSafeInvariants.t.sol +++ b/test/src/lib/LibSafeInvariants.t.sol @@ -3,7 +3,6 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; -import {IOwnable} from "../../../src/lib/LibTokenInvariants.sol"; import {LibSafeInvariants} from "../../../src/lib/LibSafeInvariants.sol"; import {LibSafeInvariantsHarness} from "./LibSafeInvariantsHarness.sol"; import {IGnosisSafe} from "../../../src/interface/IGnosisSafe.sol"; @@ -18,13 +17,8 @@ import { SafeFallbackHandlerMismatch, SafeOwnerCountMismatch, SafeOwnerMismatch, - SafeThresholdMismatch, - BeaconCodehashMismatch, - BeaconOwnerMismatch, - BeaconImplementationMismatch + SafeThresholdMismatch } from "../../../src/lib/LibSafeInvariants.sol"; -import {LibProdDeployV1} from "../../../src/lib/LibProdDeployV1.sol"; -import {IBeacon} from "@openzeppelin-contracts-5.6.1/proxy/beacon/IBeacon.sol"; /// @title LibSafeInvariantsTest /// @notice Inverted fork tests that exercise each invariant in @@ -102,7 +96,7 @@ contract LibSafeInvariantsTest is Test { /// @notice Drift in the singleton's bytecode trips /// `SafeSingletonBytecodeMismatch`. Simulated by `vm.etch`-ing alien /// bytecode at the singleton address — the codehash diverges from - /// the pinned `SAFE_V1_4_1_L2_SINGLETON_CODEHASH` even though slot + /// the pinned `LibSafeInvariants.SAFE_V1_4_1_L2_SINGLETON_CODEHASH` even though slot /// 0 still points at the canonical address. /// @dev `vm.etch` on the singleton breaks every delegate-routed /// read on the proxy, so the slot-0 fetch is mocked back to the @@ -281,69 +275,4 @@ contract LibSafeInvariantsTest is Test { vm.expectRevert(abi.encodeWithSelector(SafeThresholdMismatch.selector, address(safe), uint256(4), uint256(1))); harness.callAssertAll(safe, 4, LibSafeInvariants.expectedOwners()); } - - /// @notice `assertBeaconInvariants` trips `BeaconCodehashMismatch` when - /// the beacon's runtime codehash drifts from the pinned OZ - /// `UpgradeableBeacon` bytecode. Simulated by `vm.etch`-ing a single - /// `INVALID` opcode at the beacon address; `extcodehash` then returns the - /// hash of `0xFE`, which differs from the pinned codehash. Uses the live - /// receipt vault beacon as the victim. - function testInvertedBeaconCodehashMismatch() external { - selectBaseFork(); - address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; - bytes memory mutatedCode = hex"FE"; - vm.etch(beacon, mutatedCode); - bytes32 mutatedCodehash; - assembly ("memory-safe") { - mutatedCodehash := extcodehash(beacon) - } - vm.expectRevert( - abi.encodeWithSelector( - BeaconCodehashMismatch.selector, beacon, LibSafeInvariants.UPGRADEABLE_BEACON_CODEHASH, mutatedCodehash - ) - ); - harness.callAssertBeaconInvariants( - beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION - ); - } - - /// @notice `assertBeaconInvariants` trips `BeaconOwnerMismatch` when the - /// beacon's `owner()` differs from the expected owner. Simulated by - /// mocking `owner()` on the live receipt vault beacon to a rogue address. - function testInvertedBeaconOwnerMismatch() external { - selectBaseFork(); - address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; - address rogueOwner = address(0xBADC0DE); - vm.mockCall(beacon, abi.encodeWithSelector(IOwnable.owner.selector), abi.encode(rogueOwner)); - vm.expectRevert( - abi.encodeWithSelector( - BeaconOwnerMismatch.selector, beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, rogueOwner - ) - ); - harness.callAssertBeaconInvariants( - beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION - ); - } - - /// @notice `assertBeaconInvariants` trips `BeaconImplementationMismatch` - /// when the beacon's `implementation()` differs from the expected - /// implementation. Simulated by mocking `implementation()` on the live - /// receipt vault beacon to a rogue address. - function testInvertedBeaconImplementationMismatch() external { - selectBaseFork(); - address beacon = LibProdDeployV1.STOX_RECEIPT_VAULT_BEACON_V1; - address rogueImpl = address(0xBADBEEF); - vm.mockCall(beacon, abi.encodeWithSelector(IBeacon.implementation.selector), abi.encode(rogueImpl)); - vm.expectRevert( - abi.encodeWithSelector( - BeaconImplementationMismatch.selector, - beacon, - LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION, - rogueImpl - ) - ); - harness.callAssertBeaconInvariants( - beacon, LibProdDeployV1.BEACON_INITIAL_OWNER, LibProdDeployV1.STOX_RECEIPT_VAULT_IMPLEMENTATION - ); - } } diff --git a/test/src/lib/LibSafeInvariantsHarness.sol b/test/src/lib/LibSafeInvariantsHarness.sol index 40e225ee..9637ae49 100644 --- a/test/src/lib/LibSafeInvariantsHarness.sol +++ b/test/src/lib/LibSafeInvariantsHarness.sol @@ -3,6 +3,7 @@ pragma solidity =0.8.25; import {LibSafeInvariants} from "../../../src/lib/LibSafeInvariants.sol"; +import {LibBeaconInvariants} from "../../../src/lib/LibBeaconInvariants.sol"; import {IGnosisSafe} from "../../../src/interface/IGnosisSafe.sol"; /// @title LibSafeInvariantsHarness @@ -32,6 +33,6 @@ contract LibSafeInvariantsHarness { } function callAssertBeaconInvariants(address beacon, address expectedOwner, address expectedImpl) external view { - LibSafeInvariants.assertBeaconInvariants(beacon, expectedOwner, expectedImpl); + LibBeaconInvariants.assertBeaconInvariants(beacon, expectedOwner, expectedImpl); } }