Skip to content

Commit 5dfbc09

Browse files
committed
fix: reject block proposals in poisoned slots
1 parent 1fd63ef commit 5dfbc09

2 files changed

Lines changed: 157 additions & 0 deletions

File tree

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ export type BlockProposalValidationFailureReason =
5959
| 'failed_txs'
6060
| 'initial_state_mismatch'
6161
| 'timeout'
62+
| 'block_proposal_beyond_checkpoint'
63+
| 'checkpoint_proposal_equivocation'
6264
| 'unknown_error';
6365

6466
type ReexecuteTransactionsResult = {
@@ -146,6 +148,10 @@ type CheckpointComputationResult =
146148
| { checkpointNumber: CheckpointNumber; reason?: undefined }
147149
| { checkpointNumber?: undefined; reason: 'invalid_proposal' | 'global_variables_mismatch' };
148150

151+
type BlockProposalSlotValidationResult =
152+
| { isValid: true }
153+
| { isValid: false; reason: 'block_proposal_beyond_checkpoint' | 'checkpoint_proposal_equivocation' };
154+
149155
/** Handles block and checkpoint proposals for both validator and non-validator nodes. */
150156
export class ProposalHandler {
151157
public readonly tracer: Tracer;
@@ -164,6 +170,9 @@ export class ProposalHandler {
164170
/** Returns current validator addresses for own-proposal detection. Set via register(). */
165171
private getOwnValidatorAddresses?: () => string[];
166172

173+
/** P2P proposal pool access for deciding when retained proposals should block archiver processing. */
174+
private p2pClient?: Pick<P2P, 'getProposalsForSlot'>;
175+
167176
private checkpointProposalValidationFailureCallback?: CheckpointProposalValidationFailureCallback;
168177

169178
constructor(
@@ -213,6 +222,7 @@ export class ProposalHandler {
213222

214223
/**
215224
* Registers handlers for block and checkpoint proposals on the p2p client.
225+
* Records the p2p client so validation can inspect retained proposals.
216226
* Block proposals are registered for non-validator nodes (validators register their own enhanced handler).
217227
* The all-nodes checkpoint proposal handler is always registered for validation, caching, and pipelining.
218228
* @param archiver - Archiver reference for setting proposed checkpoints (pipelining)
@@ -224,6 +234,7 @@ export class ProposalHandler {
224234
archiver?: Pick<Archiver, 'addProposedCheckpoint' | 'getL1Constants'>,
225235
getOwnValidatorAddresses?: () => string[],
226236
): ProposalHandler {
237+
this.p2pClient = p2pClient;
227238
this.archiver = archiver;
228239
this.getOwnValidatorAddresses = getOwnValidatorAddresses;
229240

@@ -362,6 +373,16 @@ export class ProposalHandler {
362373
return { isValid: false, reason: 'invalid_proposal' };
363374
}
364375

376+
const retainedSlotValidation = await this.validateNewBlockInSlot(proposal);
377+
if (!retainedSlotValidation.isValid) {
378+
this.log.info(`Block proposal conflicts with retained proposals, skipping archiver processing`, {
379+
...proposalInfo,
380+
indexWithinCheckpoint: proposal.indexWithinCheckpoint,
381+
reason: retainedSlotValidation.reason,
382+
});
383+
return { isValid: false, blockNumber: proposal.blockNumber, reason: retainedSlotValidation.reason };
384+
}
385+
365386
// Ensure the block source is synced before checking for existing blocks,
366387
// since a proposed checkpoint prune may remove blocks we'd otherwise find.
367388
// This affects mostly the block_number_already_exists check, since a pending
@@ -500,6 +521,26 @@ export class ProposalHandler {
500521
return { isValid: true, blockNumber, reexecutionResult };
501522
}
502523

524+
private async validateNewBlockInSlot(blockProposal: BlockProposal): Promise<BlockProposalSlotValidationResult> {
525+
if (!this.p2pClient) {
526+
return { isValid: true };
527+
}
528+
529+
const { blockProposals, checkpointProposals } = await this.p2pClient.getProposalsForSlot(blockProposal.slotNumber);
530+
531+
if (checkpointProposals.length === 0) {
532+
return { isValid: true };
533+
} else if (checkpointProposals.length > 1) {
534+
return { isValid: false, reason: 'checkpoint_proposal_equivocation' };
535+
} else {
536+
const checkpointProposal = checkpointProposals[0];
537+
const terminalBlock = blockProposals.find(block => block.archive.equals(checkpointProposal.archive));
538+
return terminalBlock !== undefined && blockProposal.indexWithinCheckpoint > terminalBlock.indexWithinCheckpoint
539+
? { isValid: false, reason: 'block_proposal_beyond_checkpoint' }
540+
: { isValid: true };
541+
}
542+
}
543+
503544
private async getParentBlock(proposal: BlockProposal): Promise<'genesis' | BlockData | undefined> {
504545
const parentArchive = proposal.blockHeader.lastArchive.root;
505546
const config = this.checkpointsBuilder.getConfig();

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ describe('ValidatorClient', () => {
117117
p2pClient.getCheckpointAttestationsForSlot.mockImplementation(() => Promise.resolve([]));
118118
p2pClient.handleAuthRequestFromPeer.mockResolvedValue(StatusMessage.random());
119119
p2pClient.broadcastCheckpointAttestations.mockResolvedValue();
120+
p2pClient.getProposalsForSlot.mockResolvedValue({ blockProposals: [], checkpointProposals: [] });
120121
checkpointsBuilder = mock<FullNodeCheckpointsBuilder>();
121122
checkpointsBuilder.getConfig.mockReturnValue({
122123
l1GenesisTime: 1n,
@@ -520,6 +521,121 @@ describe('ValidatorClient', () => {
520521
expect(isValid).toBe(true);
521522
});
522523

524+
it('does not push a block proposal beyond a retained checkpoint terminal block to the archiver', async () => {
525+
validatorClient.updateConfig({ skipPushProposedBlocksToArchiver: false });
526+
validatorClient.getProposalHandler().register(p2pClient, true);
527+
528+
const signer = Secp256k1Signer.random();
529+
const emptyInHash = computeInHashFromL1ToL2Messages([]);
530+
const checkpointProposal = await makeCheckpointProposal({
531+
signer,
532+
checkpointHeader: makeCheckpointHeader(1, { slotNumber: proposal.slotNumber, inHash: emptyInHash }),
533+
archiveRoot: Fr.random(),
534+
lastBlock: {
535+
blockHeader: makeBlockHeader(1, { blockNumber, slotNumber: proposal.slotNumber }),
536+
indexWithinCheckpoint: IndexWithinCheckpoint(0),
537+
txHashes: proposal.txHashes,
538+
},
539+
});
540+
const terminalBlock = checkpointProposal.getBlockProposal()!;
541+
542+
const terminalGlobals = terminalBlock.blockHeader.globalVariables;
543+
const laterBlockHeader = makeBlockHeader(2, {
544+
lastArchive: new AppendOnlyTreeSnapshot(terminalBlock.archive, terminalBlock.blockNumber),
545+
blockNumber: BlockNumber(terminalBlock.blockNumber + 1),
546+
slotNumber: proposal.slotNumber,
547+
chainId: terminalGlobals.chainId,
548+
version: terminalGlobals.version,
549+
timestamp: terminalGlobals.timestamp,
550+
coinbase: terminalGlobals.coinbase,
551+
feeRecipient: terminalGlobals.feeRecipient,
552+
gasFees: terminalGlobals.gasFees,
553+
});
554+
const laterBlock = await makeBlockProposal({
555+
signer,
556+
blockHeader: laterBlockHeader,
557+
indexWithinCheckpoint: IndexWithinCheckpoint(1),
558+
inHash: emptyInHash,
559+
archiveRoot: Fr.random(),
560+
});
561+
562+
epochCache.getProposerAttesterAddressInSlot.mockResolvedValue(signer.address);
563+
p2pClient.getProposalsForSlot.mockResolvedValue({
564+
blockProposals: [terminalBlock, laterBlock],
565+
checkpointProposals: [checkpointProposal.toCore()],
566+
});
567+
568+
const terminalBlockData = {
569+
header: terminalBlock.blockHeader,
570+
archive: new AppendOnlyTreeSnapshot(terminalBlock.archive, terminalBlock.blockNumber),
571+
blockHash: BlockHash.random(),
572+
checkpointNumber: CheckpointNumber(1),
573+
indexWithinCheckpoint: terminalBlock.indexWithinCheckpoint,
574+
} as unknown as BlockData;
575+
blockSource.getBlockData.mockImplementation(query =>
576+
Promise.resolve('number' in query ? undefined : terminalBlockData),
577+
);
578+
579+
const blockAddedIfProcessed = {
580+
...blockBuildResult.block,
581+
header: laterBlock.blockHeader,
582+
body: { txEffects: times(laterBlock.txHashes.length, () => TxEffect.empty()) },
583+
archive: new AppendOnlyTreeSnapshot(laterBlock.archive, laterBlock.blockNumber),
584+
checkpointNumber: CheckpointNumber(1),
585+
indexWithinCheckpoint: laterBlock.indexWithinCheckpoint,
586+
} as unknown as L2Block;
587+
mockCheckpointBuilder.buildBlock.mockResolvedValue({
588+
...blockBuildResult,
589+
block: blockAddedIfProcessed,
590+
numTxs: laterBlock.txHashes.length,
591+
});
592+
worldState.fork.mockResolvedValue({
593+
close: () => Promise.resolve(),
594+
[Symbol.asyncDispose]: () => Promise.resolve(),
595+
getTreeInfo: () => Promise.resolve({ root: laterBlock.blockHeader.lastArchive.root.toBuffer() }),
596+
} as never);
597+
598+
const result = await validatorClient.getProposalHandler().handleBlockProposal(laterBlock, sender, true);
599+
600+
expect(result).toMatchObject({ isValid: false, reason: 'block_proposal_beyond_checkpoint' });
601+
expect(blockSource.addBlock).not.toHaveBeenCalled();
602+
});
603+
604+
it('does not push a block proposal to the archiver when retained checkpoint proposals equivocate', async () => {
605+
validatorClient.updateConfig({ skipPushProposedBlocksToArchiver: false });
606+
validatorClient.getProposalHandler().register(p2pClient, true);
607+
608+
const emptyInHash = computeInHashFromL1ToL2Messages([]);
609+
const checkpointProposal = await makeCheckpointProposal({
610+
checkpointHeader: makeCheckpointHeader(1, { slotNumber: proposal.slotNumber, inHash: emptyInHash }),
611+
archiveRoot: Fr.random(),
612+
lastBlock: {
613+
blockHeader: makeBlockHeader(1, { blockNumber, slotNumber: proposal.slotNumber }),
614+
indexWithinCheckpoint: IndexWithinCheckpoint(0),
615+
txHashes: proposal.txHashes,
616+
},
617+
});
618+
const equivocatedCheckpointProposal = await makeCheckpointProposal({
619+
checkpointHeader: makeCheckpointHeader(1, { slotNumber: proposal.slotNumber, inHash: emptyInHash }),
620+
archiveRoot: Fr.random(),
621+
lastBlock: {
622+
blockHeader: makeBlockHeader(1, { blockNumber, slotNumber: proposal.slotNumber }),
623+
indexWithinCheckpoint: IndexWithinCheckpoint(0),
624+
txHashes: proposal.txHashes,
625+
},
626+
});
627+
628+
p2pClient.getProposalsForSlot.mockResolvedValue({
629+
blockProposals: [proposal],
630+
checkpointProposals: [checkpointProposal.toCore(), equivocatedCheckpointProposal.toCore()],
631+
});
632+
633+
const result = await validatorClient.getProposalHandler().handleBlockProposal(proposal, sender, true);
634+
635+
expect(result).toMatchObject({ isValid: false, reason: 'checkpoint_proposal_equivocation' });
636+
expect(blockSource.addBlock).not.toHaveBeenCalled();
637+
});
638+
523639
it('uses the next wall-clock slot as the tx collection deadline for pipelined proposals', async () => {
524640
const pipelineOffsetInSlots = 1;
525641
epochCache.isProposerPipeliningEnabled.mockReturnValue(true);

0 commit comments

Comments
 (0)