Skip to content

Commit 0f0577c

Browse files
spalladinoaztec-bot
authored andcommitted
fix(validator): sync world state before forking in checkpoint proposal validation (#24694)
## Problem On a live mainnet node, checkpoint-proposal validation could throw an uncaught error out of the gossipsub message handler whenever it ran while the world-state synchronizer trailed the archiver (block source) by a block: ``` ERROR world-state:database Call CREATE_FORK failed: Error: Unable to initialize from future block: 117860 unfinalizedBlockHeight: 117859. Tree name: NullifierTree ERROR p2p:libp2p_service Error handling gossipsub message: Error: Unable to initialize from future block: 117860 unfinalizedBlockHeight: 117859. Tree name: NullifierTree ``` The archiver already had block N (so the block lookups inside checkpoint validation succeeded), but world state's `unfinalizedBlockHeight` was still N-1. When `validateCheckpointProposal` forked world state at the parent block before it had been applied, the raw tree error escaped as an uncaught ERROR-level gossipsub message. The node lost that checkpoint validation/attestation for the round and spammed error logs; it self-healed the next tick, but it should not happen. ## Root cause The two world-state fork sites were asymmetric. The block re-execution path (`reexecuteTransactions`) syncs world state *before* forking. The checkpoint validation path (`validateCheckpointProposal`) forked via `checkpointsBuilder.getFork` *without* a preceding sync. The checkpoint path does sync the block source (archiver) earlier, but that does not advance the world state, so the gap remained. ## Fix - Move the sync-before-fork invariant into `FullNodeCheckpointsBuilder.getFork`: it now calls `worldState.syncImmediate(blockNumber, blockHash)` before `worldState.fork(blockNumber)`, so the single fork helper owns the invariant rather than each caller. `syncImmediate` blocks until world state reaches the block, or throws a typed error if it genuinely cannot, instead of leaking the raw tree error. - In `validateCheckpointProposal`, look up the parent block's hash from the block source and pass it through so the sync re-syncs on a world-state reorg. Wrap the fork in a try/catch that maps any failure to a clean, non-slashable `world_state_not_synced` result, so a sync/fork failure never surfaces as an uncaught ERROR-level gossipsub message. - As a second, independent guard, check that the forked world state's archive root matches the proposal's `lastArchiveRoot` before rebuilding the checkpoint. A mismatch means world state forked from a different chain than the proposal was built on, so we fail fast with a clean, non-slashable `initial_archive_mismatch` result — mirroring the block-proposal re-execution check — instead of a confusing downstream header/archive mismatch. Both new reasons record an `unvalidated` outcome and are non-slashable, matching the other "we couldn't validate right now" / local-state-divergence reasons. Other checkpoint-builder fork sites were reviewed and need no change: the proposer's `checkpoint_proposal_job` forks at its own already-synced, coherence-checked world-state tip, and the block-proposal re-execution path already syncs before forking and performs the equivalent archive-root check. ## Tests - `checkpoint_builder.test.ts`: `getFork` syncs world state to the block before forking, and propagates a sync failure without forking. - `proposal_handler.test.ts`: forks at the parent block number passing its block hash; a fork failure returns `world_state_not_synced`; and a fork whose archive root diverges from the proposal returns `initial_archive_mismatch`. ## Notes The same bug exists on the v4 line — the v4 site is the same function, forking via `this.worldState.fork(parentBlockNumber)` directly without a preceding `syncImmediate`. This fix should be ported there as well. Fixes A-1421 (cherry picked from commit b7a7d1b)
1 parent 453695e commit 0f0577c

6 files changed

Lines changed: 202 additions & 21 deletions

File tree

yarn-project/stdlib/src/interfaces/block-builder.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { BlockNumber, CheckpointNumber } from '@aztec/foundation/branded-ty
22
import type { Fr } from '@aztec/foundation/curves/bn254';
33
import type { LoggerBindings } from '@aztec/foundation/log';
44

5+
import type { BlockHash } from '../block/block_hash.js';
56
import type { L2Block } from '../block/l2_block.js';
67
import type { ChainConfig, SequencerConfig } from '../config/chain-config.js';
78
import type { L1RollupConstants } from '../epoch-helpers/index.js';
@@ -143,7 +144,11 @@ export interface ICheckpointBlockBuilder {
143144

144145
/** Interface for creating checkpoint builders. */
145146
export interface ICheckpointsBuilder {
146-
getFork(blockNumber: BlockNumber): Promise<MerkleTreeWriteOperations>;
147+
/**
148+
* Syncs world state to `blockNumber` and returns a fork of it at that block. When `blockHash` is
149+
* provided it is verified against the synced block, triggering a resync on mismatch (reorg detection).
150+
*/
151+
getFork(blockNumber: BlockNumber, blockHash?: BlockHash): Promise<MerkleTreeWriteOperations>;
147152

148153
startCheckpoint(
149154
checkpointNumber: CheckpointNumber,

yarn-project/validator-client/src/checkpoint_builder.test.ts

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { TestDateProvider } from '@aztec/foundation/timer';
1414
import type { LightweightCheckpointBuilder } from '@aztec/prover-client/light';
1515
import type { AvmSimulator, PublicContractsDB, PublicProcessor } from '@aztec/simulator/server';
1616
import { AztecAddress } from '@aztec/stdlib/aztec-address';
17-
import { L2Block } from '@aztec/stdlib/block';
17+
import { BlockHash, L2Block } from '@aztec/stdlib/block';
1818
import type { ContractDataSource } from '@aztec/stdlib/contract';
1919
import { Gas, GasFees } from '@aztec/stdlib/gas';
2020
import {
@@ -24,6 +24,7 @@ import {
2424
type MerkleTreeWriteOperations,
2525
type PublicProcessorLimits,
2626
type PublicProcessorValidator,
27+
type WorldStateSynchronizer,
2728
} from '@aztec/stdlib/interfaces/server';
2829
import {
2930
type CheckpointGlobalVariables,
@@ -37,7 +38,7 @@ import type { TelemetryClient } from '@aztec/telemetry-client';
3738
import { describe, expect, it, jest } from '@jest/globals';
3839
import { type MockProxy, mock } from 'jest-mock-extended';
3940

40-
import { CheckpointBuilder } from './checkpoint_builder.js';
41+
import { CheckpointBuilder, FullNodeCheckpointsBuilder } from './checkpoint_builder.js';
4142

4243
describe('CheckpointBuilder', () => {
4344
let checkpointBuilder: TestCheckpointBuilder;
@@ -789,3 +790,52 @@ describe('CheckpointBuilder', () => {
789790
});
790791
});
791792
});
793+
794+
describe('FullNodeCheckpointsBuilder', () => {
795+
let worldState: MockProxy<WorldStateSynchronizer>;
796+
let builder: FullNodeCheckpointsBuilder;
797+
798+
const blockNumber = BlockNumber(5);
799+
800+
beforeEach(() => {
801+
worldState = mock<WorldStateSynchronizer>();
802+
const telemetryClient = mock<TelemetryClient>();
803+
telemetryClient.getMeter.mockReturnValue(mock());
804+
telemetryClient.getTracer.mockReturnValue(mock());
805+
806+
builder = new FullNodeCheckpointsBuilder(
807+
{ l1GenesisTime: 0n, slotDuration: 24, l1ChainId: 1, rollupVersion: 1, rollupManaLimit: 200_000_000 },
808+
worldState,
809+
mock<ContractDataSource>(),
810+
new TestDateProvider(),
811+
telemetryClient,
812+
);
813+
});
814+
815+
describe('getFork', () => {
816+
it('syncs world state to the block (with its hash) before forking', async () => {
817+
const forkResult = mock<MerkleTreeWriteOperations>();
818+
worldState.fork.mockResolvedValue(forkResult);
819+
const blockHash = BlockHash.random();
820+
821+
const result = await builder.getFork(blockNumber, blockHash);
822+
823+
expect(result).toBe(forkResult);
824+
// The block hash is relayed to syncImmediate for reorg detection.
825+
expect(worldState.syncImmediate).toHaveBeenCalledWith(blockNumber, blockHash);
826+
expect(worldState.fork).toHaveBeenCalledWith(blockNumber);
827+
// Syncing must precede the fork, otherwise the fork can hit a block the trees have not applied yet
828+
// and throw a raw "initialize from future block" tree error.
829+
expect(worldState.syncImmediate.mock.invocationCallOrder[0]).toBeLessThan(
830+
worldState.fork.mock.invocationCallOrder[0],
831+
);
832+
});
833+
834+
it('propagates a sync failure without forking', async () => {
835+
worldState.syncImmediate.mockRejectedValue(new Error('Unable to initialize from future block'));
836+
837+
await expect(builder.getFork(blockNumber)).rejects.toThrow('Unable to initialize from future block');
838+
expect(worldState.fork).not.toHaveBeenCalled();
839+
});
840+
});
841+
});

yarn-project/validator-client/src/checkpoint_builder.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
PublicProcessor,
1616
createPublicTxSimulatorForBlockBuilding,
1717
} from '@aztec/simulator/server';
18-
import { L2Block } from '@aztec/stdlib/block';
18+
import { type BlockHash, L2Block } from '@aztec/stdlib/block';
1919
import { Checkpoint } from '@aztec/stdlib/checkpoint';
2020
import type { ContractDataSource } from '@aztec/stdlib/contract';
2121
import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers';
@@ -419,8 +419,17 @@ export class FullNodeCheckpointsBuilder implements ICheckpointsBuilder {
419419
);
420420
}
421421

422-
/** Returns a fork of the world state at the given block number. */
423-
getFork(blockNumber: BlockNumber): Promise<MerkleTreeWriteOperations> {
422+
/**
423+
* Syncs world state to the given block number and returns a fork of it at that block.
424+
*
425+
* Syncing first is required: the block source (archiver) can already hold a block while world state
426+
* still trails it, and forking a not-yet-applied block throws a raw "initialize from future block"
427+
* tree error. syncImmediate blocks until world state reaches the block, or throws a typed error if it
428+
* genuinely cannot. When `blockHash` is provided it is verified against the synced block, triggering a
429+
* resync on mismatch (reorg detection).
430+
*/
431+
async getFork(blockNumber: BlockNumber, blockHash?: BlockHash): Promise<MerkleTreeWriteOperations> {
432+
await this.worldState.syncImmediate(blockNumber, blockHash);
424433
return this.worldState.fork(blockNumber);
425434
}
426435
}

yarn-project/validator-client/src/proposal_handler.test.ts

Lines changed: 79 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { TestDateProvider } from '@aztec/foundation/timer';
1010
import { type FieldsOf, unfreeze } from '@aztec/foundation/types';
1111
import type { P2P } from '@aztec/p2p';
1212
import type { BlockProposalValidator } from '@aztec/p2p/msg_validators';
13+
import { BlockHash } from '@aztec/stdlib/block';
1314
import type { BlockData, L2Block, L2BlockSink, L2BlockSource } from '@aztec/stdlib/block';
1415
import { type Checkpoint, CheckpointReexecutionTracker, type ProposedCheckpointData } from '@aztec/stdlib/checkpoint';
1516
import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers';
@@ -491,8 +492,12 @@ describe('ProposalHandler checkpoint validation', () => {
491492
let mockCheckpointBuilder: MockProxy<CheckpointBuilder>;
492493
let mockDispose: jest.Mock;
493494

494-
/** Sets up mocks so the handler passes all early checks and reaches the checkpoint rebuild path. */
495-
function setupDeepValidationMocks(computedCheckpoint: Partial<Checkpoint>) {
495+
/**
496+
* Sets up mocks so the handler passes all early checks and reaches the checkpoint rebuild path.
497+
* `forkArchiveRoot` is the archive root the forked world state reports; it must match the proposal
498+
* header's lastArchiveRoot (default Fr.ZERO, as CheckpointHeader.empty) to pass the fork archive check.
499+
*/
500+
function setupDeepValidationMocks(computedCheckpoint: Partial<Checkpoint>, forkArchiveRoot: Fr = Fr.ZERO) {
496501
// Block with matching archive so the early archive check passes
497502
const block = {
498503
archive: new AppendOnlyTreeSnapshot(archiveRoot, 1),
@@ -505,7 +510,10 @@ describe('ProposalHandler checkpoint validation', () => {
505510
blockSource.getBlocksForSlot.mockResolvedValue([block]);
506511

507512
mockDispose = jest.fn();
508-
checkpointsBuilder.getFork.mockResolvedValue({ [Symbol.asyncDispose]: mockDispose } as any);
513+
checkpointsBuilder.getFork.mockResolvedValue({
514+
[Symbol.asyncDispose]: mockDispose,
515+
getTreeInfo: () => Promise.resolve({ root: forkArchiveRoot.toBuffer() }),
516+
} as any);
509517

510518
mockCheckpointBuilder = mock<CheckpointBuilder>();
511519
mockCheckpointBuilder.completeCheckpoint.mockResolvedValue({
@@ -618,14 +626,17 @@ describe('ProposalHandler checkpoint validation', () => {
618626
toBlobFields: () => [],
619627
} as unknown as L2Block;
620628

621-
setupDeepValidationMocks({
622-
header,
623-
archive: new AppendOnlyTreeSnapshot(archiveRoot, 1),
624-
blocks: [minimalBlock],
625-
number: CheckpointNumber(1),
626-
slot: SlotNumber(1),
627-
toBlobFields: () => [],
628-
});
629+
setupDeepValidationMocks(
630+
{
631+
header,
632+
archive: new AppendOnlyTreeSnapshot(archiveRoot, 1),
633+
blocks: [minimalBlock],
634+
number: CheckpointNumber(1),
635+
slot: SlotNumber(1),
636+
toBlobFields: () => [],
637+
},
638+
lastArchiveRoot,
639+
);
629640

630641
const proposal = await makeProposal({ archiveRoot, checkpointHeader: header });
631642
const result = await handler.handleCheckpointProposal(proposal, proposalInfo);
@@ -640,6 +651,63 @@ describe('ProposalHandler checkpoint validation', () => {
640651
await handler.handleCheckpointProposal(proposal, proposalInfo);
641652
expect(mockDispose).toHaveBeenCalled();
642653
});
654+
655+
// Parent of the single block (number 1) used across the deep-validation setup.
656+
const parentBlockNumber = BlockNumber(0);
657+
658+
// getFork syncs world state to the parent block before forking (see FullNodeCheckpointsBuilder tests).
659+
// This asserts the caller forks at the parent block number (one before the checkpoint's first block),
660+
// passing the parent's block hash (looked up from the block source) for reorg detection.
661+
it('forks at the parent block number, passing its block hash', async () => {
662+
const parentBlockHash = BlockHash.random();
663+
setupDeepValidationMocks({ header: makeHeader({ totalManaUsed: new Fr(999) }) });
664+
blockSource.getBlockData.mockResolvedValue({
665+
header: makeBlockHeader(),
666+
blockHash: parentBlockHash,
667+
} as BlockData);
668+
669+
const proposal = await makeProposal({ archiveRoot, checkpointHeader: makeHeader() });
670+
await handler.handleCheckpointProposal(proposal, proposalInfo);
671+
672+
expect(checkpointsBuilder.getFork).toHaveBeenCalledWith(parentBlockNumber, parentBlockHash);
673+
});
674+
675+
// If world state forked from a different chain than the proposal was built on (e.g. a reorg), the fork's
676+
// archive root will not match the checkpoint's expected starting archive. Fail fast before rebuilding.
677+
it('returns initial_archive_mismatch when the fork archive does not match the last archive', async () => {
678+
// Fork reports a different archive root than the proposal header's lastArchiveRoot (Fr.ZERO).
679+
setupDeepValidationMocks({ header: makeHeader() }, Fr.random());
680+
681+
const proposal = await makeProposal({ archiveRoot, checkpointHeader: makeHeader() });
682+
const result = await handler.handleCheckpointProposal(proposal, proposalInfo);
683+
684+
expect(result).toEqual({
685+
isValid: false,
686+
reason: 'initial_archive_mismatch',
687+
checkpointNumber: CheckpointNumber(1),
688+
});
689+
expect(mockDispose).toHaveBeenCalled();
690+
});
691+
692+
// Regression: on a live node the archiver held block N (so getBlocksForSlot succeeds) while world state
693+
// still trailed at N-1, so forking the parent threw "Unable to initialize from future block" and the raw
694+
// tree error escaped as an uncaught gossipsub error. Validation must map any fork failure to a clean
695+
// result instead of letting it escape.
696+
it('returns world_state_not_synced when forking the parent block fails', async () => {
697+
setupDeepValidationMocks({ header: makeHeader() });
698+
checkpointsBuilder.getFork.mockRejectedValue(
699+
new Error('Unable to initialize from future block: 1 unfinalizedBlockHeight: 0. Tree name: NullifierTree'),
700+
);
701+
702+
const proposal = await makeProposal({ archiveRoot, checkpointHeader: makeHeader() });
703+
const result = await handler.handleCheckpointProposal(proposal, proposalInfo);
704+
705+
expect(result).toEqual({
706+
isValid: false,
707+
reason: 'world_state_not_synced',
708+
checkpointNumber: CheckpointNumber(1),
709+
});
710+
});
643711
});
644712

645713
// Regression for A-1218: during a reorg the archiver can still hold a stale block at the proposal's

yarn-project/validator-client/src/proposal_handler.ts

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ import type { CheckpointReexecutionTracker, ReexecutionOutcome } from '@aztec/st
2525
import { getPreviousCheckpointOutHashes, validateCheckpoint } from '@aztec/stdlib/checkpoint';
2626
import { getEpochAtSlot } from '@aztec/stdlib/epoch-helpers';
2727
import { Gas } from '@aztec/stdlib/gas';
28-
import type { ITxProvider, ValidatorClientFullConfig, WorldStateSynchronizer } from '@aztec/stdlib/interfaces/server';
28+
import type {
29+
ITxProvider,
30+
MerkleTreeWriteOperations,
31+
ValidatorClientFullConfig,
32+
WorldStateSynchronizer,
33+
} from '@aztec/stdlib/interfaces/server';
2934
import {
3035
type L1ToL2MessageSource,
3136
accumulateCheckpointOutHashes,
@@ -91,10 +96,12 @@ export type CheckpointProposalValidationFailureReason =
9196
| 'invalid_fee_asset_price_modifier'
9297
| 'last_block_not_found'
9398
| 'block_fetch_error'
99+
| 'world_state_not_synced'
94100
| 'checkpoint_already_published'
95101
| 'no_blocks_for_slot'
96102
| 'last_block_archive_mismatch'
97103
| 'too_many_blocks_in_checkpoint'
104+
| 'initial_archive_mismatch'
98105
| 'checkpoint_header_mismatch'
99106
| 'archive_mismatch'
100107
| 'out_hash_mismatch'
@@ -115,6 +122,8 @@ const CHECKPOINT_VALIDATION_REASON_TO_OUTCOME: Record<
115122
checkpoint_already_published: undefined,
116123
last_block_not_found: 'unvalidated',
117124
block_fetch_error: 'unvalidated',
125+
world_state_not_synced: 'unvalidated',
126+
initial_archive_mismatch: 'unvalidated',
118127
no_blocks_for_slot: 'unvalidated',
119128
last_block_archive_mismatch: 'invalid',
120129
too_many_blocks_in_checkpoint: 'invalid',
@@ -186,6 +195,9 @@ export const SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT: Record<
186195
['invalid_signature']: false,
187196
['last_block_not_found']: false,
188197
['block_fetch_error']: false,
198+
['world_state_not_synced']: false,
199+
// A reorg / divergent local chain, not a proposer offense (mirrors the block path's initial_state_mismatch).
200+
['initial_archive_mismatch']: false,
189201
['checkpoint_already_published']: false,
190202
};
191203

@@ -1161,9 +1173,41 @@ export class ProposalHandler {
11611173
log: this.log,
11621174
});
11631175

1164-
// Fork world state at the block before the first block
1176+
// Fork world state at the block before the first block. getFork syncs world state to the parent block
1177+
// first (see its doc): the block source (archiver) can already hold the block while world state still
1178+
// trails it by one, and forking a not-yet-applied block throws a raw tree error that would otherwise
1179+
// escape as an uncaught gossipsub error. We pass the parent's expected block hash so the sync detects a
1180+
// world-state reorg (undefined for the genesis parent, where no block exists to pin). On failure we map
1181+
// to a clean validation result rather than letting it escape.
11651182
const parentBlockNumber = BlockNumber(firstBlock.number - 1);
1166-
await using fork = await this.checkpointsBuilder.getFork(parentBlockNumber);
1183+
let forkResult: MerkleTreeWriteOperations;
1184+
try {
1185+
const parentBlockHash = (await this.blockSource.getBlockData({ number: parentBlockNumber }))?.blockHash;
1186+
forkResult = await this.checkpointsBuilder.getFork(parentBlockNumber, parentBlockHash);
1187+
} catch (err) {
1188+
this.log.warn(`Failed to fork world state at block ${parentBlockNumber} for checkpoint proposal`, {
1189+
...proposalInfo,
1190+
parentBlockNumber,
1191+
err,
1192+
});
1193+
return { isValid: false, reason: 'world_state_not_synced', checkpointNumber };
1194+
}
1195+
await using fork = forkResult;
1196+
1197+
// Verify the fork's archive root matches the checkpoint's expected starting archive (the archive after
1198+
// the parent block). A mismatch means world state forked from a different chain than the proposal was
1199+
// built on (e.g. a reorg), so recomputing the checkpoint against it would be meaningless. This mirrors
1200+
// the block-proposal re-execution check and fails fast with a clean, non-slashable result instead of a
1201+
// confusing downstream mismatch.
1202+
const forkArchiveRoot = new Fr((await fork.getTreeInfo(MerkleTreeId.ARCHIVE)).root);
1203+
if (!forkArchiveRoot.equals(proposal.checkpointHeader.lastArchiveRoot)) {
1204+
this.log.warn(`Fork archive root does not match checkpoint proposal's last archive`, {
1205+
...proposalInfo,
1206+
forkArchiveRoot: forkArchiveRoot.toString(),
1207+
expectedLastArchiveRoot: proposal.checkpointHeader.lastArchiveRoot.toString(),
1208+
});
1209+
return { isValid: false, reason: 'initial_archive_mismatch', checkpointNumber };
1210+
}
11671211

11681212
// Create checkpoint builder with all existing blocks
11691213
const checkpointBuilder = await this.checkpointsBuilder.openCheckpoint(

yarn-project/validator-client/src/validator.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,12 @@ describe('ValidatorClient', () => {
394394
} as unknown as L2Block;
395395
const disposeFork = jest.fn();
396396
blockSource.getBlocksForSlot.mockResolvedValue([checkpointBlock]);
397-
checkpointsBuilder.getFork.mockResolvedValue({ [Symbol.asyncDispose]: disposeFork } as any);
397+
checkpointsBuilder.getFork.mockResolvedValue({
398+
[Symbol.asyncDispose]: disposeFork,
399+
// Match the proposal's expected starting archive so the fork archive check passes and validation
400+
// reaches the header-mismatch offense under test.
401+
getTreeInfo: () => Promise.resolve({ root: proposalHeader.lastArchiveRoot.toBuffer() }),
402+
} as any);
398403
mockCheckpointBuilder.completeCheckpoint.mockResolvedValue({
399404
header: computedHeader,
400405
archive: new AppendOnlyTreeSnapshot(proposal.archive, blockNumber),

0 commit comments

Comments
 (0)