Merge pull request #506 from OriginTrail/feat/rfc07-pr5-gossipsub-msgid

feat: Network Relay Registry (Phase 1) + RFC 07 In-Process PeerResolver (PRs #471, #494, #496, #497, #499, #501 bundled)

Author: branarakic

.github/workflows/ci.yml

@@ -130,6 +130,19 @@ jobs:
       - name: Test audit-create-random lexer
         run: node --test scripts/audit-create-random.test.mjs
 
+      # RFC 07 §3.2 boundary gate. Bans raw `dialProtocol(` outside the
+      # network/protocol-router boundary so new protocols can't silently
+      # bypass the in-process PeerResolver and re-introduce the
+      # asymmetric-failure class RFC 07 was built to eliminate. See
+      # `scripts/audit-dial-protocol.mjs` for the full rationale and
+      # `dkgv10-spec/production_mainnet/07_IN_PROCESS_PEER_RESOLVER.md`
+      # for the architectural commitment.
+      - name: Audit dialProtocol boundary (RFC 07 §3.2)
+        run: node scripts/audit-dial-protocol.mjs
+
+      - name: Test audit-dial-protocol lexer
+        run: node --test scripts/audit-dial-protocol.test.mjs
+
       - name: Install dependencies
         run: pnpm install --frozen-lockfile
 

packages/agent/src/dkg-agent.ts

@@ -1,22 +1,7 @@
-import { multiaddr } from '@multiformats/multiaddr';
 import type { ProtocolRouter } from '@origintrail-official/dkg-core';
-import type { DiscoveryClient } from '../discovery.js';
-
-/**
- * Minimal libp2p surface the Messenger needs. Defined locally to keep
- * test mocking trivial — production code passes `node.libp2p`.
- */
-interface Libp2pLike {
-  getConnections(peerId?: unknown): Array<unknown>;
-  peerStore: {
-    merge(peer: unknown, update: { multiaddrs: unknown[] }): Promise<void>;
-  };
-}
 
 export interface MessengerDeps {
-  libp2p: Libp2pLike;
   router: ProtocolRouter;
-  discovery: DiscoveryClient;
 }
 
 export interface SendOpts {
@@ -26,32 +11,27 @@ export interface SendOpts {
 /**
  * Single outbound P2P sending primitive.
  *
- * Two responsibilities:
- *  1. Best-effort prime a /p2p-circuit relay route into the libp2p peerStore
- *     before dialling, so NAT'd peers reachable only via a circuit relay can
- *     be dialled by `dialProtocol` (which otherwise falls back to direct dial
- *     and fails for NAT'd peers without an active connection).
- *  2. Forward the bytes via `ProtocolRouter.send` (which itself owns
- *     transport-level retry on recoverable errors — see protocol-router.ts).
+ * Forwards the bytes to `ProtocolRouter.send`, which (since RFC 07 PR-3)
+ * consults `PeerResolver` before dialing — populating the libp2p
+ * peerStore with whatever multiaddrs the resolution order finds
+ * (live conn → DHT → RFC 04 registry → agents-CG). Centralising every
+ * outbound send through one entry point removes the "this code path
+ * forgot to prime the relay route" defect class behind PR #448's
+ * Laptop B invite failure.
  *
- * Centralising this in one place removes a class of "this code path forgot to
- * call ensureCircuitRelayAddress" defects (the latent bug behind PR #448's
- * Laptop B invite failure).
+ * Note: pre-PR-3 this class held its own `PeerResolver` ref and
+ * resolved before delegating to the router. After PR-3 the router does
+ * the same lookup, so doing it here too duplicated the DHT walk on
+ * every cold send. Codex review on PR #497 caught the duplication;
+ * the resolver dependency is now owned by `ProtocolRouter` alone.
  *
- * Discovery is currently SPARQL-first against the agents context graph; this
- * is preserved verbatim from the previous DKGAgent.ensureCircuitRelayAddress
- * implementation. See dkgv10-spec/production_mainnet/04_NETWORK_RELAY_REGISTRY.md
- * for the planned chain-driven replacement.
+ * See `dkgv10-spec/production_mainnet/07_IN_PROCESS_PEER_RESOLVER.md`.
  */
 export class Messenger {
-  private readonly libp2p: Libp2pLike;
   private readonly router: ProtocolRouter;
-  private readonly discovery: DiscoveryClient;
 
   constructor(deps: MessengerDeps) {
-    this.libp2p = deps.libp2p;
     this.router = deps.router;
-    this.discovery = deps.discovery;
   }
 
   async sendToPeer(
@@ -60,41 +40,6 @@ export class Messenger {
     data: Uint8Array,
     opts: SendOpts = {},
   ): Promise<Uint8Array> {
-    await this.ensureCircuitRelayAddress(peerId);
     return this.router.send(peerId, protocolId, data, opts.timeoutMs);
   }
-
-  /**
-   * Best-effort: if the peer is not currently connected and the agent
-   * registry advertises a relay for them, add a /p2p-circuit multiaddr
-   * to the peer store so dialProtocol can route through the relay.
-   *
-   * Failures are swallowed deliberately — the caller's send() will surface
-   * a proper transport error from dialProtocol if the peer is unreachable.
-   *
-   * TODO(follow-up): prefer chain-driven discovery via the network-state
-   * registry (RFC 04) for freshness; SPARQL profile lookup as last-resort
-   * fallback.
-   */
-  private async ensureCircuitRelayAddress(peerIdStr: string): Promise<void> {
-    try {
-      const { peerIdFromString } = await import('@libp2p/peer-id');
-      const peerId = peerIdFromString(peerIdStr);
-
-      const conns = this.libp2p.getConnections(peerId);
-      if (conns.length > 0) return;
-
-      const agent = await this.discovery.findAgentByPeerId(peerIdStr);
-      if (!agent?.relayAddress) return;
-
-      const circuitAddr = multiaddr(
-        `${agent.relayAddress}/p2p-circuit/p2p/${peerIdStr}`,
-      );
-      await this.libp2p.peerStore.merge(peerId, {
-        multiaddrs: [circuitAddr],
-      });
-    } catch {
-      // Best-effort — let the caller's send() surface the real error.
-    }
-  }
 }

packages/agent/src/p2p/messenger.ts

@@ -1,123 +1,80 @@
 import { describe, it, expect, vi } from 'vitest';
 import { Messenger } from '../src/p2p/messenger.js';
 import type { ProtocolRouter } from '@origintrail-official/dkg-core';
-import type { DiscoveryClient } from '../src/discovery.js';
 
-// 12D3Koo... peer ids must be valid base58 to satisfy peerIdFromString. Two
-// arbitrary valid ed25519 peer IDs taken from existing tests.
 const PEER_A = '12D3KooWAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA';
 const PEER_B = '12D3KooWQz2bQbQueABKRSjV9koF8VYsXk5TdCsUmPf5zAEZg3q6';
-const RELAY_ADDR = '/ip4/178.104.54.178/tcp/9090/p2p/12D3KooWSmU3owJvB9sFw8uApDgKrv2VBMecsGGvgAc4Gq6hB57M';
 
 interface MockSetup {
-  libp2pConnections: Array<unknown>;
   routerSendMock: ReturnType<typeof vi.fn>;
-  peerStoreMergeMock: ReturnType<typeof vi.fn>;
-  findAgentMock: ReturnType<typeof vi.fn>;
 }
 
-function makeMessenger(overrides: Partial<MockSetup> = {}): { messenger: Messenger; mocks: MockSetup; callOrder: string[] } {
-  const callOrder: string[] = [];
-
+function makeMessenger(overrides: Partial<MockSetup> = {}): {
+  messenger: Messenger;
+  mocks: MockSetup;
+} {
   const mocks: MockSetup = {
-    libp2pConnections: overrides.libp2pConnections ?? [],
-    routerSendMock: overrides.routerSendMock ?? vi.fn(async () => {
-      callOrder.push('router.send');
-      return new Uint8Array([0x01, 0x02]);
-    }),
-    peerStoreMergeMock: overrides.peerStoreMergeMock ?? vi.fn(async () => {
-      callOrder.push('peerStore.merge');
-    }),
-    findAgentMock: overrides.findAgentMock ?? vi.fn(async () => {
-      callOrder.push('discovery.findAgentByPeerId');
-      return { peerId: PEER_B, name: 'remote', agentUri: 'urn:agent:remote', relayAddress: RELAY_ADDR };
-    }),
+    routerSendMock:
+      overrides.routerSendMock ??
+      vi.fn(async () => new Uint8Array([0x01, 0x02])),
   };
 
   const messenger = new Messenger({
-    libp2p: {
-      getConnections: () => mocks.libp2pConnections,
-      peerStore: { merge: mocks.peerStoreMergeMock },
-    },
     router: { send: mocks.routerSendMock } as unknown as ProtocolRouter,
-    discovery: { findAgentByPeerId: mocks.findAgentMock } as unknown as DiscoveryClient,
   });
 
-  return { messenger, mocks, callOrder };
+  return { messenger, mocks };
 }
 
 describe('Messenger.sendToPeer', () => {
-  it('skips relay prime when peer is already connected', async () => {
-    const { messenger, mocks } = makeMessenger({
-      libp2pConnections: [{ remotePeer: { toString: () => PEER_B } }],
-    });
-
-    await messenger.sendToPeer(PEER_B, '/dkg/test/1.0.0', new Uint8Array([0xff]));
+  it('delegates to router.send with peerId / protocol / data', async () => {
+    const { messenger, mocks } = makeMessenger();
 
-    expect(mocks.findAgentMock).not.toHaveBeenCalled();
-    expect(mocks.peerStoreMergeMock).not.toHaveBeenCalled();
-    expect(mocks.routerSendMock).toHaveBeenCalledOnce();
+    const out = await messenger.sendToPeer(
+      PEER_B,
+      '/dkg/test/1.0.0',
+      new Uint8Array([0xff]),
+    );
+
+    expect(mocks.routerSendMock).toHaveBeenCalledWith(
+      PEER_B,
+      '/dkg/test/1.0.0',
+      expect.any(Uint8Array),
+      undefined,
+    );
+    expect(out).toEqual(new Uint8Array([0x01, 0x02]));
   });
 
-  it('primes /p2p-circuit multiaddr when peer not connected and profile advertises a relay', async () => {
+  it('forwards timeoutMs to router.send', async () => {
     const { messenger, mocks } = makeMessenger();
 
-    await messenger.sendToPeer(PEER_B, '/dkg/test/1.0.0', new Uint8Array([0xff]));
-
-    expect(mocks.findAgentMock).toHaveBeenCalledWith(PEER_B);
-    expect(mocks.peerStoreMergeMock).toHaveBeenCalledOnce();
-    const mergeCall = mocks.peerStoreMergeMock.mock.calls[0];
-    const multiaddrs = (mergeCall[1] as { multiaddrs: Array<{ toString(): string }> }).multiaddrs;
-    expect(multiaddrs[0].toString()).toContain('/p2p-circuit/p2p/' + PEER_B);
-    expect(mocks.routerSendMock).toHaveBeenCalledOnce();
-  });
-
-  it('skips relay prime when discovery returns no relayAddress', async () => {
-    const { messenger, mocks } = makeMessenger({
-      findAgentMock: vi.fn(async () => ({ peerId: PEER_B, name: 'remote', agentUri: 'urn:agent:remote' })),
+    await messenger.sendToPeer(PEER_A, '/dkg/test/1.0.0', new Uint8Array([0xff]), {
+      timeoutMs: 5000,
     });
 
-    await messenger.sendToPeer(PEER_B, '/dkg/test/1.0.0', new Uint8Array([0xff]));
-
-    expect(mocks.findAgentMock).toHaveBeenCalled();
-    expect(mocks.peerStoreMergeMock).not.toHaveBeenCalled();
-    expect(mocks.routerSendMock).toHaveBeenCalledOnce();
+    expect(mocks.routerSendMock).toHaveBeenCalledWith(
+      PEER_A,
+      '/dkg/test/1.0.0',
+      expect.any(Uint8Array),
+      5000,
+    );
   });
 
-  it('tolerates discovery throwing — proceeds to router.send anyway', async () => {
-    const { messenger, mocks } = makeMessenger({
-      findAgentMock: vi.fn(async () => { throw new Error('discovery boom'); }),
+  it('propagates router.send errors to the caller', async () => {
+    const { messenger } = makeMessenger({
+      routerSendMock: vi.fn(async () => {
+        throw new Error('transport boom');
+      }),
     });
 
-    await messenger.sendToPeer(PEER_B, '/dkg/test/1.0.0', new Uint8Array([0xff]));
-
-    expect(mocks.peerStoreMergeMock).not.toHaveBeenCalled();
-    expect(mocks.routerSendMock).toHaveBeenCalledOnce();
+    await expect(
+      messenger.sendToPeer(PEER_B, '/dkg/test/1.0.0', new Uint8Array([0xff])),
+    ).rejects.toThrow('transport boom');
   });
 
-  it('regression: ensureCircuitRelayAddress fires BEFORE router.send (the Laptop B bug)', async () => {
-    const { messenger, callOrder } = makeMessenger();
-
-    await messenger.sendToPeer(PEER_B, '/dkg/test/1.0.0', new Uint8Array([0xff]));
-
-    // The bug fixed by this primitive: forwardJoinRequest used to call
-    // router.send directly without first priming the relay route, causing
-    // "no reachable curator" failures for NAT'd peers. The fix is structural
-    // — every send through Messenger primes first.
-    const mergeIdx = callOrder.indexOf('peerStore.merge');
-    const sendIdx = callOrder.indexOf('router.send');
-    expect(mergeIdx).toBeGreaterThanOrEqual(0);
-    expect(sendIdx).toBeGreaterThanOrEqual(0);
-    expect(mergeIdx).toBeLessThan(sendIdx);
-  });
-
-  it('forwards timeoutMs to router.send', async () => {
-    const { messenger, mocks } = makeMessenger({
-      libp2pConnections: [{ remotePeer: { toString: () => PEER_B } }],
-    });
-
-    await messenger.sendToPeer(PEER_A, '/dkg/test/1.0.0', new Uint8Array([0xff]), { timeoutMs: 5000 });
-
-    expect(mocks.routerSendMock).toHaveBeenCalledWith(PEER_A, '/dkg/test/1.0.0', expect.any(Uint8Array), 5000);
-  });
+  // Note: Messenger no longer holds a PeerResolver — the resolver is
+  // owned by ProtocolRouter (RFC 07 PR-3) so resolution happens once
+  // per send rather than twice. The structural property "resolver
+  // primes peerStore before dialProtocol" still holds; it's just
+  // verified at the router layer now (see protocol-router-resolver.test.ts).
 });