Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions script/MigrateBeaconOwners.s.sol
Original file line number Diff line number Diff line change
@@ -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 {LibBeaconInvariants} from "../src/lib/LibBeaconInvariants.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 `LibBeaconInvariants.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 <EOA 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 `LibBeaconInvariants` on the
// first drift, before any broadcast happens.
for (uint256 i = 0; i < beaconList.length; i++) {
LibBeaconInvariants.assertBeaconInvariants(beaconList[i], LibProdDeployV1.BEACON_INITIAL_OWNER, implList[i]);
}
Comment on lines +88 to +97

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Validate the destination Safe before broadcasting ownership transfers.

The script pre-flights each beacon, but it never calls the Safe invariant bundle before transferring beacon ownership to STOX_TOKEN_OWNER_SAFE. Add a pre-broadcast LibSafeInvariants.assertAll(safe) so a drifted owner set, threshold, module, guard, fallback handler, or singleton is caught before the EOA gives the Safe control.

🛡️ Proposed pre-flight check
         IGnosisSafe safe = IGnosisSafe(LibSafeInvariants.STOX_TOKEN_OWNER_SAFE);
         address[3] memory beaconList = beacons();
         address[3] memory implList = currentImpls();
 
+        LibSafeInvariants.assertAll(safe);
+
         // Pre-flight: every beacon is in the expected EOA-owned state. Reverts
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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 `LibBeaconInvariants` on the
// first drift, before any broadcast happens.
for (uint256 i = 0; i < beaconList.length; i++) {
LibBeaconInvariants.assertBeaconInvariants(beaconList[i], LibProdDeployV1.BEACON_INITIAL_OWNER, implList[i]);
}
IGnosisSafe safe = IGnosisSafe(LibSafeInvariants.STOX_TOKEN_OWNER_SAFE);
address[3] memory beaconList = beacons();
address[3] memory implList = currentImpls();
LibSafeInvariants.assertAll(safe);
// Pre-flight: every beacon is in the expected EOA-owned state. Reverts
// with the relevant typed error from `LibBeaconInvariants` on the
// first drift, before any broadcast happens.
for (uint256 i = 0; i < beaconList.length; i++) {
LibBeaconInvariants.assertBeaconInvariants(beaconList[i], LibProdDeployV1.BEACON_INITIAL_OWNER, implList[i]);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@script/MigrateBeaconOwners.s.sol` around lines 88 - 97, The migration script
currently pre-flights beacon state but does not verify the destination Safe
before ownership transfer. In `MigrateBeaconOwners.s.sol`, use the existing
`safe` variable from `IGnosisSafe(LibSafeInvariants.STOX_TOKEN_OWNER_SAFE)` and
add a pre-broadcast `LibSafeInvariants.assertAll(safe)` alongside the existing
beacon invariant loop, so any drift in owner set, threshold, module, guard,
fallback handler, or singleton is caught before broadcasting.


// 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++) {
LibBeaconInvariants.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));
}
}
155 changes: 155 additions & 0 deletions src/lib/LibBeaconInvariants.sol
Original file line number Diff line number Diff line change
@@ -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;
Comment on lines +86 to +94

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Route the pinned beacon codehash through the deploy library.

Line 93 hardcodes the production beacon runtime codehash in a source library. That bypasses the repository’s deploy-constant contract and can drift from the current LibProdDeploy* version. Point UPGRADEABLE_BEACON_CODEHASH at the latest versioned deploy library constant instead of redeclaring the value here. As per coding guidelines, “Source contracts must reference addresses and codehashes through versioned LibProdDeploy* libraries.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/LibBeaconInvariants.sol` around lines 86 - 94, The beacon codehash is
being hardcoded in LibBeaconInvariants instead of flowing through the versioned
deploy-constant source. Update UPGRADEABLE_BEACON_CODEHASH to reference the
latest LibProdDeploy* constant rather than redeclaring the literal here, and use
the matching deploy library symbol so the invariant stays aligned with the
current production deploy library.

Source: Coding guidelines


/// @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);
}
}
}
Loading
Loading