Skip to content

feat(l1): v6 rollup upgrade deploy script, payload and runbook - #25496

Open
aminsammara wants to merge 13 commits into
nextfrom
payload-v6
Open

aminsammara wants to merge 13 commits into
nextfrom
payload-v6

Conversation

@aminsammara

@aminsammara aminsammara commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Adds the deployment path for the v6 rollup upgrade. Not ready to merge — see Outstanding below.

The main thing to review is the _config() literal in DeployRollupForUpgradeV6.s.sol. It is the complete set of deploy-time rollup configuration, one line per knob, and reviewing it is sufficient to review what the rollup will be.

What it deploys

DeployRollupForUpgradeV6.s.sol, in one broadcast:

  1. HonkVerifier
  2. Rollup — owned by the deployer at construction, which also constructs its Inbox, Outbox, FeeJuicePortal, Slasher + SlashingProposer and a fresh RewardBooster
  3. EscapeHatch, then setEscapeHatch — done here, while the deployer still owns the rollup, because that call is onlyOwner and one-shot
  4. transferOwnership(governance)
  5. V6UpgradePayload (+ a replacement FlushRewarder on chains that have one)

Nothing becomes canonical until governance executes the payload.

Configuration

  • Values track v5 production; each line records the v5 value so any intentional v6 divergence reads as a divergence.
  • Mainnet is the baseline, Sepolia a short override list, and an unrecognised chain reverts rather than silently deploying mainnet values (including the mainnet slashing vetoer).
  • REGISTRY_ADDRESS is the only environment input — fee asset, staking asset, GSE, governance and reward distributor are all derived from it and asserted against the outgoing rollup.

Verification

verify(address) asserts every configured value back off the deployed contracts: genesis roots, timings, committee, staking, entry queue, fees, rewards (including getRegistryRewardOverrides), the full slashing stack via Slasher/SlashingProposer immutables, the escape hatch, Inbox/Outbox back-pointers and versions, and that governance owns the rollup.

V6UpgradeSimulation.sol then runs the payload through the real governance lifecycle (GSEPayload wrap → propose → vote → execute) against a state snapshot and reverts it. It proves the actions execute correctly; it does not predict whether a proposal would pass, since a simulation-only voter is given a majority.

What the payload does

# Action Why
1 Registry.addRollup makes v6 canonical
2 GSE.addRollup existing attesters follow without redepositing
3 oldFlushRewarder.recover(...) carries the flush incentive over; skipped where there is no rewarder

No reward-distributor action is needed: it resolves the canonical rollup live off the registry, so its implicit pool follows v6 automatically.

Outstanding

  • vkTreeRoot, protocolContractsHash, genesisArchiveRoot — zero; run() refuses to deploy until set
  • initialEthPerFeeAsset — stale dev value, needs refreshing at deploy time
  • Registry reward override registries and amounts, mainnet and Sepolia
  • Open question in-file on the Sepolia entry-queue bootstrap set size

Notes

  • Requires generated/HonkVerifier.sol from a noir-projects bootstrap. Do not substitute a stub — it compiles and accepts every proof.
  • The protocol fee margin and recipient keep their constructor values (0 and a placeholder). Setting them is a governance call and belongs in the payload; the recipient must be set before any non-zero margin.
  • V6_UPGRADE_RUNBOOK.md covers build, inputs, pre-flight checks, dry run, deploy, proposal and post-execution checks.

Adds the deployment path for the v6 rollup upgrade.

- DeployRollupForUpgradeV6.s.sol deploys the verifier, the rollup, the
  escape hatch and the governance payload. Every configuration value is
  a literal in _config(), branched for mainnet and Sepolia, so there are
  no env-var defaults and no network-defaults.json fallbacks that could
  change what is deployed; REGISTRY_ADDRESS is the only environment
  input. verify() reads each value back off the deployed contracts and
  asserts it matches the table.
- V6UpgradePayload.sol registers the rollup with the Registry and the
  GSE, and moves the outgoing flush rewarder's unowed balance into a
  replacement bound to the new rollup.
- V6UpgradeSimulation.sol runs the payload through the real governance
  lifecycle against a state snapshot and reverts it, so a deploy cannot
  succeed while producing a payload that would fail.
- V6_UPGRADE_RUNBOOK.md documents the build, inputs, pre-flight checks,
  deploy, proposal and post-execution verification.

The escape hatch is deployed and registered before ownership moves to
governance, because setEscapeHatch is onlyOwner and one-shot.

