Skip to content

Commit 141121e

Browse files
committed
fix: reject block proposals in poisoned slots
1 parent b7abf68 commit 141121e

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
@@ -118,6 +118,7 @@ describe('ValidatorClient', () => {
118118
p2pClient.getCheckpointAttestationsForSlot.mockImplementation(() => Promise.resolve([]));
119119
p2pClient.handleAuthRequestFromPeer.mockResolvedValue(StatusMessage.random());
120120
p2pClient.broadcastCheckpointAttestations.mockResolvedValue();
121+
p2pClient.getProposalsForSlot.mockResolvedValue({ blockProposals: [], checkpointProposals: [] });
121122
checkpointsBuilder = mock<FullNodeCheckpointsBuilder>();
122123
checkpointsBuilder.getConfig.mockReturnValue({
123124
l1GenesisTime: 1n,
@@ -530,6 +531,121 @@ describe('ValidatorClient', () => {
530531
expect(isValid).toBe(true);
531532
});
532533

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

0 commit comments

Comments
 (0)