Skip to content

fix(l1): make the generic deployer refuse unusable rollup configuration - #25520

Merged
iAmMichaelConnor merged 4 commits into
nextfrom
cb/l1-generic-deployer-guards
Sep 22, 2026
Merged

iAmMichaelConnor merged 4 commits into
nextfrom
cb/l1-generic-deployer-guards

Conversation

@AztecBot

@AztecBot AztecBot commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

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 · group: slackbot · requested by Mike (@iAmMichaelConnor) · Slack thread

@AztecBot AztecBot added ci-draft Run CI on draft PRs. ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure claudebox Owned by claudebox. it can push to this PR. labels Sep 21, 2026
@iAmMichaelConnor
iAmMichaelConnor marked this pull request as ready for review September 22, 2026 16:57
@iAmMichaelConnor
iAmMichaelConnor merged commit e99e47e into next Sep 22, 2026
16 checks passed
@iAmMichaelConnor
iAmMichaelConnor deleted the cb/l1-generic-deployer-guards branch September 22, 2026 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-draft Run CI on draft PRs. ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure claudebox Owned by claudebox. it can push to this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants