Skip to content

fix(fast-inbox): insert the block's L1-to-L2 messages before executing its txs (AztecProtocol/aztec-packages#25323) - #216

Open
spalladino wants to merge 1 commit into
spl/fi-n10-docs-correctionsfrom
spl/a-2041-append-messages-before-txs
Open

spalladino wants to merge 1 commit into
spl/fi-n10-docs-correctionsfrom
spl/a-2041-append-messages-before-txs

Conversation

@spalladino

Copy link
Copy Markdown
Collaborator

CheckpointBuilder.buildBlock ran the public processor on a world-state fork that did not yet contain the
block's own streaming L1→L2 messages. The messages were appended afterwards, inside
LightweightCheckpointBuilder.addBlock, once the block was being sealed.

The proving path does it the other way round. prover-node/src/job/checkpoint-prover.ts createFork appends
the block's message leaves to its fork before re-executing the block's txs, and the block-root circuit pins
each tx's l1_to_l2_tree_snapshot to the post-append root.

So the two orderings disagree for exactly one kind of transaction: a public call that consumes a message its own
block inserts.

proposal / validation (buildBlock) proving (checkpoint-prover)
L1→L2 tree during AVM execution pre-append — the leaf is not there post-append — the leaf is there
the consuming tx reverts succeeds

The tx effects therefore differ between the block that was proposed and the block that is re-executed, the prover
throws Block header mismatch on a block that has already been attested and published, and the epoch cannot be
proven and gets pruned. Proposer and validators both go through buildBlock, so they agree with each other and
sign; nothing catches it before the prover. Any user can trigger it by sending an L1 message and consuming it
promptly enough to land in the inserting block.

The fix

Two parts.

1. buildBlock appends the block's messages to the fork before executing its txs — right after
ForkCheckpoint.new and before processor.process, so the AVM reads the same post-append tree the prover and
the circuits already use. Because the circuits pin the post-append root, only the proposer/validator side has to
move: the prover, the orchestrator and the circuits are untouched.

Appending inside the fork checkpoint is what makes the retry safe. A block that fails
(InsufficientValidTxsError, or a throw out of the processor) rolls its message leaves back together with its tx
effects, so the retry in the next sub-slot does not double-insert.

2. LightweightCheckpointBuilder.addBlock splits into two public methods over one private implementation.
sealBlock is for a caller that has already applied the state updates to the fork — that is buildBlock now —
and applyEffectsAndSealBlock is for a caller that wants the builder to insert the tx effects and the messages
itself, which is every test and mock. The old insertTxsEffects flag and the message append were always toggled
together, so a single applyStateUpdates switch replaces both. Either way the messages are accumulated into the
checkpoint's message list, so inboxRollingHash is unchanged.

What does not change

Block bodies, headers, leaf indices and the checkpoint inboxRollingHash are identical before and after — the
leaves land compactly at the same indices, and the header's l1ToL2MessageTree snapshot is read at the same
point. The only thing that moves is the tree the AVM reads during execution. No circuit, L1, p2p or
serialization change.

Port provenance

This is a port of AztecProtocol/aztec-packages #25323 (6271d5277c). That PR never merged to next — it
merged into its stack parent branch — so the node port plan recorded its content as baseline and skipped it.
Three rungs of this stack independently hit the resulting revert and logged it as an open item rather than
fixing it; this rung is that fix.

Red / green

New regression cases in validator-client/src/checkpoint_builder.test.ts, under
buildBlock with streaming L1-to-L2 messages (real world state). They run a real
NativeWorldStateService fork and a real LightweightCheckpointBuilder, because the position of the messages
relative to tx execution is invisible with a mocked fork.

On the base branch (spl/fi-n10-docs-corrections), before the source change:

  ● CheckpointBuilder › buildBlock with streaming L1-to-L2 messages (real world state)
    › the block's messages are in the fork when the public processor runs

    expect(received).toBe(expected) // Object.is equality

    Expected: 3n
    Received: 0n

    > 969 |       expect(treeSizeDuringExecution).toBe(3n);

Tests:       1 failed, 42 skipped, 3 passed, 46 total

The L1→L2 tree is empty while the txs execute. The other three cases — the two rollback cases and the
empty-list case — pass on the base branch by construction: they pin the "append inside the ForkCheckpoint"
requirement so the fix cannot be implemented by appending outside it.

After the fix:

Tests:       46 passed, 46 total    validator-client/src/checkpoint_builder.test.ts
Tests:       8 passed, 8 total      prover-client/src/light/lightweight_checkpoint_builder.test.ts

lightweight_checkpoint_builder.test.ts gains sealBlock reuses leaves already in the fork and produces the same block as applyEffectsAndSealBlock: same header, same tree size, and the same inboxRollingHash as the
default path. It cannot be run red, since sealBlock does not exist before the source change.

Also green: proposal_handler.test.ts (79), checkpoint_proposal_job.test.ts (87), the prover-client light
suite (13 including the bench test), yarn build, yarn lint.

Where the e2e coverage lives

Upstream #25323 added an e2e case, consumes a message in the same block that inserts it, which this repo never
had. It does not need porting: PR #189 already added the same assertion at
end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts:349,

expect(consumeReceipt.blockNumber).toEqual(Number(inserting.blockNumber));

which has been expected-red precisely because this fix was missing. Since this rung sits below #189, that
assertion is the e2e coverage and should go green once this lands. No duplicate case was added.

Stack position

Inserted between #186 (spl/fi-n10-docs-corrections) and #187 (spl/fi-n11-inbox-bot). #187, #189 and #190
were rebased onto it with their content unchanged.

Fixes A-2041
Fixes A-2042

…g its txs (AztecProtocol/aztec-packages#25323)

CheckpointBuilder.buildBlock ran the public processor on a world-state fork that did not yet contain the block's own
streaming L1-to-L2 messages; they were appended afterwards inside LightweightCheckpointBuilder.addBlock. The prover
node appends them to its fork before re-executing, and the block-root circuit pins each tx's l1_to_l2_tree_snapshot to
the post-append root, so a public tx consuming a message its own block inserts reverted at proposal time and succeeded
at proving time. The differing tx effects make the prover throw a block header mismatch on an already-attested block,
and the epoch cannot be proven.

buildBlock now appends the block's messages to the fork right after ForkCheckpoint.new and before processor.process,
so the AVM reads the same post-append tree the prover and the circuits use. Appending inside the fork checkpoint means
a failed block rolls the leaves back with the tx effects.

LightweightCheckpointBuilder.addBlock splits into sealBlock (caller already applied the state updates) and
applyEffectsAndSealBlock (the builder inserts tx effects and messages, then seals). Either way the messages are
accumulated into the checkpoint's message list, so inboxRollingHash is unchanged.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

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.

1 participant