refactor(fast-inbox): some cleanups by LeilaWang · Pull Request #25422 · AztecProtocol/aztec-packages

LeilaWang · 2026-09-07T19:38:29Z

Some small cleanups I did while reading the codebase. Details are left in the comments.

Rebase note (not by the original author)

Rebased from spl/fi2-message-only-docs onto spl/fi2-required-inbox-prefix, the current top of the Fast Inbox stack. The six documentation commits this branch carried are already in the base under new hashes, so only the cleanup commit was replayed.

The one conflict was sequencer-client/src/sequencer/checkpoint_proposal_job.ts. The cleanup refactors a completion state machine (completion, consumptionComplete, retained bucket sequences) that the base has since removed in favour of stateless per-block endpoint selection. It was resolved in the base's favour on architecture and in the cleanup's favour on state ownership:

StreamingCheckpointState keeps the base's two fields and the base's endpoint selection, bounds, retry and abort behaviour; the obsolete state-machine fields were not restored.
The state is now replaced rather than mutated. buildBlocksForCheckpoint takes an initialStreamingState, keeps a local one, and returns the final state in BlockBuildingResult.
buildForcedEndpointBlock (which post-dates the cleanup) now reads its state without modifying it and returns the state the tail advanced to, which the block loop adopts. Both preflights therefore see the checkpoint's final cursor including the forced tail.

The prover's error-propagation change was integrated on top of the base's proof-completion/finalization fix rather than restoring the old lifecycle; one base test that asserted the generic "did not complete block processing" message now asserts the propagated cause.

Also in the follow-up commit: the new prover tests use typed mock<T>() doubles instead of as any and a private-field reach, and a sequencer regression covers a failed block attempt leaving the cursor unchanged so its retry advances it exactly once.

LeilaWang · 2026-09-07T19:39:11Z

        self.bundle_applied = true;

-        // Real messages occupy the leading lanes; everything past `num_msgs` is zero padding.
-        assert_trailing_zeros(bundle.messages, bundle.num_msgs);


Removing this as the same check is also done in append_only_tree::append_leaves_to_snapshot below.

LeilaWang · 2026-09-07T19:45:06Z


    // Load sync point for blocks defaulting to start block
-    const { blocksSynchedTo = this.l1Constants.l1StartBlock } = await getArchiverSynchPoint(this.stores);
+    const blocksSynchedTo = (await this.stores.blocks.getSynchedL1BlockNumber()) ?? this.l1Constants.l1StartBlock;


I find it easier to see that blocksSynchedTo is the synched l1 block number by calling the block store api directly, and it also avoids another unnecessary async call in getArchiverSynchPoint.

LeilaWang · 2026-09-07T19:48:40Z


-    // Accumulate the streaming bundle now that the block is fully built, so a mid-build throw above leaves the
-    // checkpoint's message list (and thus its rolling hash) consistent with the blocks actually built.
-    this.l1ToL2Messages.push(...l1ToL2Messages);


I didn't feel comfortable that it was changing the array passed to the constructor directly. And while changing it, I found that it also made sense to replace the array with a rolling hash instead, since it's what's been built for each block added.

LeilaWang · 2026-09-07T19:52:43Z

