Skip to content

Commit 38a2fa3

Browse files
PhilWindleclaude
andauthored
feat: Validate num txs in block proposals (#20850)
This PR introduces additional block/checkpoint proposal validation to ensure `SEQ_MAX_TX_PER_BLOCK` is not breached. Also adds additional tests for existing validation criteria. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d4c76da commit 38a2fa3

11 files changed

Lines changed: 183 additions & 15 deletions

File tree

yarn-project/p2p/src/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export interface P2PConfig
3838
ChainConfig,
3939
TxCollectionConfig,
4040
TxFileStoreConfig,
41-
Pick<SequencerConfig, 'blockDurationMs' | 'expectedBlockProposalsPerSlot'> {
41+
Pick<SequencerConfig, 'blockDurationMs' | 'expectedBlockProposalsPerSlot' | 'maxTxsPerBlock'> {
4242
/** A flag dictating whether the P2P subsystem should be enabled. */
4343
p2pEnabled: boolean;
4444

yarn-project/p2p/src/msg_validators/proposal_validator/block_proposal_validator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { BlockProposal, P2PValidator } from '@aztec/stdlib/p2p';
44
import { ProposalValidator } from '../proposal_validator/proposal_validator.js';
55

66
export class BlockProposalValidator extends ProposalValidator<BlockProposal> implements P2PValidator<BlockProposal> {
7-
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean }) {
7+
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean; maxTxsPerBlock?: number }) {
88
super(epochCache, opts, 'p2p:block_proposal_validator');
99
}
1010
}

yarn-project/p2p/src/msg_validators/proposal_validator/checkpoint_proposal_validator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export class CheckpointProposalValidator
77
extends ProposalValidator<CheckpointProposal>
88
implements P2PValidator<CheckpointProposal>
99
{
10-
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean }) {
10+
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean; maxTxsPerBlock?: number }) {
1111
super(epochCache, opts, 'p2p:checkpoint_proposal_validator');
1212
}
1313
}

yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@ export abstract class ProposalValidator<TProposal extends BlockProposal | Checkp
99
protected epochCache: EpochCacheInterface;
1010
protected logger: Logger;
1111
protected txsPermitted: boolean;
12+
protected maxTxsPerBlock?: number;
1213

13-
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean }, loggerName: string) {
14+
constructor(
15+
epochCache: EpochCacheInterface,
16+
opts: { txsPermitted: boolean; maxTxsPerBlock?: number },
17+
loggerName: string,
18+
) {
1419
this.epochCache = epochCache;
1520
this.txsPermitted = opts.txsPermitted;
21+
this.maxTxsPerBlock = opts.maxTxsPerBlock;
1622
this.logger = createLogger(loggerName);
1723
}
1824

@@ -47,6 +53,14 @@ export abstract class ProposalValidator<TProposal extends BlockProposal | Checkp
4753
return { result: 'reject', severity: PeerErrorSeverity.MidToleranceError };
4854
}
4955

56+
// Max txs per block check
57+
if (this.maxTxsPerBlock !== undefined && proposal.txHashes.length > this.maxTxsPerBlock) {
58+
this.logger.warn(
59+
`Penalizing peer for proposal with ${proposal.txHashes.length} transaction(s) when max is ${this.maxTxsPerBlock}`,
60+
);
61+
return { result: 'reject', severity: PeerErrorSeverity.MidToleranceError };
62+
}
63+
5064
// Embedded txs must be listed in txHashes
5165
const hashSet = new Set(proposal.txHashes.map(h => h.toString()));
5266
const missingTxHashes =

yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator_test_suite.ts