Not deployable as-is: the genesis roots, initialEthPerFeeAsset and the
registry reward override values are left as TODOs.
@chugarchugarr

Copy link
Copy Markdown

There is one migration invariant in the runbook that looks stronger than the payload currently enforces.

The runbook says totalEarmarkedBalance must be zero, explicitly notes that subsidizeAddress is permissionless, and instructs operators to re-check immediately before execution because that value can change while the governance proposal is pending.

If zero is intended to be a hard precondition for the “reward distributor follows the canonical rollup automatically” argument, I think it should be part of the executable transition rather than only an operator procedure.

The payload already uses exactly this pattern for the execution window: check the precondition as the first action so the whole governance execution reverts before any state mutation.

Could the reward-distributor assumption be treated the same way?

totalEarmarkedBalance != 0
=> payload execution must fail before Registry.addRollup

(or the narrower predicate that captures the intended migration condition, if totalEarmarkedBalance == 0 is stronger than necessary).

A regression case would be useful:

  1. build/propose the exact payload while the precondition holds;
  2. mutate the earmarked state during the governance delay;
  3. execute the unchanged payload;
  4. assert it reverts atomically before v6 becomes canonical.

That would make the state-inheritance assumption part of what governance actually authorizes, rather than something that can drift between review and execution.

@iAmMichaelConnor

Copy link
Copy Markdown
Contributor

Hi! Some review comments as I was understanding some other things relating to L1 contracts and v6:

V6 upgrade PR: test coverage

Nothing in CI exercises V6UpgradePayload or the config table. V6UpgradeSimulation is the only thing that runs the payload, and it needs a fork, so it runs once, on deploy day. test/governance/scenario/AddRollup.t.sol and test/script/DeployRollupForUpgrade.t.sol cover the generic path and are the natural templates; nothing covers the V6 contracts themselves. V5 shipped test/periphery/V5UpgradePayload.t.sol and V5UpgradePayloadFork.t.sol; V6 should have at least the equivalent. Concretely:

  1. test/periphery/V6UpgradePayload.t.sol — unit tests, no fork.

    • getActions() shape for all four constructor combinations (window on/off × old rewarder present/absent): lengths 4/3/3/2, and order assertWithinExecutionWindow → Registry.addRollup(ROLLUP) → GSE.addRollup(ROLLUP) → oldRewarder.recover(asset, NEW_FLUSH_REWARDER, rewardsAvailable()). Decode the calldata and assert the selectors and arguments, not just the targets.
    • rewardsAvailable() is read at call time: change the old rewarder's balance between two getActions() calls and assert the recover amount moves.
    • Constructor: reverts with V6UpgradePayload__FlushRewarderRollupMismatch when the supplied rewarder serves a rollup other than the current canonical one; NEW_FLUSH_REWARDER is zero when the old one is zero; otherwise it mirrors REWARD_ASSET and rewardPerInsertion, is bound to the new rollup and owned by governance.
    • isWithinExecutionWindow boundary vectors: Mon 07:59 vs 08:00 and Fri 16:59 vs 17:00 London, in GMT and in BST (so 06:59/07:00 UTC in summer); any hour on Sat/Sun; the BST transition instants themselves (01:00 UTC on the last Sunday of March and October); a year where the last Sunday of March is the 25th and one where it is the 31st; a leap day. Plus a fuzz round-trip _yearFromDays(_daysFromCivil(y, m, d)) == y, since the date math is hand-rolled.
    • assertWithinExecutionWindow reverts with V6UpgradePayload__OutsideExecutionWindow outside the window and passes inside.
  2. Governance lifecycle without a fork — an AddRollup.t.sol-style test for V6UpgradePayload on a RollupBuilder stack (the CI version of V6UpgradeSimulation): propose → vote → execute and assert the registry's canonical rollup, GSE membership, and the flush-reward balance movement. Include the window behaviour end to end: execute outside the window reverts, the proposal is still Executable, warp inside, execute succeeds.

  3. test/script/DeployRollupForUpgradeV6.t.sol — the config table. Today nothing checks that _config() even produces a deployable rollup. run() refuses while the genesis roots are zero, and the literals can't be overridden, so make _config() virtual (or take the roots from a test hook) and have a test deploy against a local stack under vm.chainId(1) and vm.chainId(11155111), then call verify(). That catches what the constructors enforce — escape-hatch constraints, the reward-override ≤ checkpointReward × sequencerBps ceiling, duplicate registries — and any typo in the Sepolia override list, both of which are currently found only on deploy day.

