fix(p2p): validate BLOCK_TXS in BatchTxRequester

yarn-project/p2p/src/client/interface.ts

@@ -9,11 +9,7 @@ import type { ENR } from '@nethermindeth/enr';
 
 import type { P2PConfig } from '../config.js';
 import type { AuthRequest, StatusMessage } from '../services/index.js';
-import type {
-  ReqRespSubProtocol,
-  ReqRespSubProtocolHandler,
-  ReqRespSubProtocolValidators,
-} from '../services/reqresp/interface.js';
+import type { ReqRespSubProtocol, ReqRespSubProtocolHandler } from '../services/reqresp/interface.js';
 import type {
   DuplicateAttestationInfo,
   DuplicateProposalInfo,
@@ -228,11 +224,7 @@ export type P2P = P2PClient & {
   /** Clears the db. */
   clear(): Promise<void>;
 
-  addReqRespSubProtocol(
-    subProtocol: ReqRespSubProtocol,
-    handler: ReqRespSubProtocolHandler,
-    validator?: ReqRespSubProtocolValidators[ReqRespSubProtocol],
-  ): Promise<void>;
+  addReqRespSubProtocol(subProtocol: ReqRespSubProtocol, handler: ReqRespSubProtocolHandler): Promise<void>;
 
   handleAuthRequestFromPeer(authRequest: AuthRequest, peerId: PeerId): Promise<StatusMessage>;
 

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

@@ -39,11 +39,7 @@ import type { AttestationPoolApi, ProposalsForSlot } from '../mem_pools/attestat
 import type { MemPools } from '../mem_pools/interface.js';
 import type { TxPoolV2 } from '../mem_pools/tx_pool_v2/interfaces.js';
 import type { AuthRequest, StatusMessage } from '../services/index.js';
-import {
-  ReqRespSubProtocol,
-  type ReqRespSubProtocolHandler,
-  type ReqRespSubProtocolValidators,
-} from '../services/reqresp/interface.js';
+import { ReqRespSubProtocol, type ReqRespSubProtocolHandler } from '../services/reqresp/interface.js';
 import type {
   DuplicateAttestationInfo,
   DuplicateProposalInfo,
@@ -283,12 +279,8 @@ export class P2PClient extends WithTracer implements P2P {
     return this.syncPromise;
   }
 
-  addReqRespSubProtocol(
-    subProtocol: ReqRespSubProtocol,
-    handler: ReqRespSubProtocolHandler,
-    validator: ReqRespSubProtocolValidators[ReqRespSubProtocol],
-  ): Promise<void> {
-    return this.p2pService.addReqRespSubProtocol(subProtocol, handler, validator);
+  addReqRespSubProtocol(subProtocol: ReqRespSubProtocol, handler: ReqRespSubProtocolHandler): Promise<void> {
+    return this.p2pService.addReqRespSubProtocol(subProtocol, handler);
   }
 
   private initBlockStream(startingBlock?: BlockNumber) {

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

@@ -53,7 +53,10 @@ describe('p2p client integration batch txs', () => {
     epochCache = mock<EpochCache>();
     worldState = mock<WorldStateSynchronizer>();
     connectionSampler = mock<ConnectionSampler>();
-    mockP2PService = mock<BatchTxRequesterLibP2PService>({ connectionSampler });
+    mockP2PService = mock<BatchTxRequesterLibP2PService>({
+      connectionSampler,
+      validateRequestedBlockTxsConsistency: () => Promise.resolve(true),
+    });
     txValidator = {
       validateRequestedTx: () => Promise.resolve({ result: 'valid' }),
       validateRequestedTxs: txs => Promise.resolve(txs.map(() => ({ result: 'valid' }))),

yarn-project/p2p/src/services/dummy_service.ts

@@ -18,7 +18,6 @@ import type {
   ReqRespSubProtocol,
   ReqRespSubProtocolHandler,
   ReqRespSubProtocolHandlers,
-  ReqRespSubProtocolValidators,
   SubProtocolMap,
 } from './reqresp/interface.js';
 import type { GoodByeReason } from './reqresp/protocols/goodbye.js';
@@ -146,11 +145,7 @@ export class DummyP2PService implements P2PService {
     return Promise.resolve();
   }
 
-  addReqRespSubProtocol(
-    _subProtocol: ReqRespSubProtocol,
-    _handler: ReqRespSubProtocolHandler,
-    _validator?: ReqRespSubProtocolValidators[ReqRespSubProtocol],
-  ): Promise<void> {
+  addReqRespSubProtocol(_subProtocol: ReqRespSubProtocol, _handler: ReqRespSubProtocolHandler): Promise<void> {
     return Promise.resolve();
   }
 
@@ -179,6 +174,7 @@ export class DummyP2PService implements P2PService {
       peerScoring: {
         penalizePeer: (_peerId, _penalty) => {},
       },
+      validateRequestedBlockTxsConsistency: () => Promise.resolve(true),
     };
   }
 }
@@ -284,10 +280,7 @@ export class DummyPeerManager implements PeerManagerInterface {
 export class DummyReqResp implements ReqRespInterface {
   updateConfig(_config: Partial<P2PReqRespConfig>): void {}
   setShouldRejectPeer(): void {}
-  start(
-    _subProtocolHandlers: ReqRespSubProtocolHandlers,
-    _subProtocolValidators: ReqRespSubProtocolValidators,
-  ): Promise<void> {
+  start(_subProtocolHandlers: ReqRespSubProtocolHandlers): Promise<void> {
     return Promise.resolve();
   }
   stop(): Promise<void> {
@@ -317,11 +310,7 @@ export class DummyReqResp implements ReqRespInterface {
     };
   }
 
-  addSubProtocol(
-    _subProtocol: ReqRespSubProtocol,
-    _handler: ReqRespSubProtocolHandler,
-    _validator?: ReqRespSubProtocolValidators[ReqRespSubProtocol],
-  ): Promise<void> {
+  addSubProtocol(_subProtocol: ReqRespSubProtocol, _handler: ReqRespSubProtocolHandler): Promise<void> {
     return Promise.resolve();
   }
 }

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

@@ -19,7 +19,7 @@ import {
   makeCheckpointProposal,
   mockTx,
 } from '@aztec/stdlib/testing';
-import { type Tx, TxArray, TxHashArray, type TxValidator } from '@aztec/stdlib/tx';
+import { TxArray, TxHashArray } from '@aztec/stdlib/tx';
 import { type TelemetryClient, getTelemetryClient } from '@aztec/telemetry-client';
 import { ServerWorldStateSynchronizer } from '@aztec/world-state';
 
@@ -316,7 +316,7 @@ describe('LibP2PService', () => {
     });
   });
 
-  describe('validateRequestedBlockTxs', () => {
+  describe('validateRequestedBlockTxsConsistency', () => {
     function makeRequest(archiveRoot: Fr, length: number, indices: number[]): BlockTxsRequest {
       return new BlockTxsRequest(archiveRoot, new TxHashArray(), BitVector.init(length, indices));
     }
@@ -347,7 +347,7 @@ describe('LibP2PService', () => {
       const request = makeRequest(reqHash, 5, [0, 2]);
       const response = makeResponse(otherHash, 5, [0, 2], []);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.MidToleranceError);
     });
@@ -357,7 +357,7 @@ describe('LibP2PService', () => {
       const request = makeRequest(hash, 5, [0, 2]);
       const response = makeResponse(hash, 4, [0, 2], []);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.MidToleranceError);
     });
@@ -367,7 +367,7 @@ describe('LibP2PService', () => {
       const request = makeRequest(hash, 5, [0, 2, 3]);
       const response = makeResponse(hash, 5, [0, 2, 3], ['0xaaa', '0xaaa']); // duplicate
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.MidToleranceError);
     });
@@ -378,7 +378,7 @@ describe('LibP2PService', () => {
       const request = makeRequest(hash, 3, [0, 2]);
       const response = makeResponse(hash, 3, [0], ['0x1', '0x2']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.MidToleranceError);
     });
@@ -390,7 +390,7 @@ describe('LibP2PService', () => {
 
       setProposalTxHashes(service, ['0xgood0', '0xgood2', '0xgood4', '0xother', '0xother2']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.LowToleranceError);
     });
@@ -404,7 +404,7 @@ describe('LibP2PService', () => {
 
       setProposalTxHashes(service, ['0xgood0', '0xother1', '0xgood2', '0xother3', '0xgood4']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.LowToleranceError);
     });
@@ -416,9 +416,8 @@ describe('LibP2PService', () => {
 
       setProposalTxHashes(service, ['0xgood0', '0xother1', '0xgood2', '0xother3', '0xgood4']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(true);
-      expect(service.validateRequestedTxMock).toHaveBeenCalledTimes(3);
     });
 
     it('should accept partial subset when proposal exists and order matches requested indices', async () => {
@@ -429,9 +428,8 @@ describe('LibP2PService', () => {
 
       setProposalTxHashes(service, ['0xgood0', '0xother1', '0xgood2', '0xother3', '0xgood4']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(true);
-      expect(service.validateRequestedTxMock).toHaveBeenCalledTimes(2);
       expect(mockPeerManager.penalizePeer).not.toHaveBeenCalled();
     });
 
@@ -443,9 +441,8 @@ describe('LibP2PService', () => {
 
       setProposalTxHashes(service, ['0xgood0', '0xother1', '0xgood2', '0xother3', '0xother4']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(true);
-      expect(service.validateRequestedTxMock).toHaveBeenCalledTimes(0);
       expect(mockPeerManager.penalizePeer).not.toHaveBeenCalled();
     });
 
@@ -455,7 +452,7 @@ describe('LibP2PService', () => {
       const request = makeRequest(hash, 3, [1]);
       const response = makeResponse(hash, 3, [], ['0xsome']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.MidToleranceError);
     });
@@ -468,7 +465,7 @@ describe('LibP2PService', () => {
 
       setProposalTxHashes(service, ['0xgood0', '0xother1', '0xgood2', '0xother3', '0xgood4']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.LowToleranceError);
     });
@@ -481,7 +478,7 @@ describe('LibP2PService', () => {
 
       setProposalTxHashes(service, ['0xgood0', '0xother1', '0xgood2', '0xother3', '0xgood4']);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).toHaveBeenCalledWith(mockPeerId, PeerErrorSeverity.LowToleranceError);
     });
@@ -498,7 +495,7 @@ describe('LibP2PService', () => {
       };
       service.setAttestationPool(mockAttestationPool);
 
-      const ok = await service.validateRequestedBlockTxs(request, response, mockPeerId);
+      const ok = await service.validateRequestedBlockTxsConsistency(request, response, mockPeerId);
       expect(ok).toBe(false);
       expect(mockPeerManager.penalizePeer).not.toHaveBeenCalled();
     });
@@ -1315,9 +1312,6 @@ interface CreateTestLibP2PServiceOptions {
  * and allows construction with mocked dependencies.
  */
 class TestLibP2PService extends LibP2PService {
-  /** Mocked validateRequestedTx for testing. */
-  public validateRequestedTxMock: jest.Mock;
-
   /** Controls whether first-stage gossip validation passes. Set to false to simulate first-stage failure. */
   public firstStageValidationPasses = true;
 
@@ -1330,9 +1324,6 @@ class TestLibP2PService extends LibP2PService {
   /** Controls the severity returned by the failing first-stage validator. */
   public firstStageSeverity: PeerErrorSeverity = PeerErrorSeverity.LowToleranceError;
 
-  /** Stub validator returned by createRequestedTxValidator. */
-  private stubValidator: TxValidator;
-
   /** Exposed epoch cache for test configuration. */
   public testEpochCache: MockProxy<EpochCacheInterface>;
 
@@ -1387,10 +1378,6 @@ class TestLibP2PService extends LibP2PService {
     this.mockPeerDiscoveryService = resolvedPeerDiscoveryService;
 
     this.testEpochCache = epochCache;
-    this.validateRequestedTxMock = jest.fn(() => Promise.resolve());
-    this.stubValidator = {
-      validateTx: () => Promise.resolve({ result: 'valid' as const }),
-    };
   }
 
   /** Exposes the protected handleNewGossipMessage for testing. */
@@ -1429,13 +1416,13 @@ class TestLibP2PService extends LibP2PService {
     };
   }
 
-  /** Exposes the protected validateRequestedBlockTxs for testing. */
-  public override validateRequestedBlockTxs(
+  /** Exposes the protected validateRequestedBlockTxsConsistency for testing. */
+  public override validateRequestedBlockTxsConsistency(
     request: BlockTxsRequest,
     response: BlockTxsResponse,
     peerId: PeerId,
   ): Promise<boolean> {
-    return super.validateRequestedBlockTxs(request, response, peerId);
+    return super.validateRequestedBlockTxsConsistency(request, response, peerId);
   }
 
   /** Exposes the protected processBlockFromPeer for testing. */
@@ -1453,21 +1440,6 @@ class TestLibP2PService extends LibP2PService {
     return super.validateAndStoreCheckpointAttestation(peerId, attestation);
   }
 
-  /** Override to use the mock. */
-  protected override async validateRequestedTx(
-    tx: Tx,
-    peerId: PeerId,
-    _txValidator: TxValidator,
-    _requested?: Set<`0x${string}`>,
-  ): Promise<void> {
-    await this.validateRequestedTxMock(tx, peerId);
-  }
-
-  /** Override to return the stub validator. */
-  protected override createRequestedTxValidator(): TxValidator {
-    return this.stubValidator;
-  }
-
   /** Sets the attestation pool on the mempools for test setup. */
   public setAttestationPool(attestationPool: MockAttestationPoolForTests): void {
     (this.mempools as any).attestationPool = attestationPool;