Add the orchestrator beacon to the governed beacon set - #334
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`prodBeaconsForChainId` returned three beacons while production has four. The fourth is the orchestrator beacon, which after #324 holds mint/burn authority over the other three, and the governance-timelock migration walks that array — so the most governance-sensitive beacon in the system was the one surface the migration could never reach. Nothing failed: `GovernanceTimelockMigration.t.sol` only checks the enumerated three, so it stayed green. Widened to `address[4]` in `LibProdBeaconsBase` / `LibProdBeacons0_1_1` and `LibBeaconInvariants`, appended so the existing indices hold, with `ORCHESTRATOR_BEACON_INDEX` beside the three named in #325. Every consumer that iterates picks the new member up unchanged. `prodBeaconCodehashesForChainId` is new and index-aligned. The set now spans two build generations — the token beacons are the V1 build at 858 bytes, the orchestrator beacon is OZ 5.6.1 at the current optimizer settings and 728 bytes — so a single pin reverts on it. Both the library's own check and `_selectBeaconTargets` in the timelock migration took one constant; both now take the per-beacon value. Without this the widened migration reverts `MigrationBeaconCodehashMismatch` and no chain migrates at all. `MigrateBeaconOwners` keeps its own three-element list: it is a historical executed script with its own literals rather than a consumer of this accessor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3aa2d5e to
a94712a
Compare
0680737 to
bd0c0d5
Compare
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
The orchestrator beacon joins the governed set mid-lifecycle: its constructor bakes the deploy EOA as owner and the 20260818-migrate-orchestrator-beacon-owner EOA broadcast moves it to the chain's Safe. The set's two ownership sweeps (assertProdBeaconsOwnedBy and assertProdBeaconsOwnershipMigration) now accept that baked pre-state for the orchestrator index until the beacon-owner migration deadline - the same window LibOrchestratorInvariants.assertBeaconSet applies - and hard-require the expected owner after it. Live today on all three chains (closure + instance dispatched, beacon still EOA-owned) the sweeps otherwise red-lined every prod surface; with the window the fleet prod walk reaches the authoring state and proves all 41 production tokens state-preserving across the simulated upgrade per chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbsbYN4C4YDa8pu9DdudoX
a94712a to
2f96db0
Compare
Third surface of the same mid-lifecycle join: the governance-timelock migration's beacon selection required every governed beacon Safe-owned before authoring, so the still-EOA-owned orchestrator beacon tripped UnexpectedBeaconOwner on every chain. The selection now skips it while its own EOA -> Safe migration is pending (same deadline window as the ownership sweeps); a later dispatch picks it up once Safe-owned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbsbYN4C4YDa8pu9DdudoX

Closes #333. Stacked on #325.
The gap
prodBeaconsForChainIdreturned three beacons. Production has four. The fourth is the orchestrator beacon, and after #324 it holds mint/burn authority over the other three — so the most governance-sensitive beacon in the system was the one surface20260729-migrate-governance-to-timelockcould never reach, because it walks that array.Nothing failed.
GovernanceTimelockMigration.t.solonly checks the enumerated three, so it stayed green while the orchestrator beacon sat outside the migration entirely. Left alone past the 2026-10-01 deadline it is stranded, upgradable by whoever owns it, while every other beacon moves under the timelock.What changed
LibProdBeaconsBase/LibProdBeacons0_1_1:beacons()andimplementations()widened toaddress[4], orchestrator beacon and its impl appended so existing indices hold.LibBeaconInvariants:ORCHESTRATOR_BEACON_INDEX = 3beside the three constants Fleet upgrade bundle: receipt + receipt-vault beacons to audited 0.1.30 #325 added;prodBeaconsForChainIdreturnsaddress[4].prodBeaconCodehashesForChainId, new and index-aligned. The set now spans two build generations: the token beacons are the V1 build (858 bytes,0x8e95867e…), the orchestrator beacon is OZ 5.6.1 at the currentoptimizer_runs(728 bytes,0x448cd063…). A single pin reverts on it. Both_assertDeployedPinnedBeaconand_selectBeaconTargetsin the timelock migration hardcoded one constant; both now take the per-beacon value.MigrateBeaconOwnersdeliberately keeps its own three-element list: it is a historical executed script with its own literals, not a consumer of this accessor.Sequencing — this needs #317 broadcast first
The orchestrator beacon is still owned by the deploy EOA
0x8E4bdeec…on all three chains; #317 transfers it to the chain Safe and has not been run. Until it is,assertProdBeaconsOwnedByChainSafefails on the new member, because it requires one owner across the set.That is sequencing, not a code gap, and it is verified rather than assumed. Simulating #317 - a single
transferOwnershippranked from the EOA - then asserting the widened set:Draft until #317 has been broadcast on Base, Ethereum and HyperEVM.
QA
testRunAuthorsFullMigration,testRunAuthorsFullMigrationOnEthereum,testRunAuthorsFullMigrationOnHyperevm,testGovernanceCanMigrateAwayToSuccessor,testVerifyAcceptsAuthoredArtifactAndRejectsTamperall die withMigrationBeaconCodehashMismatch(0xb9DCd744…, 0x8e95867e…, 0x448cd063…). After it, all five pass. The remaining 8 failures are the owner precondition above, cleared by Orchestrator beacon-owner migration to the chain Safe #317 and demonstrated by the pranked-transfer run.prodBeaconCodehashesForChainId-> returnUPGRADEABLE_BEACON_CODEHASHin all four slots -> the five migration tests above die again with the same selector._selectBeaconTargets-> revertexpectedCodehashes[i]to the single constant -> same five die. Appending vs inserting the new member was checked by keepingRECEIPT_BEACON_INDEX/RECEIPT_VAULT_BEACON_INDEXat 0/1 and confirming Fleet upgrade bundle: receipt + receipt-vault beacons to audited 0.1.30 #325`s fleet tests still pass.0xb9DCd744b0413Dff0EDC70A5B229c7aa03734613carries 728 bytes on Base, Ethereum and HyperEVM; the V1 fleet beacon0x86e93c39…carries 858 on Base. Its owner reads0x8E4bdeec…(the deploy EOA) on all three, and the V1 beacons read0xe70d821f…(the token-owner Safe) — which is why the set is not uniform until Orchestrator beacon-owner migration to the chain Safe #317 runs.