Also, briefly

  • The runbook could point at script/UniswapLookup.s.sol for refreshing initialEthPerFeeAsset — it's the only thing in-tree that computes ethPerFeeAssetE12, and the table has it as a TODO.
  • State when the V6 files leave next (at the v6-next cut, or after execution) and add a one-line header to the three files saying so.

FYI — governance paperwork. AZUP-2 (v5) is now filed retroactively at AztecProtocol/governance#71 with every deployed address and rollup parameter verified on-chain, and a draft AZUP-3 (v6) is open at AztecProtocol/governance#72.

iAmMichaelConnor pushed a commit that referenced this pull request Sep 21, 2026
…deployer, not the mainnet path (#25519)

## Summary

Comment-only change. `l1-contracts/script/deploy/`
(`DeployAztecL1Contracts.s.sol`, `DeployRollupForUpgrade.s.sol`,
`DeployRollupLib.sol`, `RollupConfiguration.sol`) is the env-driven
deployer for tests, spartan/CLI and testnets. Nothing in the tree says
so, and a reader who finds `REAL_VERIFIER` defaulting to a mock verifier
or genesis roots defaulting to zero can reasonably conclude that mainnet
is one unset variable away from a bad deployment.

It is not. Mainnet's one-off contracts were deployed from
`AztecProtocol/ignition-contracts`, and each mainnet rollup version is
deployed by a bespoke pinned script, `DeployRollupForUpgradeV<N>.s.sol`,
that hard-codes and re-verifies that deployment's configuration: v5 from
`DeployRollupForUpgradeV5.s.sol` on the `v5-next` branch, v6 from
`DeployRollupForUpgradeV6.s.sol` (#25496).

## Change

- A doc comment on each of the four generic script files stating what
they are for, that mainnet uses the pinned per-version scripts, and that
the env defaults are test-network conveniences.
- One sentence on the root `CLAUDE.md` line that introduces
`l1-contracts/`, so agents working in the tree do not classify
generic-script defaults as mainnet findings.

No bytecode, behaviour, gas or test changes. `forge fmt --check` passes
on the four files.

---
*Created by
[claudebox](https://claudebox.work/v2/sessions/c27ea1045602719d/jobs/43)
· group: `slackbot` · requested by Mike (@iAmMichaelConnor) · [Slack
thread](https://aztecfoundation.slack.com/archives/D0B2N7W1WJD/p1789737469555569?thread_ts=1789737469.555569&cid=D0B2N7W1WJD)*
@iAmMichaelConnor

Copy link
Copy Markdown
Contributor

Also, #25520 adds some safety checks to the default deployments scripts, which these prod deployments scripts (such as the v6 scripts in this PR) might wish to consider.

@iAmMichaelConnor

Copy link
Copy Markdown
Contributor

V6UpgradePayload should probably adopt the approach of AztecProtocol/aztec-packages-private#830.

It's ~10 lines and fits the structure you already have:

  • store PREDECESSOR = _registry.getCanonicalRollup() as an immutable in the
    constructor (you already read it there for the flush-rewarder check);
  • add assertPredecessorIsCanonical() that reverts unless PREDECESSOR is still
    REGISTRY.getCanonicalRollup();
  • make it a self-targeted action ahead of Registry.addRollup, exactly like
    assertWithinExecutionWindow, so a stale payload reverts the whole proposal
    before anything is written.

Two things for V6_UPGRADE_RUNBOOK.md to go with it:

  1. A pre-signalling check. Before anyone signals or votes for a registration
    payload, verify on-chain that (a) its getActions() includes the
    assertPredecessorIsCanonical action, and (b) its PREDECESSOR() equals
    Registry.getCanonicalRollup(). A payload without the guard is exactly the
    stale-executable hazard the fix removes, and both facts are readable with
    cast call before anything is committed to.

  2. The operational consequence of the guard. Once any other registration lands,
    this payload can never execute, even if its proposal was accepted; it has to be
    redeployed (picking up the new predecessor) and taken through governance again.
    Intended, but worth stating so nobody is surprised mid-upgrade.

The 830 body has the gas numbers (≈ +10k on execute, one-off per upgrade) and the
regression test that fails against the unguarded payload and passes with the guard.

Registration is append-only with last-write-wins, and `Governance.execute` is permissionless and
only checks that a proposal is `Executable`. So two accepted registrations are a hazard rather
than a queue: if this payload is abandoned after the vote — a bug found in the rollup, a
replacement proposed — and the replacement executes first, anyone can then execute this one and
move both `Registry.getCanonicalRollup()` and GSE latest back to v6. The demotion is permanent,
because neither registry re-admits a rollup it already holds, and the bonus validator set follows
in the same transaction.

The window is narrow and sits exactly where it hurts. With mainnet timings a proposal is
executable from 40 to 47 days after creation, so two registrations overlap only if the second is
created within 7 days of the first — which is precisely what "find a bug in v6, patch it quickly"
produces. Fast remediation is what opens the hole.

The payload now records the canonical rollup at deployment as `PREDECESSOR` and asserts it is
still canonical before anything is written, so it only ever authorises the transition voters
actually approved: from that rollup, to this one. A stale payload reverts the whole execution and
changes nothing.

Placed FIRST, ahead of the execution-window check. Both are non-mutating so the order is free,
and between "this payload must never run" and "come back on Monday" the first is the one worth
surfacing.

Enforced as a self-targeted action rather than a require in `getActions`, matching the window
check for the same reason: a stale payload must stay READABLE. Explorers, `GSEPayload.amIValid`
and the deploy script's simulation all call `getActions`, and a payload that cannot describe
itself once it can no longer execute is worse to diagnose, not better.

The constructor already read `getCanonicalRollup()` for the flush-rewarder check; that read is
now unconditional and shared, so both checks refer to the same rollup by construction. One
consequence: deploying against a registry with no canonical rollup now reverts. This payload
succeeds a rollup and was never usable for a first registration.

The runbook carries the operational half — the pre-signalling checks that the guard is in the
action list and the predecessor is still canonical, and the consequence nobody should meet by
surprise: once any other registration lands this payload is dead, and registering v6 then needs a
fresh deploy and a full governance cycle, not a quick swap.

Mirrors the approach reviewed in aztec-packages-private#830 for
`RegisterNewRollupVersionPayload`, which that PR notes does not cover this payload.

Not yet tested: `test/` does not compile without `generated/HonkVerifier.sol`, so the revert path
is verified by review and compiler only. A regression test follows.
…endar

The payload had no test at all. `V6UpgradeSimulation` is the only thing that ran it, and it needs a
fork, so it runs once — on deploy day, against the state it is about to change. That is the wrong
moment to learn that the action list is malformed.

18 tests, no fork, no RollupBuilder: a real Registry plus a stub rollup is all the payload's action
list and preconditions actually need, and keeping the rollup out makes the suite cheap enough to run
on every change.

The predecessor guard is covered red-green. Removing the guard action from `getActions` fails seven
of these; removing only its wiring while keeping the function fails the ordering test alone, which is
the split that was wanted — one test owns the predicate, another owns the fact that governance is
made to call it. The stale-payload case also asserts `getActions()` still RETURNS once the payload
can no longer execute, since a payload that cannot describe itself is worse to diagnose, not safer.

The action list is checked by decoded arguments, not just targets and lengths: the addresses inside
`addRollup` and `recover` are the part that would be silently wrong. `recover`'s amount is asserted
to track the rewarder's balance between two calls, pinning that it is read when the list is built
rather than when the payload is deployed.

THE CALENDAR IS CHECKED AGAINST THE REAL CALENDAR. The date math is hand-rolled — no library, no
tabulated DST table — so a Solidity reimplementation would only reproduce whatever bug it has. The 23
vectors come from the IANA Europe/London database instead: both sides of 08:00 and 17:00 in GMT and
in BST, weekends, three leap days including one on a Sunday, and the Monday after BST starts in a
year where March's last Sunday is the 25th and in one where it is the 31st, which is what exercises
`_lastSundayOfMonth` at both ends. The transition instants themselves are Sundays, so they are closed
by the weekday rule regardless; the first observable consequence of a wrong offset is the Monday
after, and that is what is asserted.

The fuzz asserts nine open hours on a weekday and none at a weekend, a property independently
verified against the tz database across the same six years. It constrains the window's shape, not its
alignment — dropping the BST offset entirely still leaves nine hours a day and still passes — so the
vectors catch that mutation and the property catches shape errors the vectors would miss. Both are
kept, and the file says why.

Not covered here, from the same review: the governance-lifecycle test on a RollupBuilder stack, and
`DeployRollupForUpgradeV6.t.sol` for the config table.

Runs under the coverage profile locally (`FOUNDRY_PROFILE=coverage`), which remaps the verifier to
the mock; the default profile needs `generated/HonkVerifier.sol` from a noir-projects bootstrap,
which no test here touches.
Guarding the transition is only worth anything if a failed guard leaves NOTHING behind. If a
rejected execution could still land `Registry.addRollup` and stop before the rest, the guard would
have converted one hazard into a worse one: v6 canonical, the GSE never told, the flush incentive
stranded on a rewarder nobody will call again.

It cannot. `Governance.execute` is a loop of `target.call(...)` with `require(success)` per action
inside a single transaction, so a failure unwinds everything, including the actions that already
succeeded and the `Executed` flag written before the loop. That is a property of the code rather
than of our ordering, and the tests assert it both ways:

`test_StalePayloadExecutesNothingAtAll` runs the real thing — RollupBuilder stack, signal, vote,
warp to Executable — then moves canonical out from under the queued proposal and executes. It
asserts the absence of EVERY later action's effect, not just the first: canonical unchanged, version
count unchanged, GSE latest unchanged, old rewarder not drained, new rewarder not funded. It also
asserts the proposal is still `Executable` afterwards, since the rolled-back flag is what lets a
rejected attempt be retried rather than burning the proposal.

Canonical is moved after the proposal is queued, pranked as the registry's owner. Both details are
forced: signalling resolves proposers off whatever rollup is canonical, so a stub canonical before
the round breaks the proposer machinery rather than the payload; and how canonical moved is
irrelevant to a guard that only compares.

`test_AFailureAfterAddRollupRollsBackTheRegistration` pins the general property with a payload built
to fail in its SECOND action, after a real `addRollup` succeeded in its first. The registration is
gone when the transaction ends. This one does not involve the guard at all, and deliberately so:
atomicity is what makes the guard's position a readability choice rather than a safety one.

Red-green: removing the guard action fails the first test with "next call did not revert as
expected" — the stale payload executes clean through and takes canonical with it — and leaves the
second passing, which is the split that was wanted.
Nothing checked that `_config()` produces a rollup that can actually be deployed. The literals are
reviewed by eye, and the first thing that tests them is `run()` on deploy day, against mainnet, with
a broadcast in flight. The constructors have opinions — escape-hatch bounds, the reward-override
ceiling, duplicate registries, field-element ranges — and every one of them was being discovered at
the worst possible moment.

This deploys the table against a local stack under `vm.chainId(1)` and `vm.chainId(11155111)` and
calls `verify()` on the result, plus the two paths that should fail: an unsupported chain, and the
real unmodified script refusing while the genesis roots are zero.

WHAT THIS DOES AND DOES NOT CATCH. `verify()` reads the deployed rollup back against `_config()` —
the same table — so a wrong literal propagates to both sides and verifies clean. It cannot tell you
a value is wrong, only that the value is internally consistent and that the constructors accept it.
Judging the numbers stays a review question, which is what the v5 comparison on each line is for.

Three production changes, each forced by something a local stack cannot do:

- `_config()` is `virtual`, so the harness can supply the three genesis roots. `run()` still refuses
  while they are zero, and a test asserts that against the unmodified script.
- `deployedRollup` / `deployedPayload` are exposed, mirroring `rollupOutput()` on the sibling script.
  `run()` only logged them, so nothing could read back what it built.
- the governance simulation moved behind a `virtual _simulate`, because it needs forked state — real
  voters, mainnet timings — and is the one step of `run()` a local stack cannot satisfy. It is
  skipped here and runs on deploy day, against a fork, unchanged.

Three things the test had to discover, all now written down where the next person will hit them:
the genesis roots are FIELD ELEMENTS, and a stand-in that is not reduced fails with
`Rollup__FieldElementOutOfRange` for a reason unrelated to the config; the GSE thresholds are
asserted against the GSE that already exists rather than set, so the local stack is raised to
production values instead of weakening one of the few assertions that catches a rollup pointed at
the wrong GSE; and registry ownership stays with the real Governance, since `run()` resolves
`getGovernance()` and drives the payload through it.

Mainnet's config names the v5 flush rewarder by address, so the test etches one bound to the outgoing
rollup at that literal address rather than editing the table to point somewhere convenient.

Runs under `FOUNDRY_PROFILE=coverage`, which supplies the mock verifier; the default profile needs
`generated/HonkVerifier.sol` from a noir bootstrap, and no assertion here touches proof verification.
Everything needed to review this payload existed, spread across the contract's NatSpec, the
runbook's "deliberately not done", two PR descriptions and a thread. Both reviewers so far arrived
with questions the code does answer — but only to someone who reads the whole file first. This is
the page that answers them in one screen.

Deliberately NOT a restatement of the contract. It carries what a reviewer needs and the code
cannot conveniently say: the action list and when each action is present, what the payload
guarantees, what it leaves alone on purpose, the two `cast call`s to run before signalling, and the
limits — stated here rather than discovered in review.

The limits are the point. That a stale payload is permanently dead is intended but expensive during
an incident. That `totalEarmarkedBalance` is deliberately NOT enforced on-chain — because
`subsidizeAddress` is permissionless and a 1-wei call would otherwise block the upgrade forever — is
a decision that reads as an omission unless someone writes down that it was a decision. Both belong
in front of a reviewer, not in a thread.

It sits next to the contract rather than in the package README, which is about building and testing
l1-contracts and has nothing to say about any one contract. The runbook now points at it: the person
running the upgrade and the person reviewing it want different documents, and the runbook is the
operator's.
iAmMichaelConnor pushed a commit that referenced this pull request Sep 22, 2026
…on (#25520)

## Independent of the v6 upgrade PR (#25496)

This PR is **not** a dependency of, and does not depend on, the v6
upgrade PR #25496. Either can merge first, and neither needs a rebase on
the other.

- **No shared source.** `DeployRollupForUpgradeV6.s.sol`,
`V6UpgradeSimulation.sol` and `V6UpgradePayload.sol` import only `src/`
contracts, the generated `HonkVerifier`, and each other. None of them
imports `DeployRollupLib`, `RollupConfiguration`,
`DeployRollupForUpgrade` or `DeployAztecL1Contracts`, which are the only
Solidity files this PR changes. The V6 script's one mention of
`RollupConfiguration` is a comment about version derivation, and this PR
does not touch that function.
- **No shared tooling.** The V6 runbook runs `forge script` on the V6
files directly. It does not use `run_rollup_upgrade.sh`,
`test_rollup_upgrade.sh` or `stress_test_deploy.sh`, which are the shell
scripts this PR edits.
- **No contract change.** Nothing under `src/` changes, so the bytecode
v6 deploys is identical with or without this PR.

## Problem

`l1-contracts/script/deploy/` is the env-driven deployer for tests,
spartan/CLI and testnets (mainnet versions use the pinned
`DeployRollupForUpgradeV<N>.s.sol` scripts, which hard-code and verify
their configuration). The generic path forwarded whatever the
environment supplied straight into the `Rollup` constructor, and the
constructor accepts values that leave the deployed instance unusable or
mis-bound. Several defaults were also fail-open: a missing
`REAL_VERIFIER` bound a `MockVerifier`, and a missing genesis root
silently became zero.

## Change

All in the deployer, its wrappers and its tests.

`script/deploy/RollupConfiguration.sol`
- `REAL_VERIFIER` defaults to `true`; a stub verifier must be requested
explicitly.
- `VK_TREE_ROOT`, `PROTOCOL_CONTRACTS_HASH` and `GENESIS_ARCHIVE_ROOT`
must be set and non-zero.
- `NETWORK=mainnet` requires chain id 1 and chain id 1 requires
`NETWORK=mainnet`.

`script/deploy/DeployRollupLib.sol` — `validateRollupConfig`, run before
`new Rollup`:
- slot, epoch and Ethereum slot durations non-zero; `slot × epoch` fits
`uint32` (`TimeLib` multiplies without widening);
- `exitDelaySeconds` non-zero;
- `lagInEpochsForRandao ≥ 1`, validator-set lag not below it, and lag in
seconds fits `uint32`;
- each slash amount fits `uint96` (`SlashingProposer` encodes payload
amounts as `uint96`);
- `rewardDistributor` non-zero; `stakingAsset == GSE.ASSET()`.

Two values are deliberately not rejected because the node's test paths
rely on them: a zero `targetCommitteeSize` (`ValidatorSelectionLib`
treats it as "no committee"; the local network and e2e fixtures use it)
and an epoch longer than `MAX_CHECKPOINTS_PER_EPOCH` (e2e suites use 64
and 1000 to keep a run inside epoch 0).

`script/deploy/DeployRollupForUpgrade.s.sol` — `REGISTRY_ADDRESS` must
have code on the target chain.

`scripts/run_rollup_upgrade.sh` no longer needs to override
`REAL_VERIFIER`. `scripts/test_rollup_upgrade.sh` and
`scripts/stress_test_deploy.sh` export the three genesis roots, and the
two script tests set them in `setUp`.

The node's TypeScript deployer already passes `REAL_VERIFIER` and all
three genesis roots explicitly, and no in-tree, e2e or spartan
configuration uses a value these checks reject.

## Tests

New `test/script/DeployConfigValidation.t.sol` exercises each check on
values via small harnesses rather than through `vm.setEnv`, because
forge runs tests in parallel and the process environment is shared; its
`setUp` sets the same env values as the other script tests for the same
reason. Locally:

```
forge test --match-path 'test/script/*'
Ran 3 test suites: 22 tests passed, 0 failed
```

`forge fmt --check` and `forge lint` are clean on the changed files
apart from the pre-existing `unsafe-cheatcode` notes for `vm.setEnv`. CI
covers the rest of the `l1-contracts` suite and the e2e suite.

Not in this PR: wiring an `EscapeHatch` into the generic upgrade script,
which needs a configuration surface for the hatch parameters rather than
a guard.

Closes AztecProtocol/aztec-claude#840
Closes AztecProtocol/aztec-claude#845
Closes AztecProtocol/aztec-claude#1371
Closes AztecProtocol/aztec-claude#688
Closes AztecProtocol/aztec-claude#684
Closes AztecProtocol/aztec-claude#707
Closes AztecProtocol/aztec-claude#865
Closes AztecProtocol/aztec-claude#657
Closes AztecProtocol/aztec-claude#689
Closes AztecProtocol/aztec-claude#1003
Closes AztecProtocol/aztec-claude#1355
Refs AztecProtocol/aztec-claude#686
Refs AztecProtocol/aztec-claude#811
Refs AztecProtocol/aztec-claude#1145

The eight constructor-input findings are closed at the deployer layer on
purpose: the constructors in `src/` are unchanged because the mainnet
pinned scripts assert these values themselves. If contract-level guards
are wanted as well, change those lines to `Refs` before merging. #686
(zero committee) and #811 (epoch above the proof cap) are not closed
here: both values are used by the node's own test paths, so any guard
for them has to live in the pinned mainnet scripts, which already assert
them.

---
*Created by
[claudebox](https://claudebox.work/v2/sessions/c27ea1045602719d/jobs/43)
· group: `slackbot` · requested by Mike (@iAmMichaelConnor) · [Slack
thread](https://aztecfoundation.slack.com/archives/D0B2N7W1WJD/p1789737469555569?thread_ts=1789737469.555569&cid=D0B2N7W1WJD)*
The Rollup constructor's owner argument is load-bearing twice. It becomes the `Ownable` owner, and
it is forwarded into the Slasher as its immutable `GOVERNANCE` (`RollupCore.sol:243` and `:270`),
which may execute ANY slash payload with no vote, no round and no delay (`Slasher.sol:58`). The
NatSpec calls it "the address with owner privileges" and mentions only the first use.

The deploy script passed the DEPLOYER there, to run the owner-only one-shot `setEscapeHatch` before
handing ownership over. `transferOwnership` moves only the Ownable half. The Slasher's copy is
immutable, so the finished deploy left the operational deploy key able to slash any v6 attester, for
any amount, at will, for the life of the rollup — and a coordinated slash of the set halts the
chain. A leaked key is enough; no malice required.

Recovery would not have been quick. The vetoer can only veto a payload address after seeing it, or
pause slashing three days at a time. Replacing the Slasher is `SLASHER_EXECUTION_DELAY` of 60 days
plus a `LEGACY_SLASHER_DRAIN_WINDOW` of 30 during which the old one still works.

Verified before fixing, by running the deploy script in a test: rollup owner came out as governance,
`Slasher.GOVERNANCE()` as the deployer.

The fix is the shape v5 used: construct with governance and install the hatch through the payload.
The alternative — splitting owner from governance in `RollupCore` — is the better long-term answer
but changes a core contract and every caller, which is not this PR's business.

So the rollup is now owned by governance from construction, nothing owner-gated happens in the
script, and `V6UpgradePayload` gains a `setEscapeHatch` action placed before `Registry.addRollup`,
so the hatch is installed before v6 is canonical. The payload checks the hatch's `getRollup()`
back-pointer in its constructor, because `setEscapeHatch` is one-shot and a hatch bound elsewhere
would burn the only chance to install one.

`_verifySlashing` now asserts `Slasher.GOVERNANCE() == governance`. Its absence is why this passed
review: the function already read VETOER, the disable duration and every proposer immutable, so it
looked thorough. The deploy-script test asserts the same thing against a real deploy.

Escape-hatch verification moved out of `verify()`, which is a deploy-time check and can no longer
see an installed hatch, into `verifyEscapeHatch(rollup, payload)` — mirroring `verifyFlushRewarder`.
Installation itself is asserted post-execution by the simulation.

Docs corrected. `V6UpgradePayload.md` claimed "Nothing is owner-gated on the deployer", which was
true of ownership and false about power, and is exactly the assurance a reviewer would have relied
on.

Reported by a reviewer against 278155d; every line of that report was re-verified here.
The three genesis roots were placeholders that run() refused to deploy
against. Fill them in from a full build of aztec-packages at
d521f0d: vkTreeRoot and
protocolContractsHash read off the built packages, genesisArchiveRoot
from the protocol constants.

The guard against an unset genesis was asserted by instantiating the
unmodified script, which only worked while the real table held zeros.
Construct the zero case explicitly instead, so the guard stays covered.
…d overrides

The deploy tests substituted stand-in roots because the table held zeros.
It no longer does, so drop the override and let them deploy the table as
written -- the Rollup constructor now passes judgement on the real roots
rather than on a keccak stand-in.

The reward overrides were absent from the runbook's pre-deploy table
despite being unguarded by run() and silent in every failure mode: a
wrong-but-plausible registry address deploys and is simply never matched.
Give them rows and spell out how they can go wrong unnoticed.
Sepolia inherited mainnet's entry queue rates, which admit 4 validators
per epoch during bootstrap growth and drop to 1 once the set passes
bootstrapValidatorSetSize (500 / 400 = 1). That is too slow to move a
testnet queue.

Raise all three of the fields that bound a flush. They have to move
together: the bootstrap phase returns bootstrapFlushSize directly, the
normal phase floors at normalFlushSizeMin, and maxFlushSize caps both,
so raising fewer than three leaves the old value binding.
The file was formatted by a newer forge than CI pins (v1.4.1, per
build-images/src/Dockerfile), which wraps a builder chain differently.
`forge fmt --check` fails on it as committed.
The distributor resolves `canonicalRollup()` live off the registry, so the
outgoing rollup loses access to the implicit (un-earmarked) pool the moment
`Registry.addRollup` runs. Anything it should keep has to be moved into its
own earmarked bucket before that, and only governance can move it.

Two actions, placed after the predecessor guard and before `addRollup`:
`recoverFrom(PREDECESSOR, payload, EARMARK_AMOUNT)`, then `forwardEarmark`.
Drawing from the predecessor and earmarking back to it is not a no-op: it
converts pool access that is about to lapse into a balance that survives,
since `specificRecipientBalance` is keyed by address and is indifferent to
which rollup is canonical.

The round trip trips through this contract because `subsidizeAddress` pulls
via `transferFrom` and governance can never grant the allowance -- `execute`
refuses any action targeting the asset.

The distributor and reward asset are read off the registry and the rollup
rather than configured, so the payload works unchanged on a chain whose
distributor and fee asset differ, and cannot be pointed at another chain's.
Both reads are skipped when the amount is zero, so a payload that reserves
nothing does not require the chain to have a distributor at all.

EARMARK_AMOUNT is wired to zero on both chains, which omits both actions.
Adds a `setRewardConfig` action targeting PREDECESSOR, placed before
`Registry.addRollup` so the new split is in force for whatever the outgoing
rollup still settles on its way out. The new rollup is untouched and keeps
the `sequencerBps` and `checkpointReward` it was constructed with.

Gated by a flag rather than a sentinel, because both 0 and 10000 bps are
meaningful splits and neither can stand in for "leave it alone".

Two properties worth knowing when picking the values: `setRewardConfig` has
no cooldown and no step cap -- unlike `setProvingCostPerMana` and
`setProtocolFeeMargin`, which are both rate limited -- so it lands the moment
the payload executes; and the split is read at proof time, so it also reaches
checkpoints already proposed under the old split but not yet proven.

The payload construction moves into a helper purely to keep `run` inside the
EVM stack limit.

Values are left as TODOs and the flag is false on both chains, so the action
is omitted.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants