fix(nip66): address relay probe review feedback

Signed-off-by: ABHAY PANDEY <pandeyabhay967@gmail.com>

Author: Ferryx349

src/utils/relay-probe/dns-probe.ts

@@ -1,35 +1,46 @@
 import { DnsRecord, ProbeTarget } from './types'
 
 export interface DnsResolver {
-  resolve4(hostname: string): Promise<string[]>
-  resolve6(hostname: string): Promise<string[]>
-  resolveCname(hostname: string): Promise<string[]>
+  resolve4(hostname: string): Promise<DnsRecord[]>
+  resolve6(hostname: string): Promise<DnsRecord[]>
+  resolveCname(hostname: string): Promise<DnsRecord[]>
+}
+
+type RecordWithTtl = {
+  address: string
+  ttl: number
 }
 
 export const createNodeDnsResolver = (): DnsResolver => {
   // Lazy import keeps unit tests on stubbed resolvers without touching the network.
   const dns = require('dns').promises as {
-    resolve4: (hostname: string) => Promise<string[]>
-    resolve6: (hostname: string) => Promise<string[]>
+    resolve4: (hostname: string, options: { ttl: true }) => Promise<RecordWithTtl[]>
+    resolve6: (hostname: string, options: { ttl: true }) => Promise<RecordWithTtl[]>
     resolveCname: (hostname: string) => Promise<string[]>
   }
 
   return {
-    resolve4: (hostname) => dns.resolve4(hostname),
-    resolve6: (hostname) => dns.resolve6(hostname),
-    resolveCname: (hostname) => dns.resolveCname(hostname),
+    resolve4: async (hostname) => {
+      const entries = await dns.resolve4(hostname, { ttl: true })
+      return entries.map(({ address, ttl }) => ({ type: 'A' as const, value: address, ttl }))
+    },
+    resolve6: async (hostname) => {
+      const entries = await dns.resolve6(hostname, { ttl: true })
+      return entries.map(({ address, ttl }) => ({ type: 'AAAA' as const, value: address, ttl }))
+    },
+    resolveCname: async (hostname) => {
+      const values = await dns.resolveCname(hostname)
+      return values.map((value) => ({ type: 'CNAME' as const, value }))
+    },
   }
 }
 
 const collectRecords = async (resolver: DnsResolver, target: ProbeTarget): Promise<DnsRecord[]> => {
   const records: DnsRecord[] = []
 
-  const append = async (type: DnsRecord['type'], lookup: () => Promise<string[]>) => {
+  const append = async (lookup: () => Promise<DnsRecord[]>) => {
     try {
-      const values = await lookup()
-      for (const value of values) {
-        records.push({ type, value })
-      }
+      records.push(...(await lookup()))
     } catch (error: unknown) {
       const code = (error as NodeJS.ErrnoException)?.code
       if (code === 'ENOTFOUND' || code === 'ENODATA') {
@@ -40,9 +51,9 @@ const collectRecords = async (resolver: DnsResolver, target: ProbeTarget): Promi
     }
   }
 
-  await append('CNAME', () => resolver.resolveCname(target.hostname))
-  await append('A', () => resolver.resolve4(target.hostname))
-  await append('AAAA', () => resolver.resolve6(target.hostname))
+  await append(() => resolver.resolveCname(target.hostname))
+  await append(() => resolver.resolve4(target.hostname))
+  await append(() => resolver.resolve6(target.hostname))
 
   return records
 }

src/utils/relay-probe/index.ts

@@ -5,6 +5,7 @@ export {
   runProbe,
 } from './run-probe'
 export type { ProbeClients } from './run-probe'
+export { resolveDnsRecords } from './dns-probe'
 export { isNip11FetchTargetSafe } from './nip11-probe'
 export { detectNetworkType, shouldSkipDnsProbe } from './target'
 export {

src/utils/relay-probe/nip11-probe.ts

@@ -9,8 +9,8 @@ const MAX_REDIRECTS = 1
 
 const nip11DocumentSchema = z
   .object({
-    name: z.string(),
-    pubkey: pubkeySchema,
+    name: z.string().optional(),
+    pubkey: pubkeySchema.optional(),
   })
   .passthrough()
 

src/utils/relay-probe/run-probe.ts

@@ -121,8 +121,20 @@ export const runProbe = async (
   const dnsCacheTtlSeconds = options.dnsCacheTtlSeconds ?? DEFAULT_DNS_CACHE_TTL_SECONDS
   const skipDnsCache = options.skipDnsCache ?? false
 
+  const dnsStartedAt = Date.now()
+  const dnsPromise = runDnsProbe(clients, target, { dnsCacheTtlSeconds, skipDnsCache })
+  const dnsTimeout = new Promise<ProbeCheckResult<DnsResult>>((resolve) => {
+    setTimeout(() => {
+      resolve({
+        status: 'error',
+        durationMs: Date.now() - dnsStartedAt,
+        error: `DNS probe timed out after ${timeouts.dnsMs}ms`,
+      })
+    }, timeouts.dnsMs).unref()
+  })
+
   const [dns, tls, wsRtt, nip11] = await Promise.all([
-    runDnsProbe(clients, target, { dnsCacheTtlSeconds, skipDnsCache }),
+    Promise.race([dnsPromise, dnsTimeout]),
     runTimedProbe(() => probeTls(clients.tls, target, timeouts.tlsMs)),
     runTimedProbe(() => probeWebSocketRtt(clients.ws, target, timeouts.wsRttMs)),
     runTimedProbe(() => probeNip11(clients.nip11, target.nip11Url, timeouts.nip11Ms)),

src/utils/relay-probe/ws-rtt-probe.ts

@@ -11,7 +11,7 @@ export const createNodeWebSocketConnector = (): WebSocketConnector => {
     measureOpenRtt: (address, timeoutMs) =>
       new Promise<number>((resolve, reject) => {
         const startedAt = Date.now()
-        const socket = new WebSocket(address, { timeout: timeoutMs })
+        const socket = new WebSocket(address, { handshakeTimeout: timeoutMs })
         let settled = false
 
         const finish = (error?: Error, rttOpenMs?: number) => {

test/unit/utils/relay-probe-dns.spec.ts

@@ -0,0 +1,23 @@
+import { expect } from 'chai'
+
+import { parseProbeTarget, resolveDnsRecords } from '../../../src/utils/relay-probe/index'
+
+describe('relay-probe dns-probe', () => {
+  it('collects A, AAAA, and CNAME records with TTL when available', async () => {
+    const target = parseProbeTarget('wss://relay.example.com')
+    const records = await resolveDnsRecords(
+      {
+        resolveCname: async () => [{ type: 'CNAME', value: 'cdn.example.com' }],
+        resolve4: async () => [{ type: 'A', value: '93.184.216.34', ttl: 300 }],
+        resolve6: async () => [{ type: 'AAAA', value: '2606:2800:220:1:248:1893:25c8:1946', ttl: 120 }],
+      },
+      target,
+    )
+
+    expect(records).to.deep.equal([
+      { type: 'CNAME', value: 'cdn.example.com' },
+      { type: 'A', value: '93.184.216.34', ttl: 300 },
+      { type: 'AAAA', value: '2606:2800:220:1:248:1893:25c8:1946', ttl: 120 },
+    ])
+  })
+})

test/unit/utils/relay-probe-run.spec.ts

@@ -0,0 +1,143 @@
+import { expect } from 'chai'
+
+import { clearDnsProbeCache, runProbe } from '../../../src/utils/relay-probe/index'
+
+const PUBKEY = '22e804d26ed16b68db5259e78449e96dab5d464c8f470bda3eb1a70467f2c793'
+
+const makeClients = (overrides: Record<string, unknown> = {}) => ({
+  dns: {
+    resolve4: async () => [{ type: 'A' as const, value: '93.184.216.34', ttl: 300 }],
+    resolve6: async () => [],
+    resolveCname: async () => {
+      throw Object.assign(new Error('ENODATA'), { code: 'ENODATA' })
+    },
+  },
+  tls: {
+    connect: async () => ({
+      valid: true,
+      issuer: 'Example CA',
+      subject: 'relay.example.com',
+      expiresAt: new Date('2027-01-01T00:00:00.000Z'),
+      daysUntilExpiry: 365,
+    }),
+  },
+  ws: { measureOpenRtt: async () => 42 },
+  nip11: {
+    fetch: async () => ({
+      statusCode: 200,
+      name: 'relay.example.com',
+      pubkey: PUBKEY,
+    }),
+  },
+  ...overrides,
+})
+
+describe('runProbe', () => {
+  afterEach(() => {
+    clearDnsProbeCache()
+  })
+
+  it('returns structured per-check results for a clearnet relay', async () => {
+    const result = await runProbe('wss://relay.example.com', {}, makeClients())
+
+    expect(result.target.hostname).to.equal('relay.example.com')
+    expect(result.dns.status).to.equal('ok')
+    expect(result.dns.data?.records).to.deep.include({ type: 'A', value: '93.184.216.34', ttl: 300 })
+    expect(result.tls.status).to.equal('ok')
+    expect(result.wsRtt.status).to.equal('ok')
+    expect(result.nip11.status).to.equal('ok')
+  })
+
+  it('skips DNS for onion destinations', async () => {
+    const result = await runProbe(
+      'wss://abc123def456.onion',
+      {},
+      makeClients({
+        dns: {
+          resolve4: async () => {
+            throw new Error('DNS should not be called for onion hosts')
+          },
+          resolve6: async () => [],
+          resolveCname: async () => [],
+        },
+      }),
+    )
+
+    expect(result.target.networkType).to.equal('tor')
+    expect(result.dns.status).to.equal('skipped')
+  })
+
+  it('uses the DNS cache on repeated probes', async () => {
+    let resolveCount = 0
+    const clients = makeClients({
+      dns: {
+        resolve4: async () => {
+          resolveCount += 1
+          return [{ type: 'A' as const, value: '93.184.216.34', ttl: 300 }]
+        },
+        resolve6: async () => [],
+        resolveCname: async () => {
+          throw Object.assign(new Error('ENODATA'), { code: 'ENODATA' })
+        },
+      },
+    })
+
+    await runProbe('wss://relay.example.com', {}, clients)
+    const second = await runProbe('wss://relay.example.com', {}, clients)
+
+    expect(resolveCount).to.equal(1)
+    expect(second.dns.data?.fromCache).to.equal(true)
+  })
+
+  it('surfaces probe errors without failing the full run', async () => {
+    const result = await runProbe(
+      'wss://relay.example.com',
+      {},
+      makeClients({
+        tls: {
+          connect: async () => {
+            throw new Error('certificate expired')
+          },
+        },
+      }),
+    )
+
+    expect(result.tls.status).to.equal('error')
+    expect(result.tls.error).to.include('certificate expired')
+  })
+
+  it('times out stalled DNS probes without blocking other checks', async () => {
+    const result = await runProbe(
+      'wss://relay.example.com',
+      { timeouts: { dnsMs: 50, tlsMs: 50, wsRttMs: 50, nip11Ms: 50 } },
+      makeClients({
+        dns: {
+          resolve4: () => new Promise(() => undefined),
+          resolve6: async () => [],
+          resolveCname: async () => {
+            throw Object.assign(new Error('ENODATA'), { code: 'ENODATA' })
+          },
+        },
+      }),
+    )
+
+    expect(result.dns.status).to.equal('error')
+    expect(result.dns.error).to.include('DNS probe timed out')
+    expect(result.tls.status).to.equal('ok')
+  }).timeout(5000)
+
+  it('accepts NIP-11 documents without name or pubkey fields', async () => {
+    const result = await runProbe(
+      'wss://relay.example.com',
+      {},
+      makeClients({
+        nip11: {
+          fetch: async () => ({ statusCode: 200 }),
+        },
+      }),
+    )
+
+    expect(result.nip11.status).to.equal('ok')
+    expect(result.nip11.data?.name).to.equal(undefined)
+  })
+})

test/unit/utils/relay-probe-target.spec.ts

@@ -0,0 +1,38 @@
+import { expect } from 'chai'
+
+import {
+  detectNetworkType,
+  isNip11FetchTargetSafe,
+  parseProbeTarget,
+  shouldSkipDnsProbe,
+} from '../../../src/utils/relay-probe/index'
+
+describe('relay-probe target parsing', () => {
+  it('parses wss relay URLs into HTTP and WebSocket probe targets', () => {
+    const target = parseProbeTarget('wss://relay.example.com/nostream')
+
+    expect(target.hostname).to.equal('relay.example.com')
+    expect(target.networkType).to.equal('clearnet')
+    expect(target.httpOrigin).to.equal('https://relay.example.com')
+    expect(target.nip11Url).to.equal('https://relay.example.com/nostream/')
+    expect(target.wsUrl).to.equal('wss://relay.example.com/nostream')
+  })
+
+  it('detects tor and i2p destinations', () => {
+    expect(detectNetworkType('abc123.onion')).to.equal('tor')
+    expect(detectNetworkType('relay.i2p')).to.equal('i2p')
+    expect(shouldSkipDnsProbe('tor')).to.equal(true)
+  })
+
+  it('rejects non-websocket relay URLs', () => {
+    expect(() => parseProbeTarget('https://relay.example.com')).to.throw('ws:// or wss://')
+  })
+})
+
+describe('relay-probe safety helpers', () => {
+  it('rejects unsafe NIP-11 fetch targets', () => {
+    expect(isNip11FetchTargetSafe('https://relay.example.com/')).to.equal(true)
+    expect(isNip11FetchTargetSafe('http://127.0.0.1/')).to.equal(false)
+    expect(isNip11FetchTargetSafe('ftp://relay.example.com/')).to.equal(false)
+  })
+})