Lines changed: 144 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { EpochCacheInterface } from '@aztec/epoch-cache';
2+
import { NoCommitteeError } from '@aztec/ethereum/contracts';
23
import type { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer';
34
import type { EthAddress } from '@aztec/foundation/eth-address';
45
import {
@@ -9,12 +10,13 @@ import {
910
} from '@aztec/stdlib/p2p';
1011
import type { TxHash } from '@aztec/stdlib/tx';
1112

13+
import { jest } from '@jest/globals';
1214
import type { MockProxy } from 'jest-mock-extended';
1315

1416
export interface ProposalValidatorTestParams<TProposal extends BlockProposal | CheckpointProposal> {
1517
validatorFactory: (
1618
epochCache: EpochCacheInterface,
17-
opts: { txsPermitted: boolean },
19+
opts: { txsPermitted: boolean; maxTxsPerBlock?: number },
1820
) => { validate: (proposal: TProposal) => Promise<ValidationResult> };
1921
makeProposal: (options?: any) => Promise<TProposal>;
2022
makeHeader: (epochNumber: number | bigint, slotNumber: number | bigint, blockNumber: number | bigint) => any;
@@ -105,6 +107,26 @@ export function sharedProposalValidatorTests<TProposal extends BlockProposal | C
105107
expect(result).toEqual({ result: 'ignore' });
106108
});
107109

110+
it('returns mid tolerance error if proposal has invalid signature', async () => {
111+
const currentProposer = getSigner();
112+
const header = makeHeader(1, 100, 100);
113+
const mockProposal = await makeProposal({
114+
blockHeader: header,
115+
lastBlockHeader: header,
116+
signer: currentProposer,
117+
});
118+
119+
// Override getSender to return undefined (invalid signature)
120+
jest.spyOn(mockProposal as any, 'getSender').mockReturnValue(undefined);
121+
122+
mockGetProposer(getAddress(currentProposer), getAddress());
123+
const result = await validator.validate(mockProposal);
124+
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError });
125+
126+
// Should not try to resolve proposer if signature is invalid
127+
expect(epochCache.getProposerAttesterAddressInSlot).not.toHaveBeenCalled();
128+
});
129+
108130
it('returns mid tolerance error if proposer is not current proposer for current slot', async () => {
109131
const currentProposer = getSigner();
110132
const nextProposer = getSigner();
@@ -152,6 +174,34 @@ export function sharedProposalValidatorTests<TProposal extends BlockProposal | C
152174
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError });
153175
});
154176

177+
it('accepts proposal when proposer is undefined (open committee)', async () => {
178+
const currentProposer = getSigner();
179+
const header = makeHeader(1, 100, 100);
180+
const mockProposal = await makeProposal({
181+
blockHeader: header,
182+
lastBlockHeader: header,
183+
signer: currentProposer,
184+
});
185+
186+
epochCache.getProposerAttesterAddressInSlot.mockResolvedValue(undefined);
187+
const result = await validator.validate(mockProposal);
188+
expect(result).toEqual({ result: 'accept' });
189+
});
190+
191+
it('returns low tolerance error when getProposerAttesterAddressInSlot throws NoCommitteeError', async () => {
192+
const currentProposer = getSigner();
193+
const header = makeHeader(1, 100, 100);
194+
const mockProposal = await makeProposal({
195+
blockHeader: header,
196+
lastBlockHeader: header,
197+
signer: currentProposer,
198+
});
199+
200+
epochCache.getProposerAttesterAddressInSlot.mockRejectedValue(new NoCommitteeError());
201+
const result = await validator.validate(mockProposal);
202+
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.LowToleranceError });
203+
});
204+
155205
it('returns undefined if proposal is valid for current slot and proposer', async () => {
156206
const currentProposer = getSigner();
157207
const nextProposer = getSigner();
@@ -226,5 +276,98 @@ export function sharedProposalValidatorTests<TProposal extends BlockProposal | C
226276
expect(result).toEqual({ result: 'accept' });
227277
});
228278
});
279+
280+
describe('embedded tx validation', () => {
281+
it('returns mid tolerance error if embedded txs are not listed in txHashes', async () => {
282+
const currentProposer = getSigner();
283+
const txHashes = getTxHashes(2);
284+
const header = makeHeader(1, 100, 100);
285+
const mockProposal = await makeProposal({
286+
blockHeader: header,
287+
lastBlockHeader: header,
288+
signer: currentProposer,
289+
txHashes,
290+
});
291+
292+
// Create a fake tx whose hash is NOT in txHashes
293+
const fakeTxHash = getTxHashes(1)[0];
294+
const fakeTx = { getTxHash: () => fakeTxHash, validateTxHash: () => Promise.resolve(true) };
295+
Object.defineProperty(mockProposal, 'txs', { get: () => [fakeTx], configurable: true });
296+
297+
mockGetProposer(getAddress(currentProposer), getAddress());
298+
const result = await validator.validate(mockProposal);
299+
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError });
300+
});
301+
302+
it('returns low tolerance error if embedded tx has invalid tx hash', async () => {
303+
const currentProposer = getSigner();
304+
const txHashes = getTxHashes(2);
305+
const header = makeHeader(1, 100, 100);
306+
const mockProposal = await makeProposal({
307+
blockHeader: header,
308+
lastBlockHeader: header,
309+
signer: currentProposer,
310+
txHashes,
311+
});
312+
313+
// Create a fake tx whose hash IS in txHashes but validateTxHash returns false
314+
const fakeTx = { getTxHash: () => txHashes[0], validateTxHash: () => Promise.resolve(false) };
315+
Object.defineProperty(mockProposal, 'txs', { get: () => [fakeTx], configurable: true });
316+
317+
mockGetProposer(getAddress(currentProposer), getAddress());
318+
const result = await validator.validate(mockProposal);
319+
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.LowToleranceError });
320+
});
321+
});
322+
323+
describe('maxTxsPerBlock validation', () => {
324+
it('rejects proposal when txHashes exceed maxTxsPerBlock', async () => {
325+
const validatorWithMaxTxs = validatorFactory(epochCache, { txsPermitted: true, maxTxsPerBlock: 2 });
326+
const currentProposer = getSigner();
327+
const header = makeHeader(1, 100, 100);
328+
const mockProposal = await makeProposal({
329+
blockHeader: header,
330+
lastBlockHeader: header,
331+
signer: currentProposer,
332+
txHashes: getTxHashes(3),
333+
});
334+
335+
mockGetProposer(getAddress(currentProposer), getAddress());
336+
const result = await validatorWithMaxTxs.validate(mockProposal);
337+
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError });
338+
});
339+
340+
it('accepts proposal when txHashes count equals maxTxsPerBlock', async () => {
341+
const validatorWithMaxTxs = validatorFactory(epochCache, { txsPermitted: true, maxTxsPerBlock: 2 });
342+
const currentProposer = getSigner();
343+
const header = makeHeader(1, 100, 100);
344+
const mockProposal = await makeProposal({
345+
blockHeader: header,
346+
lastBlockHeader: header,
347+
signer: currentProposer,
348+
txHashes: getTxHashes(2),
349+
});
350+
351+
mockGetProposer(getAddress(currentProposer), getAddress());
352+
const result = await validatorWithMaxTxs.validate(mockProposal);
353+
expect(result).toEqual({ result: 'accept' });
354+
});
355+
356+
it('accepts proposal when maxTxsPerBlock is not set (unlimited)', async () => {
357+
// Default validator has no maxTxsPerBlock
358+
const currentProposer = getSigner();
359+
const header = makeHeader(1, 100, 100);
360+
const mockProposal = await makeProposal({
361+
blockHeader: header,
362+
lastBlockHeader: header,
363+
signer: currentProposer,
364+
txHashes: getTxHashes(10),
365+
});
366+
367+
mockGetProposer(getAddress(currentProposer), getAddress());
368+
const result = await validator.validate(mockProposal);
369+
expect(result).toEqual({ result: 'accept' });
370+
});
371+
});
229372
});
230373
}

