Skip to content

Commit 55c43ea

Browse files
committed
fix: reject block proposals in poisoned slots
1 parent 8d5dd3d commit 55c43ea

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
@@ -58,6 +58,8 @@ export type BlockProposalValidationFailureReason =
5858
| 'failed_txs'
5959
| 'initial_state_mismatch'
6060
| 'timeout'
61+
| 'block_proposal_beyond_checkpoint'
62+
| 'checkpoint_proposal_equivocation'
6163
| 'unknown_error';
6264

6365
type ReexecuteTransactionsResult = {
@@ -145,6 +147,10 @@ type CheckpointComputationResult =
145147
| { checkpointNumber: CheckpointNumber; reason?: undefined }
146148
| { checkpointNumber?: undefined; reason: 'invalid_proposal' | 'global_variables_mismatch' };
147149

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

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

168177
constructor(
@@ -212,6 +221,7 @@ export class ProposalHandler {
212221

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

@@ -361,6 +372,16 @@ export class ProposalHandler {
361372
return { isValid: false, reason: 'invalid_proposal' };
362373
}
363374

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

523+
private async validateNewBlockInSlot(blockProposal: BlockProposal): Promise<BlockProposalSlotValidationResult> {
524+
if (!this.p2pClient) {
525+
return { isValid: true };
526+
}
527+
528+
const { blockProposals, checkpointProposals } = await this.p2pClient.getProposalsForSlot(blockProposal.slotNumber);
529+
530+
if (checkpointProposals.length === 0) {
531+
return { isValid: true };
532+
} else if (checkpointProposals.length > 1) {
533+
return { isValid: false, reason: 'checkpoint_proposal_equivocation' };
534+
} else {
535+
const checkpointProposal = checkpointProposals[0];
536+
const terminalBlock = blockProposals.find(block => block.archive.equals(checkpointProposal.archive));
537+
return terminalBlock !== undefined && blockProposal.indexWithinCheckpoint > terminalBlock.indexWithinCheckpoint
538+
? { isValid: false, reason: 'block_proposal_beyond_checkpoint' }
539+
: { isValid: true };
540+
}
541+
}
542+
502543
private async getParentBlock(proposal: BlockProposal): Promise<'genesis' | BlockData | undefined> {
503544
const parentArchive = proposal.blockHeader.lastArchive.root;
504545
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,
@@ -529,6 +530,121 @@ describe('ValidatorClient', () => {
529530
expect(isValid).toBe(true);
530531
});
531532

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

0 commit comments

Comments
 (0)