From 916056485f957343dd31c031511cd1050c456978 Mon Sep 17 00:00:00 2001
From: oliver <54379826+ok@users.noreply.github.com>
Date: Sun, 20 Sep 2026 18:16:12 +0200
Subject: [PATCH] [fix] Re-probe reachability while the relay is firewalled

dht-rpc probes once at bootstrap and skips its periodic re-check while
the public host is unchanged, so one lost probe round left a correctly
forwarded relay firewalled until restart. Re-run the probe while
firewalled, from 1 minute backing off to 15, and keep the firewalled
gauge current.
---
 .claude/lessons.md        |  7 +++
 OPERATIONS.md             |  9 ++++
 src/relay.js              | 14 ++++++
 src/reprobe.js            | 57 ++++++++++++++++++++++++
 test/unit/reprobe.test.js | 92 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 179 insertions(+)
 create mode 100644 src/reprobe.js
 create mode 100644 test/unit/reprobe.test.js

diff --git a/.claude/lessons.md b/.claude/lessons.md
index dc93659..1b17e7c 100644
--- a/.claude/lessons.md
+++ b/.claude/lessons.md
@@ -41,6 +41,13 @@ port.** The startup log therefore reads as though `--port` was ignored, which se
 debugging down the wrong path entirely. Log the *configured* UDP port explicitly at
 startup, and read a port mismatch as "firewalled" before reading it as "misconfigured".
 