yarn-project/p2p/src/services/libp2p/libp2p_service.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,13 @@ export class LibP2PService extends WithTracer implements P2PService {
222222
this.protocolVersion,
223223
);
224224

225-
this.blockProposalValidator = new BlockProposalValidator(epochCache, { txsPermitted: !config.disableTransactions });
225+
this.blockProposalValidator = new BlockProposalValidator(epochCache, {
226+
txsPermitted: !config.disableTransactions,
227+
maxTxsPerBlock: config.maxTxsPerBlock,
228+
});
226229
this.checkpointProposalValidator = new CheckpointProposalValidator(epochCache, {
227230
txsPermitted: !config.disableTransactions,
231+
maxTxsPerBlock: config.maxTxsPerBlock,
228232
});
229233
this.checkpointAttestationValidator = config.fishermanMode
230234
? new FishermanAttestationValidator(epochCache, mempools.attestationPool, telemetry)

yarn-project/sequencer-client/src/config.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { type P2PConfig, p2pConfigMappings } from '@aztec/p2p/config';
1313
import { AztecAddress } from '@aztec/stdlib/aztec-address';
1414
import {
1515
type ChainConfig,
16+
DEFAULT_MAX_TXS_PER_BLOCK,
1617
type SequencerConfig,
1718
chainConfigMappings,
1819
sharedSequencerConfigMappings,
@@ -37,7 +38,7 @@ export type { SequencerConfig };
3738
*/
3839
export const DefaultSequencerConfig: ResolvedSequencerConfig = {
3940
sequencerPollingIntervalMS: 500,
40-
maxTxsPerBlock: 32,
41+
maxTxsPerBlock: DEFAULT_MAX_TXS_PER_BLOCK,
4142
minTxsPerBlock: 1,
4243
buildCheckpointIfEmpty: false,
4344
publishTxsWithProposals: false,
@@ -77,11 +78,6 @@ export const sequencerConfigMappings: ConfigMappingsType<SequencerConfig> = {
7778
description: 'The number of ms to wait between polling for checking to build on the next slot.',
7879
...numberConfigHelper(DefaultSequencerConfig.sequencerPollingIntervalMS),
7980
},
80-
maxTxsPerBlock: {
81-
env: 'SEQ_MAX_TX_PER_BLOCK',
82-
description: 'The maximum number of txs to include in a block.',
83-
...numberConfigHelper(DefaultSequencerConfig.maxTxsPerBlock),
84-
},
8581
minTxsPerBlock: {
8682
env: 'SEQ_MIN_TX_PER_BLOCK',
8783
description: 'The minimum number of txs to include in a block.',

yarn-project/stdlib/src/config/sequencer-config.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
1-
import type { ConfigMappingsType } from '@aztec/foundation/config';
1+
import { type ConfigMappingsType, numberConfigHelper } from '@aztec/foundation/config';
22

33
import type { SequencerConfig } from '../interfaces/configs.js';
44

5+
/** Default maximum number of transactions per block. */
6+
export const DEFAULT_MAX_TXS_PER_BLOCK = 32;
7+
58
/**
69
* Partial sequencer config mappings for fields that need to be shared across packages.
710
* The full sequencer config mappings remain in sequencer-client, but shared fields
811
* (like blockDurationMs needed by both p2p and sequencer-client) are defined here
912
* to avoid duplication.
1013
*/
1114
export const sharedSequencerConfigMappings: ConfigMappingsType<
12-
Pick<SequencerConfig, 'blockDurationMs' | 'expectedBlockProposalsPerSlot'>
15+
Pick<SequencerConfig, 'blockDurationMs' | 'expectedBlockProposalsPerSlot' | 'maxTxsPerBlock'>
1316
> = {
1417
blockDurationMs: {
1518
env: 'SEQ_BLOCK_DURATION_MS',
@@ -26,4 +29,9 @@ export const sharedSequencerConfigMappings: ConfigMappingsType<
2629
parseEnv: (val: string) => (val ? parseInt(val, 10) : 0),
2730
defaultValue: 0,
2831
},
32+
maxTxsPerBlock: {
33+
env: 'SEQ_MAX_TX_PER_BLOCK',
34+
description: 'The maximum number of txs to include in a block.',
35+
...numberConfigHelper(DEFAULT_MAX_TXS_PER_BLOCK),
36+
},
2937
};

yarn-project/stdlib/src/interfaces/validator.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export type ValidatorClientConfig = ValidatorHASignerConfig & {
6262
};
6363

6464
export type ValidatorClientFullConfig = ValidatorClientConfig &
65-
Pick<SequencerConfig, 'txPublicSetupAllowList' | 'broadcastInvalidBlockProposal'> &
65+
Pick<SequencerConfig, 'txPublicSetupAllowList' | 'broadcastInvalidBlockProposal' | 'maxTxsPerBlock'> &
6666
Pick<
6767
SlasherConfig,
6868
'slashBroadcastedInvalidBlockPenalty' | 'slashDuplicateProposalPenalty' | 'slashDuplicateAttestationPenalty'
@@ -93,6 +93,7 @@ export const ValidatorClientFullConfigSchema = zodFor<Omit<ValidatorClientFullCo
9393
ValidatorClientConfigSchema.extend({
9494
txPublicSetupAllowList: z.array(AllowedElementSchema).optional(),
9595
broadcastInvalidBlockProposal: z.boolean().optional(),
96+
maxTxsPerBlock: z.number().optional(),
9697
slashBroadcastedInvalidBlockPenalty: schemas.BigInt,
9798
slashDuplicateProposalPenalty: schemas.BigInt,
9899
slashDuplicateAttestationPenalty: schemas.BigInt,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export function createBlockProposalHandler(
2929
const metrics = new ValidatorMetrics(deps.telemetry);
3030
const blockProposalValidator = new BlockProposalValidator(deps.epochCache, {
3131
txsPermitted: !config.disableTransactions,
32+
maxTxsPerBlock: config.maxTxsPerBlock,
3233
});
3334
return new BlockProposalHandler(
3435
deps.checkpointsBuilder,

0 commit comments

Comments
 (0)