diff --git a/Cargo.lock b/Cargo.lock index 252ae108..4a404a7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3038,7 +3038,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.254.82" +version = "0.254.83" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index d0d58869..01be5e8e 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.82" +version = "0.254.83" # 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/SPEC.md b/SPEC.md index 729c9c87..de236ca0 100644 --- a/SPEC.md +++ b/SPEC.md @@ -9598,6 +9598,15 @@ UDP flow. Since the node prefers the relay tier, that is the answer it gets. Thi into a coin permanently with collateral behind it, so it takes NC-12's discipline: sources are untrusted and must AGREE, never trusted individually. Readings that disagree corroborate neither. +**`dig.getNetworkInfo`'s `reflexive_addr` field carries provenance, because agreement cannot be +checked without it.** The node MUST publish `null` when no STUN tier has ever answered — never a +fabricated, stale, or last-known value, since a visible `null` is harmless and a wrong address is +not. Once a tier has answered, the node MUST publish a JSON array of one object per reading, each +naming its reporting tier as `source` and the mapping as `addr` (`[{"source": "relay", "addr": +"203.0.113.7:9444"}]`). A bare string or a bare list of strings MUST NOT be used for a reading the +node wants eligible for corroboration: neither carries a reporter identity, so two such entries are +indistinguishable from one reporter repeating itself, and can never satisfy the paragraph above. + **The address FAMILY MUST NOT be a rejection criterion.** That defect is an address-family CROSSING, not an IPv6 one: the same server answers an IPv6 caller correctly. IPv6 is both the working case and the §5.2-preferred one, so a rule distrusting IPv6 answers would discard correct discovery while diff --git a/crates/dig-node-core/src/lib.rs b/crates/dig-node-core/src/lib.rs index 2369f3be..80fb67a0 100644 --- a/crates/dig-node-core/src/lib.rs +++ b/crates/dig-node-core/src/lib.rs @@ -4312,26 +4312,47 @@ impl Node { } /// `dig.getNetworkInfo` — this node's own network posture: its `peer_id`, network id, listen - /// address, candidate addresses, reachability, and relay-reservation state. Reads the shared - /// [`peer::PeerStatus`] so it reflects the live pool/relay state (or "not running" in the FFI - /// path). Never touches the chain or an upstream. + /// address, candidate addresses, discovered reflexive address, reachability, and + /// relay-reservation state. Reads the shared [`peer::PeerStatus`] so it reflects the live + /// pool/relay/reflexive state (or "not running"/`null` in the FFI path, before bring-up, or on a + /// host no STUN tier has ever answered). Never touches the chain or an upstream. pub fn network_info(&self) -> Value { let peer_id = self.peer_id_hex(); let network_id = peer::effective_network_label_from_env(); let genesis = hex::encode(peer::genesis_challenge_from_env()); let endpoint = peer::relay_url_from_env(); let port = peer::peer_port_from_env(); + // This node's own server-reflexive reading, if the peer-network bring-up's STUN walk has + // ever answered (dig-node#567) — `None` on a not-yet-started or relay-less offline node, + // which `reflexive_addr` below reports honestly as `null` rather than guessing. + let reflexive = self.peer_status.reflexive(); // The node's REAL advertised candidate addresses, ordered IPv6-first (ecosystem HARD RULE): - // a routable IPv6 address (when discoverable) precedes the IPv4 fallback. `listen_addr` reports - // the primary (IPv6-preferred) advertised endpoint — a dialable address, NOT the wildcard bind + // a routable IPv6 address (when discoverable) precedes the IPv4 fallback, and the reflexive + // address — when known — leads its family group ahead of the local-only fallback, because it + // is the one a stranger behind a DIFFERENT NAT can actually dial. `listen_addr` reports the + // primary (IPv6-preferred) advertised endpoint — a dialable address, NOT the wildcard bind // address (`[::]` / `0.0.0.0`) the listener binds. (The listener itself binds `[::]` dual-stack; // that wildcard is a bind target, never a dialable candidate to report to peers.) - let candidates = net::advertised_socket_addrs(port, net::advertise_loopback_from_env()); + let candidates = net::advertised_socket_addrs_with_reflexive( + port, + net::advertise_loopback_from_env(), + reflexive.map(|(addr, _)| addr), + ); let candidate_addresses: Vec = candidates.iter().map(|a| a.to_string()).collect(); let listen = candidate_addresses .first() .cloned() .unwrap_or_else(|| format!("[::]:{port}")); + // The shape `PublicAddress::from_network_info` (dig-node-service) reads back out: `null` + // when nothing has answered, else a ONE-element array naming the reporting tier as `source` + // — never a bare string or a bare list, both of which that adapter reads as carrying NO + // provenance and therefore incapable of ever corroborating a second, independent reading + // (dig-node#566). Fail closed: a `null` here is visible and harmless; inventing a value + // this node never actually measured is not — see dig-node#567. + let reflexive_addr = match reflexive { + Some((addr, source)) => json!([{ "source": source, "addr": addr.to_string() }]), + None => Value::Null, + }; let snap = self .peer_status .snapshot_json(&endpoint, &network_id, &genesis); @@ -4349,7 +4370,7 @@ impl Node { // (#1372). Byte-identical to the canonical mainnet genesis when unconfigured. "genesis": genesis, "listen_addr": listen, - "reflexive_addr": Value::Null, + "reflexive_addr": reflexive_addr, "candidate_addresses": candidate_addresses, "reachability": reachability, "relay": snap["relay"], @@ -16791,4 +16812,68 @@ mod tests { } } } + + /// Before any STUN tier has ever answered, `reflexive_addr` MUST stay `null` — the fail-closed + /// default dig-node#567 requires (a fabricated or last-known value here would be staked into a + /// mirror coin's memo on chain, so guessing is strictly worse than reporting nothing). + #[test] + fn network_info_reports_null_reflexive_addr_before_any_stun_tier_answers() { + let (node, _td) = test_node(Some([6u8; 32])); + let info = node.network_info(); + assert_eq!( + info["reflexive_addr"], + Value::Null, + "an undiscovered reflexive address must read as null, never a guess: {info}" + ); + } + + /// Once the peer-network bring-up's STUN walk answers, the reading MUST reach BOTH surfaces + /// dig-node#567 names: `reflexive_addr` (what the mirror-advertise pass parses back out via + /// `PublicAddress::from_network_info`) and `candidate_addresses` (what `dign network-info` + /// renders as `candidates:` — the exact line the bug was measured against on a real host). + /// + /// `reflexive_addr` is asserted by STRUCTURE, not merely "is not null": a bare string or a bare + /// list would also read as "populated" but carries no provenance, and + /// `PublicAddress::from_network_info` treats that as un-corroboratable — silently defeating the + /// ticket's requirement that the source travel with the address. Only the one-element + /// `[{"source", "addr"}]` shape satisfies both surfaces at once. + #[test] + fn network_info_publishes_a_discovered_reflexive_address_with_its_source_and_folds_it_into_candidates( + ) { + let (node, _td) = test_node(Some([7u8; 32])); + // A documentation-range address (RFC 5737) so it can never collide with a real address this + // test host happens to own — the assertion below must hold on every machine, not just one + // lucky one. + let discovered: std::net::SocketAddr = "203.0.113.7:9444".parse().unwrap(); + node.peer_status.set_reflexive(discovered, "relay"); + + let info = node.network_info(); + + let reflexive = info["reflexive_addr"] + .as_array() + .expect("reflexive_addr must be an array once a tier has answered"); + assert_eq!( + reflexive.len(), + 1, + "exactly one reading has been recorded: {info}" + ); + assert_eq!(reflexive[0]["source"], json!("relay"), "{info}"); + assert_eq!( + reflexive[0]["addr"], + json!(discovered.to_string()), + "{info}" + ); + + let candidates: Vec = info["candidate_addresses"] + .as_array() + .expect("candidate_addresses array") + .iter() + .map(|v| v.as_str().unwrap().parse().expect("a socket addr")) + .collect(); + assert!( + candidates.contains(&discovered), + "the discovered reflexive address must reach the candidate list an operator reads via \ + `dign network-info`, not only the new field: {candidates:?}" + ); + } } diff --git a/crates/dig-node-core/src/peer.rs b/crates/dig-node-core/src/peer.rs index a0316301..891d574f 100644 --- a/crates/dig-node-core/src/peer.rs +++ b/crates/dig-node-core/src/peer.rs @@ -221,6 +221,21 @@ pub struct PeerStatus { peer_id: std::sync::Mutex>, /// The most recent peer-network error (best-effort diagnostics). last_error: std::sync::Mutex>, + /// This node's discovered server-reflexive address — the NAT mapping of its dig-peer socket a + /// stranger actually dials — paired with the label of whichever STUN tier reported it + /// ([`crate::net::StunSource::label`]). `None` is a real, standing state, not an unset default: + /// no tier has answered (yet, or ever, on a relay-less offline host), and `dig.getNetworkInfo` + /// must say so rather than guess (dig-node#567). + /// + /// Set ONCE, by [`Self::set_reflexive`] from the peer-network bring-up's STUN walk + /// (`StunPlan::discover_reflexive`), and never cleared: there is no periodic re-probe today, so + /// clearing it on some other signal would trade a real reading for a worse one — an + /// unconditional `None` — rather than a better one. Downstream reachability-over-time is a + /// SEPARATE fact already tracked by `relay_reserved` above; this field only ever answers "what + /// did the STUN walk see". Whether that reading may be STAKED on (agreement with a second + /// source, global routability, a currently-held path) is a mirror-crate concern applied + /// downstream, never decided here. + reflexive: std::sync::Mutex>, } impl PeerStatus { @@ -274,6 +289,23 @@ impl PeerStatus { *self.last_error.lock().unwrap() = Some(error); } + /// Record this node's discovered server-reflexive address and the label of whichever STUN + /// tier reported it (`StunSource::label()`), called once from the peer-network bring-up after + /// `StunPlan::discover_reflexive` answers. A later call overwrites the reading — the bring-up + /// runs this exactly once today, so overwriting vs. first-write-wins is not yet a live + /// question, but overwrite is the correct choice if a second caller is ever added: the newer + /// STUN transaction is the better measurement of the node's CURRENT mapping. + pub fn set_reflexive(&self, addr: std::net::SocketAddr, source: &'static str) { + *self.reflexive.lock().unwrap() = Some((addr, source)); + } + + /// This node's discovered server-reflexive address and its reporting tier, or `None` when no + /// STUN tier has ever answered. Read by [`crate::Node::network_info`] to populate + /// `reflexive_addr` and to fold the address into the advertised candidate set (dig-node#567). + pub fn reflexive(&self) -> Option<(std::net::SocketAddr, &'static str)> { + *self.reflexive.lock().unwrap() + } + /// Whether the peer network is running. pub fn is_running(&self) -> bool { self.running.load(Ordering::Relaxed) @@ -2683,6 +2715,11 @@ async fn run_peer_network(node: Arc) -> Result<(), String> { .map(|d| d.server) .or_else(|| stun_plan.primary()); if let Some(d) = stun_discovery { + // Publish the reading onto the shared status so `dig.getNetworkInfo`'s `reflexive_addr` + // stops reporting a hard-coded `null` once something has actually answered (dig-node#567) — + // the whole reason a discovered address never reached a mirror-bond advertisement despite + // #561 shipping the discovery itself. + status.set_reflexive(d.addr, d.source.label()); println!( "dig-node peer network: STUN server for reflexive discovery: {} (source: {})", d.server, diff --git a/crates/dig-node-service/src/mirror/advertise.rs b/crates/dig-node-service/src/mirror/advertise.rs index 2da78c7e..d569768d 100644 --- a/crates/dig-node-service/src/mirror/advertise.rs +++ b/crates/dig-node-service/src/mirror/advertise.rs @@ -275,9 +275,10 @@ impl PublicAddress { /// Reads one pass's view out of `dig.getNetworkInfo`'s answer. /// /// The `reflexive_addr` key is accepted in three shapes, and anything that does not parse is - /// dropped. That tolerance is deliberate: the key is hard-coded `null` in `dig-node-core` today - /// (`dig_ecosystem#3198` is adding the producer), so this adapter is written against a shape - /// that does not exist yet, and it must not dictate one to the lane building it. + /// dropped. `dig-node-core` publishes the `[{"source", "addr"}]` shape once a STUN tier has + /// answered (dig-node#567); the tolerance for the other two shapes stays regardless, since a + /// future producer — or a hand-built test fixture — is still free to use them, and every one + /// of the three is handled identically here: only the named-source array can corroborate. /// /// | shape | read as | /// |---|---| @@ -481,7 +482,9 @@ impl Effective { /// of every already-configured node exactly as it shipped. /// 3. **No known public address is reported BEFORE the liveness gate**, because it is the more /// fundamental answer and it is the one true on a node whose relay is held but reports no -/// reflexive address — the state every host is in until `dig_ecosystem#3198` lands. +/// reflexive address — still reachable on a relay-less/offline host, or one whose STUN walk +/// (`dig_ecosystem#3198`/#561) has never answered (dig-node#567 wires that discovery into +/// `dig.getNetworkInfo`; it does not guarantee a tier answers). pub fn effective_urls(operator: &Advertised, address: &PublicAddress) -> Effective { if operator.can_advertise() { return Effective { @@ -1210,9 +1213,10 @@ mod tests { /// No reflexive address at all means no advertisement — and the reason names the ADDRESS, never /// the operator's configuration. /// - /// This is every host's state until `dig_ecosystem#3198` lands a producer, so the sentence it - /// yields is the one an operator actually reads today. Telling them to configure something - /// would send them to a remedy that cannot work. + /// This remains a real state on a relay-less/offline host, or one whose STUN walk has never + /// answered (dig-node#567 wires the discovery `dig_ecosystem#3198`/#561 already produces into + /// `dig.getNetworkInfo`; it does not make discovery infallible). Telling an operator in that + /// state to configure something would send them to a remedy that cannot work. #[test] fn no_known_address_advertises_nothing_and_blames_the_address_not_the_operator() { let unknown = PublicAddress { @@ -1404,11 +1408,11 @@ mod tests { /// The adapter reads the snapshot `dig.getNetworkInfo` actually returns, in all three shapes. /// - /// `reflexive_addr` is hard-coded `null` in `dig-node-core` today, so the null case is the - /// SHIPPED one and the rest are written against the shape `dig_ecosystem#3198` will produce. - /// The producer does not exist yet to settle which, so all three are accepted and the two that - /// carry no provenance can never corroborate — a bare list is one reporter repeating itself, - /// not two reporters agreeing. + /// `null` remains a real, shipped state (a relay-less/offline host, or one no STUN tier has + /// ever answered); `dig-node-core` publishes the named-source array once a tier does answer + /// (dig-node#567). All three shapes stay accepted here regardless, and the two that carry no + /// provenance can never corroborate — a bare list is one reporter repeating itself, not two + /// reporters agreeing. #[test] fn the_network_info_adapter_reads_the_address_the_provenance_and_the_relay() { let null = serde_json::json!({ diff --git a/crates/dig-node-service/tests/mirror_advertised_urls.rs b/crates/dig-node-service/tests/mirror_advertised_urls.rs index 551633de..1ceb1fb9 100644 --- a/crates/dig-node-service/tests/mirror_advertised_urls.rs +++ b/crates/dig-node-service/tests/mirror_advertised_urls.rs @@ -27,9 +27,11 @@ //! //! # The addresses here are SYNTHETIC, and that is the design //! -//! `dig-node-core` hard-codes `reflexive_addr` to `null` (`dig_ecosystem#3198` is adding the -//! producer), so no real host can supply one yet. What is under test is the plumbing between an -//! address and the coin, which a supplied address exercises exactly. +//! `dig-node-core` now publishes a real `reflexive_addr` once a STUN tier answers (dig-node#567), +//! but this file still hands `effective_urls_from_env` a hand-built [`PublicAddress`] rather than a +//! live one: what is under test is the plumbing between an address and the coin, which a supplied, +//! DETERMINISTIC address exercises exactly, without depending on a real STUN round trip succeeding +//! in CI. mod support; @@ -240,10 +242,10 @@ fn own_peer_id() -> String { /// A node that knows its public address and holds a path to the network. /// -/// The address is SYNTHETIC and that is deliberate: `dig-node-core` hard-codes `reflexive_addr` to -/// `null` (`dig_ecosystem#3198` is adding the producer), so no real host can supply one yet. The -/// composition under test is the plumbing between the address and the coin, which is exactly what a -/// supplied address exercises. +/// The address is SYNTHETIC and that is deliberate: a hand-built [`PublicAddress`] is a +/// deterministic fixture, independent of whether a real STUN round trip succeeds on the machine +/// running this test. The composition under test is the plumbing between the address and the coin, +/// which is exactly what a supplied address exercises. /// TWO sources, because one is never enough: `relay.dig.net` answers STUN with its own load /// balancer's address (`relay.dig.net#11`), well-formed every time, and dig-node prefers the /// relay tier — so a single-source fixture would model the one case that must never reach a coin. @@ -415,10 +417,12 @@ fn the_derived_address_reaches_the_coin_beside_the_peer_declaration() { /// **A node that does not know its public address creates NOTHING, and says why in the operator's /// own terms.** /// -/// This is every host's state today — `reflexive_addr` is hard-coded `null` — so the sentence this -/// produces is the one an operator actually reads. The assertion is on the WORDING as much as on -/// the refusal: a node that cannot know its own address has no configuration to fix, and sending -/// them to `DIG_MIRROR_ADVERTISE_URLS` would be sending them to a remedy that cannot work. +/// `reflexive_addr` still reports `null` on a relay-less/offline host, or before the peer-network +/// bring-up's STUN walk has ever answered (dig-node#567) — a real, reachable state, not merely a +/// startup default — so the sentence this produces is one an operator can actually read. The +/// assertion is on the WORDING as much as on the refusal: a node that cannot know its own address +/// has no configuration to fix, and sending them to `DIG_MIRROR_ADVERTISE_URLS` would be sending +/// them to a remedy that cannot work. #[test] fn no_public_address_creates_nothing_and_does_not_blame_the_operators_configuration() { let dir = tempfile::tempdir().expect("a temp dir"); @@ -435,7 +439,8 @@ fn no_public_address_creates_nothing_and_does_not_blame_the_operators_configurat let runtime = tokio::runtime::Runtime::new().expect("a tokio runtime"); // No operator value AND no address: `PublicAddress::default()` is exactly what - // `from_network_info` reads off the shipped `dig.getNetworkInfo` answer today. + // `from_network_info` reads off a `dig.getNetworkInfo` answer on a host no STUN tier has + // ever answered (a relay-less/offline node, or one still mid-bring-up). let advertised = with_advertise_env("", || effective_urls_from_env(&PublicAddress::default())); assert_eq!( advertised.state,