+      const l1ToL2LeafCount = (header: BlockHeader) => header.state.l1ToL2MessageTree.nextAvailableLeafIndex;
+      const checkpointStartCount = l1ToL2LeafCount(this.previousBlockHeader);
+      const checkpointEndCount = l1ToL2LeafCount(this.checkpoint.blocks.at(-1)!.header);
+      if (this.l1ToL2Messages.length !== checkpointEndCount - checkpointStartCount) {


Added this simple check to throw an error early if the number of messages doesn't match, instead of waiting until later to find out when the hash doesn't match with the value in the built block's header.

LeilaWang · 2026-09-07T20:36:53Z

      blocksInCheckpoint: L2Block[];
      /** The last block's proposal, held back to travel with the checkpoint proposal instead of being gossiped. */
      blockPendingBroadcast: BlockProposal | undefined;
+      streamingState: StreamingCheckpointState;


Changes made in this file around streamingState is so that it doesn't get modified under the hood when being passed around.

…tail block When a proposer runs out of sub-slots with the consumption cursor at a message prefix that is not a live L1 Inbox bucket end, it appends one transaction-less block to reach one so the checkpoint can be published at all. That block still writes its own block-end fields, but checkpoint blob accounting only ever reserved the current block's end fields, so ordinary packing could fill the checkpoint until the tail no longer fits. Packing now holds back one block's worth of end fields — `getNumBlockEndBlobFields()`, measured at 7 fields and 224 bytes for a real transaction-less block — while the checkpoint can still gain another block, releases it on the last block the checkpoint can hold, and lets the actual tail consume it by writing its own end fields. The checkpoint end marker is still deducted once from the total, so a tail appended to an existing checkpoint costs seven fields rather than eight. The transaction allowance is floored at zero so a full checkpoint reports no room instead of a negative one. This is local proposer packing policy: re-executing a peer's proposal reserves nothing, so no otherwise valid proposal becomes rejectable. Reserving blob space does not reserve build time or guarantee the extra block can be built. Message, per-block, block-count, timing and total blob limits are unchanged.

The tail reservation guard already requires isBuildingProposal, so the hoisted remainingBlocks and its `: 1` fallback bought nothing: inline the count into the guard and let the fair-share divisor keep the local it had before. `Math.max(1, n) > 1` and `n > 1` agree, so packing is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…cepting a proposal Before a node returns or records a checkpoint proposal as valid, it now confirms through an L1 read that the position the checkpoint finishes at closes a live Inbox bucket committing to the rolling hash the proposal signed. This strengthens a previously content-only acceptance policy; it is not a fix for a confirmed defect. The check runs in all-nodes validation, not just committee attestation, so a proposal without endpoint evidence cannot become this node's accepted optimistic checkpoint parent. The count comes from the authenticated last block of the coherent slot snapshot validation already reads, paired with the signed rolling hash — no unsigned proposer hint. The resolver answers with the closest boundary at or below the bound, so only an exact total counts: with buckets ending at 200 and 400, a checkpoint ending at 200 with the matching hash passes, and one ending at 256, or at 200 with a different hash, does not. Intermediate blocks may still end anywhere. The gate never fails open, and never treats a local uncertainty as misconduct. An unreadable view is separated in diagnostics from a view that answers without a live boundary there; both refuse to validate now, reach neither slashing nor the invalid-slot marker nor a peer penalty, and are not remembered as the proposal's verdict, so a changing head cannot poison it. Reads are retried briefly inside the slot's existing duty budget, so a provider that catches up in time still yields a valid verdict and a stalled read cannot hold the acceptance path open. Cached-valid reuse re-confirms the endpoint against a fresh view. The skip flag records nothing as valid and is documented as testing-only. The proposer's own fast path is taken only by the node that built the checkpoint, which resolved that bucket end against the live Inbox while building its final block. Historical ingestion and proof processing stay outside the gate, since old checkpoints may reference evicted endpoints. Cost is one head read plus one eth_call per checkpoint proposal validated.

An endpoint refusal used to be withheld from the validation cache, so an RPC hiccup during the all-nodes callback threw away a content verdict that cost a full checkpoint rebuild and block re-execution, and the attestation callback moments later rebuilt the whole checkpoint again inside the same duty budget. The cached-valid branch also made a second, separate endpoint call. The gate is now one step in handleCheckpointProposal: the content verdict is computed once (cached or fresh) and cached unconditionally, and only then is the endpoint confirmed, against the last block read for the same proposal. Refusals stay non-slashable, set no invalid-slot marker and no peer penalty, record an unvalidated outcome, and are never remembered as the proposal's verdict, so a recovered L1 view still yields a valid verdict on the next call — now without rebuilding. Blob upload moves next to the content verdict so it still fires once per proposal. Also: the README no longer claims the check "closes" a live bucket, and names settlement as the L1-only check it does not replace; the internal endpoint-check module is no longer re-exported; and the three fake Inboxes in the tests collapse into one helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… valid outcome Four fixes from a review of the endpoint gate. An endpoint refusal no longer overwrites a `valid` outcome this node already recorded for the same checkpoint. The gate runs on both calls p2p makes for one proposal, so a second look failing on a local RPC problem used to downgrade the slot to `unvalidated` — which the sentinel counts as a missed proposal for that slot's proposer, feeding epoch performance and the inactivity signal. Only that exact checkpoint is protected; another archive at the slot still records normally. The test helper asserting refusals is renamed to say what it checks: the refusal is not slashable and marks no invalid slot, but it does record `unvalidated`, which is not a neutral outcome. The verdict is bound to the identity of the block it was read at, not to a height. The head is read for its number and hash, the resolution is pinned to that number, and the block is read again afterwards: a provider serving a stale fork, or one the chain reorged under, answers a call at a height as readily as the canonical chain, so an answer whose block is no longer the one at that height is refused as unverifiable rather than passed. A provider that lags uniformly is still invisible from here, and the README says so. The advertised two-second ceiling is now a real bound. It was only handed to `retryUntil`, which checks its deadline after an attempt returns, so one stalled RPC consumed the whole remaining duty; it is now a race, via a new `DutyBudget.runWithin`, and the abandoned loop checks the signal instead of starting another read. Tracker pruning no longer runs in the acceptance path. It reads L1 tips, and the restructure had put it on the cached path too, where the validator calls this method directly without an outer timeout — a hanging tips read stalled the attestation. It is bookkeeping, so it runs detached, and the pipelining parent is not recorded at all once the duty has been stopped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…not against the proposer An endpoint refusal recorded `unvalidated`, which the sentinel reports as `checkpoint-unvalidated` and counts as a missed proposal for the slot's proposer. So a node whose own L1 RPC was down through the retry window charged someone else's validator for it, on the first evaluation of an otherwise content-valid proposal — inactivity accounting this gate was never meant to feed. Both endpoint reasons now record `unverifiable`, the outcome for a proposal this observer could not check against anything outside itself, which is counted against nobody. The monotonic protection covers it too, so a later endpoint failure still cannot retract a `valid` this node recorded for the same checkpoint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…termined invalid The tracker keys its per-slot entry by slot alone, and the protection against a local-inability outcome replacing a determination only covered `valid`, and only for the very checkpoint that produced it. An equivocating proposer whose second proposal for the slot reached the endpoint gate and could not be checked therefore erased what the first one established. Uncertainty now never overwrites a verdict in either direction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A stored message row keeps the L1 height it was first observed at, and recovery accepted any candidate recorded at or below the persisted finality marker as an anchor with no event lookup. That recorded height is never refreshed while the log agrees with the Inbox at the captured head, so a message re-mined to a higher block keeps its old, lower height — which can sit below the marker while the message itself is above it, unfinalized and replaceable. When such a message is later replaced, recovery keeps a prefix L1 no longer has: it rolls back from N to N, refetches, fails to chain the replacement onto the retained prefix, and restarts on the same false anchor. The node then makes no further progress on messages until it is restarted. Every anchor is now a message a bounded event lookup positively found on L1 at the same index and rolling hash. The per-pass lookup budget, the lookup window bounded above by the captured head, the deployment-block fallback and the single-transaction rollback are unchanged, and no new cache or persisted state is introduced. The finality marker is still written and advanced monotonically on authenticated syncs: what is removed is reliance on stale placement, not finality itself. The cost is that a recovery which used to stop early keeps walking backwards, and may prune proposed blocks whose messages sit below the marker; those messages return through ordinary forward ingestion.

…ad backwards addBlock triggers a sync it does not await, so the deployment-refill fixture left a pass in flight that had captured the pre-reorg head. Dropping the finalized-height shortcut made that pass do two event lookups and a rollback instead of returning at once, so it could now commit after the pass for the new, lower head and leave the old height as the synced one. The fixture failed about three times in ten; draining the pending sync before moving the head fixes it, 30 runs clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The signed Inbox message-prefix reference is now a required field on a standalone BlockProposal and on the final block embedded in a CheckpointProposal, serialized without a presence flag ahead of the optional SignedTxs bundle and always included in the signing payload. The end-of-buffer fallback is gone: a proposal that omits or truncates the reference is malformed input rather than a valid zero-message proposal. The final block's reference is checked against the checkpoint header's inboxRollingHash whenever a final block is present, and a zero-new-message block re-states the prefix its parent ended at instead of leaving the field unset. Also links the three deferred backlog limitations from the code that still carries them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… block attempt Replaces the `as any` stubs and the private `runPromise` reach in the checkpoint prover's message-slicing tests with typed `mock<T>()` doubles and a completion signal off the sub-tree, and asserts the bundles each block received rather than raw call counts. Adds a sequencer regression for the explicit consumption state: a first block attempt that fails on valid txs must leave the cursor untouched, so its retry re-derives the same range and the checkpoint advances the cursor exactly once. Updates the handoff-join expectation to the propagated processing error, and drops the mock builder's fallback for the now-required `l1ToL2Messages`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

LeilaWang commented Sep 7, 2026

View reviewed changes

LeilaWang force-pushed the lw/fast-inbox-cleanup branch from b3f5d87 to 48043e7 Compare September 7, 2026 20:30

LeilaWang commented Sep 7, 2026

View reviewed changes

spalladino force-pushed the spl/fi2-message-only-docs branch from 145907b to f0d1f1b Compare September 8, 2026 00:55

spalladino force-pushed the lw/fast-inbox-cleanup branch from 48043e7 to f2322aa Compare September 10, 2026 03:55

spalladino changed the base branch from spl/fi2-message-only-docs to spl/fi2-required-inbox-prefix September 10, 2026 03:55

spalladino added this pull request to stack #25441 September 10, 2026 03:56

spalladino force-pushed the spl/fi2-required-inbox-prefix branch from 7341b54 to 230668b Compare September 10, 2026 04:04

spalladino requested review from IlyasRidhuan, iAmMichaelConnor, iakovenkos and ledwards2225 as code owners September 10, 2026 04:04

spalladino force-pushed the lw/fast-inbox-cleanup branch from f2322aa to ee6c609 Compare September 10, 2026 04:04

spalladino and others added 8 commits September 10, 2026 02:10

spalladino force-pushed the spl/fi2-required-inbox-prefix branch from 230668b to 2f815c7 Compare September 10, 2026 05:12

spalladino force-pushed the lw/fast-inbox-cleanup branch from ee6c609 to 259713d Compare September 10, 2026 05:13

spalladino and others added 2 commits September 10, 2026 02:41

spalladino force-pushed the spl/fi2-required-inbox-prefix branch from 2f815c7 to 477b650 Compare September 10, 2026 05:41

Cleanups.

504acdc

spalladino force-pushed the lw/fast-inbox-cleanup branch from 259713d to 011ee1f Compare September 10, 2026 05:42

spalladino removed this pull request from stack #25441 September 10, 2026 11:49

spalladino force-pushed the spl/fi2-required-inbox-prefix branch 3 times, most recently from c46e794 to e95e153 Compare September 10, 2026 14:30

spalladino mentioned this pull request Sep 10, 2026

feat(circuits): add InboxParity variants for size 4 and 16 #25463

Open

Navigation Menu

Uh oh!

refactor(fast-inbox): some cleanups - #25422

refactor(fast-inbox): some cleanups#25422
LeilaWang wants to merge 12 commits into
spl/fi2-required-inbox-prefixfrom
lw/fast-inbox-cleanup

LeilaWang commented Sep 7, 2026 •

edited by spalladino

Loading

Uh oh!

LeilaWang Sep 7, 2026

Uh oh!

LeilaWang Sep 7, 2026

Uh oh!

LeilaWang Sep 7, 2026

Uh oh!

LeilaWang Sep 7, 2026

Uh oh!

LeilaWang Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

LeilaWang commented Sep 7, 2026 • edited by spalladino Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LeilaWang Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

LeilaWang Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

LeilaWang Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

LeilaWang Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

LeilaWang Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LeilaWang commented Sep 7, 2026 •

edited by spalladino

Loading