+**dht-rpc probes reachability once, and never again while the public host is unchanged.**
+Its periodic re-check is guarded by `_lastHost === _nat.host` ("do not recheck the same
+network"), and the probe needs 3 of 5 ping-backs, so one unlucky round at startup left a
+correctly forwarded relay firewalled for hours. The tell: firewalled from the first seconds
+of a run, settings untouched, and a plain restart fixes it. `src/reprobe.js` re-runs
+`_updateNetworkState()` while firewalled; re-check that hook on any hyperdht bump.
+
 **On Docker Desktop, `firewalled: true` and a 503 from the probe are the expected
 result.** The VM's NAT means the relay is genuinely not publicly reachable, so a local
 Docker run smoke-tests boot and serve — never reachability. Don't debug it as a failure.
diff --git a/OPERATIONS.md b/OPERATIONS.md
index d085247..4e3a0b7 100644
--- a/OPERATIONS.md
+++ b/OPERATIONS.md
@@ -79,6 +79,15 @@ curl -s localhost:9200/readyz
 `firewalled: true` → clients cannot reach you. Check, in order: the host firewall,
 the cloud security group, and whether the UDP port is actually forwarded.
 
+The verdict can also simply be wrong for a while. HyperDHT probes reachability once
+at startup — it needs 3 of 5 remote nodes to ping the port back — and on its own
+never probes again while the public address is unchanged, so one lost round used to
+leave a correctly forwarded relay firewalled until the next restart. The relay now
+re-runs the probe itself while firewalled: after 1 minute, then backing off to every
+15. A relay that flips to reachable minutes after a restart, with
+`reachability re-probe passed` in the log, was that case. It does not apply under
+`MIRALL_RELAY_ASSUME_REACHABLE`, where nothing was probed to begin with.
+
 Two states the page separates that `/readyz` does not:
 
 - **Assumed reachable.** `MIRALL_RELAY_ASSUME_REACHABLE` makes `firewalled` read
diff --git a/src/relay.js b/src/relay.js
index ddac8f7..069f4cc 100644
--- a/src/relay.js
+++ b/src/relay.js
@@ -22,6 +22,7 @@ import Relay from 'blind-relay'
 import b4a from 'b4a'
 import { bootstrapNodes } from './config.js'
 import { resolveSeed, keyPairFromSeed, publicKeyZ32 } from './keys.js'
+import { startReprobe } from './reprobe.js'
 
 // udx's socket.address() returns { host, family, port } — NOT Node's dgram shape
 // with `address`. Normalised here so the status snapshot has one contract and a
@@ -47,6 +48,7 @@ export class RelayNode {
     this.dht = null
     this.server = null // hyperdht server (our listening identity)
     this.relay = null // the single blind-relay.Server
+    this.reprobe = null
     this.keyPair = null
     this.ready = false
     this.closing = false
@@ -158,6 +160,17 @@ export class RelayNode {
       )
     }
 
+    // Asserted reachability was never probed, so there is nothing to re-run.
+    if (!cfg.assumeReachable) {
+      this.reprobe = startReprobe(this.dht, {
+        onResult: (firewalled) => {
+          this.metrics?.m.dhtFirewalled.set(firewalled ? 1 : 0)
+          if (!firewalled) logger?.info({ publicKey: this.publicKeyZ32 }, 'reachability re-probe passed — the relay is now reachable')
+        },
+        onUnsupported: () => logger?.warn('this dht-rpc has no re-probe hook — a firewalled verdict will stand until restart')
+      })
+    }
+
     logger?.info({
       publicKey: this.publicKeyZ32,
       firewalled: this.firewalled,
@@ -266,6 +279,7 @@ export class RelayNode {
     this.ready = false
     this.metrics?.m.ready.set(0)
 
+    this.reprobe?.stop()
     this.meter?.stop()
     this.firewall?.stop()
 
diff --git a/src/reprobe.js b/src/reprobe.js
new file mode 100644
index 0000000..bc05582
--- /dev/null
+++ b/src/reprobe.js
@@ -0,0 +1,57 @@
+// Re-runs dht-rpc's reachability probe while the node is firewalled.
+//
+// WHY IT EXISTS: dht-rpc probes once at bootstrap, and its periodic re-check is
+// skipped while the observed public host is unchanged ("do not recheck the same
+// network"). The probe needs 3 of 5 remote nodes to ping back within a timeout,
+// so one unlucky round — common in the first seconds after a restart — left a
+// correctly forwarded relay firewalled until the next restart, hours later.
+//
+// _updateNetworkState is dht-rpc internals: the same call its own tick makes,
+// minus the same-host guard. Everything about it is optional, so a dht-rpc that
+// drops it costs the self-heal and nothing else.
+const INTERVAL_MS = 60_000
+const MAX_INTERVAL_MS = 15 * 60_000
+
+export function startReprobe (dht, {
+  onResult = null,
+  onUnsupported = null,
+  intervalMs = INTERVAL_MS,
+  maxIntervalMs = MAX_INTERVAL_MS,
+  setTimer = setTimeout,
+  clearTimer = clearTimeout
+} = {}) {
+  let timer = null
+  let stopped = false
+  let wait = intervalMs
+
+  function schedule () {
+    if (stopped || !dht.firewalled) return
+    timer = setTimer(probe, wait)
+    timer.unref?.()
+    wait = Math.min(wait * 2, maxIntervalMs)
+  }
+
+  async function probe () {
+    timer = null
+    try {
+      await dht._updateNetworkState()
+    } catch { /* a failed probe is just another firewalled result */ }
+    if (stopped) return
+    onResult?.(!!dht.firewalled)
+    schedule()
+  }
+
+  if (typeof dht._updateNetworkState !== 'function') {
+    if (dht.firewalled) onUnsupported?.()
+  } else {
+    schedule()
+  }
+
+  return {
+    stop () {
+      stopped = true
+      if (timer) clearTimer(timer)
+      timer = null
+    }
+  }
+}
diff --git a/test/unit/reprobe.test.js b/test/unit/reprobe.test.js
new file mode 100644
index 0000000..d209095
--- /dev/null
+++ b/test/unit/reprobe.test.js
@@ -0,0 +1,92 @@
+// The reachability re-probe. Driven by a stand-in DHT and a hand-cranked timer,
+// so every branch runs without a network or a clock.
+import test from 'node:test'
+import assert from 'node:assert/strict'
+import { startReprobe } from '../../src/reprobe.js'
+
+function fakeTimers () {
+  const pending = []
+  return {
+    pending,
+    setTimer: (fn, ms) => { const t = { fn, ms, unref () {} }; pending.push(t); return t },
+    clearTimer: (t) => { const i = pending.indexOf(t); if (i !== -1) pending.splice(i, 1) },
+    async fire () { const t = pending.shift(); await t.fn(); return t.ms }
+  }
+}
+
+function fakeDht (passesOnProbe) {
+  const dht = {
+    firewalled: true,
+    probes: 0,
+    async _updateNetworkState () {
+      dht.probes++
+      if (dht.probes === passesOnProbe) dht.firewalled = false
+    }
+  }
+  return dht
+}
+
+test('a reachable node is never re-probed', () => {
+  const timers = fakeTimers()
+  const dht = { firewalled: false, _updateNetworkState: () => assert.fail('must not probe') }
+  startReprobe(dht, timers)
+  assert.equal(timers.pending.length, 0)
+})
+
+test('a firewalled node is re-probed until a probe passes, then left alone', async () => {
+  const timers = fakeTimers()
+  const dht = fakeDht(3)
+  const results = []
+  startReprobe(dht, { ...timers, onResult: (firewalled) => results.push(firewalled) })
+
+  await timers.fire()
+  await timers.fire()
+  assert.equal(dht.firewalled, true)
+  await timers.fire()
+  assert.equal(dht.firewalled, false)
+  assert.equal(dht.probes, 3)
+  assert.equal(timers.pending.length, 0, 'nothing is scheduled once it is reachable')
+  assert.deepEqual(results, [true, true, false])
+})
+
+test('the wait doubles up to a ceiling, so a truly closed port is not hammered', async () => {
+  const timers = fakeTimers()
+  startReprobe(fakeDht(Infinity), { ...timers, intervalMs: 1000, maxIntervalMs: 5000 })
+  const waits = []
+  for (let i = 0; i < 5; i++) waits.push(await timers.fire())
+  assert.deepEqual(waits, [1000, 2000, 4000, 5000, 5000])
+})
+
+test('a probe that throws does not end the retries', async () => {
+  const timers = fakeTimers()
+  const dht = { firewalled: true, _updateNetworkState: async () => { throw new Error('socket closed') } }
+  startReprobe(dht, timers)
+  await timers.fire()
+  assert.equal(timers.pending.length, 1)
+})
+
+test('stop cancels the pending probe, and one in flight schedules nothing after it', async () => {
+  const timers = fakeTimers()
+  const reprobe = startReprobe(fakeDht(Infinity), timers)
+  reprobe.stop()
+  assert.equal(timers.pending.length, 0)
+
+  const late = fakeTimers()
+  const dht = fakeDht(Infinity)
+  const running = startReprobe(dht, late)
+  const probe = late.fire()
+  running.stop()
+  await probe
+  assert.equal(late.pending.length, 0)
+})
+
+test('a dht-rpc without the private hook degrades to doing nothing', () => {
+  // The hook is dht-rpc internals. If a bump removes it the relay must still
+  // start; it just loses the self-heal, and says so once.
+  const timers = fakeTimers()
+  let warned = 0
+  const reprobe = startReprobe({ firewalled: true }, { ...timers, onUnsupported: () => warned++ })
+  assert.equal(timers.pending.length, 0)
+  assert.equal(warned, 1)
+  reprobe.stop()
+})