Orchestrator beacon owner is pinned to the hot deploy EOA as permanent expected state, and the timelock migration structurally cannot reach it

[thedavidmeister]

From the adversarial audit of #316 at pr:316 scope (survived refutation; 9 of 56 findings survived).

assertBeaconSet pins the orchestrator beacon owner to BEACON_INITIAL_OWNER = 0x8E4bdeec... - documented across this repo as "rainlang.eth, the deploy EOA" - and asserts it as the permanent expected state, with no migration window and no deadline. Cron CI therefore never red-lines it.

Every other beacon surface has moved the other way:

• LibBeaconInvariants.sol:93-101 explicitly warns "do not conflate the two" against PROD_BEACON_OWNER = STOX_TOKEN_OWNER_SAFE.
• Other surfaces assert through assertProdBeaconsOwnershipMigration(pre, post, deadline).
• CLAUDE.md states production onlyOwner surfaces are held by a governance timelock.

The timelock migration cannot pick this beacon up, by construction rather than by omission:

• LibBeaconInvariants.prodBeaconsForChainId returns a fixed address[3].
• 20260729-migrate-governance-to-timelock.s.sol captures address[3] beaconImplsBefore.
• The orchestrator beacon is a fourth beacon, outside that array.
• GovernanceTimelockMigration.t.sol consequently stays green past the 2026-10-01 deadline, because it only checks the enumerated three.

Impact once #324 lands: the orchestrator holds DEPOSIT/WITHDRAW on every production vault, so a single upgradeTo from that one EOA key replaces the implementation holding mint/burn authority across Base, Ethereum and HyperEVM. The pin as written asserts that arrangement is correct forever.

Widening the address[3] to include the orchestrator beacon is the part the migration needs; the pin also needs a deadline so CI fails if the handover has not happened.

Related: #326 (the same audit, the instance address being claimable now).

[thedavidmeister]

Decided: the fix is NOT widening prodBeaconsForChainId, and it does not belong in #317.

Sequencing is deliberate and stays as-is. #317 targets the token-owner Safe on purpose. Read from Base: the governance timelock 0x48ba1371… owns nothing yet, the V1 receipt beacon 0x86e93c39… is owned by 0xe70d821f… (STOX_TOKEN_OWNER_SAFE), and the orchestrator beacon 0xb9dcd744… is owned by 0x8e4bdeec… (the deploy EOA). The whole production fleet is Safe-owned. Routing this one beacon to the timelock early would make it the only surface under a 48h delay (TIMELOCK_MIN_DELAY, LibTimelockInvariants.sol:104), ahead of every beacon beside it, and would force the rest of the orchestrator setup — the #324 cutover, the #325 fleet upgrade, any bring-up fix — into schedule → 48h → execute cycles one step at a time. Finish setup under the Safe, move the fleet once. CodeRabbit recommended the opposite on #317 and was answered there with this evidence.

Why not widen the array. 20260729-migrate-governance-to-timelock is typed to address[3] at capture (:311), post-state assert (:456-460) and the ownership transfer (:553), so the orchestrator beacon cannot enter it. But widening that specific accessor is wrong twice over:

1. Fleet upgrade bundle: receipt + receipt-vault beacons to audited 0.1.30 #325 declares its own address[3] memory beacons = prodBeaconsForChainId(...) and indexes beacons[0]/[1] for the fleet-upgrade interlock. Widening in Orchestrator beacon-owner migration to the chain Safe #317 breaks Fleet upgrade bundle: receipt + receipt-vault beacons to audited 0.1.30 #325 and cascades a rebase through a reviewed stack.
2. prodBeaconsForChainId is the token beacon set — receipt, receipt vault, wrapped token vault — and every consumer indexes it positionally on that basis. The orchestrator beacon is a different class; making it element [3] pushes it into the fleet-upgrade interlock, the cross-chain parity tests and both ownership tests, none of which want it.

The shape instead: a separate accessor for "beacons under governance", which is the set the timelock migration actually needs and is not the same set as "the three token beacons". Separate PR against main, after the stack lands.

This is not optional and it is time-boxed. It must land before 20260729-migrate-governance-to-timelock runs. If the fleet migration executes first, the orchestrator beacon is skipped silently — GovernanceTimelockMigration.t.sol only checks the enumerated three and stays green — and it is stranded Safe-owned with no deadline forcing the hop. Deadline on the fleet migration is 2026-10-01.

[thedavidmeister]

Correcting the previous comment: the "different class of thing" argument was wrong, and it was the weaker half of what I wrote.

ST0xOrchestrator is documented as the "Singleton mint/burn proxy for the whole ST0x receipt-vault set. One instance (behind a beacon proxy) serves every token... It holds the vault-side DEPOSIT + WITHDRAW roles." It is not a separate category sitting beside the token beacons — it is the authority layer over them, and after #324 it holds mint/burn for the entire fleet. That makes its beacon the most governance-sensitive of the four, not the one with the weakest claim to being in the governed set.

So there is no case for a parallel "governed beacons" accessor on category grounds. The production beacon set has four members and prodBeaconsForChainId is under-sized; that is the whole defect.

What survives from the previous comment:

• The sequencing. Unchanged and still evidence-backed: the timelock owns nothing today, the fleet is Safe-owned, and moving one beacon early would put the newest component alone under a 48h delay while the rest of setup still needs doing. Orchestrator beacon-owner migration to the chain Safe #317 targets the Safe correctly.
• Not in Orchestrator beacon-owner migration to the chain Safe #317. Still correct, but for a mechanical reason only, not a conceptual one: Fleet upgrade bundle: receipt + receipt-vault beacons to audited 0.1.30 #325 declares its own address[3] memory beacons = prodBeaconsForChainId(...) and indexes beacons[0]/[1], so widening the return type breaks a stacked, reviewed PR. That is an argument about ordering, not about what belongs in the set.
• The deadline. Unchanged. This must land before 20260729-migrate-governance-to-timelock runs, or the orchestrator beacon is silently skipped and stranded Safe-owned.

