feat(fast-inbox): streaming inbox node integration, flip, and cleanup (umbrella) - #24796
feat(fast-inbox): streaming inbox node integration, flip, and cleanup (umbrella)#24796spalladino wants to merge 184 commits into
Conversation
a6dbe2a to
bd8896e
Compare
7d1d7d3 to
9e12351
Compare
spalladino
left a comment
spalladino
left a comment
There was a problem hiding this comment.
Got as far as sequencer-client (excluded)
| // The fee recipient/value below are sourced from the supplied headers, so the header hashes must be validated here | ||
| // as well as on the submit path - otherwise an off-chain caller of this getter would assemble public inputs from | ||
| // unverified fee fields and only discover the mismatch when the on-chain proof reverts. | ||
| verifyHeaders(_start, _end, _headers); |
There was a problem hiding this comment.
Why wasn't this needed before?
There was a problem hiding this comment.
It was needed before but missing — a pre-existing gap, not something this stack introduced. Base commit 8b1ea21 (2026-06-18, "protect epoch-proof fees against an unsound verifier") started sourcing fee recipient/value from the caller-supplied headers in this getter without validating them; only the submit path called verifyHeaders. An off-chain caller assembling public inputs would only discover a header mismatch when the on-chain proof reverted, so ada3771 added the call here as a drive-by fix.
Side effect worth fixing: submitEpochRootProof now hashes the headers twice (directly at :112 and again through this getter via verifyEpochRootProof) — work item filed to dedupe with an internal variant that skips validation on the already-verified submit path.
| bytes32 expectedPreviousInboxRollingHash = STFLib.getInboxRollingHash(_start - 1); | ||
| require( | ||
| expectedPreviousInboxRollingHash == _args.previousInboxRollingHash, | ||
| Errors.Rollup__InvalidPreviousInboxRollingHash( | ||
| expectedPreviousInboxRollingHash, _args.previousInboxRollingHash | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Why don't we need to do the same for the endInboxRollingHash?
There was a problem hiding this comment.
Because the end is already pinned to storage transitively: the checkpoint root writes the parity end into the checkpoint header's inboxRollingHash, this getter sources checkpointHeaderHashes from the stored getHeaderHash(...), and the circuit binds end_inbox_rolling_hash to the last checkpoint's header. A wrong _args.endInboxRollingHash therefore produces public inputs that don't match the proof, and verification fails. The start has no such coverage — the previous epoch's header hashes are not among this proof's public inputs — so it needs the explicit anchor, mirroring previousArchive.
An explicit end check (STFLib.getInboxRollingHash(_end)) would be redundant for soundness but would give a clean error instead of Rollup__InvalidProof and mirror the endArchive check. One SLOAD; happy to add it if you want the symmetry.
| v.header.inboxRollingHash, | ||
| _args.bucketHint, | ||
| v.header.slotNumber, | ||
| STFLib.getInboxMsgTotal(checkpointNumber - 1) |
There was a problem hiding this comment.
This is requiring yet another SLOAD, right? Any way we can optimize that?
There was a problem hiding this comment.
Yes — one extra cold SLOAD to read the parent's consumed total. It can be made warm: inboxMsgTotal and inboxConsumedBucket (uint64 each) currently open a fresh slot, but the temp-log slot holding CompressedSlot slotNumber (uint32) has 28 bytes free, and propose already SLOADs the parent's slot for the slot-progression check (STFLib.getSlotNumber(pendingCheckpointNumber) at ProposeLib.sol:372). Packing the counters there makes this read warm (100 gas) and merges the write into the existing slot-number SSTORE, which also drops one of the two extra stores flagged on CheckpointLog. Work item filed.
| bytes32 inboxRollingHash; | ||
| uint64 inboxMsgTotal; | ||
| uint64 inboxConsumedBucket; |
There was a problem hiding this comment.
These are two extra SSTOREs. How do these compare vs what we were storing on the Inbox?
There was a problem hiding this comment.
Layout today: inboxRollingHash takes one new slot and the two uint64 counters pack into a second new slot, so propose pays 2 extra zero→nonzero SSTOREs. The legacy path paid ~1 warm Inbox slot write at consume() (per-message frontier-tree writes were paid by senders, as the bucket-ring writes are now). Net measured effect on propose is +7,770 avg gas in the bench (195,988 → 203,758), all stack changes included.
Packing the counters next to CompressedSlot slotNumber (see the ProposeLib thread) eliminates one of the two; the rolling-hash slot has to stay — it anchors the epoch proof's start boundary.
| function testFuzzSendAndConsume( | ||
| DataStructures.L1ToL2Msg[] memory _messagesFirstBatch, | ||
| DataStructures.L1ToL2Msg[] memory _messagesSecondBatch, | ||
| uint256 _numTreesToConsumeFirstBatch, | ||
| uint256 _numTreesToConsumeSecondBatch | ||
| ) public { |
There was a problem hiding this comment.
Do we have an equivalent fuzzing test in the new suite? If not, let's add it.
There was a problem hiding this comment.
No equivalent — InboxBuckets.t.sol is all deterministic, and only the single-message testFuzzInsert survived in Inbox.t.sol. Work item filed: fuzz message batches across multiple L1 blocks with rollover, asserting the rolling-hash chain recomputes from the leaves, cumulative totals stay monotone, the per-bucket cap holds, and snapshot boundaries align with L1 blocks. The consume-side/wraparound interleaving fuzz is already planned in A-1390's harness (tiny test ring), so this one stays send-path-focused.
| // Streaming Inbox (AZIP-22 Fast Inbox): messages are inserted per block via `addBlock`, so `l1ToL2Messages` here | ||
| // is empty and the up-front checkpoint-wide insertion is skipped. | ||
| insertMessagesPerBlock: boolean = false, |
There was a problem hiding this comment.
We're post-flip now. No need for a flag to control pre vs post flip behaviour. Just write the code for post-flip.
| @@ -165,14 +177,15 @@ export class LightweightCheckpointBuilder { | |||
| public async addBlock( | |||
| globalVariables: GlobalVariables, | |||
| txs: ProcessedTx[], | |||
| opts: { insertTxsEffects?: boolean; expectedEndState?: StateReference } = {}, | |||
| opts: { insertTxsEffects?: boolean; expectedEndState?: StateReference; l1ToL2Messages?: Fr[] } = {}, | |||
There was a problem hiding this comment.
l1ToL2Messages should not be an "opt", it's now a first-class thing we add to each block. Promote it to a proper argument.
| case 'rollup-block-root-first': | ||
| return this.prover.getBlockRootFirstRollupProof(rollup.inputs, signal, provingState.epochNumber); | ||
| case 'rollup-block-root-first-single-tx': | ||
| return this.prover.getBlockRootSingleTxFirstRollupProof(rollup.inputs, signal, provingState.epochNumber); | ||
| case 'rollup-block-root-first-empty-tx': | ||
| return this.prover.getBlockRootEmptyTxFirstRollupProof(rollup.inputs, signal, provingState.epochNumber); |
There was a problem hiding this comment.
I should've surfaced this during the circuits review (probably should funnel this comment to the circuits agent), but why do we still need the first flavor of block root circuit? Can't we merge it with the others? Isn't the only difference whether it's allowed to be fully empty?
| blockProofs: Promise<{ | ||
| blockProofOutputs: PublicInputsAndRecursiveProof< | ||
| BlockRollupPublicInputs, | ||
| typeof NESTED_RECURSIVE_ROLLUP_HONK_PROOF_LENGTH | ||
| >[]; | ||
| inboxParityProof: PublicInputsAndRecursiveProof<ParityPublicInputs>; | ||
| }>; |
There was a problem hiding this comment.
Should either rename blockProofs, or split this into two different fields, one for blockProofs and one for inboxParityProof.
| private async deriveCheckpointConsumedMessages(previousBlockHeader: BlockHeader, lastBlock: L2Block): Promise<Fr[]> { | ||
| const startLeafCount = BigInt(previousBlockHeader.state.l1ToL2MessageTree.nextAvailableLeafIndex); | ||
| const endLeafCount = BigInt(lastBlock.header.state.l1ToL2MessageTree.nextAvailableLeafIndex); | ||
| if (endLeafCount <= startLeafCount) { | ||
| return []; | ||
| } | ||
| const startBucket = await this.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(startLeafCount); | ||
| const endBucket = await this.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(endLeafCount); | ||
| if (startBucket === undefined || endBucket === undefined) { | ||
| throw new Error( | ||
| `Cannot resolve consumed messages for checkpoint ending at block ${lastBlock.number} from the Inbox buckets`, | ||
| ); | ||
| } | ||
| return this.l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets(startBucket.seq, endBucket.seq); | ||
| } |
There was a problem hiding this comment.
"Get all message between these indices" (ie between startLeafCount and endLeafCount) should probably be a method of the messageStore, so clients don't need to bother with explicit buckets.
bbfaf57 to
c78ad70
Compare
Pure library components for the Fast Inbox (AZIP-22), consumed by nothing yet: new MAX_L1_TO_L2_MSGS_PER_BLOCK / MAX_L1_TO_L2_MSGS_PER_CHECKPOINT constants, a variable-length frontier-based append to an AppendOnlyTreeSnapshot at arbitrary (non-aligned) indices, an absorb-only poseidon message-bundle sponge (L1ToL2MessageSponge), and the rolling sha256 chain helper (accumulate_inbox_rolling_hash) matching the L1 truncated-to-field policy. No circuit interface changes.
Replaces the per-leaf frontier walk (MaxLeaves x TreeHeight poseidon hashes) with a level-by-level batched merge: the batch is prepended with the pending left sibling when it starts as a right child, dangling odd nodes become the new frontier entries, and remaining nodes are paired into the next level with lane bounds halving per level. Total cost drops to ~MaxLeaves + 3 x TreeHeight hashes (1,166 for 1024 leaves in the height-36 tree vs ~36,900 before). Same signature, semantics, and error messages. Adds an exhaustive small-tree sweep over every (start, num) combination and a fail-closed test for appending to a completely full tree.
…ator message fetch (A-1388) Drops the no-op first-in-checkpoint padding alias and its stale docs in world-state, and removes the obsolete non-first-block-empty-bundle transitional test. The node public-calls simulator no longer fetches next-checkpoint messages via the removed per-checkpoint API and drops its now-unused l1ToL2MessageSource dependency, simulating against the fork's current tree (streaming Inbox consumes per block). Rewrites the THREAT_MODEL inHash/consume passages to the consensus rolling-hash / bucket model.
…ts (A-1388) The optional bucket-reference tail keeps proposals that omit it round-tripping cleanly; it does not make the wire byte-identical to the pre-inHash-removal format. Reword the toBuffer/fromBuffer comments to describe only the tail's unset case.
… tests (A-1388) The env sweep removed AZTEC_INBOX_LAG from scripts/network-defaults.json but both deploy-script test setUps still readUint the key, which reverts (vm.parseJsonUint on a missing path), failing the suites before any test runs. The deploy configuration no longer consumes the env var.
…proposal formats (A-1388) The attestation store persists raw BlockProposal and CheckpointAttestation buffers. Both changed shape in this stack (the checkpoint header lost inHash and the block-proposal wire format dropped its zeroed inHash), and stored checkpoint attestations are decoded without a tolerant fallback, so a store written by a pre-Fast-Inbox node would throw on read. Bump 2 -> 3 to wipe stale pools, matching the archiver's no-migration bump.
…388) Removes the dead AZTEC_INBOX_LAG entries from the spartan environment profiles and the governance-upgrade tutorial, rewrites the validator and sequencer README sections that still described the legacy inHash flow, and drops e2e comments that still explained fixture settings in terms of the deleted inboxLag / L1ToL2MessagesNotReadyError behavior. Versioned docs snapshots are left untouched.
…mple inbox ABIs (A-1386)
…(A-1388) Finding the cut point for an L1-block rollback walked the messages backwards from the tip, deserializing every message being removed just to learn where the removal starts. A bucket lives entirely within one L1 block and its snapshot now records that block plus the bucket's first message index, so scan the bucket snapshots instead — a few records per L1 block rather than one per message.
…sage leaf index (A-1388) Blocks consume Inbox messages in order into consecutive L1-to-L2 tree leaves, so a message is available at a tip exactly when that tip's leaf count has grown past the message's index — one comparison against data every block header already carries. `isL1ToL2MessageReady` now does that via `getL1ToL2MessageIndex`, which also drops its stale claim that messages land in the first block of a checkpoint. The `getL1ToL2MessageCheckpoint` node RPC method existed only to answer this question and needed a binary search over block records to do it, so delete it: node service, world-state queries, interface, schema, and its callers. `getL1ToL2MessageIndex` takes its place in the operator API reference.
…ing public calls (A-1388) A transaction that consumes an L1-to-L2 message sent moments ago failed public simulation until a block actually consumed the message, because the simulation fork stops at the tip's message tree. Predict the bundle the next block would take — the sequencer's own bucket selection, so lag eligibility and the per-block/per-checkpoint caps match what will be built — and append it to the fork. The prediction treats the next block as non-final (the censorship cutoff only widens consumption on a checkpoint's last block, which the node cannot know) and derives its cursor from the fork's own leaf count, so it can never re-insert messages the fork already holds. Best-effort: unsynced buckets or a missing header leave the simulation on the bare tip, as before.
…ts generator (A-1434) Add INBOX_LAG_SECONDS and MAX_L1_TO_L2_MSGS_PER_CHECKPOINT to the Solidity constants allowlist so the generator emits them into ConstantsGen.sol, matching the values already present in constants.gen.ts. ProposeLib and its consumption test now read the generated Constants library instead of the hand-declared file-scope copies, giving L1 and TS a single source of truth.
…neration (A-1435) Make the prover-client regenerate_rollup_sample_inputs suite the sole owner of every block-root, block-merge, checkpoint, tx-merge, and root rollup Prover.toml, and stop the e2e full.test dump from writing the block-root and checkpoint samples it overlapped on. The two paths previously both wrote rollup-block-root-first, rollup-block-root-first-single-tx, rollup-checkpoint-root-single-block, rollup-checkpoint-merge, and rollup-root, so the committed fixtures drifted depending on which ran last. The e2e dump now regenerates only what needs real client-proved transactions the simulated orchestrator cannot produce: the private-kernel circuits and the transaction-base rollups. A dedicated three-tx scenario restores rollup-tx-merge coverage (dropped when orchestrator_single_checkpoint.test.ts was replaced), so the suite covers every rollup circuit at or above the transaction merge. Update the regen docs (barretenberg cpp CLAUDE.md, update-prover-toml and gate-counts skills) to the two-command split and drop the dead orchestrator_single_checkpoint references.
…en and fix stale regen command patterns (A-1435) Post-flip a zero-tx non-first block carrying a message bundle is a live block shape routed through the msgs-only block root, so the regen suite gains a scenario with a per-block message distribution that dumps rollup-block-root-msgs-only, and the circuit joins the CI nargo-execute list. The first regen run creates crates/rollup-block-root-msgs-only/Prover.toml, which must be committed alongside the other regenerated tomls. Also fixes the documented e2e regen command: the prover full test lives at single-node/prover/server/full.test.ts, so the old e2e_prover/full.test jest pattern matches nothing.
…-1435) makeCheckpoint concentrates every scenario message into the first block, whose bundle pads to MAX_L1_TO_L2_MSGS_PER_BLOCK (256), so seeding scenarios with the per-checkpoint cap (1024) threw on regeneration. Also distribute messages across both blocks of the rollup-block-root scenario so the committed non-first block-root sample carries a non-empty bundle.
…rio notes (A-1435)
…ader dedupe (A-1373) Regenerates gas_benchmark.md, gas_benchmark_results.json and gas_report.json at the top of the stack so the committed numbers reflect the deduped checkpoint-header verification on submitEpochRootProof, the explicit end inbox rolling-hash check, and the packing of the inbox consumption counts into the checkpoint log's slot-number slot. Benchmark averages (no L1-to-L2 messages consumed in this scenario): - No validators: propose 203,758 -> 199,557; submitEpochRootProof 1,137,731 -> 994,378. - Validators: propose 332,170 -> 327,969; submitEpochRootProof 1,719,902 -> 1,575,427. The gas report, whose Rollup suite does consume messages, moves further on propose: 289,076 -> 266,918 average.
…bridge tutorials (A-1435) The executable snippets already match the streaming Inbox, but the surrounding prose still described the checkpoint-era behaviour: a removed MessageSent field list, and a two-block lag before an L1-to-L2 message could be claimed. A message is now claimable as soon as an L2 block includes it, which the network allows once the message is at least 12 seconds old. The block-forcing helpers stay, since a local network only produces blocks when transactions are submitted.
c78ad70 to
7a0adec
Compare
… barretenberg (A-1435)
… and index-bound append (A-1435)
|
Superseded by #25039. The Fast Inbox stack has been regrouped from 20 per-issue PRs (plus 2 umbrellas) down to 3 area PRs plus an umbrella, and rebased onto New structure: #25036 (circuits + L1) → #25037 (node + flip) → #25038 (cleanup), with #25039 as the full-stack umbrella. The original branch for this PR is left on the remote as a recovery point. Closing here to cut the rebase and review overhead of maintaining 22 PRs; the work is not abandoned. |
1 participant
Umbrella / consolidated PR for phase 2 of the Fast Inbox (AZIP-22) work.
This PR spans the phase-2 stack — from the topmost branch down to
merge-train/spartan— so CI runs once over the combined diff and reviewers get a single consolidated view. It sits on top of the phase-1 (circuits + L1) umbrella #24774: until the phase-1 PRs merge, this diff includes them too.What it contains, at a high level
Phase 2 lands the node integration, the consensus flip, the streaming e2e coverage, and the legacy-path cleanup for the Fast Inbox:
Rollup.proposevalidates streaming inbox consumption against per-checkpoint temp-log records (genesis{0,0,0}),ProposeArgsgains an unsignedbucketHint,MAX_L1_TO_L2_MSGS_PER_BLOCKdrops 1024 → 256, every block carries its own L1-to-L2 tree root in the blob, the prover slices a checkpoint's messages per block at compact indices, and thestreamingInboxflag is removed — streaming is the only path.consume(),LAG), the checkpoint header'sinHash(noir/TS/Solidity in lockstep), the node's legacy per-checkpoint message paths and the 128-bit sync hash (node reads and, via the leftover sweep, the L1 field and event arg too), theAZTEC_INBOX_LAGenv sweep, inbox constants emitted from the generator, and a single canonical path for rollup sample-input regeneration.Contained PRs (stack order, bottom → top)
inHash(A-1387)Merge notes
Prover.tomlsample inputs were regenerated on a bb-capable box and committed at the top of the stack (bbfaf57daa), including the newrollup-block-root-msgs-only/Prover.toml— the circuit joined the regen scenarios and the CInargo executelist in this stack. All regenerated inputs passnargo execute.ARCHIVER_DB_VERSION7 → 9 across the stack and the p2p attestation store 2 → 3; no migrations — nodes resync (fresh rollup instance per release line).inHash(preimage changes in noir/TS/Solidity lockstep, fixtures regenerated), the block-proposal p2p wire format drops its zeroedinHash, the per-block blob encoding always carries the L1-to-L2 root, and the per-block message cap is 256. All are release-boundary changes for fresh networks.EpochProofExtLibsplit keepingRollupOperationsExtLibdeployable post-flip, streaming mocks for the validator/prover-node unit suites, a pxe L2 tips snapshot regen for the inHash-less header (read-defaultable, no schema bump), and docs-example Inbox ABIs kept in lockstep with theMessageSentevent.ProvingJobResultZod union was missing aBLOCK_ROOT_MSGS_ONLY_ROLLUPmember (feat(fast-inbox): add block_root_msgs_only_rollup circuit for no-tx blocks with messages (A-1375) #24612 / A-1375, where the request type is introduced), which stalled epochs decoding a message-only block-root result; and the proposebucketHintwas lost when a checkpoint's final block failed to build after a message-only first block consumed a bucket (feat(fast-inbox): flip consensus to streaming inbox (A-1384) #24789 / A-1384). Both carry unit-level round-trip / RED-GREEN regression tests. The relocated proving-scheduling test (base of the stack) also swapped its parity reference forPUBLIC_VM: no parity name type-checks across the whole stack (INBOX_PARITYexists only at A-1427+,PARITY_BASEonly at A-1374/A-1375, since A-1427 collapsesPARITY_BASE/PARITY_ROOTintoINBOX_PARITY), whereasPUBLIC_VMis stable everywhere and stays lower-priority thanPRIVATE_TX_BASE_ROLLUP. Verified with realtscat A-1374, A-1384, and the top.'latest'tip while these suites anchor the PXE to'checkpointed'. Post-flip per-block inbox insertion lets the proposed chain reach a message's consume-checkpoint a checkpoint before it is published, so readiness flipped true while the PXE had not synced the message and the private consume simulated too early — a deterministic failure onl1_to_l2.test.tsfrom the flip up. The helper now gates on the configured PXE sync tip (CrossChainMessagingTest.pxeSyncChainTip, defaulting to'latest'). Test-only, unchanged A-1384→top; box-verified green on all three cross-chain suites (l1_to_l2,streaming_inbox,l1_to_l2_inbox_drift).block state does not match world state) on the first cross-chain-consuming block. Post-flip the blob carries a per-block root; reconstruction now uses each block's own. Surfaced ascross_chain_public_message+token_bridge(only followers reconstruct via the archiver, so validation never caught it). Adata_retrieval.test.tsunit pins per-block reconstruction; both e2es green at a-1384. A full box sweep of every L1→L2/inbox/cross-chain/proving e2e at a-1384 is now green.streaming_inbox.test.tstests 1–2 (mid-checkpoint inclusion, latency bound) flake on their first CI runs, add tighterror_regexentries to.test_patterns.ymlfor the specific assertion rather than broad skips. None are pre-added.Review and land the individual PRs above; this umbrella exists only to consolidate CI and provide a full-stack diff.
Pass-8 CI fixes (test-only, both surfaced by fail-fast unmasking): (1) the world-state
integration.test.tsmock archiver never registered Inbox buckets, so the post-flip synchronizer reconstructed empty bundles and diverged (block state does not match world state) — fixed at A-1384 by registering genesis + per-checkpoint buckets inMockPrefilledArchiver.setPrefilled; (2) thel1_publisher.integration.test.tsinbox-consumption modeling (hardcoded rolling hash / bucket hint, then a wrong checkpoint-depth shift register) reverted on L1 — fixed at A-1387 by mirroring the real Inbox buckets and reusing the productionselectInboxBucketForBlock(INBOX_LAG_SECONDS is an L1-time cutoff). A-1388's own rewrite of that file dissolves into the A-1387 version. Box-verified: world-state 12/12 (a-1384, top), l1_publisher 13/13 (a-1387, a-1388); full tsc green at a-1384/a-1387/a-1388/top.Leftover sweep (post-review of this umbrella diff): five flip/cleanup leftovers found while reviewing this diff were fixed in their owning branches. The legacy 128-bit keccak inbox rolling hash is now fully removed on L1 (
InboxState.rollingHash, theMessageSentbytes16arg, its keccak accumulation, the four Solidity test suites asserting it, and the three live docs-example inbox ABIs) at A-1388 — the earliest branch whose TS no longer reads it. The staleIRollup.solPublicInputArgsnatspec now describes the implemented epoch-proof anchoring (A-1386). The inertisFirstBlockblob-fixture parameter is removed (A-1384). The deadInboxLeafclass is deleted (A-1388). The checkpoint-builder comments now mark up-front whole-checkpoint message insertion as test-only (A-1388).Gas benchmarks refreshed (top of stack):
gas_report.json,gas_benchmark.md, andgas_benchmark_results.jsonregenerated via./bootstrap.sh gas_report/gas_benchmarkat the pinned foundry v1.4.1. Headlines:sendL2Messagemean gas 53,020 → 44,796 (−15.5%),Inboxbytecode −19%;propose+~4% mean from thebucketHintcalldata and the added inbox-consumption validation. Much of the raw diff is pre-existing baseline staleness (the committed report predated the block→checkpoint rename and theOutbox.getRootDatasignature change), now corrected. Known quirk for future regens: threeRollup.t.soltests (testExtraBlobs,testRevertInvalidCoinbase,testRevertInvalidTimestamp) fail only underFORGE_GAS_REPORT=true— a Foundryvm.expectRevertcall-depth interaction with theIInbox.getBucket()staticcall inProposeLib.validateInboxConsumption, invisible to normal CI; a real fix or a--no-match-testentry inbootstrap.sh gas_reportis a follow-up.