From 3ad3f8e73e3927c5f2c503a8c79655c690dd0d5e Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sat, 5 Sep 2026 08:33:03 -0700 Subject: [PATCH 1/7] chore: open lane for reflexive-discovery hardening batch Four items: cross-family STUN reject, standing-fallback doc fix, stun. preference, control.config.setMirrorAdvertiseUrls. See dispatch for full spec. From 840d2bdc527b5984f167f10eca802c5228d446f5 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sat, 5 Sep 2026 09:18:29 -0700 Subject: [PATCH 2/7] feat(peer)!: reflexive-discovery hardening batch Four items in one PR (root Cargo.toml bump dirties every sibling PR in this repo, so batch rather than split): 1. Reject a cross-family STUN answer and fall through the discovery ladder. The measured dig-relay defect (fixed server-side in 0.19.7) answered an IPv4 caller with the load balancer's own IPv6 address -- a well-formed Binding transaction naming something other than the querying socket. `StunPlan::discover_reflexive` now discards a cross-family answer and keeps walking, exactly as a non-answering server falls through. 2. Correct the "INTERIM" framing on the public-STUN fallback tier: it is a standing, deliberate last resort (a single STUN source, even the relay, is a single point of trust), not scaffolding to delete once relay STUN works. dig_ecosystem#3198 stays open (agreement between two sources). 3. Prefer a dedicated `stun.` DNS name over the bare relay host, derived from whatever relay endpoint the operator configured (never hardcoded -- dig-relay is GPL-2.0 and self-hosted). `stun_server_from_relay` delegates to `stun_servers_from_relay` (already true before this change), so the two derivations can never disagree about which host to try first. 4. Serve `control.config.setMirrorAdvertiseUrls` (dig-node-control-interface 0.33.0) plus the additive `ConfigResult.mirror_advertise` field. The override is persisted to config.json and consulted by `advertised_urls_from_env`'s new precedence (env > persisted > none), so `requires_restart: true` is a genuine promise: nothing can rewrite a running process's environment, but the operator half of the mirror task is captured once at bring-up and a restart re-reads it. Also: bump dig-ip 0.1.2->0.1.3 and dig-nat 0.21.0->0.21.1 (touched crates' dig-*/chia-* deps to latest, CLAUDE.md #2.4b). SPEC.md #19.2 updated for items 1-3. Co-Authored-By: Claude --- Cargo.lock | 35 +- SPEC.md | 28 +- crates/dig-node-core/Cargo.toml | 2 +- .../dig-node-core/src/seams/dig_peer/net.rs | 307 +++++++++++++++--- crates/dig-node-service/Cargo.toml | 2 +- crates/dig-node-service/src/control.rs | 239 +++++++++++++- .../dig-node-service/src/mirror/advertise.rs | 85 ++++- crates/dig-wallet/Cargo.toml | 2 +- 8 files changed, 621 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4a404a7f..47456ced 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1948,7 +1948,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] @@ -2772,9 +2772,9 @@ dependencies = [ [[package]] name = "dig-ip" -version = "0.1.2" +version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c9d0dd76d4fa9a133bd25ff24e10173d7769a8ccd32d79cbc44715d5f78de0a" +checksum = "e8154534a970d8c626c3510b9abb402fec67e84583c68b4a685c62397aacbe91" dependencies = [ "futures", "tokio", @@ -2920,9 +2920,9 @@ dependencies = [ [[package]] name = "dig-nat" -version = "0.21.0" +version = "0.21.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f32d4a8ddbb282a8326dfa4a23d1af37f3d6fdd9d66bd67cba6b7e46658cb915" +checksum = "0a3fc5d85d0009b2e7d1e07b7e17bb174db17fb12559be3cda8383dee23cefc8" dependencies = [ "arc-swap", "async-trait", @@ -2964,14 +2964,15 @@ dependencies = [ [[package]] name = "dig-node-control-interface" -version = "0.31.0" +version = "0.33.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "714e4022a13278a2c329dcc7b16a24cbf800502027579b80838b1eafa4caafde" +checksum = "810b4320c97b98f54f455673e09cf04a459bfd6b7b172dc95d079fe36d9f96d1" dependencies = [ "async-trait", "semver", "serde", "serde_json", + "url", ] [[package]] @@ -3885,7 +3886,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -4521,7 +4522,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.6.5", + "socket2 0.5.10", "system-configuration", "tokio", "tower-service", @@ -4772,7 +4773,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -5765,7 +5766,7 @@ dependencies = [ "quinn-udp", "rustc-hash 2.1.3", "rustls", - "socket2 0.6.5", + "socket2 0.5.10", "thiserror 2.0.20", "tokio", "tracing", @@ -5803,9 +5804,9 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2 0.6.5", + "socket2 0.5.10", "tracing", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -6542,7 +6543,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6555,7 +6556,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -7267,7 +7268,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix 1.1.4", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -8401,7 +8402,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] diff --git a/SPEC.md b/SPEC.md index de236ca0..d73ec780 100644 --- a/SPEC.md +++ b/SPEC.md @@ -6998,12 +6998,28 @@ per-method timeout so a dial never hangs. ### 19.2. STUN reflexive-address discovery -The node discovers its server-reflexive (public) transport address via STUN (RFC 5389) against the STUN -server co-located with the relay (`:3478`, derived from `DIG_RELAY_URL`). The STUN endpoints -are resolved across **both address families** (every A + AAAA record) and the Binding transaction is run -**IPv6-first with IPv4 fallback** (§5.2): the IPv6 STUN server is attempted first and IPv4 is used only -when the IPv6 server is absent/unreachable — the reflexive address is never nulled merely because IPv6 -failed. +The node discovers its server-reflexive (public) transport address via STUN (RFC 5389), walking a +precedence-ordered plan (`StunPlan`, dig_ecosystem#3198) of up to three tiers, each already IPv6-first +within itself: an **operator override** (`DIG_STUN_SERVER`, when set — never silently bypassed in favor +of a default), the **DIG relay's co-located STUN endpoint** (the intended steady state), and a **public +third-party fallback** (Google + Cloudflare) reached only when the relay tier answers nothing. The relay +reclaims the role automatically the moment it answers again — no code change, no redeploy. Within the +relay tier, the node derives the endpoint from `DIG_RELAY_URL`'s host and PREFERS a dedicated +`stun.` DNS name over the bare relay host when it resolves; an unresolvable `stun.` name is +skipped silently, never an error, so a relay operator who has not adopted the convention keeps working +exactly as before, and one who has activates the dedicated endpoint by DNS alone. Every candidate +endpoint — whichever tier or host it came from — is resolved across **both address families** (every A ++ AAAA record) and the Binding transaction is run **IPv6-first with IPv4 fallback** (§5.2): the IPv6 +endpoint is attempted first and IPv4 is used only when the IPv6 endpoint is absent/unreachable — the +reflexive address is never nulled merely because IPv6 failed. + +A STUN answer whose address family differs from the family of the server that answered it is +**discarded, not returned** — the walk falls through to the next server exactly as a non-answering one +does. This guards against a server that completes an otherwise well-formed Binding transaction (correct +magic cookie, correctly echoed transaction id) but names an address belonging to a different network path +than the one queried — measured: a dual-stack load balancer answering an IPv4 caller with its own IPv6 +address. Such an answer describes something other than this node's queried transport and MUST NOT be +believed just because the transaction otherwise succeeded. The reflexive query is run from a UDP socket bound to the node's **ACTUAL listen port** (the peer-RPC port peers dial), not a throwaway ephemeral socket. The advertised candidate is therefore diff --git a/crates/dig-node-core/Cargo.toml b/crates/dig-node-core/Cargo.toml index 4f739fb7..551f54c0 100644 --- a/crates/dig-node-core/Cargo.toml +++ b/crates/dig-node-core/Cargo.toml @@ -144,7 +144,7 @@ chia-traits = "0.36.1" # path keys + orders candidate addresses via `dig_ip::Family` and aggregates them source-tagged + # de-duplicated via `dig_ip::PeerCandidates` — never a hand-rolled family sort (epic #1020, #1032). # A leaf crate (no transitive DIG deps), so the crates.io release is used directly. -dig-ip = "0.1.2" +dig-ip = "0.1.3" # The canonical dig-app ⇄ dig-node IPC session/signing contract (engine role-half + wire types + # domain builders + frame bounds). SSOT shared with dig-app so the two halves can never drift (#1080). # Pinned EXACTLY: 0.3.0 is the first release on this line and the caret range would silently accept diff --git a/crates/dig-node-core/src/seams/dig_peer/net.rs b/crates/dig-node-core/src/seams/dig_peer/net.rs index a3d77d84..b0ad10a6 100644 --- a/crates/dig-node-core/src/seams/dig_peer/net.rs +++ b/crates/dig-node-core/src/seams/dig_peer/net.rs @@ -297,7 +297,9 @@ pub fn advertise_loopback_from_env() -> bool { // -- Shared NAT-traversal config (#385) -------------------------------------------------------------- /// The RFC-5389 STUN port the DIG relay co-locates with its relay host (`relay.dig.net:3478`). A node -/// derives its STUN server from the relay endpoint (`:STUN_PORT`) — dig-nat L7 spec §3. +/// derives its STUN server from the relay endpoint, preferring a dedicated `stun.` DNS +/// name over the bare relay host when it resolves (see [`stun_servers_from_relay`]) — dig-nat L7 +/// spec §3. pub const STUN_PORT: u16 = 3478; /// The shared [`dig_nat::NatConfig`] for EVERY node peer dial (DHT lookups, multi-source range @@ -437,35 +439,72 @@ pub fn parse_relay_host(endpoint: &str) -> Option { parse_relay_endpoint(endpoint).map(|e| e.host) } -/// Resolve the DIG STUN servers (`:STUN_PORT`) from the relay endpoint URL across BOTH -/// address families — every A + AAAA record — ordered IPv6-first (§5.2, `dig_ip::Family`). The caller -/// MUST NOT pre-collapse to one family: the reflexive discovery below races IPv6 first and falls back -/// to IPv4, so it needs a STUN endpoint per family (#1393). Best-effort blocking DNS resolution; -/// returns an empty vec when the host can't be parsed/resolved. Call off the async runtime (e.g. via -/// `spawn_blocking`). +/// Resolve the DIG STUN servers from the relay endpoint URL across BOTH address families — every +/// A + AAAA record — ordered IPv6-first (§5.2, `dig_ip::Family`), preferring a DEDICATED +/// `stun.` DNS name over the bare relay host when it resolves (see +/// [`prefer_dedicated_stun_host`]). The caller MUST NOT pre-collapse to one family: the reflexive +/// discovery below races IPv6 first and falls back to IPv4, so it needs a STUN endpoint per family +/// (#1393). Best-effort blocking DNS resolution throughout; returns an empty vec when the relay +/// endpoint itself can't be parsed. Call off the async runtime (e.g. via `spawn_blocking`). +/// +/// # Why `stun.`, not a hardcoded `stun.relay.dig.net` +/// +/// dig-relay is GPL-2.0 and self-hosting is supported (SYSTEM.md); hardcoding DIG's own hostname +/// would silently break every self-hosted relay's node peers. Deriving the CONVENTION from +/// whatever relay endpoint the operator already configured means a self-hoster who adopts the +/// `stun.` naming activates the dedicated endpoint with a DNS change alone — no dig-node release, +/// no config flag, and every node that dials through THIS function picks it up identically: +/// [`stun_server_from_relay`] (the traversal-ladder / DHT-transport single-endpoint feed) delegates +/// here rather than deriving its own list, so the two can never talk to different hosts. pub fn stun_servers_from_relay(relay_endpoint: &str) -> Vec { - use std::net::ToSocketAddrs; let Some(host) = parse_relay_host(relay_endpoint) else { return Vec::new(); }; - let mut addrs: Vec = match (host.as_str(), STUN_PORT).to_socket_addrs() { - Ok(iter) => iter.collect(), - Err(_) => return Vec::new(), - }; + let dedicated = resolve_host_both_families(&format!("stun.{host}")); + let bare = resolve_host_both_families(&host); + prefer_dedicated_stun_host(dedicated, bare) +} + +/// Every `A` + `AAAA` record for `host` at [`STUN_PORT`], in whatever order the resolver returns +/// them. Empty (never an error) when `host` doesn't resolve — an absent DNS name is a normal, +/// silently-skipped outcome for every STUN tier in this module, not a fault. Best-effort blocking +/// DNS; call off the async runtime. +fn resolve_host_both_families(host: &str) -> Vec { + use std::net::ToSocketAddrs; + (host, STUN_PORT) + .to_socket_addrs() + .map(Iterator::collect) + .unwrap_or_default() +} + +/// Merge a dedicated STUN host's resolved addresses ahead of the bare relay host's own, so the +/// dedicated endpoint is tried first WITHIN each address family once the final IPv6-first sort +/// runs — an identical address named by both is kept only once. Pure over already-resolved +/// addresses, so the preference itself is unit-testable without live DNS (the same seam +/// [`StunPlan::from_tiers`] uses for tier precedence). +fn prefer_dedicated_stun_host(dedicated: Vec, bare: Vec) -> Vec { + let mut seen = std::collections::HashSet::new(); + let mut addrs: Vec = dedicated.into_iter().chain(bare).collect(); + addrs.retain(|a| seen.insert(*a)); // IPv6-first: `dig_ip::Family` orders V6 before V4 (the ecosystem's canonical family sort). + // Stable, so within each family the dedicated host's entries (pushed first, above) stay ahead + // of the bare host's. addrs.sort_by_key(dig_ip::Family::of); addrs } -/// Resolve the single IPv6-first DIG STUN server (`:STUN_PORT`) for the traversal-ladder -/// hole-punch tier + DHT transport (a single reflexive-input endpoint, dig-nat L7 spec §3). The -/// reflexive-advertise path uses [`stun_servers_from_relay`] instead (it needs a per-family endpoint). -/// `None` when the host can't be parsed/resolved. Call off the async runtime. +/// Resolve the single IPv6-first DIG STUN server for the traversal-ladder hole-punch tier + DHT +/// transport (a single reflexive-input endpoint, dig-nat L7 spec §3), preferring the dedicated +/// `stun.` endpoint exactly as [`stun_servers_from_relay`] does — it delegates to that +/// function rather than re-deriving the host, so the two can never disagree about which server to +/// try first. The reflexive-advertise path uses [`stun_servers_from_relay`] directly instead (it +/// needs every per-family endpoint, not just the first). `None` when the host can't be +/// parsed/resolved. Call off the async runtime. pub fn stun_server_from_relay(relay_endpoint: &str) -> Option { stun_servers_from_relay(relay_endpoint).into_iter().next() } -// -- Public STUN fallback (INTERIM - dig_ecosystem#3198) ---------------------------------------- +// -- Public STUN fallback (STANDING, LAST-RESORT - dig_ecosystem#3198) -------------------------- /// Which of the STUN tiers actually answered this node's Binding request. /// @@ -483,9 +522,14 @@ pub enum StunSource { /// The STUN server co-located with the DIG relay (`:STUN_PORT`) - the intended /// steady state, and the tier this node prefers whenever it answers. Relay, - /// A public third-party STUN server from [`PUBLIC_STUN_SERVERS`]. **Interim only** - /// (dig_ecosystem#3198): reached solely when the relay tier yields nothing, so the relay - /// reclaims the role the moment it starts answering - with no code change and no redeploy. + /// A public third-party STUN server from [`PUBLIC_STUN_SERVERS`]. A STANDING fallback, + /// retained deliberately and reached only LAST — solely when the relay tier yields nothing — + /// so the relay reclaims the role the moment it starts answering, with no code change and no + /// redeploy. Kept rather than removed once the relay works: a single STUN source is a single + /// point of trust, the relay can itself be wrong or briefly unreachable, and a node with no + /// reflexive address publishes nothing and earns nothing. `dig_ecosystem#3198` tracks + /// requiring TWO sources to agree before a reading is used durably/on-chain — this tier + /// existing is what makes that corroboration possible at all. Public, } @@ -515,14 +559,24 @@ impl StunSource { pub const STUN_SERVER_ENV: &str = "DIG_STUN_SERVER"; /// The public STUN servers this node falls back to when the DIG relay answers nothing -/// (dig_ecosystem#3198). **Interim**: the relay is meant to serve this role (dig-nat L7 spec §3) and -/// reclaims it automatically the moment it answers, because the relay tier is tried first. +/// (dig_ecosystem#3198). A STANDING fallback, kept deliberately rather than deleted once the relay +/// works: the relay is preferred (dig-nat L7 spec §3) and reclaims the role automatically the +/// moment it answers, because the relay tier is tried first and this one is tried LAST — but a +/// single STUN source (even the relay) is a single point of trust, and a node with no reflexive +/// address at all publishes nothing and earns nothing. Measured justification: before dig-relay +/// 0.19.7 fixed a family-tag defect, `relay.dig.net` answered every IPv4 caller with the load +/// balancer's OWN IPv6 address — this tier was the only source that answered honestly during that +/// window, which is exactly the failure mode a single-source design cannot survive. /// /// More than one operator on purpose. A single third-party host would make one company's outage an /// outage of every DIG node's address discovery, which is a worse dependency than the one being /// worked around. Both entries publish A **and** AAAA records, so the IPv6-first walk (§5.2) has a /// real IPv6 endpoint to try rather than falling to IPv4 by default. /// +/// This tier is reached LAST on purpose, not merely last in this list: a third party learning +/// every DIG node's address is a real privacy cost, so it is paid only once every closer-to-home +/// option (operator override, then the DIG relay) has already failed to answer. +/// /// Note the ports differ and are NOT [`STUN_PORT`] for every host: Google serves STUN on 19302. pub const PUBLIC_STUN_SERVERS: &[(&str, u16)] = &[("stun.l.google.com", 19302), ("stun.cloudflare.com", 3478)]; @@ -730,15 +784,25 @@ impl StunPlan { let Ok(socket) = bind_stun_socket(port, server.is_ipv6()) else { continue; }; - if let Ok(result) = + let Ok(result) = dig_nat::stun::query_reflexive_address(&socket, server, timeout).await - { - return Some(ReflexiveDiscovery { - addr: reflexive_candidate(result, port), - source: *source, - server, - }); + else { + continue; + }; + // An answer whose family differs from the family of the server we queried is not + // about THIS query and must be discarded rather than believed just because the + // transaction otherwise completed (correct cookie, correctly echoed transaction + // id): a dual-stack load balancer has been measured answering an IPv4 caller with + // its OWN IPv6 address. Fall through exactly as a non-answering server does, all + // the way to a lower-precedence tier if nothing in this one answers honestly. + if dig_ip::Family::of(&result) != dig_ip::Family::of(&server) { + continue; } + return Some(ReflexiveDiscovery { + addr: reflexive_candidate(result, port), + source: *source, + server, + }); } } None @@ -769,7 +833,7 @@ pub fn stun_fallback_warning( match discovery { Some(d) if d.source == StunSource::Public && plan.has_source(StunSource::Relay) => { Some(format!( - "the DIG relay answered no STUN binding request; this node's reflexive address came from the PUBLIC fallback server {} instead. This is interim (dig_ecosystem#3198) - the relay is preferred and resumes automatically once it answers.", + "the DIG relay answered no STUN binding request; this node's reflexive address came from the PUBLIC fallback server {} instead. The relay is preferred and this node will use it again automatically the next time it answers, with no restart needed; dig_ecosystem#3198 tracks requiring a second source to agree before this reading is used durably.", d.server )) } @@ -1189,6 +1253,60 @@ mod tests { assert_eq!(parse_relay_host("relay.dig.net"), None); } + // -- `stun.` preference: pure over already-resolved addresses, so it is testable --- + // -- without live DNS, the same seam `StunPlan::from_tiers` uses for tier precedence. ---------- + + /// The dedicated `stun.` endpoint leads the bare host's own answer WITHIN each + /// family — proving the preference survives the final IPv6-first sort rather than being + /// scrambled by it. Four DISTINCT addresses so the ordering has something real to get wrong: + /// a implementation that only ever resolved the bare host (today's code) cannot produce this + /// order at all, because it never has the dedicated addresses to place. + #[test] + fn the_dedicated_stun_host_leads_the_bare_hosts_answer_within_each_family() { + let dedicated_v6: SocketAddr = "[2001:db8::1]:3478".parse().unwrap(); + let dedicated_v4: SocketAddr = "100.64.1.1:3478".parse().unwrap(); + let bare_v6: SocketAddr = "[2001:db8::2]:3478".parse().unwrap(); + let bare_v4: SocketAddr = "100.64.2.2:3478".parse().unwrap(); + + let merged = prefer_dedicated_stun_host( + vec![dedicated_v6, dedicated_v4], + vec![bare_v6, bare_v4], + ); + + assert_eq!(merged, vec![dedicated_v6, bare_v6, dedicated_v4, bare_v4]); + } + + /// The measured shape at `relay.dig.net`: `stun.`'s AAAA points at the SAME dualstack + /// NLB the bare host's own AAAA already names, while its A record is a NEW IPv4-only NLB. + /// The identical IPv6 entry must survive only ONCE — trying the same server twice wastes a + /// full STUN timeout for zero benefit — while the genuinely new IPv4 entry is kept. + #[test] + fn an_address_named_by_both_hosts_is_tried_only_once() { + let shared_v6: SocketAddr = "[2606:4700::1]:3478".parse().unwrap(); + let new_v4: SocketAddr = "100.64.9.9:3478".parse().unwrap(); + + let merged = prefer_dedicated_stun_host(vec![shared_v6, new_v4], vec![shared_v6]); + + assert_eq!( + merged, + vec![shared_v6, new_v4], + "the shared address must appear exactly once: {merged:?}" + ); + } + + /// A relay operator who has not adopted the `stun.` convention: the dedicated name resolves to + /// nothing (as if NXDOMAIN), and the bare host's own answer is used, completely unchanged from + /// today's behaviour. No error, no empty result — the preference is additive. + #[test] + fn an_unresolvable_dedicated_stun_host_falls_back_to_the_bare_host_unchanged() { + let bare_v6: SocketAddr = "[2001:db8::9]:3478".parse().unwrap(); + let bare_v4: SocketAddr = "100.64.3.3:3478".parse().unwrap(); + + let merged = prefer_dedicated_stun_host(Vec::new(), vec![bare_v6, bare_v4]); + + assert_eq!(merged, vec![bare_v6, bare_v4]); + } + /// **Proves:** a relay endpoint the node cannot read yields NO destination, in each of the four /// ways the pre-#285 parser invented one — an unknown scheme, an unparsable port, embedded /// userinfo, and a fragment. Each malformed input is paired with the well-formed endpoint it is @@ -1444,38 +1562,65 @@ mod tests { assert_eq!(reflexive, None); } - // -- Public STUN fallback (INTERIM - dig_ecosystem#3198) -------------------------------------- + // -- Public STUN fallback (STANDING, LAST-RESORT - dig_ecosystem#3198) -------------------------- /// Encode a STUN Binding success response carrying `mapped` in XOR-MAPPED-ADDRESS, echoing - /// `txid` (RFC 5389 §15.2, IPv4 form). + /// `txid` (RFC 5389 §15.2). `mapped`'s own variant selects the wire family (0x01 IPv4 / 0x02 + /// IPv6) and XOR key (the 32-bit cookie alone for IPv4; cookie‖transaction-id for IPv6) — so + /// this ONE encoder builds both an honest same-family answer and, for the cross-family guard + /// tests below, an answer whose family deliberately does not match the server socket it came + /// from (the measured dig-relay defect: an IPv4-bound endpoint answering with an IPv6 address). /// /// Written out here rather than borrowed from dig-nat on purpose: a fake server built from /// dig-nat's own encoder would round-trip that crate against itself and pass even if both halves /// were wrong together. Encoding from the RFC means these tests exercise dig-nat's real parser. - fn encode_binding_success(txid: &[u8; 12], mapped: std::net::SocketAddrV4) -> Vec { + fn encode_binding_success(txid: &[u8; 12], mapped: SocketAddr) -> Vec { let cookie = dig_nat::stun::MAGIC_COOKIE; - let mut attr = Vec::with_capacity(12); + let cookie_be = cookie.to_be_bytes(); + let port_xor = (mapped.port() ^ (cookie >> 16) as u16).to_be_bytes(); + let (family, addr_xor): (u8, Vec) = match mapped.ip() { + IpAddr::V4(v4) => { + let xored = v4 + .octets() + .iter() + .zip(cookie_be.iter()) + .map(|(a, b)| a ^ b) + .collect(); + (0x01, xored) + } + IpAddr::V6(v6) => { + // RFC 5389 §15.2: the IPv6 XOR key is the 32-bit cookie followed by the 96-bit txid. + let mut key = [0u8; 16]; + key[..4].copy_from_slice(&cookie_be); + key[4..].copy_from_slice(txid); + let xored = v6.octets().iter().zip(key.iter()).map(|(a, b)| a ^ b).collect(); + (0x02, xored) + } + }; + + let mut attr = Vec::with_capacity(8 + addr_xor.len()); attr.extend_from_slice(&dig_nat::stun::ATTR_XOR_MAPPED_ADDRESS.to_be_bytes()); - attr.extend_from_slice(&8u16.to_be_bytes()); + attr.extend_from_slice(&((4 + addr_xor.len()) as u16).to_be_bytes()); attr.push(0); // reserved - attr.push(0x01); // address family: IPv4 - attr.extend_from_slice(&(mapped.port() ^ (cookie >> 16) as u16).to_be_bytes()); - attr.extend_from_slice(&(u32::from(*mapped.ip()) ^ cookie).to_be_bytes()); + attr.push(family); + attr.extend_from_slice(&port_xor); + attr.extend_from_slice(&addr_xor); let mut msg = Vec::with_capacity(20 + attr.len()); msg.extend_from_slice(&dig_nat::stun::BINDING_SUCCESS.to_be_bytes()); msg.extend_from_slice(&(attr.len() as u16).to_be_bytes()); - msg.extend_from_slice(&cookie.to_be_bytes()); + msg.extend_from_slice(&cookie_be); msg.extend_from_slice(txid); msg.extend_from_slice(&attr); msg } - /// A fake STUN server on loopback that answers every Binding request with `mapped`. Returns the - /// address to point a tier at. - async fn spawn_fake_stun(mapped: &str) -> SocketAddr { - let mapped: std::net::SocketAddrV4 = mapped.parse().unwrap(); - let socket = tokio::net::UdpSocket::bind("127.0.0.1:0").await.unwrap(); + /// A fake STUN server bound to `bind_addr` that answers every Binding request with `mapped` + /// (any family, independent of `bind_addr`'s own family — see [`encode_binding_success`]). + /// Returns the address to point a tier at. + async fn spawn_fake_stun_at(bind_addr: &str, mapped: &str) -> SocketAddr { + let mapped: SocketAddr = mapped.parse().unwrap(); + let socket = tokio::net::UdpSocket::bind(bind_addr).await.unwrap(); let addr = socket.local_addr().unwrap(); tokio::spawn(async move { let mut buf = [0u8; 512]; @@ -1493,6 +1638,12 @@ mod tests { addr } + /// A fake STUN server on IPv4 loopback that answers every Binding request with `mapped`. + /// The common case ([`spawn_fake_stun_at`] with the family that every pre-existing test needs). + async fn spawn_fake_stun(mapped: &str) -> SocketAddr { + spawn_fake_stun_at("127.0.0.1:0", mapped).await + } + /// A real, free loopback UDP port with nothing listening on it - the stand-in for a relay whose /// DNS resolves perfectly and which answers nothing, the measured defect. Bound then dropped, so /// the port is genuinely free rather than a hard-coded guess that could collide with a live @@ -1536,8 +1687,8 @@ mod tests { } /// The relay RECLAIMS the role the moment it answers - no code change, no redeploy. Both tiers - /// are configured and both work here, and the relay still wins; that ordering is what makes this - /// fallback interim rather than a permanent dependency on a third party. + /// are configured and both work here, and the relay still wins; that ordering is what keeps this + /// fallback a genuine LAST resort rather than a permanent redirection away from the relay. #[tokio::test] async fn a_relay_that_answers_is_never_overtaken_by_the_public_tier() { let listen_port = free_local_port(); @@ -1555,6 +1706,70 @@ mod tests { assert_eq!(discovery.addr.ip().to_string(), "100.64.1.1"); } + /// **The revert-proof for the cross-family STUN guard.** A relay bound to IPv4 — the node + /// queried it over IPv4 — but answering with an IPv6 address is the measured dig-relay defect: + /// a dual-stack load balancer's own IPv6 address, not the caller's, with an otherwise perfectly + /// well-formed Binding response (correct cookie, correct echoed transaction id). That answer is + /// not about this node's query and must be discarded, falling through the ladder exactly as a + /// silent relay does (#3198's fallback), to the public tier's honest, same-family answer. + /// + /// The public tier is the honest CONTROL, not incidental set-dressing: without it this test + /// could not distinguish "the bogus answer was rejected and the walk continued" from "discovery + /// broke and returns None for any reason" — the two look identical if the only tier configured + /// is the lying one. + #[tokio::test] + async fn a_cross_family_stun_answer_is_discarded_and_the_ladder_falls_through() { + let listen_port = free_local_port(); + // Bound on IPv4 loopback; answers with an IPv6 address anyway (the LB-reflects-itself bug). + let relay = spawn_fake_stun("[2606:4700::dead:beef]:1234").await; + let public = spawn_fake_stun("100.64.7.7:41234").await; + let plan = StunPlan::from_tiers(Vec::new(), vec![relay], vec![public]); + + let discovery = plan + .discover_reflexive(listen_port, Duration::from_millis(400)) + .await + .expect("the public tier answered honestly, so a reflexive address MUST be discovered"); + + assert_eq!( + discovery.source, + StunSource::Public, + "the relay's cross-family answer must be discarded rather than returned: {discovery:?}" + ); + assert_eq!(discovery.addr.ip().to_string(), "100.64.7.7"); + } + + /// The reverse direction: an IPv6-bound server answering with an IPv4 address is equally not an + /// answer to this node's query, and is discarded the same way. Skips gracefully where the host + /// has no IPv6 loopback (mirrors `dual_stack_bind_accepts_an_ipv4_loopback_client` above). + #[tokio::test] + async fn a_cross_family_stun_answer_is_discarded_the_other_direction_too() { + if !crate::peer::tests::is_ipv6_loopback_available().await { + eprintln!( + "skipping a_cross_family_stun_answer_is_discarded_the_other_direction_too: no \ + IPv6 loopback in this environment" + ); + return; + } + let listen_port = free_local_port(); + // 100.64.x.x (RFC 6598 shared address space), not a documentation range: dig-nat's own + // `is_usable_reflexive_addr` rejects 203.0.113.0/24 et al. regardless of family, which would + // make this test pass for THAT reason instead of the cross-family guard under test. + let relay = spawn_fake_stun_at("[::1]:0", "100.64.5.5:1234").await; + let public = spawn_fake_stun("100.64.7.7:41234").await; + let plan = StunPlan::from_tiers(Vec::new(), vec![relay], vec![public]); + + let discovery = plan + .discover_reflexive(listen_port, Duration::from_millis(400)) + .await + .expect("the public tier answered honestly, so a reflexive address MUST be discovered"); + + assert_eq!( + discovery.source, + StunSource::Public, + "the IPv6 relay's IPv4 answer must be discarded rather than returned: {discovery:?}" + ); + } + /// The operator override outranks BOTH defaults. An operator running a private relay must never /// be silently redirected to a third party, so a configured endpoint is consulted before the /// relay and long before any public server. diff --git a/crates/dig-node-service/Cargo.toml b/crates/dig-node-service/Cargo.toml index 726e465f..de412741 100644 --- a/crates/dig-node-service/Cargo.toml +++ b/crates/dig-node-service/Cargo.toml @@ -92,7 +92,7 @@ async-trait = "0.1" # "0.6.0" the suite went green over a `control.wallet.coinById` the contract had never heard of -- # it checked nothing about the very method the change added. A caret range keeps the pin moving # with the published catalog instead of silently narrowing what CI can see. -dig-node-control-interface = "0.31" +dig-node-control-interface = "0.33" # The deterministic mirror-coin collateral model: the per-epoch requirement, the controller # multiplier, the small-network handicap, the floor clamp, and the safety-margin arithmetic. diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index 84fe49d1..6f385c51 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -60,13 +60,13 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use dig_node_control_interface::params::{ - WalletCoinByIdParams, WalletCoinSpendParams, WalletCoinsByParentParams, WalletCoinsParams, - WalletReservationsReserveParams, + SetMirrorAdvertiseUrlsParams, WalletCoinByIdParams, WalletCoinSpendParams, + WalletCoinsByParentParams, WalletCoinsParams, WalletReservationsReserveParams, }; use dig_node_control_interface::results::{ - AutomatedSpend, SpendAsset, SpendAuthority, SpendChainReference, SpendFailureStage, - SpendOutcome, SpendsListResult, WalletOperatorAddressResult, - WalletOperatorAddressUnavailableReason, + AutomatedSpend, MirrorAdvertiseState, MirrorAdvertiseView, SetMirrorAdvertiseUrlsResult, + SpendAsset, SpendAuthority, SpendChainReference, SpendFailureStage, SpendOutcome, + SpendsListResult, WalletOperatorAddressResult, WalletOperatorAddressUnavailableReason, }; use dig_node_control_interface::ControlMethod; use dig_node_core::seams::dig_peer::peer_network::PeerNetwork as _; @@ -178,6 +178,7 @@ pub const CONTROL_METHODS: &[&str] = &[ "control.status", "control.config.get", "control.config.setUpstream", + "control.config.setMirrorAdvertiseUrls", "control.log.setLevel", "control.cache.get", "control.cache.setCap", @@ -245,6 +246,7 @@ pub const OWNED_CONTROL_METHODS: &[&str] = &[ "control.status", "control.config.get", "control.config.setUpstream", + "control.config.setMirrorAdvertiseUrls", "control.log.setLevel", "control.cache.get", "control.cache.setCap", @@ -782,6 +784,13 @@ const PINNED_KEY: &str = "pinned_stores"; /// `control.config.setUpstream`; read by `Config::from_env` on next start). pub const UPSTREAM_OVERRIDE_KEY: &str = "upstream_override"; +/// The config.json key for the persisted mirror-advertise-URLs override (set via +/// `control.config.setMirrorAdvertiseUrls`, dig-node-control-interface 0.33.0; read by +/// [`crate::mirror::advertise::advertised_urls_from_env`] on next start — that read is what makes +/// this override's `requires_restart: true` promise become TRUE the next time the process starts, +/// rather than never, since nothing can rewrite a running process's own environment). +pub const MIRROR_ADVERTISE_URLS_OVERRIDE_KEY: &str = "mirror_advertise_urls_override"; + /// Read the pinned-store list from the node's config.json. Each entry is a /// canonical lowercase 64-hex store id (optionally with a pinned root, kept as a /// `{store_id, root?}` object). Missing/blank config → empty list. @@ -905,6 +914,47 @@ pub fn set_upstream_override(config_path: &Path, upstream: &str) -> std::io::Res }) } +/// Read the persisted mirror-advertise-URLs override from config.json, if any. Missing file, +/// missing key, or an empty array all read as `None` — "no override, use the derived default" — +/// matching [`SetMirrorAdvertiseUrlsParams::validated`]'s refusal of an explicit empty list: this +/// key is never WRITTEN as `[]`, only removed, so an empty array seen here would mean the file was +/// edited by hand rather than through this control call. +pub fn read_mirror_advertise_urls_override_from(config_path: &Path) -> Option> { + let txt = std::fs::read_to_string(config_path).ok()?; + let v: Value = serde_json::from_str(&txt).ok()?; + let urls: Vec = v + .get(MIRROR_ADVERTISE_URLS_OVERRIDE_KEY) + .and_then(|u| u.as_array())? + .iter() + .filter_map(|entry| entry.as_str().map(str::to_string)) + .collect(); + (!urls.is_empty()).then_some(urls) +} + +/// Read the persisted mirror-advertise-URLs override from the real config path. +pub fn read_mirror_advertise_urls_override() -> Option> { + read_mirror_advertise_urls_override_from(&dig_node_core::config_path()) +} + +/// Persist (`Some`) or clear (`None`) the mirror-advertise-URLs override in config.json. `urls` is +/// assumed already validated (non-empty, well-formed) by the caller — this function only writes +/// or removes the key, exactly as [`set_upstream_override`] does for its own key. +pub fn set_mirror_advertise_urls_override( + config_path: &Path, + urls: Option<&[String]>, +) -> std::io::Result<()> { + update_config(config_path, |v| match urls { + None => { + if let Some(obj) = v.as_object_mut() { + obj.remove(MIRROR_ADVERTISE_URLS_OVERRIDE_KEY); + } + } + Some(urls) => { + v[MIRROR_ADVERTISE_URLS_OVERRIDE_KEY] = json!(urls); + } + }) +} + /// Is a value a canonical lowercase 64-hex string (a store id / root)? PURE. pub fn is_hex64(s: &str) -> bool { s.len() == 64 && s.bytes().all(|b| b.is_ascii_hexdigit()) @@ -1014,6 +1064,9 @@ async fn dispatch_owned(ctx: &ControlCtx, id: Value, method: &str, params: &Valu "control.status" => control_ok(id, status(ctx).await), "control.config.get" => control_ok(id, config_get(ctx)), "control.config.setUpstream" => config_set_upstream(ctx, id, params), + "control.config.setMirrorAdvertiseUrls" => { + config_set_mirror_advertise_urls(ctx, id, params) + } "control.log.setLevel" => log_set_level(id, params), "control.cache.get" => control_ok(id, cache_get()), "control.cache.setCap" => cache_set_cap(id, params), @@ -1138,6 +1191,21 @@ async fn status(ctx: &ControlCtx) -> Value { fn config_get(ctx: &ControlCtx) -> Value { let (dir, shared) = (crate::meta::cache_dir(), crate::meta::cache_shared()); let port = ctx.addr.rsplit(':').next().unwrap_or(""); + // dig-node-control-interface 0.33.0's additive `ConfigResult.mirror_advertise` (dig-node#570): + // the SAME view `control.config.setMirrorAdvertiseUrls` reports, from the operator's CURRENTLY + // effective configuration (env var, else the persisted override — see + // `crate::mirror::advertise::advertised_urls_from_env`) rather than from a fresh set/clear. + let operator = crate::mirror::advertise::configured_operator_urls(); + let operator_override = (!operator.accepted.is_empty() || !operator.rejected.is_empty()) + .then(|| { + operator + .accepted + .iter() + .cloned() + .chain(operator.rejected.iter().map(|(entry, _)| entry.clone())) + .collect() + }); + let mirror_advertise = mirror_advertise_view(ctx, &operator, operator_override); json!({ "addr": ctx.addr, "port": port, @@ -1147,6 +1215,7 @@ fn config_get(ctx: &ControlCtx) -> Value { "cache_shared": shared, "config_path": ctx.config_path.display().to_string(), "sync_available": ctx.sync_available, + "mirror_advertise": mirror_advertise, }) } @@ -1200,6 +1269,107 @@ fn config_set_upstream(ctx: &ControlCtx, id: Value, params: &Value) -> Value { } } +/// Map this service's own [`crate::mirror::advertise::AdvertiseState`] to the WIRE +/// `MirrorAdvertiseState` dig-node-control-interface 0.33.0 declares. +/// +/// An explicit match, never a string round-trip through `.label()`: both enums already agree +/// character-for-character on their six spellings (`AdvertiseState::label()` IS the snake_case +/// form `MirrorAdvertiseState`'s own `#[serde(rename_all = "snake_case")]` produces), but a match +/// makes a variant added to EITHER enum without a corresponding arm here a COMPILE error, never a +/// silently-wrong or silently-missing wire label. +fn to_wire_advertise_state( + state: crate::mirror::advertise::AdvertiseState, +) -> MirrorAdvertiseState { + use crate::mirror::advertise::AdvertiseState as Local; + match state { + Local::Override => MirrorAdvertiseState::AdvertisingOverride, + Local::Derived => MirrorAdvertiseState::AdvertisingDerived, + Local::Off => MirrorAdvertiseState::Off, + Local::NoPublicAddress => MirrorAdvertiseState::NoPublicAddress, + Local::Uncorroborated => MirrorAdvertiseState::UncorroboratedAddress, + Local::NoRelay => MirrorAdvertiseState::NoRelay, + } +} + +/// The mirror advertise-URL view both `control.config.get` and +/// `control.config.setMirrorAdvertiseUrls` report — ONE function, so the two surfaces can never +/// compute this differently. `operator` is the already-decided operator override (what a set/clear +/// just persisted, or what the environment currently holds); `operator_override` is the verbatim +/// value [`MirrorAdvertiseView::operator_override`] reports alongside it. +fn mirror_advertise_view( + ctx: &ControlCtx, + operator: &crate::mirror::advertise::Advertised, + operator_override: Option>, +) -> MirrorAdvertiseView { + let address = + crate::mirror::advertise::PublicAddress::from_network_info(&ctx.node.network_info()); + let effective = crate::mirror::advertise::effective_urls(operator, &address); + MirrorAdvertiseView { + urls: effective.urls, + operator_override, + state: to_wire_advertise_state(effective.state), + } +} + +/// `control.config.setMirrorAdvertiseUrls` (dig-node-control-interface 0.33.0, dig-node#570) — +/// override (`Some`, non-empty, well-formed) or clear (`None`) the URLs this node advertises in +/// its own mirror-coin memos. +/// +/// # `requires_restart` is unconditionally `true`, and that is the honest answer today +/// +/// [`crate::server`]'s mirror-creation task reads the operator's configured URLs via +/// [`crate::mirror::advertise::configured_operator_urls`] exactly ONCE, for the life of that task +/// (deliberately — see that function's own doc: re-reading it per pass would only let the +/// bring-up warning drift from what is actually published). No control call can rewrite a running +/// process's already-captured state, so this override cannot take effect before a restart. It DOES +/// take effect at the NEXT one: the persisted write below is what +/// [`crate::mirror::advertise::advertised_urls_from_env`] consults, so the promise this field +/// makes is genuine, not merely unimplemented. +fn config_set_mirror_advertise_urls(ctx: &ControlCtx, id: Value, params: &Value) -> Value { + let parsed: SetMirrorAdvertiseUrlsParams = match serde_json::from_value(params.clone()) { + Ok(p) => p, + Err(e) => { + return control_error( + id, + ErrorCode::InvalidParams, + format!("control.config.setMirrorAdvertiseUrls: {e}"), + ) + } + }; + let parsed = match parsed.validated() { + Ok(p) => p, + Err(e) => return control_error(id, ErrorCode::InvalidParams, e.message), + }; + + if let Err(e) = set_mirror_advertise_urls_override(&ctx.config_path, parsed.urls.as_deref()) { + return control_error( + id, + ErrorCode::ControlError, + format!("failed to persist mirror-advertise-urls override: {e}"), + ); + } + + // What THIS call just persisted, not what the running process's own environment still says + // (which cannot change) -- so the response matches what actually took effect on disk. + let operator = match &parsed.urls { + Some(urls) => crate::mirror::advertise::Advertised { + accepted: urls.clone(), + rejected: Vec::new(), + }, + None => crate::mirror::advertise::Advertised::default(), + }; + let mirror_advertise = mirror_advertise_view(ctx, &operator, parsed.urls); + + control_ok( + id, + serde_json::to_value(SetMirrorAdvertiseUrlsResult { + mirror_advertise, + requires_restart: true, + }) + .unwrap_or_default(), + ) +} + /// Cache view (cap/used/dir/shared) — reuses the dig-node crate's resolvers. /// /// `capsule_bytes`/`response_bytes` split the same total, so `used_bytes` being large while @@ -6940,6 +7110,65 @@ mod tests { assert_eq!(read_upstream_override_from(&config_path), None); } + /// The persistence half of `control.config.setMirrorAdvertiseUrls` (dig-node#570): a set + /// round-trips, a second set REPLACES rather than appends, and `None` clears -- the same + /// three-state contract `SetMirrorAdvertiseUrlsParams` documents on the wire. + #[test] + fn mirror_advertise_urls_override_roundtrips_replaces_and_clears() { + let dir = tempfile::Builder::new() + .prefix("dig-node-mirror-advertise-test-") + .tempdir() + .expect("a scratch dir"); + let config_path = dir.path().join("config.json"); + assert_eq!(read_mirror_advertise_urls_override_from(&config_path), None); + + let first = vec!["dig://[2001:db8::7]:9776".to_string()]; + set_mirror_advertise_urls_override(&config_path, Some(&first)).unwrap(); + assert_eq!( + read_mirror_advertise_urls_override_from(&config_path), + Some(first) + ); + + // A second SET replaces the list rather than merging into it. + let second = vec![ + "dig://relay.example:9776".to_string(), + "dig://[2001:db8::8]:9776".to_string(), + ]; + set_mirror_advertise_urls_override(&config_path, Some(&second)).unwrap(); + assert_eq!( + read_mirror_advertise_urls_override_from(&config_path), + Some(second) + ); + + // `None` clears it. + set_mirror_advertise_urls_override(&config_path, None).unwrap(); + assert_eq!(read_mirror_advertise_urls_override_from(&config_path), None); + } + + /// The pin registry and the upstream override already prove `update_config` preserves sibling + /// keys (`update_config_preserves_dig_node_keys`); this proves the mirror-advertise-urls key + /// joins that set rather than clobbering it — a THIRD service-owned key living beside the other + /// two in the same file. + #[test] + fn mirror_advertise_urls_override_coexists_with_pins_and_upstream() { + let dir = tempfile::Builder::new() + .prefix("dig-node-mirror-advertise-coexist-test-") + .tempdir() + .expect("a scratch dir"); + let config_path = dir.path().join("config.json"); + + let store = "f".repeat(64); + add_pin(&config_path, &store, None).unwrap(); + set_upstream_override(&config_path, "https://example.test").unwrap(); + set_mirror_advertise_urls_override(&config_path, Some(&["dig://h:1".to_string()])).unwrap(); + + let v: Value = + serde_json::from_str(&std::fs::read_to_string(&config_path).unwrap()).unwrap(); + assert_eq!(v["pinned_stores"][0]["store_id"], json!(store)); + assert_eq!(v["upstream_override"], json!("https://example.test")); + assert_eq!(v["mirror_advertise_urls_override"], json!(["dig://h:1"])); + } + /// (#1851 leg-2) `control.wallet.balance` MUST emit `balance`/`pending` as JSON **numbers**, /// matching `dig-node-control-interface` 0.3.0's `WalletBalanceResult { balance: u64, .. }` /// and dig-app-core's `BalanceResponse { balance: u64 }`. This is the property under test — diff --git a/crates/dig-node-service/src/mirror/advertise.rs b/crates/dig-node-service/src/mirror/advertise.rs index d569768d..7a5057cd 100644 --- a/crates/dig-node-service/src/mirror/advertise.rs +++ b/crates/dig-node-service/src/mirror/advertise.rs @@ -155,9 +155,43 @@ impl Advertised { } } -/// Reads the operator's advertised-URL list from the environment. +/// The precedence [`advertised_urls_from_env`] applies, pure over already-read inputs so it is +/// unit-testable without process env mutation or a real config path — the same "drive the pure +/// decision with fixture data, let the impure wrapper do the reading" split this module's own +/// [`effective_urls`] already uses: +/// +/// 1. **The live environment variable, when non-blank.** A deploy/CI export must never be +/// silently shadowed by a saved control-plane choice — the same precedence `Config::from_env`'s +/// upstream resolution already uses. +/// 2. **Else the persisted override**, written by `control.config.setMirrorAdvertiseUrls` +/// (dig-node#570). This is what makes that call's `requires_restart: true` promise genuine: +/// nothing can rewrite a running process's own environment, but the NEXT process start reads +/// this precedence fresh and picks the persisted value up. +/// 3. **Else nothing.** +fn advertised_urls_precedence(env_value: Option, persisted: Option>) -> Advertised { + if let Some(v) = env_value.filter(|v| !v.trim().is_empty()) { + return parse_advertised_urls(&v); + } + match persisted { + Some(urls) => parse_advertised_urls(&urls.join(",")), + None => Advertised::default(), + } +} + +/// [`advertised_urls_from_env`] for an explicit config path (tests) — see that function for the +/// precedence this implements. +pub fn advertised_urls_effective_from(config_path: &std::path::Path) -> Advertised { + advertised_urls_precedence( + std::env::var(ADVERTISE_URLS_ENV).ok(), + crate::control::read_mirror_advertise_urls_override_from(config_path), + ) +} + +/// Reads the operator's advertised-URL list: the live environment variable when set, else the +/// persisted `control.config.setMirrorAdvertiseUrls` override (dig-node#570) — see +/// [`advertised_urls_precedence`] for the exact rule and why the override is honoured here at all. pub fn advertised_urls_from_env() -> Advertised { - parse_advertised_urls(&std::env::var(ADVERTISE_URLS_ENV).unwrap_or_default()) + advertised_urls_effective_from(&dig_node_core::config_path()) } /// The URL scheme a derived entry is published under. @@ -828,6 +862,53 @@ fn is_this_machine_only_v4(ip: Ipv4Addr) -> bool { mod tests { use super::*; + // -- Persisted mirror-advertise-URLs override precedence (dig-node#570) ------------------- + // Pure over already-read inputs (no env mutation, no filesystem), the same testing style + // every other decision in this module uses. + + /// The live environment variable wins outright when non-blank, regardless of what a + /// persisted override holds — a deploy/CI export must never be silently shadowed by a saved + /// control-plane choice. + #[test] + fn the_env_value_wins_over_a_persisted_override_when_present() { + let result = advertised_urls_precedence( + Some("https://env.example".to_string()), + Some(vec!["https://persisted.example".to_string()]), + ); + assert_eq!(result.accepted, vec!["https://env.example".to_string()]); + } + + /// No env value at all: the persisted override is used — this is what makes + /// `control.config.setMirrorAdvertiseUrls`'s `requires_restart: true` promise become true the + /// next time the process starts. + #[test] + fn a_persisted_override_is_used_when_the_env_var_is_absent() { + let result = + advertised_urls_precedence(None, Some(vec!["https://persisted.example".to_string()])); + assert_eq!(result.accepted, vec!["https://persisted.example".to_string()]); + } + + /// A BLANK env value (set but empty/whitespace) counts as absent, not as an explicit "advertise + /// nothing" — matching `SetMirrorAdvertiseUrlsParams::validated`'s own refusal of an explicit + /// empty list: this module has no way to tell "unset" from "set to nothing" through a bare env + /// string, so it reads a blank the same permissive way it always has. + #[test] + fn a_blank_env_value_falls_back_to_the_persisted_override_too() { + let result = advertised_urls_precedence( + Some(" ".to_string()), + Some(vec!["https://persisted.example".to_string()]), + ); + assert_eq!(result.accepted, vec!["https://persisted.example".to_string()]); + } + + /// Neither present: nothing to advertise, exactly `Advertised::default()` — the situation + /// every real dig-node is in before this override was ever set. + #[test] + fn neither_env_nor_persisted_yields_nothing() { + let result = advertised_urls_precedence(None, None); + assert_eq!(result, Advertised::default()); + } + /// **Every operator-facing line this module emits reads as a sentence.** /// /// All three of them shipped corrupted: a `\` string continuation lost its backslash and baked diff --git a/crates/dig-wallet/Cargo.toml b/crates/dig-wallet/Cargo.toml index d33f7a59..b8cad9fb 100644 --- a/crates/dig-wallet/Cargo.toml +++ b/crates/dig-wallet/Cargo.toml @@ -78,7 +78,7 @@ axum = "0.7" # The PUBLISHED control-plane contract. `BalanceAsset` is re-exported from it rather than # restated here: the wire spelling of an asset and the asset a read scopes to must be the same # value, and two enums naming the same tokens agree only until one of them gains a variant. -dig-node-control-interface = "0.31" +dig-node-control-interface = "0.33" # The ecosystem's single definition of a Chia offer id (dig_ecosystem#3140, #283): # `sha256(spend_bundle.to_bytes())`, the value Chia's `Offer.name()`, Sage and dexie use. This # crate once derived the id itself from the offered coin set alone, which collided across offers From 06fd53cf44e6bea5830124bb2b3e2af0a67111f2 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sat, 5 Sep 2026 09:31:26 -0700 Subject: [PATCH 3/7] style: cargo fmt the reflexive-discovery batch Co-Authored-By: Claude --- SPEC.md | 3 ++- .../dig-node-core/src/seams/dig_peer/net.rs | 18 ++++++++----- crates/dig-node-service/src/control.rs | 27 ++++++++++++------- .../dig-node-service/src/mirror/advertise.rs | 15 ++++++++--- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/SPEC.md b/SPEC.md index d73ec780..06fb0a23 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1684,8 +1684,9 @@ lowercase 64-hex; a capsule reference is `storeId:rootHash`. Malformed refs yiel | Method | Params | Result (essentials) | |---|---|---| | `control.status` | — | `running`, `service`, `version`, `commit`, `protocol`, `uptime_secs`, `addr`, `upstream`, `cache`, `hosted_store_count`, `cached_capsule_count`, `pinned_store_count`, `sync.available`, `logging` (`initialized`, `dir`, `file_logging`, `file_error` — a START-UP verdict; see §20.1) | -| `control.config.get` | — | `addr`, `port`, `upstream`, `upstream_override`, `cache_dir`, `cache_shared`, `config_path`, `sync_available` | +| `control.config.get` | — | `addr`, `port`, `upstream`, `upstream_override`, `cache_dir`, `cache_shared`, `config_path`, `sync_available`, `mirror_advertise` (dig-node-control-interface 0.33.0's `MirrorAdvertiseView`: `urls`, `operator_override`, `state`) | | `control.config.setUpstream` | `upstream` (URL string; blank clears) | `upstream` (normalized), `requires_restart: true` — persisted, effective on next start (§3.4) | +| `control.config.setMirrorAdvertiseUrls` | `urls` (non-empty list of absolute URLs to override with; omitted/`null` clears; an explicit empty list is `INVALID_PARAMS`) | `mirror_advertise` (`MirrorAdvertiseView`: the effective `urls`/`operator_override`/`state` THIS write produced), `requires_restart: true` — persisted to config.json and consulted by `advertised_urls_from_env`'s env-else-persisted precedence on the NEXT start; the currently-running mirror task already captured its operator URLs once at bring-up and cannot observe this write live. Ordinary tier (installs no principal). `INVALID_PARAMS` on an explicit empty list or a non-absolute URL. | | `control.log.setLevel` | `filter` (an `EnvFilter` directive, e.g. `debug` or `info,dig_node_core=debug`) | `filter` (echoed) — live-applied via the `dig-logging` reload handle, effective immediately, NOT persisted (§11); `INVALID_PARAMS` on a missing/malformed directive, `CONTROL_ERROR` when logging is not installed in the process | | `control.cache.get` | — | `cap_bytes`, `used_bytes`, `capsule_bytes`, `response_bytes`, `dir`, `shared` | | `control.profile.putBody` | `store_id`, `root` (64-hex), `body_b64` (standard padded base64 of the DPB bytes) | `stored: true`, `store_id`, `root`, `body_bytes`, `announced_to_peers`, `unreachable_peers`. `announced_to_peers` is a TRUE delivery count — peers the 223 announce was actually sent to, excluding lazy and NAT-bound peers that are connected but cannot be pushed to — so `0` does NOT mean failure and MUST NOT be treated as one: the body is persisted either way and the periodic re-announce reaches whoever connects later. `unreachable_peers` reports that connected-but-unreachable remainder, so a caller can distinguish "no peers exist" from "peers exist and none could be reached". The node MUST independently resolve the root on chain and refuse unless the chain confirms exactly that root AND the bytes hash to it (§22.3). A refusal is an ERROR, never an `Ok` carrying `stored: false`. A decoded body above `MAX_BODY_BYTES` (4 MiB) is `INVALID_PARAMS` before anything is persisted. | diff --git a/crates/dig-node-core/src/seams/dig_peer/net.rs b/crates/dig-node-core/src/seams/dig_peer/net.rs index b0ad10a6..5e618301 100644 --- a/crates/dig-node-core/src/seams/dig_peer/net.rs +++ b/crates/dig-node-core/src/seams/dig_peer/net.rs @@ -482,7 +482,10 @@ fn resolve_host_both_families(host: &str) -> Vec { /// runs — an identical address named by both is kept only once. Pure over already-resolved /// addresses, so the preference itself is unit-testable without live DNS (the same seam /// [`StunPlan::from_tiers`] uses for tier precedence). -fn prefer_dedicated_stun_host(dedicated: Vec, bare: Vec) -> Vec { +fn prefer_dedicated_stun_host( + dedicated: Vec, + bare: Vec, +) -> Vec { let mut seen = std::collections::HashSet::new(); let mut addrs: Vec = dedicated.into_iter().chain(bare).collect(); addrs.retain(|a| seen.insert(*a)); @@ -1268,10 +1271,8 @@ mod tests { let bare_v6: SocketAddr = "[2001:db8::2]:3478".parse().unwrap(); let bare_v4: SocketAddr = "100.64.2.2:3478".parse().unwrap(); - let merged = prefer_dedicated_stun_host( - vec![dedicated_v6, dedicated_v4], - vec![bare_v6, bare_v4], - ); + let merged = + prefer_dedicated_stun_host(vec![dedicated_v6, dedicated_v4], vec![bare_v6, bare_v4]); assert_eq!(merged, vec![dedicated_v6, bare_v6, dedicated_v4, bare_v4]); } @@ -1593,7 +1594,12 @@ mod tests { let mut key = [0u8; 16]; key[..4].copy_from_slice(&cookie_be); key[4..].copy_from_slice(txid); - let xored = v6.octets().iter().zip(key.iter()).map(|(a, b)| a ^ b).collect(); + let xored = v6 + .octets() + .iter() + .zip(key.iter()) + .map(|(a, b)| a ^ b) + .collect(); (0x02, xored) } }; diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index 6f385c51..0f7fb1d9 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -1196,8 +1196,8 @@ fn config_get(ctx: &ControlCtx) -> Value { // effective configuration (env var, else the persisted override — see // `crate::mirror::advertise::advertised_urls_from_env`) rather than from a fresh set/clear. let operator = crate::mirror::advertise::configured_operator_urls(); - let operator_override = (!operator.accepted.is_empty() || !operator.rejected.is_empty()) - .then(|| { + let operator_override = + (!operator.accepted.is_empty() || !operator.rejected.is_empty()).then(|| { operator .accepted .iter() @@ -1326,14 +1326,21 @@ fn mirror_advertise_view( /// [`crate::mirror::advertise::advertised_urls_from_env`] consults, so the promise this field /// makes is genuine, not merely unimplemented. fn config_set_mirror_advertise_urls(ctx: &ControlCtx, id: Value, params: &Value) -> Value { - let parsed: SetMirrorAdvertiseUrlsParams = match serde_json::from_value(params.clone()) { - Ok(p) => p, - Err(e) => { - return control_error( - id, - ErrorCode::InvalidParams, - format!("control.config.setMirrorAdvertiseUrls: {e}"), - ) + // A caller omitting `params` entirely (`null`, distinct from the KAT's documented `{}` clear + // form) is read the same lenient way `config_set_upstream`'s `params.get(...)` reads a missing + // field: as "nothing was sent", never as a malformed request. + let parsed: SetMirrorAdvertiseUrlsParams = if params.is_null() { + SetMirrorAdvertiseUrlsParams { urls: None } + } else { + match serde_json::from_value(params.clone()) { + Ok(p) => p, + Err(e) => { + return control_error( + id, + ErrorCode::InvalidParams, + format!("control.config.setMirrorAdvertiseUrls: {e}"), + ) + } } }; let parsed = match parsed.validated() { diff --git a/crates/dig-node-service/src/mirror/advertise.rs b/crates/dig-node-service/src/mirror/advertise.rs index 7a5057cd..582a9618 100644 --- a/crates/dig-node-service/src/mirror/advertise.rs +++ b/crates/dig-node-service/src/mirror/advertise.rs @@ -168,7 +168,10 @@ impl Advertised { /// nothing can rewrite a running process's own environment, but the NEXT process start reads /// this precedence fresh and picks the persisted value up. /// 3. **Else nothing.** -fn advertised_urls_precedence(env_value: Option, persisted: Option>) -> Advertised { +fn advertised_urls_precedence( + env_value: Option, + persisted: Option>, +) -> Advertised { if let Some(v) = env_value.filter(|v| !v.trim().is_empty()) { return parse_advertised_urls(&v); } @@ -885,7 +888,10 @@ mod tests { fn a_persisted_override_is_used_when_the_env_var_is_absent() { let result = advertised_urls_precedence(None, Some(vec!["https://persisted.example".to_string()])); - assert_eq!(result.accepted, vec!["https://persisted.example".to_string()]); + assert_eq!( + result.accepted, + vec!["https://persisted.example".to_string()] + ); } /// A BLANK env value (set but empty/whitespace) counts as absent, not as an explicit "advertise @@ -898,7 +904,10 @@ mod tests { Some(" ".to_string()), Some(vec!["https://persisted.example".to_string()]), ); - assert_eq!(result.accepted, vec!["https://persisted.example".to_string()]); + assert_eq!( + result.accepted, + vec!["https://persisted.example".to_string()] + ); } /// Neither present: nothing to advertise, exactly `Advertised::default()` — the situation From d70bb7367550c4dc388a2d84ed2bdbde6e81e143 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sat, 5 Sep 2026 09:36:13 -0700 Subject: [PATCH 4/7] feat(cli): add the dign config set-mirror-advertise-urls verb The CLI-parity gate (cli_covers_every_node_control_method, #426) failed on control.config.setMirrorAdvertiseUrls -- every control.* method the node resolves needs a dign verb or the CLI silently falls behind the WS surface the extension drives. Adds ControlAction::ConfigSetMirrorAdvertiseUrls, its wire_params/method/pretty-print arms, and the clap subcommand (zero arguments clears the override, matching the wire's None-clears contract). Co-Authored-By: Claude --- SPEC.md | 4 +++- crates/dig-node-service/src/control_cli.rs | 24 ++++++++++++++++++++++ crates/dig-node-service/src/entrypoint.rs | 15 ++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/SPEC.md b/SPEC.md index 06fb0a23..e4532b9b 100644 --- a/SPEC.md +++ b/SPEC.md @@ -2619,7 +2619,9 @@ token-free by design; it is not a control-parity subcommand and this rule does n - `info` → `control.status` — the rich node status (version, uptime, cache, hosted-store + cached-capsule counts, §21 sync availability). DISTINCT from `status` (§8.3), which is an unauthenticated `/health` liveness probe; `info` is the token-gated detailed view. -- `config [get]` → `control.config.get`; `config set-upstream ` → `control.config.setUpstream`. +- `config [get]` → `control.config.get`; `config set-upstream ` → `control.config.setUpstream`; + `config set-mirror-advertise-urls [...]` → `control.config.setMirrorAdvertiseUrls` (no + arguments clears the override). - `cache [get]` → `control.cache.get`; `cache set-cap ` → `control.cache.setCap`; `cache clear` → `control.cache.clear`. - `stores [list]` → `control.hostedStores.list`; `stores pin|unpin|status ` → diff --git a/crates/dig-node-service/src/control_cli.rs b/crates/dig-node-service/src/control_cli.rs index f4b09807..c20f72ae 100644 --- a/crates/dig-node-service/src/control_cli.rs +++ b/crates/dig-node-service/src/control_cli.rs @@ -42,6 +42,10 @@ pub enum ControlAction { ConfigGet, /// `control.config.setUpstream` — persist the upstream DIG RPC override (next-start effective). ConfigSetUpstream { url: String }, + /// `control.config.setMirrorAdvertiseUrls` — override (non-empty list) or clear (`None`) the + /// URLs this node advertises in its own mirror-coin memos (dig-node#570, next-start effective — + /// see that method's own doc for why nothing can make it live before a restart). + ConfigSetMirrorAdvertiseUrls { urls: Option> }, /// `control.cache.get` — cache cap/used/dir/shared. CacheGet, /// `control.cache.setCap` — set the on-disk cache size cap (bytes; floored at 64 MiB). @@ -220,6 +224,9 @@ impl ControlAction { ControlAction::Info => "control.status", ControlAction::ConfigGet => "control.config.get", ControlAction::ConfigSetUpstream { .. } => "control.config.setUpstream", + ControlAction::ConfigSetMirrorAdvertiseUrls { .. } => { + "control.config.setMirrorAdvertiseUrls" + } ControlAction::CacheGet => "control.cache.get", ControlAction::CacheSetCap { .. } => "control.cache.setCap", ControlAction::CacheClear => "control.cache.clear", @@ -298,6 +305,14 @@ impl ControlAction { match self { ControlAction::ConfigSetUpstream { url } => json!({ "upstream": url }), + // `None` clears the override, wired as an ABSENT `urls` key (not `null` / `[]`) so the + // node's own `#[serde(default)]` reads it exactly like the KAT's documented `{}` clear + // form — see `SetMirrorAdvertiseUrlsParams`'s own doc for why an explicit empty list is + // refused rather than accepted as either meaning. + ControlAction::ConfigSetMirrorAdvertiseUrls { urls } => match urls { + Some(urls) => json!({ "urls": urls }), + None => json!({}), + }, // Basis points, never a percentage and never a float. A 1 bp margin (0.01%) is a legal // choice and any conversion to whole percent would erase it. ControlAction::CollateralMarginSet { margin_bp } => json!({ "margin_bp": margin_bp }), @@ -460,6 +475,7 @@ pub fn cli_covered_control_methods() -> Vec<&'static str> { ControlAction::Info.method(), ControlAction::ConfigGet.method(), ControlAction::ConfigSetUpstream { url: String::new() }.method(), + ControlAction::ConfigSetMirrorAdvertiseUrls { urls: None }.method(), ControlAction::CacheGet.method(), ControlAction::CacheSetCap { bytes: 0 }.method(), ControlAction::CacheClear.method(), @@ -644,6 +660,14 @@ fn summarize(method: &str, result: &Value) -> String { "upstream set to {} (effective on next node start)", result["upstream"].as_str().unwrap_or("?"), ), + "control.config.setMirrorAdvertiseUrls" => format!( + "mirror advertise state: {} · urls: {} (effective on next node start)", + result["mirror_advertise"]["state"].as_str().unwrap_or("?"), + result["mirror_advertise"]["urls"] + .as_array() + .map(|a| a.len()) + .unwrap_or(0), + ), "control.cache.get" => format!( "cache {} / {} bytes used/cap · {}", result["used_bytes"].as_u64().unwrap_or(0), diff --git a/crates/dig-node-service/src/entrypoint.rs b/crates/dig-node-service/src/entrypoint.rs index d58334de..e807f78d 100644 --- a/crates/dig-node-service/src/entrypoint.rs +++ b/crates/dig-node-service/src/entrypoint.rs @@ -246,6 +246,13 @@ enum ConfigCommand { /// The upstream RPC URL (blank clears the override). url: String, }, + /// Override (or clear) the URLs this node advertises in its own mirror-coin memos + /// (dig-node#570, effective on next node start). + SetMirrorAdvertiseUrls { + /// Absolute URL(s) to advertise. Omit entirely to CLEAR the override and revert to this + /// node's derived (self-discovered) address. + urls: Vec, + }, } /// `dig-node cache` sub-actions. With none, prints the cache config. @@ -1028,6 +1035,14 @@ fn config_action(cmd: Option) -> ControlAction { match cmd { None | Some(ConfigCommand::Get) => ControlAction::ConfigGet, Some(ConfigCommand::SetUpstream { url }) => ControlAction::ConfigSetUpstream { url }, + Some(ConfigCommand::SetMirrorAdvertiseUrls { urls }) => { + // An empty CLI invocation clears the override, exactly like an omitted `urls` on the + // wire (`SetMirrorAdvertiseUrlsParams`'s own `None` case) — never an explicit `[]`, + // which the node refuses as ambiguous. + ControlAction::ConfigSetMirrorAdvertiseUrls { + urls: (!urls.is_empty()).then_some(urls), + } + } } } From d76b6d73bd6d82528807c527824bef76dee9bb5e Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sat, 5 Sep 2026 09:39:27 -0700 Subject: [PATCH 5/7] fix(cli): stop leaking an internal ticket number into --help text no_help_text_exposes_an_internal_ticket_number caught it: the new config set-mirror-advertise-urls subcommand's doc comment (rendered verbatim by clap) named dig-node#570. Reworded both the clap-rendered doc and its clap-agnostic ControlAction sibling. Co-Authored-By: Claude --- crates/dig-node-service/src/control_cli.rs | 4 ++-- crates/dig-node-service/src/entrypoint.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/dig-node-service/src/control_cli.rs b/crates/dig-node-service/src/control_cli.rs index c20f72ae..daf79f6c 100644 --- a/crates/dig-node-service/src/control_cli.rs +++ b/crates/dig-node-service/src/control_cli.rs @@ -43,8 +43,8 @@ pub enum ControlAction { /// `control.config.setUpstream` — persist the upstream DIG RPC override (next-start effective). ConfigSetUpstream { url: String }, /// `control.config.setMirrorAdvertiseUrls` — override (non-empty list) or clear (`None`) the - /// URLs this node advertises in its own mirror-coin memos (dig-node#570, next-start effective — - /// see that method's own doc for why nothing can make it live before a restart). + /// URLs this node advertises in its own mirror-coin memos (next-start effective — see that + /// method's own doc for why nothing can make it live before a restart). ConfigSetMirrorAdvertiseUrls { urls: Option> }, /// `control.cache.get` — cache cap/used/dir/shared. CacheGet, diff --git a/crates/dig-node-service/src/entrypoint.rs b/crates/dig-node-service/src/entrypoint.rs index e807f78d..29dc6778 100644 --- a/crates/dig-node-service/src/entrypoint.rs +++ b/crates/dig-node-service/src/entrypoint.rs @@ -246,8 +246,8 @@ enum ConfigCommand { /// The upstream RPC URL (blank clears the override). url: String, }, - /// Override (or clear) the URLs this node advertises in its own mirror-coin memos - /// (dig-node#570, effective on next node start). + /// Override (or clear) the URLs this node advertises in its own mirror-coin memos, + /// effective on next node start. SetMirrorAdvertiseUrls { /// Absolute URL(s) to advertise. Omit entirely to CLEAR the override and revert to this /// node's derived (self-discovered) address. From 53830407b613fd92d4e19a131f28b71a12b61721 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sat, 5 Sep 2026 09:46:47 -0700 Subject: [PATCH 6/7] fix(clippy): collapse the nested urls match clippy flagged Co-Authored-By: Claude --- crates/dig-node-service/src/control_cli.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/dig-node-service/src/control_cli.rs b/crates/dig-node-service/src/control_cli.rs index daf79f6c..d227cf8b 100644 --- a/crates/dig-node-service/src/control_cli.rs +++ b/crates/dig-node-service/src/control_cli.rs @@ -309,10 +309,10 @@ impl ControlAction { // node's own `#[serde(default)]` reads it exactly like the KAT's documented `{}` clear // form — see `SetMirrorAdvertiseUrlsParams`'s own doc for why an explicit empty list is // refused rather than accepted as either meaning. - ControlAction::ConfigSetMirrorAdvertiseUrls { urls } => match urls { - Some(urls) => json!({ "urls": urls }), - None => json!({}), - }, + ControlAction::ConfigSetMirrorAdvertiseUrls { urls: Some(urls) } => { + json!({ "urls": urls }) + } + ControlAction::ConfigSetMirrorAdvertiseUrls { urls: None } => json!({}), // Basis points, never a percentage and never a float. A 1 bp margin (0.01%) is a legal // choice and any conversion to whole percent would erase it. ControlAction::CollateralMarginSet { margin_bp } => json!({ "margin_bp": margin_bp }), From e966e353353817656eb517c416253bfea1c3c904 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sat, 5 Sep 2026 11:30:52 -0700 Subject: [PATCH 7/7] fix(control): drop resetCoinDb from KNOWN_UNPUBLISHED and bump to 0.254.84 dig-node-control-interface 0.32 published control.wallet.resetCoinDb (ControlMethod::WalletResetCoinDb), so the conformance gate's the_unpublished_list_still_describes_real_drift test correctly flags the now-stale exemption. The published method's own requires_master_token() returns false, so removing the local exemption changes nothing about the tier the DIG App relies on for this call -- the paired tier is now delegated to the published contract instead of carved out locally by name, which is the exact end state the exemption list documents itself as working toward. Also bumps the workspace version to 0.254.84: main had caught up to 0.254.83 while this branch sat unbumped, failing the version-increment gate. Co-Authored-By: Claude --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/dig-node-service/src/control.rs | 21 ++++++++++----------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 47456ced..3db9345d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3039,7 +3039,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.254.83" +version = "0.254.84" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index 01be5e8e..f58eb43b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ edition = "2021" # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.254.83" +version = "0.254.84" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over # it, so silent wrapping in release would turn a length bug into a memory/logic hazard. diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index 0f7fb1d9..f7751f31 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -323,18 +323,17 @@ pub const DELEGATED_CONTROL_METHODS: &[&str] = &[ /// tests stayed green because a method absent from the contract is absent from both sides of every /// comparison they make. Granting the ordinary tier is now a reviewable one-line edit to this list /// instead of a side effect of editing an unrelated one. -/// `control.wallet.resetCoinDb` (dig-node#384) is listed because the contract has not published it -/// yet, and BOTH consequences of listing it were weighed rather than inherited: the conformance -/// gate tolerates the publish drift, and the method keeps the PAIRED tier. The second is the one -/// that matters, and it is the intended answer — the DIG App drives this reset and holds a paired -/// token, so master-tiering it would make the feature unreachable by its only consumer. It is -/// destructive, and what bounds it is loopback-only + a token + `confirm: true` on the wire + a -/// refusal while a spend is in flight, not tier alone. -/// -/// **Remove this entry the moment `dig-node-control-interface` publishes the method** — the +/// `control.wallet.resetCoinDb` (dig-node#384) is no longer here: `dig-node-control-interface` +/// 0.32 published it (`ControlMethod::WalletResetCoinDb`), and its own `requires_master_token()` +/// returns `false` — the same PAIRED tier this exemption used to grant by name, now delegated to +/// the contract instead of carved out locally. That is the intended end state this list exists to +/// reach: the DIG App still drives the reset with a paired token, unaffected by the move, because +/// [`requires_master_token_given`] consults the published contract FIRST and this list only ever +/// widens the master set, never narrows it. +/// +/// **Remove an entry the moment `dig-node-control-interface` publishes its method** — the /// `the_unpublished_list_still_describes_real_drift` test fails until it is. -pub const KNOWN_UNPUBLISHED_CONTROL_METHODS: &[&str] = - &["control.peers.ping", "control.wallet.resetCoinDb"]; +pub const KNOWN_UNPUBLISHED_CONTROL_METHODS: &[&str] = &["control.peers.ping"]; /// Does this control method require the MASTER control token, never a paired one? PURE. ///