Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion yarn-project/p2p/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export interface P2PConfig
ChainConfig,
TxCollectionConfig,
TxFileStoreConfig,
Pick<SequencerConfig, 'blockDurationMs' | 'expectedBlockProposalsPerSlot'> {
Pick<SequencerConfig, 'blockDurationMs' | 'expectedBlockProposalsPerSlot' | 'maxTxsPerBlock'> {
/** A flag dictating whether the P2P subsystem should be enabled. */
p2pEnabled: boolean;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { BlockProposal, P2PValidator } from '@aztec/stdlib/p2p';
import { ProposalValidator } from '../proposal_validator/proposal_validator.js';

export class BlockProposalValidator extends ProposalValidator<BlockProposal> implements P2PValidator<BlockProposal> {
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean }) {
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean; maxTxsPerBlock?: number }) {
super(epochCache, opts, 'p2p:block_proposal_validator');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export class CheckpointProposalValidator
extends ProposalValidator<CheckpointProposal>
implements P2PValidator<CheckpointProposal>
{
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean }) {
constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean; maxTxsPerBlock?: number }) {
super(epochCache, opts, 'p2p:checkpoint_proposal_validator');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@ export abstract class ProposalValidator<TProposal extends BlockProposal | Checkp
protected epochCache: EpochCacheInterface;
protected logger: Logger;
protected txsPermitted: boolean;
protected maxTxsPerBlock?: number;

constructor(epochCache: EpochCacheInterface, opts: { txsPermitted: boolean }, loggerName: string) {
constructor(
epochCache: EpochCacheInterface,
opts: { txsPermitted: boolean; maxTxsPerBlock?: number },
loggerName: string,
) {
this.epochCache = epochCache;
this.txsPermitted = opts.txsPermitted;
this.maxTxsPerBlock = opts.maxTxsPerBlock;
this.logger = createLogger(loggerName);
}

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

// Max txs per block check
if (this.maxTxsPerBlock !== undefined && proposal.txHashes.length > this.maxTxsPerBlock) {
this.logger.warn(
`Penalizing peer for proposal with ${proposal.txHashes.length} transaction(s) when max is ${this.maxTxsPerBlock}`,
);
return { result: 'reject', severity: PeerErrorSeverity.MidToleranceError };
}

// Embedded txs must be listed in txHashes
const hashSet = new Set(proposal.txHashes.map(h => h.toString()));
const missingTxHashes =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { EpochCacheInterface } from '@aztec/epoch-cache';
import { NoCommitteeError } from '@aztec/ethereum/contracts';
import type { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer';
import type { EthAddress } from '@aztec/foundation/eth-address';
import {
Expand All @@ -9,12 +10,13 @@ import {
} from '@aztec/stdlib/p2p';
import type { TxHash } from '@aztec/stdlib/tx';

import { jest } from '@jest/globals';
import type { MockProxy } from 'jest-mock-extended';

export interface ProposalValidatorTestParams<TProposal extends BlockProposal | CheckpointProposal> {
validatorFactory: (
epochCache: EpochCacheInterface,
opts: { txsPermitted: boolean },
opts: { txsPermitted: boolean; maxTxsPerBlock?: number },
) => { validate: (proposal: TProposal) => Promise<ValidationResult> };
makeProposal: (options?: any) => Promise<TProposal>;
makeHeader: (epochNumber: number | bigint, slotNumber: number | bigint, blockNumber: number | bigint) => any;
Expand Down Expand Up @@ -105,6 +107,26 @@ export function sharedProposalValidatorTests<TProposal extends BlockProposal | C
expect(result).toEqual({ result: 'ignore' });
});

it('returns mid tolerance error if proposal has invalid signature', async () => {
const currentProposer = getSigner();
const header = makeHeader(1, 100, 100);
const mockProposal = await makeProposal({
blockHeader: header,
lastBlockHeader: header,
signer: currentProposer,
});

// Override getSender to return undefined (invalid signature)
jest.spyOn(mockProposal as any, 'getSender').mockReturnValue(undefined);

mockGetProposer(getAddress(currentProposer), getAddress());
const result = await validator.validate(mockProposal);
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError });

// Should not try to resolve proposer if signature is invalid
expect(epochCache.getProposerAttesterAddressInSlot).not.toHaveBeenCalled();
});

it('returns mid tolerance error if proposer is not current proposer for current slot', async () => {
const currentProposer = getSigner();
const nextProposer = getSigner();
Expand Down Expand Up @@ -152,6 +174,34 @@ export function sharedProposalValidatorTests<TProposal extends BlockProposal | C
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError });
});

it('accepts proposal when proposer is undefined (open committee)', async () => {
const currentProposer = getSigner();
const header = makeHeader(1, 100, 100);
const mockProposal = await makeProposal({
blockHeader: header,
lastBlockHeader: header,
signer: currentProposer,
});

epochCache.getProposerAttesterAddressInSlot.mockResolvedValue(undefined);
const result = await validator.validate(mockProposal);
expect(result).toEqual({ result: 'accept' });
});

it('returns low tolerance error when getProposerAttesterAddressInSlot throws NoCommitteeError', async () => {
const currentProposer = getSigner();
const header = makeHeader(1, 100, 100);
const mockProposal = await makeProposal({
blockHeader: header,
lastBlockHeader: header,
signer: currentProposer,
});

epochCache.getProposerAttesterAddressInSlot.mockRejectedValue(new NoCommitteeError());
const result = await validator.validate(mockProposal);
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.LowToleranceError });
});

it('returns undefined if proposal is valid for current slot and proposer', async () => {
const currentProposer = getSigner();
const nextProposer = getSigner();
Expand Down Expand Up @@ -226,5 +276,98 @@ export function sharedProposalValidatorTests<TProposal extends BlockProposal | C
expect(result).toEqual({ result: 'accept' });
});
});

describe('embedded tx validation', () => {
it('returns mid tolerance error if embedded txs are not listed in txHashes', async () => {
const currentProposer = getSigner();
const txHashes = getTxHashes(2);
const header = makeHeader(1, 100, 100);
const mockProposal = await makeProposal({
blockHeader: header,
lastBlockHeader: header,
signer: currentProposer,
txHashes,
});

// Create a fake tx whose hash is NOT in txHashes
const fakeTxHash = getTxHashes(1)[0];
const fakeTx = { getTxHash: () => fakeTxHash, validateTxHash: () => Promise.resolve(true) };
Object.defineProperty(mockProposal, 'txs', { get: () => [fakeTx], configurable: true });

mockGetProposer(getAddress(currentProposer), getAddress());
const result = await validator.validate(mockProposal);
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError });
});

it('returns low tolerance error if embedded tx has invalid tx hash', async () => {
const currentProposer = getSigner();
const txHashes = getTxHashes(2);
const header = makeHeader(1, 100, 100);
const mockProposal = await makeProposal({
blockHeader: header,
lastBlockHeader: header,
signer: currentProposer,
txHashes,
});

// Create a fake tx whose hash IS in txHashes but validateTxHash returns false
const fakeTx = { getTxHash: () => txHashes[0], validateTxHash: () => Promise.resolve(false) };
Object.defineProperty(mockProposal, 'txs', { get: () => [fakeTx], configurable: true });

mockGetProposer(getAddress(currentProposer), getAddress());
const result = await validator.validate(mockProposal);
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.LowToleranceError });
});
});

describe('maxTxsPerBlock validation', () => {
it('rejects proposal when txHashes exceed maxTxsPerBlock', async () => {
const validatorWithMaxTxs = validatorFactory(epochCache, { txsPermitted: true, maxTxsPerBlock: 2 });
const currentProposer = getSigner();
const header = makeHeader(1, 100, 100);
const mockProposal = await makeProposal({
blockHeader: header,
lastBlockHeader: header,
signer: currentProposer,
txHashes: getTxHashes(3),
});

mockGetProposer(getAddress(currentProposer), getAddress());
const result = await validatorWithMaxTxs.validate(mockProposal);
expect(result).toEqual({ result: 'reject', severity: PeerErrorSeverity.MidToleranceError });
});

it('accepts proposal when txHashes count equals maxTxsPerBlock', async () => {
const validatorWithMaxTxs = validatorFactory(epochCache, { txsPermitted: true, maxTxsPerBlock: 2 });
const currentProposer = getSigner();
const header = makeHeader(1, 100, 100);
const mockProposal = await makeProposal({
blockHeader: header,
lastBlockHeader: header,
signer: currentProposer,
txHashes: getTxHashes(2),
});

mockGetProposer(getAddress(currentProposer), getAddress());
const result = await validatorWithMaxTxs.validate(mockProposal);
expect(result).toEqual({ result: 'accept' });
});

it('accepts proposal when maxTxsPerBlock is not set (unlimited)', async () => {
// Default validator has no maxTxsPerBlock
const currentProposer = getSigner();
const header = makeHeader(1, 100, 100);
const mockProposal = await makeProposal({
blockHeader: header,
lastBlockHeader: header,
signer: currentProposer,
txHashes: getTxHashes(10),
});

mockGetProposer(getAddress(currentProposer), getAddress());
const result = await validator.validate(mockProposal);
expect(result).toEqual({ result: 'accept' });
});
});
});
}
6 changes: 5 additions & 1 deletion yarn-project/p2p/src/services/libp2p/libp2p_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,13 @@ export class LibP2PService<T extends P2PClientType = P2PClientType.Full> extends
this.protocolVersion,
);

this.blockProposalValidator = new BlockProposalValidator(epochCache, { txsPermitted: !config.disableTransactions });
this.blockProposalValidator = new BlockProposalValidator(epochCache, {
txsPermitted: !config.disableTransactions,
maxTxsPerBlock: config.maxTxsPerBlock,
});
this.checkpointProposalValidator = new CheckpointProposalValidator(epochCache, {
txsPermitted: !config.disableTransactions,
maxTxsPerBlock: config.maxTxsPerBlock,
});
this.checkpointAttestationValidator = config.fishermanMode
? new FishermanAttestationValidator(epochCache, mempools.attestationPool, telemetry)
Expand Down
8 changes: 2 additions & 6 deletions yarn-project/sequencer-client/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { type P2PConfig, p2pConfigMappings } from '@aztec/p2p/config';
import { AztecAddress } from '@aztec/stdlib/aztec-address';
import {
type ChainConfig,
DEFAULT_MAX_TXS_PER_BLOCK,
type SequencerConfig,
chainConfigMappings,
sharedSequencerConfigMappings,
Expand All @@ -37,7 +38,7 @@ export type { SequencerConfig };
*/
export const DefaultSequencerConfig: ResolvedSequencerConfig = {
sequencerPollingIntervalMS: 500,
maxTxsPerBlock: 32,
maxTxsPerBlock: DEFAULT_MAX_TXS_PER_BLOCK,
minTxsPerBlock: 1,
buildCheckpointIfEmpty: false,
publishTxsWithProposals: false,
Expand Down Expand Up @@ -77,11 +78,6 @@ export const sequencerConfigMappings: ConfigMappingsType<SequencerConfig> = {
description: 'The number of ms to wait between polling for checking to build on the next slot.',
...numberConfigHelper(DefaultSequencerConfig.sequencerPollingIntervalMS),
},
maxTxsPerBlock: {
env: 'SEQ_MAX_TX_PER_BLOCK',
description: 'The maximum number of txs to include in a block.',
...numberConfigHelper(DefaultSequencerConfig.maxTxsPerBlock),
},
minTxsPerBlock: {
env: 'SEQ_MIN_TX_PER_BLOCK',
description: 'The minimum number of txs to include in a block.',
Expand Down
12 changes: 10 additions & 2 deletions yarn-project/stdlib/src/config/sequencer-config.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import type { ConfigMappingsType } from '@aztec/foundation/config';
import { type ConfigMappingsType, numberConfigHelper } from '@aztec/foundation/config';

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

/** Default maximum number of transactions per block. */
export const DEFAULT_MAX_TXS_PER_BLOCK = 32;

/**
* Partial sequencer config mappings for fields that need to be shared across packages.
* The full sequencer config mappings remain in sequencer-client, but shared fields
* (like blockDurationMs needed by both p2p and sequencer-client) are defined here
* to avoid duplication.
*/
export const sharedSequencerConfigMappings: ConfigMappingsType<
Pick<SequencerConfig, 'blockDurationMs' | 'expectedBlockProposalsPerSlot'>
Pick<SequencerConfig, 'blockDurationMs' | 'expectedBlockProposalsPerSlot' | 'maxTxsPerBlock'>
> = {
blockDurationMs: {
env: 'SEQ_BLOCK_DURATION_MS',
Expand All @@ -26,4 +29,9 @@ export const sharedSequencerConfigMappings: ConfigMappingsType<
parseEnv: (val: string) => (val ? parseInt(val, 10) : 0),
defaultValue: 0,
},
maxTxsPerBlock: {
env: 'SEQ_MAX_TX_PER_BLOCK',
description: 'The maximum number of txs to include in a block.',
...numberConfigHelper(DEFAULT_MAX_TXS_PER_BLOCK),
},
};
3 changes: 2 additions & 1 deletion yarn-project/stdlib/src/interfaces/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export type ValidatorClientConfig = ValidatorHASignerConfig & {
};

export type ValidatorClientFullConfig = ValidatorClientConfig &
Pick<SequencerConfig, 'txPublicSetupAllowList' | 'broadcastInvalidBlockProposal'> &
Pick<SequencerConfig, 'txPublicSetupAllowList' | 'broadcastInvalidBlockProposal' | 'maxTxsPerBlock'> &
Pick<
SlasherConfig,
'slashBroadcastedInvalidBlockPenalty' | 'slashDuplicateProposalPenalty' | 'slashDuplicateAttestationPenalty'
Expand Down Expand Up @@ -93,6 +93,7 @@ export const ValidatorClientFullConfigSchema = zodFor<Omit<ValidatorClientFullCo
ValidatorClientConfigSchema.extend({
txPublicSetupAllowList: z.array(AllowedElementSchema).optional(),
broadcastInvalidBlockProposal: z.boolean().optional(),
maxTxsPerBlock: z.number().optional(),
slashBroadcastedInvalidBlockPenalty: schemas.BigInt,
slashDuplicateProposalPenalty: schemas.BigInt,
slashDuplicateAttestationPenalty: schemas.BigInt,
Expand Down
1 change: 1 addition & 0 deletions yarn-project/validator-client/src/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export function createBlockProposalHandler(
const metrics = new ValidatorMetrics(deps.telemetry);
const blockProposalValidator = new BlockProposalValidator(deps.epochCache, {
txsPermitted: !config.disableTransactions,
maxTxsPerBlock: config.maxTxsPerBlock,
});
return new BlockProposalHandler(
deps.checkpointsBuilder,
Expand Down
1 change: 1 addition & 0 deletions yarn-project/validator-client/src/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter)
const metrics = new ValidatorMetrics(telemetry);
const blockProposalValidator = new BlockProposalValidator(epochCache, {
txsPermitted: !config.disableTransactions,
maxTxsPerBlock: config.maxTxsPerBlock,
});
const blockProposalHandler = new BlockProposalHandler(
checkpointsBuilder,
Expand Down
Loading