Skip to content

RAI-789 feat(safe): retarget threshold migration to 3-of-6 against post-rotation roster #204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hardyjosh wants to merge 1 commit into from
Closed

RAI-789 feat(safe): retarget threshold migration to 3-of-6 against post-rotation roster #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions script/MigrateMultisigThreshold.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ error VerifyExpectedSingleTx(uint256 actualCount);

/// @title MigrateMultisigThreshold
/// @notice Forge script that authors the ST0x token-owner Safe's
/// multisig threshold migration (1-of-4 -> 3-of-4). Performs an
/// multisig threshold migration (1-of-6 -> 3-of-6 against the post-rotation roster). Performs an
/// exhaustive on-chain pre-flight via `LibSafeInvariants.assertAll`
/// (proxy codehash, singleton + bytecode, version, modules, guard,
/// fallback handler, uniform vault ownership, expected owner set,
Expand Down Expand Up @@ -58,7 +58,7 @@ contract MigrateMultisigThreshold is Script {

/// @notice Human-readable name embedded in the emitted Tx Builder
/// JSON's `meta.name`. Visible to signers in the Safe Tx Builder UI.
string internal constant BUNDLE_NAME = "ST0x Safe threshold 1->3";
string internal constant BUNDLE_NAME = "ST0x Safe threshold 1->3 (post-rotation roster)";

/// @notice Output path (relative to the project root) for the Tx
/// Builder JSON artifact. Picked up by the multisig-artifact GH
Expand All @@ -74,7 +74,7 @@ contract MigrateMultisigThreshold is Script {
/// `vm.prank`.
function run() external {
IGnosisSafe safe = IGnosisSafe(LibProdSafes.STOX_TOKEN_OWNER_SAFE);
// Pre-flight: every immutable invariant plus the pinned 4-owner
// Pre-flight: every immutable invariant plus the pinned 6-owner
// roster plus the pinned current threshold. Defaults from
// `LibProdSafes` (no-arg overload). Reverts with the relevant
// typed error from the underlying library on first mismatch.
Expand Down
75 changes: 47 additions & 28 deletions src/lib/LibProdSafes.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,22 @@ pragma solidity ^0.8.25;
/// @title LibProdSafes
/// @notice Production Safe constants for the ST0x token-owner multisig on
/// Base. Pinned addresses, codehashes, slot values, and the expected owner
/// set (currently 4 owners) are derived from live on-chain state and the
/// canonical Safe deployment manifest, then re-asserted from fork tests so
/// that drift between this file and reality trips CI rather than slipping
/// into a migration script.
/// @dev Scope: the multisig threshold migration raises this Safe from
/// 1-of-4 to 3-of-4. (Originally specced as 1-of-5 -> 3-of-5; the owner
/// roster was reduced to four on 2026-05-18 via the `RemovedOwner` event
/// at block 46156528, before the threshold migration was executed.)
/// set (6 owners post-rotation) are derived from live on-chain state and
/// the canonical Safe deployment manifest, then re-asserted from fork
/// tests so that drift between this file and reality trips CI rather than
/// slipping into a migration script.
/// @dev Scope: this file pins the *post-rotation* roster (6 ST0x
/// governance signers) and the threshold migration script targets 3-of-6
/// against that roster. The owner rotation itself is done manually via
/// the Safe UI under the current 1-of-N threshold (no rotation script
/// needed — at threshold 1 a single signer can execute each
/// `addOwnerWithThreshold` / `removeOwner` call via the Safe UI). The
/// signers' addresses are pinned as `address(0)` placeholders until each
/// signer's control of the address they nominate has been verified
/// off-chain (signer-coordination tracker is held privately); placeholders
/// mean every script and fork test that asserts against `expectedOwners()`
/// deliberately fails until verification completes AND the manual roster
/// swap has executed on-chain.
/// @dev Sources:
/// - Safe v1.4.1 L2 singleton & proxy: github.com/safe-global/safe-deployments
/// under `src/assets/v1.4.1/safe_l2.json` (chainId 8453 entry). Both the
Expand Down Expand Up @@ -65,8 +73,8 @@ library LibProdSafes {
address constant SAFE_V1_4_1_COMPATIBILITY_FALLBACK_HANDLER = 0xfd0732Dc9E303f09fCEf3a7388Ad10A83459Ec99;

/// @notice The Safe that owns every ST0x receipt vault on Base. Subject
/// of the threshold migration (1 -> 3, against the current 4-owner
/// roster).
/// of the threshold migration (1 -> 3, against the post-rotation
/// 6-owner roster).
/// https://basescan.org/address/0xe70d821f3462A074E63b42D0aac6523faAe1D611
address constant STOX_TOKEN_OWNER_SAFE = 0xe70d821f3462a074e63b42d0AaC6523faAe1d611;

Expand All @@ -77,25 +85,29 @@ library LibProdSafes {
/// in the same PR that records the live post-execution state.
uint256 constant STOX_TOKEN_OWNER_SAFE_THRESHOLD = 1;

/// @notice Owner #1 of `STOX_TOKEN_OWNER_SAFE`. Order matches
/// `getOwners()` (Safe-internal linked-list order).
address constant STOX_TOKEN_OWNER_SAFE_OWNER_1 = 0x19f95a84aa1C48A2c6a7B2d5de164331c86D030C;
/// @notice Owner #1 of `STOX_TOKEN_OWNER_SAFE`. **PLACEHOLDER**
/// (`address(0)`) until the post-rotation roster is published and each
/// signer's verified address replaces the corresponding literal.
/// @dev Order matches `getOwners()` (Safe-internal linked-list order)
/// against the post-rotation roster. The owner rotation itself is done
/// manually via the Safe UI under the current 1-of-N threshold (no
/// rotation script); only the threshold raise is scripted.
address constant STOX_TOKEN_OWNER_SAFE_OWNER_1 = address(0);

/// @notice Owner #2 of `STOX_TOKEN_OWNER_SAFE`. Order matches
/// `getOwners()` (Safe-internal linked-list order).
address constant STOX_TOKEN_OWNER_SAFE_OWNER_2 = 0x8f6bF4A948Af2Fc74eE34982C4435a7C013D1A52;
/// @notice Owner #2 of `STOX_TOKEN_OWNER_SAFE`. **PLACEHOLDER**.
address constant STOX_TOKEN_OWNER_SAFE_OWNER_2 = address(0);

/// @notice Owner #3 of `STOX_TOKEN_OWNER_SAFE`. Order matches
/// `getOwners()` (Safe-internal linked-list order). This slot previously
/// held the now-removed owner `0x691AcCd4...`; after the 2026-05-18
/// `RemovedOwner` event at block 46156528 the linked list shifted up
/// and what was owner #4 became owner #3.
address constant STOX_TOKEN_OWNER_SAFE_OWNER_3 = 0x91E2AF6Ee6bc5d0f7AA1644Bb94957932629d2DB;
/// @notice Owner #3 of `STOX_TOKEN_OWNER_SAFE`. **PLACEHOLDER**.
address constant STOX_TOKEN_OWNER_SAFE_OWNER_3 = address(0);

/// @notice Owner #4 of `STOX_TOKEN_OWNER_SAFE`. Order matches
/// `getOwners()` (Safe-internal linked-list order). Formerly owner #5
/// before the 2026-05-18 roster reduction.
address constant STOX_TOKEN_OWNER_SAFE_OWNER_4 = 0xBF8a5DE7BaAFaD46495217d467F43ae305cb900f;
/// @notice Owner #4 of `STOX_TOKEN_OWNER_SAFE`. **PLACEHOLDER**.
address constant STOX_TOKEN_OWNER_SAFE_OWNER_4 = address(0);

/// @notice Owner #5 of `STOX_TOKEN_OWNER_SAFE`. **PLACEHOLDER**.
address constant STOX_TOKEN_OWNER_SAFE_OWNER_5 = address(0);
Comment on lines +88 to +107

@coderabbitai coderabbitai Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Name each placeholder slot after its intended signer and hardware brand.

expectedOwners() is order-sensitive and feeds the default LibSafeInvariants.assertAll() path, so these five slots are not interchangeable. Leaving them as generic Owner #1..#5 placeholders makes the post-ceremony hydration step ambiguous; swapping any two real addresses will trip the pinned owner-order invariant and block the migration. The PR objective called out NatSpec-named placeholders for exactly this reason.

🤖 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/LibProdSafes.sol` around lines 88 - 107, The placeholder owner
constants STOX_TOKEN_OWNER_SAFE_OWNER_1..5 must be renamed to explicitly
identify each intended signer and their hardware brand (e.g.,
STOX_TOKEN_OWNER_SAFE_ALICE_LEDGER, STOX_TOKEN_OWNER_SAFE_BOB_TREZOR, etc.) so
the order-sensitive expectedOwners() / LibSafeInvariants.assertAll() path is
unambiguous; update the constant names (keeping the same address(0) placeholders
for now) to the NatSpec-named form that reflects the real signer and device, and
ensure the declaration order matches the post-rotation roster / Safe getOwners()
linked-list order.


/// @notice Owner #6 of `STOX_TOKEN_OWNER_SAFE`. **PLACEHOLDER**.
address constant STOX_TOKEN_OWNER_SAFE_OWNER_6 = address(0);

/// @notice Returns the expected owner set for `STOX_TOKEN_OWNER_SAFE` in
/// the exact order returned by `getOwners()` against an unpinned Base
Expand All @@ -105,14 +117,21 @@ library LibProdSafes {
/// 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 four owners of the ST0x token-owner Safe in
/// @dev Six entries post-rotation. The roster uses a mixed-vendor
/// hardware-wallet policy: the 3-of-6 threshold combined with the
/// vendor mix enforces that no single-vendor subset can reach quorum
/// on its own, so a single-vendor compromise cannot sign a tx without
/// recruiting a different-vendor signer.
/// @return The six owners of the ST0x token-owner Safe in
/// `getOwners()` order.
function expectedOwners() internal pure returns (address[] memory) {
address[] memory owners = new address[](4);
address[] memory owners = new address[](6);
owners[0] = STOX_TOKEN_OWNER_SAFE_OWNER_1;
owners[1] = STOX_TOKEN_OWNER_SAFE_OWNER_2;
owners[2] = STOX_TOKEN_OWNER_SAFE_OWNER_3;
owners[3] = STOX_TOKEN_OWNER_SAFE_OWNER_4;
owners[4] = STOX_TOKEN_OWNER_SAFE_OWNER_5;
owners[5] = STOX_TOKEN_OWNER_SAFE_OWNER_6;
return owners;
}
}
4 changes: 2 additions & 2 deletions test/script/MigrateMultisigThresholdTest.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ contract MigrateMultisigThresholdTest is Test {

// Smoke-check the JSON shape.
string memory bundleName = vm.parseJsonString(json, ".meta.name");
assertEq(bundleName, "ST0x Safe threshold 1->3", "meta.name pinned");
assertEq(bundleName, "ST0x Safe threshold 1->3 (post-rotation roster)", "meta.name pinned");
bool hasFirstTx = vm.keyExistsJson(json, ".transactions[0].to");
bool hasSecondTx = vm.keyExistsJson(json, ".transactions[1].to");
assertTrue(hasFirstTx, "first transaction present");
Expand Down Expand Up @@ -113,7 +113,7 @@ contract MigrateMultisigThresholdTest is Test {
/// If even one receipt vault has its `owner()` pointing somewhere
/// other than the Safe, the migration must abort before producing an
/// artifact (the migration would otherwise lock the wrong Safe into
/// 3-of-4 without controlling the vaults).
/// 3-of-6 without controlling the vaults).
function testRunRejectsVaultOwnershipDrift() external {
selectBaseFork();
address rogueOwner = address(0xBADC0DE);
Expand Down
Loading