Uh oh!
There was an error while loading. Please reload this page.
fix(wallet)!: move the Sage-parity mTLS listener off Sage's RPC port 9257 - #262
Conversation
The node's wallet mTLS listener defaulted to 127.0.0.1:9257 — Sage's own RPC port. dig-node is an auto-starting OS service and Sage is a desktop app, so after a reboot dig-node took the socket first; a Sage client that then opened 9257 met our mutual-TLS listener, presented no cert we accept, and was rejected by OUR server with a handshake_failure that surfaces as an opaque client-side OpenSSL error. The parity that matters is of the method surface, not the port. The listener now defaults to 9776, beside the rest of the DIG cluster (9777 wallet HTTP mirror, 9778 control RPC, 9779 dig-app identity). 9257 is never bound, not even as a fallback: a conditional bind reinstates the same race. The bind stays best-effort, but is no longer silent — losing the port used to be invisible on both sides, so the contention could not be diagnosed from the machine it happened on. A failed bind now logs at WARN naming the port and is published on control.status, so `dign info` reports "wallet mTLS UNAVAILABLE (port 9776 held by another process ...)". BREAKING CHANGE: the wallet mTLS listener moved from port 9257 to 9776. A node-class client configured against 9257 must be pointed at 9776. Closes#260 Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
PASS — gate verdict (correctness, one thorough pass)
Head reviewed: 38533a2f96b019878322298d06e610ddac52a237 (resolved from remote). Read-only except one disclosed revert-proof mutation, described at the end.
1. Is 9257 genuinely and unconditionally vacated? YES
DEFAULT_MTLS_PORT is a plain const = 9776 in crates/dig-wallet/src/sage/transport.rs:56 with exactly ONE production call site — crates/dig-node-service/src/server.rs:2036, crate::wallet_mtls::spawn(DEFAULT_MTLS_PORT, ...). There is no env var, no config field, no fallback and no conditional branch that can resolve the mTLS bind back to 9257. bind_and_record binds the port it is handed and nothing else; on failure it returns None rather than retrying elsewhere, so there is no second-chance path either.
SAGE_RPC_PORT is used in exactly two places, both prohibition: the doc comment and RESERVED in the port test. It is never passed to a bind.
The four surviving 9257 literals in dig-node-core/src/seams/dig_peer/* are DHT test fixtures for REMOTE candidate addresses (CandidateAddr::direct("203.0.113.7", 9257)) — no bind, no listener, documentation-range addresses. Not a finding.
2. Is 9776 actually free? YES — re-derived independently
Re-ran a word-boundary sweep for \b9776\b across every submodule under modules/ (ripgrep, gitignore-respecting, so target/, node_modules/, dist*/, vendor/, .gitnexus/ excluded): ZERO hits. 9776 collides with none of the reserved sets — Chia (8444, 8555, 8447, 8446, 8559, 8560, 9256, 55400, 18444, 58444), Sage (9257), or DIG (9444, 9445, 9446, 9450, 9777, 9778, 9779). It sits in the gap directly below the DIG cluster, which is the right neighbourhood for it.
3. Is the silent failure gone? YES, and it is still non-fatal
Both halves hold:
- Still non-fatal.
wallet_mtls::spawnreturns early on a failed bind (let Some(listener) = bind_and_record(port) else { return; }); nothing propagates andserve_with_shutdowncontinues to the localhost bind. A busy port cannot stop the node. - No longer silent, on three surfaces. A WARN log naming the port and the OS error verbatim;
control.statuscarrieswallet_mtlsunconditionally (control.rs:944); anddign infoalways appends the clause, sincewallet_mtls_clauseis a non-optional argument to thecontrol.statussummary format.
The old state — unavailable with nothing anywhere saying so — is not reachable, because the clause is emitted on every control.status render rather than only when a listener exists. The catch-all arm answering "wallet mTLS state unknown" for an unrecognised or absent value is the right default: it declines to assert health rather than guessing listening. That is the correct fail-closed direction for a diagnostic. NotStarted is honestly modelled for paths that never reach serve, and reported as "not started" rather than conflated with a failure.
4. The ! and the bump — consistent
0.127.0 -> 0.128.0 is a MINOR, which under 0.x is the correct encoding of a breaking change (0.x minor IS the breaking increment; a major would be wrong here). dig-wallet 0.26.0 -> 0.27.0 matches, and the public DEFAULT_MTLS_PORT value change is genuinely breaking for a consumer. The version-increment gate is green and the manifests agree.
The commit carries a real BREAKING CHANGE: footer stating the migration in plain user language — "the wallet mTLS listener moved from port 9257 to 9776. A node-class client configured against 9257 must be pointed at 9776." git-cliff surfaces that verbatim in the changelog. That is the sentence that matters, and it is present.
5. Client sweep — confirmed, including the deliberate non-change
Nothing dials the old listener.
dig-appcrates/dig-app-core/src/loopback/guard.rs:12andSPEC.md:3737— both PROSE ONLY (a doc comment and a spec sentence). No dial, no constant, no config default. Verified by grepping the whole repo for 9257 across .rs/.ts/.tsx: those two lines are the only hits.dig-chrome-extensionSPEC.md:1694— prose only.ChainSourceSetting.tsx:166was correctly LEFT ALONE. The field issageUrl(confirmed atchainSourceSetting.test.tsx:41-43, which round-trips the entered value intosageUrlstorage). It points at the user OWN Sage instance — the very process this PR stops squatting. Changing that placeholder to 9776 would point user Sage configuration at OUR listener, and would be the actual regression. The reasoning holds; leaving it is correct.
Test quality — one revert proof reproduced, control verified
I reproduced the port proof on a committed tree, then restored:
- Changed
DEFAULT_MTLS_PORTback to 9257 and nothing else.default_mtls_port_is_not_sages_rpc_portFAILED attransport.rs:323with "DEFAULT_MTLS_PORT 9257 is bound by another wallet or Chia service". Restored the file from a backup copy;git status --porcelainempty; test green again. The test is load-bearing, not decorative. a_lost_bind_is_recorded_and_a_won_bind_names_its_portandinfo_summary_reports_a_lost_wallet_mtls_portboth pass on the restored tree.
The bind test control genuinely discriminates. It holds a real listener on a real ephemeral port, asserts the bind is refused and the state records Unavailable{port}, then DROPS the squatter and re-binds the SAME port, asserting Some(listener) and Listening(taken). A recorder that returned failure unconditionally fails the second half; one that returned success unconditionally fails the first. Reusing the same port across both halves is the detail that makes it a control rather than two independent assertions, and holding a real socket rather than inventing a busy port number means the bind path itself is exercised. The single-test grouping is correctly justified: the state is process-global, so two test fns would interleave and observe each other writes.
The dign info test likewise asserts the healthy case does NOT contain "UNAVAILABLE", so a clause hardcoded to unavailable would not pass.
Merge preconditions
All five required contexts asserted BY NAME from branch protection: Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage — all SUCCESS. unresolvedReviewThreads=0, mergeStateStatus=CLEAN. The script BLOCKED result is solely draft=true, which is correct and intended until the gate returned. Leaving DRAFT per dispatch.
Source overlap with #258 — none found
Confirmed disjoint at source level: this PR touches dig-node-service + dig-wallet only. Shared files are Cargo.toml, Cargo.lock, SPEC.md as stated. This merges first; #258 rebases after.
Non-gating observations (resolving these myself, they do not block merge)
- Cross-repo doc drift is real and currently unticketed.
dig-apploopback/guard.rs:12+SPEC.md:3737anddig-chrome-extensionSPEC.md:1694still describe 9257 as a dig-node port. Harmless at runtime, but a reimplementer reading either SPEC would target the wrong port. A ticket search in both repos returns nothing, so this needs a follow-up in each repo. NON-GATING — docs only, no dial, and this is a user-facing outage fix that must not wait on two other repos. - No operator override for the port.
DEFAULT_MTLS_PORTis a hardcoded const with no config surface, so an operator who happens to have 9776 occupied cannot relocate the listener. Acceptable today precisely because this PR makes that failure VISIBLE rather than silent, and adding config would widen scope on an ASAP fix. Worth a later ticket, not now.
The one plain sentence
No — dig-node cannot take Sage port by any route: the only mTLS bind site passes a single hardcoded DEFAULT_MTLS_PORT = 9776 const with no env, config, conditional or fallback path back to 9257, and a test fails the build if anyone moves it onto a reserved port.
Disclosure of shared-state mutation
I mutated one line in the lane own worktree (worktrees/dig-node-260), which the dispatch designated for rebuild: transport.rs:56 9776 -> 9257, ran the test, then restored from a file copy (cp, never git checkout). Post-restore git status --porcelain is EMPTY, no stash, no commits, HEAD unchanged at 38533a2f96b019878322298d06e610ddac52a237. target/ left in place as instructed.
Uh oh!
There was an error while loading. Please reload this page.
DO NOT MERGE — gate round in progress.
Closes#260. Parent: https://github.com/DIG-Network/dig_ecosystem/issues/260-tracking (orchestrator epic to be linked by the orchestrator).
What changed
The wallet mTLS listener no longer binds Sage's RPC port.
DEFAULT_MTLS_PORT9257(Sage's RPC)9776NON-FATAL, logged at WARN, invisible everywhere elsecontrol.status.wallet_mtls, rendered indign info9257is never bound — not as a fallback, not "if free". A conditional bind reinstates exactlythe race that caused the bug: dig-node is an auto-starting OS service and Sage is a desktop app,
so dig-node wins after a reboot and a Sage client that reaches our mutual-TLS listener is
rejected by OUR server with
handshake_failure, which surfaces as a client-side OpenSSL errorthree layers from the cause.
9776sits with the rest of the DIG cluster (9777wallet HTTP mirror,9778control RPC,9779dig-app identity) so the number reads as ours.Sage's port, confirmed from Sage — not from our own comments
xch-dev/sage,crates/sage-config/src/config.rs:69:Confirmed. (Sage ships RPC disabled by default; the reporting user had enabled it, which is
the normal case for anyone who uses it.)
What an operator now sees when the port is contested
and under
--json,result.wallet_mtls = {"state":"unavailable","port":9776,"reason":"...", "detail":"..."}. Healthy readswallet mTLS :9776; a node that never reached the serve pathreads
not started. The bind stays non-fatal — a busy port must not stop the node — it is justnever invisible again.
Blast radius checked
gitnexus was NOT used: no per-worktree index exists here and building one costs more than the
symbol is worth —
DEFAULT_MTLS_PORTis apub constwhose every use is a literal textualreference. Fallback per CLAUDE.md §2.0 bound (2): exhaustive ripgrep of the symbol, of the
literal
9257, and of everyTcpListener::bindin both crates, plus a word-boundary sweep ofevery ecosystem submodule except
dig-browser.DEFAULT_MTLS_PORT— 1 definition, 1 consumer (server.rsserve path). No other crate reads it.serve_dual(mtls_port, ..)— no callers anywhere in the workspace; its port comes from the caller.9257in this repo is either the new namedSAGE_RPC_PORTconstant, prose aboutthe bug, or a
CandidateAddrtest fixture indig-node-corethat is an arbitrary peer port.Risk: breaking but narrow — one listener, loopback only, with a live alternative surface.
Tests, each proven load-bearing by reverting only its fix
dig-walletdefault_mtls_port_is_not_sages_rpc_port9257dig-node-servicewallet_mtls::a_lost_bind_is_recorded_and_a_won_bind_names_its_portset_state(Unavailable)deleted (silent again)dig-node-servicecontrol_cli::info_summary_reports_a_lost_wallet_mtls_portThe bind test holds a real listener on a real ephemeral port for the failing half, then
releases it and re-binds the same port as the control — so a recorder that reported failure
unconditionally would fail the second half. The port test pins both directions:
SAGE_RPC_PORTis asserted to be 9257, and
DEFAULT_MTLS_PORTasserted outside the reserved set (Sage + the tenChia binds), so the guard cannot go vacuous by the constant drifting.
Full suites:
cargo test -p dig-wallet -p dig-node-service— all green (604 + 386 unit, plusevery integration target).
cargo clippy --workspace --all-targets -- -D warningsclean,cargo fmt --allno-op.Clients
No client in the ecosystem dials our old listener — swept every submodule except
dig-browser.Two documentation drifts exist in repos this PR does not touch and are reported to the
orchestrator rather than reached into:
dig-app:crates/dig-app-core/src/loopback/guard.rs:12andSPEC.md:3737describe9257asa dig-node port.
dig-chrome-extension:SPEC.md:1694says an MV3 extension cannot present "the9257mTLSclient cert" — the port is now 9776. Its
ChainSourceSettingplaceholderhttp://localhost:9257is correct and must stay: that field points the extension at the user's own Sage, which is
the very thing this PR stops us from squatting.
Version
0.127.0→0.128.0(workspace / the shipped binary) anddig-wallet0.26.0→0.27.0.Breaking under 0.x is a minor bump. Migration: one line — a node-class client configured against
127.0.0.1:9257moves to127.0.0.1:9776.SPEC.md(§18.1, §18.22, §7.12 transport list) now states the port, the prohibition, and that afailed bind MUST be reported.
DEVELOPMENT_LOG.mdrecords both halves.CHANGELOG.mdisgit-cliff-generated from the commit, which carries the user's language.