Revert "feat: detect announce IP changes automatically (#21606)"

This reverts commit 04f7026, reversing
changes made to 6b12e5e.

Author: AztecBot

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

@@ -57,7 +57,7 @@ export async function createP2PClient(
   telemetry: TelemetryClient = getTelemetryClient(),
   deps: P2PClientDeps = {},
 ) {
-  const config = configureP2PClientAddresses({
+  const config = await configureP2PClientAddresses({
     ...inputConfig,
     dataStoreMapSizeKb: inputConfig.p2pStoreMapSizeKb ?? inputConfig.dataStoreMapSizeKb,
   });

yarn-project/p2p/src/services/discv5/discV5_service.ts

@@ -96,7 +96,7 @@ export class DiscV5Service extends EventEmitter implements PeerDiscoveryService
         lookupTimeout: 2000,
         requestTimeout: 2000,
         allowUnverifiedSessions: true,
-        enrUpdate: config.queryForIp && !p2pIp, // Enable native ENR IP discovery when no static IP is configured
+        enrUpdate: !p2pIp ? true : false, // If no p2p IP is set, enrUpdate can automatically resolve it
         ...configOverrides.config,
       },
       metricsRegistry,
@@ -129,11 +129,9 @@ export class DiscV5Service extends EventEmitter implements PeerDiscoveryService
   private onMultiaddrUpdated(m: Multiaddr) {
     // We want to update our tcp port to match the udp port
     // p2pBroadcastPort is optional on config, however it is set to default within the p2p client factory
-    const address = m.nodeAddress().address;
-    const multiAddrTcp = multiaddr(convertToMultiaddr(address, this.config.p2pBroadcastPort!, 'tcp'));
+    const multiAddrTcp = multiaddr(convertToMultiaddr(m.nodeAddress().address, this.config.p2pBroadcastPort!, 'tcp'));
     this.enr.setLocationMultiaddr(multiAddrTcp);
     this.logger.info('Multiaddr updated', { multiaddr: multiAddrTcp.toString() });
-    this.emit('ip:changed', address);
   }
 
   public async start(): Promise<void> {

yarn-project/p2p/src/services/discv5/discv5_service.test.ts

@@ -111,16 +111,12 @@ describe('Discv5Service', () => {
     await stopNodes(node1, node2);
   });
 
-  it('should automatically resolve p2p ip if not set and queryForIp is true', async () => {
+  it('should automatically resolve p2p ip if not set', async () => {
     const extraNodes = 3;
     const nodes: DiscV5Service[] = [];
 
-    // Create a node with no p2pIp and queryForIp=true -- enrUpdate should be enabled
-    const node = await createNode({
-      p2pIp: undefined,
-      queryForIp: true,
-      config: { addrVotesToUpdateEnr: 1, pingInterval: 200 },
-    });
+    // Create a node with no p2pIp
+    const node = await createNode({ p2pIp: undefined, config: { addrVotesToUpdateEnr: 1, pingInterval: 200 } });
     await node.start();
     nodes.push(node);
 
@@ -138,19 +134,12 @@ describe('Discv5Service', () => {
 
     expect(node.getEnr().ip).toEqual(undefined);
 
-    // ip:changed should be emitted when the ENR IP is resolved
-    let discoveredIp: string | undefined;
-    node.on('ip:changed', (ip: string) => {
-      discoveredIp = ip;
-    });
-
     await runDiscoveryUntil(nodes, () => node.getEnr().ip !== undefined);
 
-    // Expect IP has been updated, tcp and udp ports match, and ip:changed event was emitted
+    // Expect it's IP has been updated, and that the tcp and udp ports are the same
     expect(node.getEnr().ip).not.toEqual(undefined);
     expect(node.getEnr().tcp).not.toEqual(undefined);
     expect(node.getEnr().tcp).toEqual(node.getEnr().udp);
-    expect(discoveredIp).toEqual(node.getEnr().ip?.toString());
 
     await stopNodes(...nodes);
   });

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

@@ -24,8 +24,6 @@ import { ServerWorldStateSynchronizer } from '@aztec/world-state';
 import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals';
 import type { Message, PeerId } from '@libp2p/interface';
 import { TopicValidatorResult } from '@libp2p/interface';
-import type { ConnectionManager } from '@libp2p/interface-internal';
-import { multiaddr } from '@multiformats/multiaddr';
 import { type MockProxy, mock } from 'jest-mock-extended';
 
 import { type P2PConfig, p2pConfigMappings } from '../../config.js';
@@ -38,8 +36,6 @@ import type { MemPools } from '../../mem_pools/interface.js';
 import type { TxPoolV2 } from '../../mem_pools/tx_pool_v2/interfaces.js';
 import type { TransactionValidator } from '../../msg_validators/tx_validator/factory.js';
 import type { PubSubLibp2p } from '../../util.js';
-import { convertToMultiaddr } from '../../util.js';
-import { DummyPeerDiscoveryService } from '../dummy_service.js';
 import type { PeerManagerInterface } from '../peer-manager/interface.js';
 import type { ReqRespInterface } from '../reqresp/interface.js';
 import { BitVector } from '../reqresp/protocols/block_txs/bitvector.js';
@@ -1055,148 +1051,6 @@ describe('LibP2PService', () => {
       expect(allNodesCheckpointReceivedCallback).toHaveBeenCalledWith(expect.any(Object), expect.anything());
     });
   });
-
-  describe('discv5 ip:changed bridge (queryForIp)', () => {
-    const p2pPort = 40400;
-    const firstIp = '203.0.113.5';
-    const secondIp = '198.51.100.2';
-
-    function createQueryForIpService() {
-      const peerDiscovery = new DummyPeerDiscoveryService();
-      const addressManager = {
-        removeObservedAddr: jest.fn(),
-        addObservedAddr: jest.fn(),
-        confirmObservedAddr: jest.fn(),
-      };
-      const mockPeerId = mock<PeerId>({ toString: () => MOCK_PEER_ID });
-      const nodeState = { status: 'stopped' as string };
-      const mockNode = {
-        get status() {
-          return nodeState.status;
-        },
-        set status(v: string) {
-          nodeState.status = v;
-        },
-        peerId: mockPeerId,
-        start: jest.fn(() => {
-          nodeState.status = 'started';
-        }),
-        stop: jest.fn(() => {
-          nodeState.status = 'stopped';
-        }),
-        services: {
-          pubsub: {
-            subscribe: jest.fn(),
-            addEventListener: jest.fn(),
-            removeEventListener: jest.fn(),
-            getMeshPeers: jest.fn(() => []),
-          },
-          components: {
-            addressManager,
-            connectionManager: {} as unknown as ConnectionManager,
-          },
-        },
-      } as unknown as PubSubLibp2p;
-
-      const config: P2PConfig = {
-        ...getDefaultConfig(p2pConfigMappings),
-        seenMessageCacheSize: 1000,
-        debugP2PInstrumentMessages: false,
-        disableTransactions: true,
-        l1ChainId: 1,
-        rollupVersion: 1,
-        l1Contracts: { rollupAddress: EthAddress.random() },
-        queryForIp: true,
-        p2pIp: undefined,
-        p2pPort,
-        p2pDiscoveryDisabled: true,
-        peerCheckIntervalMS: 60_000, // Long enough that heartbeat won't run during this unit test
-      };
-
-      const mockPeerManager = mock<PeerManagerInterface>();
-      mockPeerManager.initializePeers.mockResolvedValue(undefined);
-      mockPeerManager.stop.mockResolvedValue(undefined);
-      mockPeerManager.heartbeat.mockResolvedValue(undefined);
-
-      const mockReqResp = mock<ReqRespInterface>();
-      mockReqResp.start.mockResolvedValue(undefined);
-      mockReqResp.stop.mockResolvedValue(undefined);
-
-      const mempools = mock<MemPools>();
-      const archiver = mock<L2BlockSource & ContractDataSource>();
-      const epochCache = mock<EpochCacheInterface>();
-      const mockProofVerifier = mock<ClientProtocolCircuitVerifier>({
-        verifyProof: () => Promise.resolve({ valid: true, durationMs: 1, totalDurationMs: 1 }),
-      });
-      const mockWorldStateSynchronizer = mock<ServerWorldStateSynchronizer>();
-
-      const service = new LibP2PService(
-        config,
-        mockNode,
-        peerDiscovery,
-        mockReqResp,
-        mockPeerManager,
-        mempools,
-        archiver,
-        epochCache,
-        mockProofVerifier,
-        mockWorldStateSynchronizer,
-        getTelemetryClient(),
-        createLogger('p2p:test:queryForIp'),
-      );
-
-      return { service, peerDiscovery, addressManager, config };
-    }
-
-    it('registers observed announce address when discv5 emits ip:changed', async () => {
-      const { service, peerDiscovery, addressManager } = createQueryForIpService();
-      const expectedAddr = multiaddr(convertToMultiaddr(firstIp, p2pPort, 'tcp'));
-
-      await service.start();
-      peerDiscovery.emit('ip:changed', firstIp);
-
-      expect(addressManager.addObservedAddr).toHaveBeenCalledWith(expectedAddr);
-      expect(addressManager.confirmObservedAddr).toHaveBeenCalledWith(expectedAddr);
-      expect(addressManager.removeObservedAddr).not.toHaveBeenCalled();
-
-      await service.stop();
-    });
-
-    it('removes previous observed address when ip:changed fires again with a new IP', async () => {
-      const { service, peerDiscovery, addressManager } = createQueryForIpService();
-      const firstAddr = multiaddr(convertToMultiaddr(firstIp, p2pPort, 'tcp'));
-      const secondAddr = multiaddr(convertToMultiaddr(secondIp, p2pPort, 'tcp'));
-
-      await service.start();
-      peerDiscovery.emit('ip:changed', firstIp);
-      addressManager.removeObservedAddr.mockClear();
-      addressManager.addObservedAddr.mockClear();
-      addressManager.confirmObservedAddr.mockClear();
-
-      peerDiscovery.emit('ip:changed', secondIp);
-
-      expect(addressManager.removeObservedAddr).toHaveBeenCalledWith(firstAddr);
-      expect(addressManager.addObservedAddr).toHaveBeenCalledWith(secondAddr);
-      expect(addressManager.confirmObservedAddr).toHaveBeenCalledWith(secondAddr);
-
-      await service.stop();
-    });
-
-    it('unsubscribes from ip:changed on stop so later emits are ignored', async () => {
-      const { service, peerDiscovery, addressManager } = createQueryForIpService();
-
-      await service.start();
-      peerDiscovery.emit('ip:changed', firstIp);
-      addressManager.addObservedAddr.mockClear();
-      addressManager.confirmObservedAddr.mockClear();
-
-      await service.stop();
-      peerDiscovery.emit('ip:changed', secondIp);
-
-      expect(addressManager.addObservedAddr).not.toHaveBeenCalled();
-      expect(addressManager.confirmObservedAddr).not.toHaveBeenCalled();
-    });
-  });
 });
 
 /** Mock type for tx objects used in block txs validation tests. */

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

@@ -51,10 +51,9 @@ import { yamux } from '@chainsafe/libp2p-yamux';
 import { bootstrap } from '@libp2p/bootstrap';
 import { identify } from '@libp2p/identify';
 import { type Message, type MultiaddrConnection, type PeerId, TopicValidatorResult } from '@libp2p/interface';
-import type { AddressManager, ConnectionManager } from '@libp2p/interface-internal';
+import type { ConnectionManager } from '@libp2p/interface-internal';
 import { mplex } from '@libp2p/mplex';
 import { tcp } from '@libp2p/tcp';
-import { multiaddr } from '@multiformats/multiaddr';
 import { ENR } from '@nethermindeth/enr';
 import { createLibp2p } from 'libp2p';
 
@@ -182,10 +181,6 @@ export class LibP2PService extends WithTracer implements P2PService {
   private validatorCheckpointReceivedCallback: P2PCheckpointReceivedCallback;
 
   private gossipSubEventHandler: (e: CustomEvent<GossipsubMessage>) => void;
-  private ipChangedHandler?: (ip: string) => void;
-
-  /** Discovered public IP address (set when queryForIp is enabled and no static IP was configured). */
-  private discoveredP2pIp?: string;
 
   private instrumentation: P2PInstrumentation;
 
@@ -457,9 +452,8 @@ export class LibP2PService extends WithTracer implements P2PService {
             topics: topicScoreParams,
           }),
         }) as (components: GossipSubComponents) => GossipSub,
-        components: (components: { connectionManager: ConnectionManager; addressManager: AddressManager }) => ({
+        components: (components: { connectionManager: ConnectionManager }) => ({
           connectionManager: components.connectionManager,
-          addressManager: components.addressManager,
         }),
       },
       logger: createLibp2pComponentLogger(logger.module, logger.getBindings()),
@@ -518,10 +512,10 @@ export class LibP2PService extends WithTracer implements P2PService {
 
     // Get listen & announce addresses for logging
     const { p2pIp, p2pPort } = this.config;
-    if (!p2pIp && !this.config.queryForIp) {
+    if (!p2pIp) {
       throw new Error('Announce address not provided.');
     }
-    const announceTcpMultiaddr = p2pIp ? convertToMultiaddr(p2pIp, p2pPort, 'tcp') : undefined;
+    const announceTcpMultiaddr = convertToMultiaddr(p2pIp, p2pPort, 'tcp');
 
     // Create request response protocol handlers
     const txHandler = reqRespTxHandler(this.mempools);
@@ -575,31 +569,6 @@ export class LibP2PService extends WithTracer implements P2PService {
     if (!this.config.p2pDiscoveryDisabled) {
       await this.peerDiscoveryService.start();
     }
-
-    // When queryForIp is enabled and no static IP was configured, bridge discv5 IP discovery to libp2p.
-    // Discv5 discovers our public IP via peer WHOAREYOU exchanges (enrUpdate=true) and emits 'ip:changed'.
-    // We confirm the discovered address in the libp2p AddressManager so it appears in getMultiaddrs()
-    // and is pushed to all connected peers via the identify protocol.
-    if (this.config.queryForIp && !p2pIp) {
-      this.ipChangedHandler = (ip: string) => {
-        const addressManager = this.node.services.components.addressManager;
-        const newAddr = multiaddr(convertToMultiaddr(ip, this.config.p2pPort, 'tcp'));
-
-        // Remove old discovered IP if one exists
-        if (this.discoveredP2pIp) {
-          const oldAddr = multiaddr(convertToMultiaddr(this.discoveredP2pIp, this.config.p2pPort, 'tcp'));
-          addressManager.removeObservedAddr(oldAddr);
-        }
-
-        addressManager.addObservedAddr(newAddr);
-        addressManager.confirmObservedAddr(newAddr);
-        // Store discovered IP
-        this.discoveredP2pIp = ip;
-        this.logger.info('Public IP discovered via discv5', { ip });
-      };
-      this.peerDiscoveryService.on('ip:changed', this.ipChangedHandler);
-    }
-
     this.discoveryRunningPromise = new RunningPromise(
       async () => {
         await this.peerManager.heartbeat();
@@ -612,7 +581,7 @@ export class LibP2PService extends WithTracer implements P2PService {
     this.logger.info(`Started P2P service`, {
       listen: this.config.listenAddress,
       port: this.config.p2pPort,
-      announce: announceTcpMultiaddr ?? 'pending (queryForIp=true)',
+      announce: announceTcpMultiaddr,
       peerId: this.node.peerId.toString(),
     });
   }
@@ -625,12 +594,6 @@ export class LibP2PService extends WithTracer implements P2PService {
     // Remove gossip sub listener
     this.node.services.pubsub.removeEventListener(GossipSubEvent.MESSAGE, this.gossipSubEventHandler);
 
-    // Remove ip:changed listener if registered
-    if (this.ipChangedHandler) {
-      this.peerDiscoveryService.off('ip:changed', this.ipChangedHandler);
-      this.ipChangedHandler = undefined;
-    }
-
     // Stop peer manager
     this.logger.debug('Stopping peer manager...');
     await this.peerManager.stop();

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

@@ -201,13 +201,6 @@ export interface PeerDiscoveryService extends EventEmitter {
   on(event: 'peer:discovered', listener: (enr: ENR) => void): this;
   emit(event: 'peer:discovered', enr: ENR): boolean;
 
-  /**
-   * Event emitted when our public IP is discovered or changes via discv5 peer interactions.
-   * Only emitted when enrUpdate is enabled (i.e. queryForIp=true and no static p2pIp).
-   */
-  on(event: 'ip:changed', listener: (ip: string) => void): this;
-  emit(event: 'ip:changed', ip: string): boolean;
-
   getStatus(): PeerDiscoveryState;
 
   getEnr(): ENR | undefined;

yarn-project/p2p/src/test-helpers/mock-pubsub.ts

@@ -15,7 +15,6 @@ import {
   type TopicValidatorResult,
   TypedEventEmitter,
 } from '@libp2p/interface';
-import type { AddressManager, ConnectionManager } from '@libp2p/interface-internal';
 
 import type { P2PConfig } from '../config.js';
 import type { MemPools } from '../mem_pools/interface.js';
@@ -213,14 +212,6 @@ export class MockPubSub implements PubSubLibp2p {
   get services() {
     return {
       pubsub: this.gossipSub,
-      components: {
-        addressManager: {
-          removeObservedAddr: () => {},
-          addObservedAddr: () => {},
-          confirmObservedAddr: () => {},
-        } as unknown as AddressManager,
-        connectionManager: {} as unknown as ConnectionManager,
-      },
     };
   }