chore: merge v5-next into merge-train/spartan-v5 (raw, conflict markers) - #24221
Merged
PhilWindle merged 17 commits intoJun 22, 2026
Conversation
Port of #24186 to the v5 release line. Bind per-checkpoint fee recipient/value to the committee-attested checkpoint header instead of prover-supplied calldata, so reward distribution and the epoch proof's fee-region public inputs are derived from headers verified on L1 and no longer depend on a sound verifier. - L1: add ProposedHeader.accumulatedFees (hashed in ProposedHeaderLib); replace SubmitEpochRootProofArgs.fees[] with headers[]; submitEpochRootProof rehashes each supplied header against the stored hash; RewardLib and getEpochProofPublicInputs read recipient/value from the verified headers. - Circuits: add accumulated_fees to CheckpointHeader (hash + serialize) and set it in the checkpoint-root composer; bump CHECKPOINT_HEADER_LENGTH. - Off-chain: thread checkpoint headers from the prover node's epoch session through the proof-publishing service to the L1 submit call; archiver, sequencer and stdlib carry the new field. - PXE: bump PXE_DATA_SCHEMA_VERSION (8 -> 9) for the checkpoint serialization change and regenerate the storage compatibility snapshots.
The committed pinned-build.tar.gz is used unconditionally in place of compiling the protocol circuits, so it must track circuit source changes. Adding the accumulated_fees field to the checkpoint header changed the checkpoint_root, checkpoint_root_single_block and root rollup circuits; without re-pinning, the prover ran stale 12-field checkpoint circuits and produced a checkpointHeaderHashes value that disagreed with the freshly compiled 13-field Solidity, failing epoch proof submission (e2e_simple: Timeout awaiting isProven).
Move verifyHeaders from submitEpochRootProof into getEpochProofPublicInputs. The on-chain submit path still runs it via verifyEpochRootProof, and the prover-publisher's off-chain getEpochProofPublicInputs preflight now validates the headers before publishing instead of skipping the check. Since the fee recipient/value public inputs are sourced from the supplied headers, this catches a header mismatch off-chain rather than reverting on submit.
A malicious proposer could store header or archive field values >= the BN254 scalar field modulus on L1. Off-chain archivers decode those storage slots into Fr elements, so an out-of-range value bricks honest archivers' L1 sync. Reject such values at propose time: - validateHeader: blockHeadersHash, outHash, feeRecipient, accumulatedFees - propose: the new checkpoint archive root Other header fields are already safe (equality-checked against field-reduced on-chain values, or type-bounded), so they are left unchecked. This is the L1 half of A-1254; the archiver-side defense-in-depth is a separate follow-up PR.
#24199) ## Summary Part of A-1254. A malicious proposer can currently store checkpoint header or archive field values that are `>=` the BN254 scalar field modulus (`Constants.P`) on L1. Off-chain archivers decode those storage slots into `Fr` elements, so an out-of-range value bricks honest archivers' L1 sync. This is the L1 half of the fix: reject such values at write time so they can never reach L1 storage. ## Changes A new helper `FieldLib.requireValidFieldElement(bytes32)` wraps the `value < Constants.P` check and reverts with `Errors.Rollup__FieldElementOutOfRange(bytes32 value)`. It is used to range-check the attacker-controlled, `Fr`-decoded values that are not otherwise constrained: - `ProposeLib.validateHeader` — header fields `blockHeadersHash`, `outHash`, `feeRecipient`, `accumulatedFees`. - `ProposeLib.propose` — the new checkpoint archive root (`_args.archive`), which is not part of the header, checked before the digest/header validation/storage. - `STFLib.initialize` — the genesis archive root, which is written to `archives[0]` at deployment and propagates into the first header's `lastArchiveRoot`. ## Why the other fields are already safe - `lastArchiveRoot`, `inHash`, `blobsHash` — equality-constrained to on-chain values that are already field-reduced (`tipArchive`, `inbox.consume()`, `blobsHashesCommitment`). - `totalManaUsed` — bounded by `<= FeeLib.getManaLimit()`, and the mana limit is capped at `type(uint32).max`, far below `P`. - `gasFees` (uint128), `coinbase` (address), `slotNumber` / `timestamp` — type-bounded or equality-checked against slot-derived values. ## Tests New `test/RollupFieldRange.t.sol`: - Each of the five guarded propose fields reverts with `Rollup__FieldElementOutOfRange` when set to `Constants.P` (boundary) and to `type(uint256).max`. - The genesis archive root reverts at deploy time when out of range. - An otherwise-valid header with every guarded field at `Constants.P - 1` proposes successfully, confirming the boundary is exclusive and legitimate field elements are not rejected. Verified red/green for both the propose checks and the genesis check; the broader `RollupTest`, escape-hatch, and fees suites remain green. ## Follow-up The archiver-side defense-in-depth (decoding out-of-range values without bricking sync) is a separate follow-up PR that will close A-1254.
…ield run_rollup_upgrade.sh deploys with a random GENESIS_ARCHIVE_ROOT. The rollup now rejects a genesis archive root >= the BN254 scalar field modulus (Rollup__FieldElementOutOfRange), so a full 32-byte random value fails ~81% of the time. Generate 31 random bytes with a zero top byte so the value is always a valid field element.
The previous re-pin generated VKs via the build cache, leaving a verification key that did not match its recompiled circuit after the CHECKPOINT_HEADER_LENGTH bump. Real epoch proofs then failed at the root-rollup pairing check (bb_prover_full_rollup), which only runs under full CI. Rebuild with NO_CACHE so every VK is derived locally from its own ACIR; validated by running bb_prover_full_rollup with real proofs (root-rollup proof now verifies).
The prior re-pin was generated with a stale local bb (predating the ECCVM fixed-VK / chonk-verifier / IPA changes now in the tree), so the rollup VKs embedded the old ECCVM VK. Locally that bb proves and verifies self-consistently, but full CI builds bb from current source and proves the new ECCVM against the old baked VK, failing the root-rollup pairing check (bb_prover_full_rollup: 'Failed to verify proof from key'). Rebuilt bb from source and regenerated both the noir and mock protocol-circuit pins so their VKs match the current bb. Validated locally with real proofs.
BEGIN_COMMIT_OVERRIDE feat(txe): auto-generate oracle serialization roundtrip tests (#24138) END_COMMIT_OVERRIDE
This is a defense-in-depth change: it protects the fee juice portal in the event of a catastrophic soundness bug. ## Problem The epoch-proof `_fees` array (per-checkpoint `recipient`/`value`) was prover-supplied calldata, bound only by the epoch proof. If the verifier were unsound (e.g. returned `true` unconditionally), a prover could forge fee recipients/values and steal fees. In an extreme case, this could result in the complete drain of the canonical fee juice portal. ## Fix Bind the fee value to the committee, not the proof. The committee already attests to the checkpoint `headerHash` (which commits `coinbase`) at propose time. We add the fee **value** to the checkpoint header as `accumulatedFees`, so both halves of a fee entry are committed when the checkpoint is proposed and stored on L1. At proof submission the prover supplies the full checkpoint headers; L1 rehashes each with the existing `ProposedHeaderLib.hash` and requires equality with the stored header hash. Rewards and the proof's fee-region public inputs are then derived from these verified headers so fees and checkpoint rewards no longer depends on the verifier. ## Design note The `fees[FeeRecipient]` array in the rollup circuit/merge/root public inputs are not strictly needed anymore. They are left there to minimize the changeset. Removing the array can be a follow-up. Further, there are cheaper options where we could break up what the proposer stores when they publish a checkpoint, and have the checkpoint header hash be a nested computation like H(fee_recipient, amount, H(...rest)). We can investigate those if needed.
## Why we are doing this Utility functions had no notion of `msg_sender`, so a utility that wanted to know its caller had to accept it as a parameter. `HandshakeRegistry::get_app_siloed_secret` took an explicit `caller` argument and siloed the returned secret by it, so a hostile contract could pass another address and read an app's siloed secret it shouldn't see. ## Our fix - `UtilityContext` gains a `msg_sender`, exposed as `self.msg_sender()` and `self.context.maybe_msg_sender()`, mirroring the private and public contexts. - It is set only for cross-contract calls (utility-to-utility or private-to-utility), where it is the calling contract's address taken from the call graph and so cannot be forged. A top-level call (invoked directly by a wallet or dapp) has no caller: `maybe_msg_sender()` is `none` and `msg_sender()` panics. The `from` supplied when simulating still only controls note/key visibility; it is never exposed as a sender. - `HandshakeRegistry::get_app_siloed_secret` drops the forgeable `caller` parameter and silos by `self.msg_sender()`, so a contract can only retrieve secrets siloed to its own address. - `TestEnvironment` gains `ExecuteUtilityOptions::with_from` to simulate a cross-contract caller in tests without routing through an actual nested call. Fixes F-671 --------- Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
AztecBot
added
ci-draft
## Conflict resolution (PR 2 of 2) Stacked on top of the raw merge #24221. This is the **reviewable diff** — it resolves the single conflict from merging `v5-next` into `merge-train/spartan-v5`. Once both are green, merge this into #24221, then #24221 into `merge-train/spartan-v5`, which catches the train up to its base and makes the long-lived `merge-train/spartan-v5` → `v5-next` PR conflict-free. ### Conflicts resolved (1) **`yarn-project/end-to-end/src/e2e_nested_utility_calls.test.ts`** — both sides added content at the same point in the `authorizeUtilityCall hook` describe block: - **train (`ours`)** added a doc comment above the existing `denies cross-contract utility call from private function when hook returns false` test (from the e2e annotation pass on the train). - **base `v5-next` (`theirs`)** added a new `nested utility call sees the calling contract as its msg_sender` test (from `feat(aztec-nr)!: add msg_sender to the utility context` #24062). **Resolution:** keep both — the new `msg_sender` test followed by the doc-commented `denies…private function` test. No code was dropped from either side. The `delegate_get_msg_sender` contract method the new test exercises is brought in by the merge (it lives in `noir-projects/noir-contracts/contracts/test/nested_utility_contract`, non-conflicting). No fixtures or generated files were involved. PR 1's CI is red by design (conflict markers committed there); this PR's CI is the one to watch. --- *Created by [claudebox](https://claudebox.work/v2/sessions/51d528a3438f2849) · group: `slackbot`*
PhilWindle
marked this pull request as ready for review
June 22, 2026 11:48
PhilWindle
requested review from
LeilaWang,
just-mitch and
nventuro
as code owners
June 22, 2026 11:48
PhilWindle
approved these changes
Jun 22, 2026
This was referenced Jun 22, 2026
PhilWindle
added a commit
that referenced
this pull request
Jun 22, 2026
## Merge `v5-next` into `merge-train/spartan-v5` — MUST merge as a merge commit (do not squash) ### Why the conflicts came back The previous stack (#24221/#24222) resolved the conflict correctly, but #24221 was **squash-merged** onto the train. A squash creates a brand-new single commit and does **not** record `v5-next` as a parent, so git still sees the train and `v5-next` as diverged (`git merge-base --is-ancestor origin/v5-next origin/merge-train/spartan-v5` → false). The merge-train auto-pull then re-attempts `v5-next → train` and hits the same conflict again. ### What this PR does This is a real **merge commit** of `v5-next` into the train (two parents: train tip `eb7d64d` + `v5-next` `4df7243`), with the one conflict resolved. The resulting tree is **identical to the current train** (0 file changes) because the content already landed via the earlier squash — this PR exists purely to record `v5-next` as an ancestor and advance the merge base. After this lands, `v5-next` is an ancestor of the train and the long-lived `merge-train/spartan-v5` → `v5-next` PR is conflict-free. ### Conflict resolved (1) `yarn-project/end-to-end/src/e2e_nested_utility_calls.test.ts` — kept the train's doc comment above the `denies…private function` test; the `msg_sender` test from `v5-next` is already present. ###⚠️ Merge method **Merge this with "Create a merge commit"** (or `merge_pr merge_method=merge`). A squash or rebase merge will recreate the same divergence and the conflicts will return. --- *Created by [claudebox](https://claudebox.work/v2/sessions/51d528a3438f2849) · group: `slackbot`*
rangozd
pushed a commit
to rangozd/aztec-packages
that referenced
this pull request
Aug 5, 2026
BEGIN_COMMIT_OVERRIDE fix(p2p): re-seed discovery from persisted peer ENRs after restart (AztecProtocol#24169) docs(e2e): annotate e2e tests with setup/category notes (AztecProtocol#24191) chore: merge v5-next into merge-train/spartan-v5 (raw, conflict markers) (AztecProtocol#24221) fix(archiver): index zero-field logs under empty tag instead of throwing (A-1253) (AztecProtocol#24212) fix(prover-node): report awaiting-root and publishing-proof phases in EpochSession (A-1212) (AztecProtocol#24216) fix(p2p): bound declared contract-class bytecode length before allocating (A-1258) (AztecProtocol#24213) fix(p2p): frame gossipsub msgId and restrict allowedTopics (A-1256) (AztecProtocol#24214) chore: merge v5-next into merge-train/spartan-v5 (AztecProtocol#24226) fix(p2p): guard ENR address parsing against malformed TCP fields (A-1255) (AztecProtocol#24215) END_COMMIT_OVERRIDE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Raw merge — do not merge alone
This is PR 1 of a two-PR stack that catches
merge-train/spartan-v5up to its basev5-nextso the long-lived train PR (merge-train/spartan-v5→v5-next) becomes conflict-free again.This PR is the raw merge of
v5-nextintomerge-train/spartan-v5with conflict markers committed — it is red by design. Review and merge it only together with the stacked resolution PR (PR 2, headcb/merge-train-spartan-v5-fix), which targets this branch.Parents
merge-train/spartan-v5@bc2c924919fe8880ee9572df779b78f653ef2e15v5-next@4df72438bf01f125e791af7e8bb11a613964457eConflicted files (1)
yarn-project/end-to-end/src/e2e_nested_utility_calls.test.tsResolution lives in the stacked fix PR.
Created by claudebox · group:
slackbot