Positional indexing is the thing to be careful about when widening — beacons[0]/[1] are load-bearing in #325 and in the migration's own capture/assert/transfer — so the orchestrator beacon should be appended, never inserted.

[thedavidmeister]

Refining the fix shape again — "widen address[3] to address[4]" was the wrong prescription, and the accessors added in #325 (95bc67c) are a half-measure by design.

Those accessors (receiptBeaconForChainId / receiptVaultBeaconForChainId) still do prodBeaconsForChainId(chainId)[RECEIPT_BEACON_INDEX] internally. They reduce the number of files that know the layout from five to one, which unblocks this issue, but the representation is still a tuple whose meaning is carried by position. The index constants they need are the tell.

Looking at what the consumers actually want:

• _selectBeaconTargets (20260729-migrate-governance-to-timelock.s.sol:552-566) iterates the whole set, checking codehash and owner on each. It needs completeness, not order.
• 20260729-migrate-governance-to-timelock.t.sol:380 and :424 take [0] meaning "the receipt beacon". They need a name, not a position.

Nothing needs an array whose index carries meaning. So the fix is a named struct rather than a wider array:

• fields for receipt, receipt vault, wrapped token vault, and the orchestrator beacon
• the migration iterates the struct, so a new field is picked up automatically instead of requiring every consumer to agree about a new length
• RECEIPT_BEACON_INDEX / RECEIPT_VAULT_BEACON_INDEX are deleted; the role accessors read fields
• the two test sites read the named field instead of [0]

Scope note, correcting an earlier comment on this issue: I said widening would cascade through the open stack. That was only true of #325, which is now decoupled. 20260729-migrate-governance-to-timelock and its tests are on main and are not in the stack, so this change is free to touch them. The reason it belongs here rather than in #325 is scope, not breakage.

Deadline is unchanged: this must land before the fleet migration runs, or the orchestrator beacon is silently skipped and stranded Safe-owned.

[thedavidmeister]

Withdrawing the struct suggestion — it does not work. Solidity has no reflection, so a struct cannot be iterated. _selectBeaconTargets would have to hand-build an array from named fields, and that construction site is precisely where a fourth beacon gets forgotten. The struct relocates the fragility rather than removing it.

Array plus named index constants is the right shape, which is what #325 (95bc67c) already put in place. _selectBeaconTargets loops i < beacons.length, so the work here is:

1. append the orchestrator beacon to LibProdBeaconsBase.beacons() and LibProdBeacons0_1_1.beacons(), widening to address[4]
2. add ORCHESTRATOR_BEACON_INDEX = 3 beside the existing role constants
3. the migration picks it up with no further change, because it iterates rather than indexing

One obstacle in that loop that has to be solved at the same time. _selectBeaconTargets (20260729-migrate-governance-to-timelock.s.sol:557-561) checks every beacon against a single constant:

if (codehash != LibBeaconInvariants.UPGRADEABLE_BEACON_CODEHASH) {
    revert MigrationBeaconCodehashMismatch(...);
}

That is the V1 pin 0x8e95867e… (858-byte build). The orchestrator beacon is 0x448cd063… (728 bytes, OZ 5.6.1 at optimizer_runs = 2000) — measured on Base, Ethereum and HyperEVM. So the moment it enters the array, the migration reverts MigrationBeaconCodehashMismatch on it and no chain migrates at all.

The loop needs a per-beacon expected codehash rather than one constant, in the same way #317 (72f3c81) made assertBeaconInvariants take the codehash as a parameter instead of hardcoding V1. An index-aligned expected-codehash array beside the beacon array is the obvious fit, since both are already positional and the index constants now name the positions.

Previous two comments on the fix shape are superseded by this one.

[thedavidmeister]

Final shape, superseding the struct proposal above.

#325 is now 0680737: the role accessors from 95bc67c are reverted, the array keeps its address[3] shape, and the bare indices are replaced with RECEIPT_BEACON_INDEX / RECEIPT_VAULT_BEACON_INDEX / WRAPPED_TOKEN_VAULT_BEACON_INDEX in LibBeaconInvariants.

Why the accessors were wrong: they still did prodBeaconsForChainId(chainId)[INDEX] internally, so the positional coupling moved into the library rather than going away — and they broke the unit test, which then built its expectation from the same accessor the script calls, so the assertions would have held for whatever that accessor returned.

Why the struct was wrong: Solidity cannot iterate a struct. _selectBeaconTargets would have to hand-build an array from named fields, and that construction site is exactly where a fourth beacon gets forgotten. The array iterates for free.

So the work here is:

1. append the orchestrator beacon in LibProdBeaconsBase.beacons() and LibProdBeacons0_1_1.beacons(), widening the return to address[4]
2. add ORCHESTRATOR_BEACON_INDEX = 3 beside the three existing constants
3. _selectBeaconTargets picks it up unchanged, because it loops i < beacons.length
4. give that loop a per-beacon expected codehash — it currently checks every beacon against UPGRADEABLE_BEACON_CODEHASH (the V1 pin 0x8e95867e…, 858-byte build), and the orchestrator beacon is 0x448cd063… (728 bytes), so it would revert MigrationBeaconCodehashMismatch and take down every chain's migration. Orchestrator beacon-owner migration to the chain Safe #317 solved the same problem for assertBeaconInvariants by taking the codehash as a parameter; an index-aligned expected-codehash array is the fit here, now that the indices are named.

Deadline unchanged: must land before the fleet migration runs.