Skip to content

Commit 5031497

Browse files
authored
chore(p2p): simplify IBatchRequestTxValidator (#23373)
Complexity not needed anymore.
1 parent a2baf79 commit 5031497

6 files changed

Lines changed: 32 additions & 65 deletions

File tree

yarn-project/p2p/src/client/test/p2p_client.integration_batch_txs.test.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { sleep } from '@aztec/foundation/sleep';
99
import { emptyChainConfig } from '@aztec/stdlib/config';
1010
import type { WorldStateSynchronizer } from '@aztec/stdlib/interfaces/server';
1111
import { makeBlockHeader, makeBlockProposal, mockTx } from '@aztec/stdlib/testing';
12-
import { Tx, TxHash } from '@aztec/stdlib/tx';
12+
import { Tx, TxHash, type TxValidator } from '@aztec/stdlib/tx';
1313

1414
import { describe, expect, it, jest } from '@jest/globals';
1515
import { type MockProxy, mock } from 'jest-mock-extended';
@@ -20,7 +20,6 @@ import type { AttestationPool } from '../../mem_pools/attestation_pool/attestati
2020
import type { TxPoolV2 } from '../../mem_pools/tx_pool_v2/interfaces.js';
2121
import { BatchTxRequester } from '../../services/reqresp/batch-tx-requester/batch_tx_requester.js';
2222
import type { BatchTxRequesterLibP2PService } from '../../services/reqresp/batch-tx-requester/interface.js';
23-
import type { IBatchRequestTxValidator } from '../../services/reqresp/batch-tx-requester/tx_validator.js';
2423
import type { ConnectionSampler } from '../../services/reqresp/connection-sampler/connection_sampler.js';
2524
import { RequestTracker } from '../../services/tx_collection/request_tracker.js';
2625
import { generatePeerIdPrivateKeys } from '../../test-helpers/generate-peer-id-private-keys.js';
@@ -39,7 +38,7 @@ describe('p2p client integration batch txs', () => {
3938

4039
let mockP2PService: MockProxy<BatchTxRequesterLibP2PService>;
4140
let connectionSampler: MockProxy<ConnectionSampler>;
42-
let txValidator: IBatchRequestTxValidator;
41+
let txValidator: TxValidator;
4342

4443
let logger: Logger;
4544
let p2pBaseConfig: P2PConfig;
@@ -58,8 +57,7 @@ describe('p2p client integration batch txs', () => {
5857
validateRequestedBlockTxsConsistency: () => Promise.resolve(true),
5958
});
6059
txValidator = {
61-
validateRequestedTx: () => Promise.resolve({ result: 'valid' }),
62-
validateRequestedTxs: txs => Promise.resolve(txs.map(() => ({ result: 'valid' }))),
60+
validateTx: () => Promise.resolve({ result: 'valid' }),
6361
};
6462

6563
logger = createLogger('p2p:test:integration:batch');

yarn-project/p2p/src/services/reqresp/batch-tx-requester/batch_tx_requester.test.ts

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { sleep } from '@aztec/foundation/sleep';
1010
import { DateProvider } from '@aztec/foundation/timer';
1111
import { type BlockProposal, PeerErrorSeverity } from '@aztec/stdlib/p2p';
1212
import { makeBlockHeader, makeBlockProposal } from '@aztec/stdlib/testing';
13-
import { Tx, TxArray, TxHash, type TxValidationResult } from '@aztec/stdlib/tx';
13+
import { Tx, TxArray, TxHash, type TxValidationResult, type TxValidator } from '@aztec/stdlib/tx';
1414

1515
import { describe, expect, it, jest } from '@jest/globals';
1616
import type { PeerId } from '@libp2p/interface';
@@ -26,16 +26,12 @@ import { BatchTxRequester } from './batch_tx_requester.js';
2626
import { DEFAULT_BATCH_TX_REQUESTER_BAD_PEER_THRESHOLD, DEFAULT_BATCH_TX_REQUESTER_TX_BATCH_SIZE } from './config.js';
2727
import type { BatchTxRequesterLibP2PService, IPeerPenalizer } from './interface.js';
2828
import { type IPeerCollection, PeerCollection, RATE_LIMIT_EXCEEDED_PEER_CACHE_TTL } from './peer_collection.js';
29-
import type { IBatchRequestTxValidator } from './tx_validator.js';
3029

3130
/** Mock tx validator for testing that always returns valid */
32-
class AlwaysValidTxValidator implements IBatchRequestTxValidator {
33-
validateRequestedTx(_tx: Tx): Promise<TxValidationResult> {
31+
class AlwaysValidTxValidator implements TxValidator {
32+
validateTx(_tx: Tx): Promise<TxValidationResult> {
3433
return Promise.resolve({ result: 'valid' });
3534
}
36-
validateRequestedTxs(txs: Tx[]): Promise<TxValidationResult[]> {
37-
return Promise.resolve(txs.map(() => ({ result: 'valid' })));
38-
}
3935
}
4036

4137
const TEST_TIMEOUT = 15_000;
@@ -49,7 +45,7 @@ describe('BatchTxRequester', () => {
4945
let connectionSampler: MockProxy<ConnectionSampler>;
5046
let reqResp: MockProxy<ReqRespInterface>;
5147
let mockP2PService: MockProxy<BatchTxRequesterLibP2PService>;
52-
let txValidator: IBatchRequestTxValidator;
48+
let txValidator: TxValidator;
5349

5450
beforeEach(async () => {
5551
logger = createLogger('test');
@@ -1191,13 +1187,12 @@ describe('BatchTxRequester', () => {
11911187

11921188
const invalidTxIndices = new Set([2, 3, 7]); // Mark transactions at indices 2, 3, and 7 as invalid
11931189

1194-
const customValidator: IBatchRequestTxValidator = {
1195-
validateRequestedTx: (tx: Tx) => {
1190+
const customValidator: TxValidator = {
1191+
validateTx: (tx: Tx) => {
11961192
const txIndex = missing.findIndex(h => h.equals(tx.txHash));
11971193
const isInvalid = invalidTxIndices.has(txIndex);
11981194
return Promise.resolve(isInvalid ? { result: 'invalid', reason: ['test invalid'] } : { result: 'valid' });
11991195
},
1200-
validateRequestedTxs: (txs: Tx[]) => Promise.all(txs.map(tx => customValidator.validateRequestedTx(tx))),
12011196
};
12021197

12031198
const { mockImplementation } = createRequestLogger(blockProposal, new Set(), peerTransactions);
@@ -1271,13 +1266,12 @@ describe('BatchTxRequester', () => {
12711266
// Validator that rejects transactions at specific indices
12721267
// Even indices are rejected, odd indices are accepted
12731268
const invalidTxIndices = new Set([0, 2, 4, 6, 8, 10]);
1274-
const mixedValidator: IBatchRequestTxValidator = {
1275-
validateRequestedTx: (tx: Tx) => {
1269+
const mixedValidator: TxValidator = {
1270+
validateTx: (tx: Tx) => {
12761271
const txIndex = missing.findIndex(h => h.equals(tx.txHash));
12771272
const isInvalid = invalidTxIndices.has(txIndex);
12781273
return Promise.resolve(isInvalid ? { result: 'invalid', reason: ['test invalid'] } : { result: 'valid' });
12791274
},
1280-
validateRequestedTxs: (txs: Tx[]) => Promise.all(txs.map(tx => mixedValidator.validateRequestedTx(tx))),
12811275
};
12821276

12831277
const { mockImplementation } = createRequestLogger(blockProposal, new Set(), peerTransactions);
@@ -1329,8 +1323,8 @@ describe('BatchTxRequester', () => {
13291323
connectionSampler.getPeerListSortedByConnectionCountAsc.mockReturnValue([peer]);
13301324

13311325
// Validator that throws errors for specific transactions
1332-
const throwingValidator: IBatchRequestTxValidator = {
1333-
validateRequestedTx: (tx: Tx) => {
1326+
const throwingValidator: TxValidator = {
1327+
validateTx: (tx: Tx) => {
13341328
const txIndex = missing.findIndex(h => h.equals(tx.txHash));
13351329

13361330
// Throw error for transactions at indices 1 and 3
@@ -1345,7 +1339,6 @@ describe('BatchTxRequester', () => {
13451339

13461340
return Promise.resolve({ result: 'valid' });
13471341
},
1348-
validateRequestedTxs: (txs: Tx[]) => Promise.all(txs.map(tx => throwingValidator.validateRequestedTx(tx))),
13491342
};
13501343

13511344
const peerTransactions = new Map([[peer.toString(), Array.from({ length: txCount }, (_, i) => i)]]);
@@ -1698,13 +1691,12 @@ describe('BatchTxRequester', () => {
16981691

16991692
const invalidTxIndices = new Set([1, 6]); // Mark some transactions as invalid
17001693

1701-
const customValidator: IBatchRequestTxValidator = {
1702-
validateRequestedTx: (tx: Tx) => {
1694+
const customValidator: TxValidator = {
1695+
validateTx: (tx: Tx) => {
17031696
const txIndex = missing.findIndex(h => h.equals(tx.txHash));
17041697
const isInvalid = invalidTxIndices.has(txIndex);
17051698
return Promise.resolve(isInvalid ? { result: 'invalid', reason: ['test invalid'] } : { result: 'valid' });
17061699
},
1707-
validateRequestedTxs: (txs: Tx[]) => Promise.all(txs.map(tx => customValidator.validateRequestedTx(tx))),
17081700
};
17091701

17101702
const { mockImplementation } = createRequestLogger(blockProposal, new Set(), peerTransactions);

yarn-project/p2p/src/services/reqresp/batch-tx-requester/batch_tx_requester.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { FifoMemoryQueue, type ISemaphore, Semaphore } from '@aztec/foundation/q
44
import { sleep } from '@aztec/foundation/sleep';
55
import { DateProvider } from '@aztec/foundation/timer';
66
import { PeerErrorSeverity } from '@aztec/stdlib/p2p';
7-
import { Tx, TxArray, TxHash } from '@aztec/stdlib/tx';
7+
import { Tx, TxArray, TxHash, type TxValidator } from '@aztec/stdlib/tx';
88

99
import type { PeerId } from '@libp2p/interface';
1010

@@ -21,7 +21,7 @@ import {
2121
import type { BatchTxRequesterLibP2PService, BatchTxRequesterOptions, ITxMetadataCollection } from './interface.js';
2222
import { MissingTxMetadataCollection } from './missing_txs.js';
2323
import { type IPeerCollection, PeerCollection } from './peer_collection.js';
24-
import { BatchRequestTxValidator, type IBatchRequestTxValidator } from './tx_validator.js';
24+
import { createBatchRequestTxValidator } from './tx_validator.js';
2525

2626
/*
2727
* Tries to fetch all missing transaction until deadline is hit.
@@ -51,7 +51,7 @@ export class BatchTxRequester {
5151
private readonly txsMetadata: ITxMetadataCollection;
5252
private readonly smartRequesterSemaphore: ISemaphore;
5353
private readonly txQueue: FifoMemoryQueue<Tx>;
54-
private readonly txValidator: IBatchRequestTxValidator;
54+
private readonly txValidator: TxValidator;
5555
private readonly smartParallelWorkerCount: number;
5656
private readonly dumbParallelWorkerCount: number;
5757
private readonly txBatchSize: number;
@@ -78,7 +78,7 @@ export class BatchTxRequester {
7878
this.opts.dumbParallelWorkerCount ?? DEFAULT_BATCH_TX_REQUESTER_DUMB_PARALLEL_WORKER_COUNT;
7979
this.txBatchSize = this.opts.txBatchSize ?? DEFAULT_BATCH_TX_REQUESTER_TX_BATCH_SIZE;
8080
this.txQueue = new FifoMemoryQueue(this.logger);
81-
this.txValidator = this.opts.txValidator ?? new BatchRequestTxValidator(this.p2pService.txValidatorConfig);
81+
this.txValidator = this.opts.txValidator ?? createBatchRequestTxValidator(this.p2pService.txValidatorConfig);
8282

8383
if (this.opts.peerCollection) {
8484
this.peers = this.opts.peerCollection;
@@ -509,7 +509,7 @@ export class BatchTxRequester {
509509
const validationResults = await Promise.allSettled(
510510
newTxs.map(async tx => ({
511511
tx,
512-
isValid: (await this.txValidator.validateRequestedTx(tx)).result === 'valid',
512+
isValid: (await this.txValidator.validateTx(tx)).result === 'valid',
513513
})),
514514
);
515515

yarn-project/p2p/src/services/reqresp/batch-tx-requester/interface.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
import type { ISemaphore } from '@aztec/foundation/queue';
22
import type { PeerErrorSeverity } from '@aztec/stdlib/p2p';
3-
import type { Tx, TxHash } from '@aztec/stdlib/tx';
3+
import type { Tx, TxHash, TxValidator } from '@aztec/stdlib/tx';
44

55
import type { PeerId } from '@libp2p/interface';
66

77
import type { ConnectionSampler } from '../connection-sampler/connection_sampler.js';
88
import type { BlockTxsRequest, BlockTxsResponse } from '../index.js';
99
import type { ReqRespInterface } from '../interface.js';
1010
import type { IPeerCollection } from './peer_collection.js';
11-
import type { BatchRequestTxValidatorConfig, IBatchRequestTxValidator } from './tx_validator.js';
11+
import type { BatchRequestTxValidatorConfig } from './tx_validator.js';
1212

1313
export interface IPeerPenalizer {
1414
penalizePeer(peerId: PeerId, penalty: PeerErrorSeverity): void;
@@ -57,5 +57,5 @@ export interface BatchTxRequesterOptions {
5757
semaphore?: ISemaphore;
5858
peerCollection?: IPeerCollection;
5959
/** Optional tx validator for testing - if not provided, one is created from p2pService.txValidatorConfig */
60-
txValidator?: IBatchRequestTxValidator;
60+
txValidator?: TxValidator;
6161
}
Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ClientProtocolCircuitVerifier } from '@aztec/stdlib/interfaces/server';
2-
import { Tx, type TxValidationResult, type TxValidator } from '@aztec/stdlib/tx';
2+
import type { TxValidator } from '@aztec/stdlib/tx';
33

44
import { createTxValidatorForOnDemandReceivedTxs } from '../../../msg_validators/index.js';
55

@@ -9,29 +9,9 @@ export interface BatchRequestTxValidatorConfig {
99
proofVerifier: ClientProtocolCircuitVerifier;
1010
}
1111

12-
export interface IBatchRequestTxValidator {
13-
validateRequestedTx(tx: Tx): Promise<TxValidationResult>;
14-
validateRequestedTxs(txs: Tx[]): Promise<TxValidationResult[]>;
15-
}
16-
17-
export class BatchRequestTxValidator implements IBatchRequestTxValidator {
18-
readonly txValidator: TxValidator;
19-
constructor(private readonly config: BatchRequestTxValidatorConfig) {
20-
this.txValidator = BatchRequestTxValidator.createRequestedTxValidator(this.config);
21-
}
22-
23-
public async validateRequestedTx(tx: Tx): Promise<TxValidationResult> {
24-
return await this.txValidator.validateTx(tx);
25-
}
26-
27-
public async validateRequestedTxs(txs: Tx[]): Promise<TxValidationResult[]> {
28-
return await Promise.all(txs.map(tx => this.validateRequestedTx(tx)));
29-
}
30-
31-
static createRequestedTxValidator(config: BatchRequestTxValidatorConfig): TxValidator {
32-
return createTxValidatorForOnDemandReceivedTxs(config.proofVerifier, {
33-
l1ChainId: config.l1ChainId,
34-
rollupVersion: config.rollupVersion,
35-
});
36-
}
12+
export function createBatchRequestTxValidator(config: BatchRequestTxValidatorConfig): TxValidator {
13+
return createTxValidatorForOnDemandReceivedTxs(config.proofVerifier, {
14+
l1ChainId: config.l1ChainId,
15+
rollupVersion: config.rollupVersion,
16+
});
3717
}

yarn-project/p2p/src/testbench/p2p_client_testbench_worker.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import type { DataStoreConfig } from '@aztec/stdlib/kv-store';
2323
import { type BlockProposal, P2PMessage } from '@aztec/stdlib/p2p';
2424
import { ChonkProof } from '@aztec/stdlib/proofs';
2525
import { makeAztecAddress, makeBlockHeader, makeBlockProposal, mockTx } from '@aztec/stdlib/testing';
26-
import { Tx, TxHash, type TxValidationResult } from '@aztec/stdlib/tx';
26+
import { Tx, TxHash, type TxValidationResult, type TxValidator } from '@aztec/stdlib/tx';
2727
import { type TelemetryClient, getTelemetryClient } from '@aztec/telemetry-client';
2828

2929
import type { Message, PeerId } from '@libp2p/interface';
@@ -38,7 +38,6 @@ import { LibP2PService } from '../services/index.js';
3838
import type { PeerManager } from '../services/peer-manager/peer_manager.js';
3939
import { BatchTxRequester } from '../services/reqresp/batch-tx-requester/batch_tx_requester.js';
4040
import type { BatchTxRequesterLibP2PService } from '../services/reqresp/batch-tx-requester/interface.js';
41-
import type { IBatchRequestTxValidator } from '../services/reqresp/batch-tx-requester/tx_validator.js';
4241
import { RateLimitStatus } from '../services/reqresp/rate-limiter/rate_limiter.js';
4342
import type { ReqResp } from '../services/reqresp/reqresp.js';
4443
import type { PeerDiscoveryService } from '../services/service.js';
@@ -276,10 +275,8 @@ async function runAggregatorBenchmark(
276275
}
277276
}
278277

279-
const noopTxValidator: IBatchRequestTxValidator = {
280-
validateRequestedTx: (_tx: Tx): Promise<TxValidationResult> => Promise.resolve({ result: 'valid' }),
281-
validateRequestedTxs: (txs: Tx[]): Promise<TxValidationResult[]> =>
282-
Promise.resolve(txs.map(() => ({ result: 'valid' }))),
278+
const noopTxValidator: TxValidator = {
279+
validateTx: (_tx: Tx): Promise<TxValidationResult> => Promise.resolve({ result: 'valid' }),
283280
};
284281

285282
timer = new Timer();

0 commit comments

Comments
 (0)