diff --git a/crates/dig-node-core/src/lib.rs b/crates/dig-node-core/src/lib.rs index c30c1d58..53728f35 100644 --- a/crates/dig-node-core/src/lib.rs +++ b/crates/dig-node-core/src/lib.rs @@ -582,15 +582,16 @@ pub struct Node { /// /// A slot rather than a constructor argument for the same reason [`Node::mirror_pointers`] is /// one: the FFI/browser path has no state directory and must keep constructing a `Node` - /// without one. Nothing installs it in production yet — nothing in dig-node funds a - /// distributor today (`rewards::port`'s module doc, blocker 2). WHICH ticket owns the startup - /// wiring that would call [`Node::install_funded_distributor_registry`] with the node's state - /// directory is tracked separately, and it is NOT dig_ecosystem#3268, whose scope is the claim - /// loop and `ClaimStatus` and which names neither this registry nor that call. Until a ticket - /// wires it the slot stays empty, and + /// without one. `dig-node-service`'s startup path installs a real, state-dir-backed registry + /// (dig_ecosystem#3292); until that install runs (e.g. a harness with `enable_chain_sync: + /// false`, or the FFI/browser path) the slot stays empty and /// [`Node::funded_distributors_read`] answers /// [`rewards::funded::NotConfiguredReason::NoStateDirectory`] — UNKNOWN, deliberately never an - /// empty funded set. + /// empty funded set. Even once installed, the registry starts with no record on disk (writing + /// one is dig_ecosystem#3291, a separate operator-declaration ticket — the node cannot observe + /// its own funding because funding spends from a wallet it never holds), so a fresh production + /// node reads [`rewards::funded::NotConfiguredReason::NoRecordWritten`] — still UNKNOWN, by + /// design, not a defect of the installer. funded_distributors: OnceLock, /// The chain seam `dig.getRewardDistributor` / `dig.listRewardDistributorCommitments` /// (dig_ecosystem#3269 units 1-2) read through — [`rewards::port::RewardsChainPort`]. @@ -642,13 +643,12 @@ impl Node { /// if a registry is already installed, in which case NOTHING changed — a second install must /// not be able to swap a live registry for an inert one behind a caller's back. /// - /// Called from tests today: no production startup path installs one, so clippy's non-test - /// lib target sees no production caller and `allow(dead_code)` stands in for it. Remove the - /// attribute when that wiring lands. Its owning ticket is tracked separately and is NOT - /// dig_ecosystem#3268 (claim loop + `ClaimStatus`), which names neither this registry nor this - /// call — do not read the attribute as a claim about #3268's scope. - #[cfg_attr(not(test), allow(dead_code))] - pub(crate) fn install_funded_distributor_registry( + /// `pub` because this is the INJECTION POINT, mirroring + /// [`Node::install_reward_chain_port`]: `dig-node-service`'s startup path builds a + /// state-dir-backed registry and installs it here (dig_ecosystem#3292). Being callable from + /// outside does NOT relax the single-install discipline: a second install still returns + /// `false` and changes nothing. + pub fn install_funded_distributor_registry( &self, registry: rewards::funded::FundedDistributorRegistry, ) -> bool { @@ -9488,6 +9488,77 @@ mod tests { } } + /// **Proves:** dig_ecosystem#3280 — a malformed `launcher_id` is refused with `-32602`, the + /// same validator `dig.getRewardDistributor` already uses (`parse_launcher_id_arg`), instead + /// of silently filtering to an empty `items` list that reads exactly like "I looked and found + /// nothing" (SPEC §12.5 clause 6's "reassuring zero", on the input side rather than the read + /// side). Registers a handle first so a filter bug that matches everything cannot pass this + /// test by accident: the malformed request must be refused before any filter runs. + /// **Catches:** a malformed `launcher_id` degrading to `Consulted { items: [] }`. + #[test] + fn get_reward_prover_status_with_a_malformed_launcher_id_is_refused() { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let (node, _td) = test_node(None); + node.register_reward_prover_status(crate::rewards::state::StatusHandle::new( + sample_reward_prover_status([0x11u8; 32]), + )); + + let resp = rt.block_on(handle_rpc( + &node, + json!({ + "jsonrpc":"2.0","id":1,"method":"dig.getRewardProverStatus", + "params": {"launcher_id": "zz"} + }), + crate::download::ReadOrigin::Local, + crate::download::RequestProvenance::FirstParty, + )); + + assert_eq!( + resp["error"]["code"], + json!(-32602), + "a malformed launcher_id must be refused, not filtered to an empty list: {resp}" + ); + assert!( + resp.get("result").is_none(), + "an error response must carry no result: {resp}" + ); + } + + /// **Proves:** a well-formed but UNKNOWN `launcher_id` still answers `consulted` with an + /// empty `items` list — distinct from the malformed case above, which must be `-32602`. + /// **Catches:** widening the malformed-input refusal to also swallow legitimate misses. + #[test] + fn get_reward_prover_status_with_an_unknown_well_formed_launcher_id_is_empty_not_refused() { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let (node, _td) = test_node(None); + node.register_reward_prover_status(crate::rewards::state::StatusHandle::new( + sample_reward_prover_status([0x11u8; 32]), + )); + + let resp = rt.block_on(handle_rpc( + &node, + json!({ + "jsonrpc":"2.0","id":1,"method":"dig.getRewardProverStatus", + "params": {"launcher_id": hex::encode([0x99u8; 32])} + }), + crate::download::ReadOrigin::Local, + crate::download::RequestProvenance::FirstParty, + )); + + assert_eq!(resp["result"]["statuses"]["outcome"], json!("consulted")); + assert_eq!( + resp["result"]["statuses"]["items"], + json!([]), + "an unknown but well-formed id is a legitimate empty answer, not an error: {resp}" + ); + } + /// **Proves:** with nothing registered, `dig.getRewardProverStatus` answers /// `{"statuses": []}` — SPEC §2.4 clause 1's "not distributing" render — never blank, `null`, /// or an omitted `result`. **Catches:** an absent-record case that renders as nothing rather diff --git a/crates/dig-node-core/src/rewards/funded.rs b/crates/dig-node-core/src/rewards/funded.rs index 8f4ea39e..b19afb3b 100644 --- a/crates/dig-node-core/src/rewards/funded.rs +++ b/crates/dig-node-core/src/rewards/funded.rs @@ -176,7 +176,6 @@ impl FundedDistributorRegistry { /// A registry persisting to `dir`. The directory is created on first write, not here, so /// constructing one is infallible and side-effect free. - #[cfg_attr(not(test), allow(dead_code))] #[must_use] pub fn with_state_dir(dir: &Path) -> Self { Self { @@ -201,7 +200,6 @@ impl FundedDistributorRegistry { /// [`FundedDistributorsRead::PersistedStateCorrupt`] until a human resolves it. That is the /// same "leave the corrupt file exactly as it is on disk" posture /// `rewards_claim::engine::ClaimEngine::persist_fee_window` takes, plus a forensic copy. - #[cfg_attr(not(test), allow(dead_code))] #[must_use] pub fn read(&self) -> FundedDistributorsRead { let Some(path) = self.record_path() else { @@ -819,8 +817,12 @@ mod tests { ); } - /// **Catches:** a future variant added to the not-an-answer half of - /// [`FundedDistributorsRead`] that `determined` reports as a renderable set. + /// **Catches:** a regression on the not-an-answer half of [`FundedDistributorsRead`] listed + /// below reporting a renderable set. It does NOT by itself catch a future variant added to the + /// enum — that would need adding here too. The guard that actually forces the issue is + /// [`FundedDistributorsRead::determined`]'s wildcard-free match (this module, above): a new + /// variant left unhandled there fails to COMPILE, which is what makes adding a variant without + /// updating this array a build error rather than a silent gap. #[test] fn every_not_an_answer_outcome_is_undetermined() { let undetermined = [ diff --git a/crates/dig-node-core/src/seams/dig_rpc/dispatch.rs b/crates/dig-node-core/src/seams/dig_rpc/dispatch.rs index 256d08f2..4f1ecf55 100644 --- a/crates/dig-node-core/src/seams/dig_rpc/dispatch.rs +++ b/crates/dig-node-core/src/seams/dig_rpc/dispatch.rs @@ -922,10 +922,21 @@ impl RpcDispatch for Node { // mapped explicitly by `reward_prover_status_to_wire`. Some(Method::GetRewardProverStatus) => { let params = req.get("params").cloned().unwrap_or(json!({})); - let filter_launcher_id = params - .get("launcher_id") - .and_then(Value::as_str) - .map(str::to_ascii_lowercase); + // dig_ecosystem#3280: `launcher_id` is OPTIONAL here (unlike + // `GetRewardDistributor`'s required param), so absence is "no filter" and is not + // itself an error. A PRESENT value goes through the same validator + // `GetRewardDistributor` (below) and `ListRewardDistributorCommitments` already + // use, so a malformed value is refused with `-32602` instead of silently + // filtering to an empty list — the same "reassuring zero" defect this epic exists + // to kill, just on the input side rather than the read side. + let filter_launcher_id: Option<[u8; 32]> = if params.get("launcher_id").is_some() { + match parse_launcher_id_arg(¶ms) { + Ok(id) => Some(id), + Err(msg) => return rpc_err(&id, -32602, &msg), + } + } else { + None + }; let snapshots = node.reward_prover_status_snapshots(); // dig_ecosystem#3269 fix939: a zeroed `launcher_id` or `store_id` is never a real // distributor's or module's IDENTITY — see `is_missing_identity`/`zeroed_fields`. @@ -980,7 +991,7 @@ impl RpcDispatch for Node { true }) .filter(|s| match &filter_launcher_id { - Some(want) => hex::encode(s.launcher_id).eq_ignore_ascii_case(want), + Some(want) => &s.launcher_id == want, None => true, }) .map(reward_prover_status_to_wire) diff --git a/crates/dig-node-service/src/server.rs b/crates/dig-node-service/src/server.rs index e8543ba3..c4a9edb2 100644 --- a/crates/dig-node-service/src/server.rs +++ b/crates/dig-node-service/src/server.rs @@ -2227,6 +2227,27 @@ where // node in the network — a correct answer, and a permanently unchanging one. bring_up_collateral_records(); + // This node's funder-ownership registry (dig_ecosystem#3285), installed against the same + // hardened state dir the control token lives in (`state.state_dir`, resolved above). Unlike + // the chain-reading installs below, this is pure local state — no chain source, no + // `enable_chain_sync` gate — so it installs unconditionally, including under an integration + // harness with sync disabled. Until this call existed nothing installed a registry in any + // shipped binary, so `dig.listRewardDistributors`'s `funded` half answered `not_consulted` on + // every production node regardless of what an operator had funded (dig_ecosystem#3292). + // `install_funded_distributor_registry`'s `OnceLock` means a second call here (there is none) + // would simply be refused, not double-installed. The registry itself starts EMPTY on a fresh + // node — no record on disk until dig_ecosystem#3291's writer runs — so the correct read right + // after this call is `NotConfigured(NoRecordWritten)`, not a funded set; that is success, not + // a bug. + if !state.node.install_funded_distributor_registry( + dig_node_core::rewards::funded::FundedDistributorRegistry::with_state_dir(&state.state_dir), + ) { + tracing::warn!( + "install_funded_distributor_registry declined a second install: a funder-ownership \ + registry was already installed on this Node" + ); + } + // The CENSUS half (#400). `bring_up_collateral_records` writes epoch 1, which is derivable // from nothing; this is what lets the node record epoch n. It runs detached and on a timer // because a census depends on the chain having moved: an epoch that has begun by the clock is diff --git a/crates/dig-node-service/tests/funded_distributor_registry_startup_3292.rs b/crates/dig-node-service/tests/funded_distributor_registry_startup_3292.rs new file mode 100644 index 00000000..c6ce54db --- /dev/null +++ b/crates/dig-node-service/tests/funded_distributor_registry_startup_3292.rs @@ -0,0 +1,219 @@ +//! dig_ecosystem#3292: `Node::install_funded_distributor_registry` has a real, non-test caller. +//! +//! Before this ticket the installer was `pub(crate)`, so `dig-node-service` -- the crate that +//! owns `state.state_dir` and constructs the real `Node` -- could not call it at all. This file +//! does not compile against the pre-fix source (a `pub(crate)` method is invisible to this +//! integration-test crate, a separate compilation unit), which is this ticket's RED. + +use dig_node_core::rewards::funded::{FundedDistributorsRead, NotConfiguredReason}; +use dig_node_core::Node; + +/// Mirrors `rewards_chain_port_a3.rs`'s `install_reward_chain_port_refuses_a_second_install` +/// shape for the funder-ownership registry: install from a tempdir, once-only, and the read +/// after install is the correct "no record written yet" UNKNOWN -- never a reassuring empty +/// funded set (dig_ecosystem#3285's whole point; #3291's writer, not this ticket, is what would +/// ever populate it). +#[test] +fn install_funded_distributor_registry_installs_a_tempdir_backed_registry_once() { + let dir = tempfile::tempdir().expect("temp dir"); + let registry = + dig_node_core::rewards::funded::FundedDistributorRegistry::with_state_dir(dir.path()); + + // Sanity on the registry itself, independent of `Node`: a fresh state dir with no record + // file reads `NotConfigured(NoRecordWritten)`, not `FundsNothing` -- the property the + // installer must not collapse. + assert_eq!( + registry.read(), + FundedDistributorsRead::NotConfigured(NotConfiguredReason::NoRecordWritten), + "a fresh state dir with no record file must read as UNKNOWN, never a funded set" + ); + + // `Node` exposes no lighter test constructor to an external integration-test crate -- + // `Node::from_env()` is the same constructor `rewards_chain_port_a3.rs`'s own test uses for + // the identical reason. + let node = Node::from_env(); + + let first_install = node.install_funded_distributor_registry(registry.clone()); + assert!(first_install, "the first install must be accepted"); + + let second_install = node.install_funded_distributor_registry(registry); + assert!( + !second_install, + "the second install must be refused, changing nothing" + ); +} + +/// The slice of a source file before its own `#[cfg(test)]` module -- i.e. what actually ships. +/// Duplicated from `rewards_chain_port_a3.rs`'s identical helper because this integration test is +/// a separate compilation unit and cannot import a private helper from that file. +fn production_region(source: &str) -> &str { + match source.find("#[cfg(test)]") { + Some(test_module_start) => &source[..test_module_start], + None => source, + } +} + +/// The offset of `needle`'s first occurrence in `haystack`, or a panic naming `msg` -- so a +/// deleted call site fails LOUDLY (with a reason) rather than as a silent `None` swallowed by +/// `.unwrap_or`. +fn must_find(haystack: &str, needle: &str, msg: &str) -> usize { + haystack.find(needle).unwrap_or_else(|| panic!("{msg}")) +} + +/// Closes the gap `server_startup_calls_install_funded_distributor_registry_in_production_code` +/// (above) cannot: that test is a pure CONTAINS check, so it is blind to the install call being +/// *moved* rather than deleted. dig_ecosystem#3292 originally shipped with exactly that shape -- +/// the call existed (as a `pub(crate)` no-op nobody drove), just not on a path that ran. The +/// mutation this guards is moving the call inside `if config.enable_chain_sync { .. }`: an +/// integration harness (and every test in this crate) runs with `enable_chain_sync: false` +/// specifically so nothing dials the network, so a call gated behind that flag would silently +/// stop running under the harness AND on any deployment that (for whatever future reason) ships +/// with sync disabled -- reintroducing dig_ecosystem#3292 with a passing `contains(..)` test +/// beside it. Ordering position in the source is the only signal available to a black-box, +/// separate-compilation-unit integration test: see this file's module doc below for why a true +/// runtime distinction between "installed" and "never installed" is not reachable from here. +#[test] +fn install_call_precedes_the_enable_chain_sync_gate_in_production_code() { + let server_source = production_region(include_str!("../src/server.rs")); + + let install_offset = must_find( + server_source, + "install_funded_distributor_registry(", + "server.rs's production startup path must call \ + `Node::install_funded_distributor_registry`, or dig_ecosystem#3292 has regressed \ + (mutation (a): the call was deleted)", + ); + let chain_sync_gate_offset = must_find( + server_source, + "if config.enable_chain_sync {", + "server.rs no longer spells its chain-sync gate as `if config.enable_chain_sync {` -- \ + update this test's needle to match, it is not itself evidence of a regression", + ); + + assert!( + install_offset < chain_sync_gate_offset, + "install_funded_distributor_registry's call site ({install_offset}) is no longer before \ + the `enable_chain_sync` gate ({chain_sync_gate_offset}): it has moved to or past that \ + gate, which means a node/harness running with chain sync disabled would start with NO \ + funder-ownership registry installed -- dig_ecosystem#3292's exact regression shape \ + (mutation (b))" + ); +} + +/// Boots the REAL production startup path (`serve_with_shutdown`, the same function `dig-node +/// run` calls) end to end on an ephemeral loopback port, with chain sync disabled (as every +/// harness must -- see [`must_find`]'s caller's doc). Proves the install call site is reachable +/// and executes without panicking or erroring as part of an actual startup, which +/// `install_funded_distributor_registry_installs_a_tempdir_backed_registry_once` (calling the +/// installer directly, never through `serve_with_shutdown`) cannot: that test would still pass if +/// `server.rs` never called the installer at all. +/// +/// # Why this test cannot ALSO assert "installed, not merely not-panicking" +/// +/// It would if it could. `Node::funded_distributor_registry` / `Node::funded_distributors_read` +/// are `pub(crate)` to `dig-node-core` -- invisible to `dig-node-service` itself, let alone to +/// this separate integration-test compilation unit -- so there is no accessor this test can call. +/// `AppState`'s `node: Arc` field is private with no getter, and `serve_with_shutdown` +/// returns only `io::Result<()>` once shutdown resolves, so no caller outside `server.rs` ever +/// holds the `Node` `serve_with_shutdown` built. +/// +/// The one externally-reachable read, `dig.listRewardDistributors`, does not help either: +/// `dispatch.rs`'s handler matches +/// `FundedDistributorsRead::NotConfigured(_) | PersistedStateCorrupt { .. } | IoFailed { .. }` as +/// ONE wildcarded arm and answers `Half::NotConsulted` for all three, with no `reason` field on +/// the wire. A registry that was never installed reads `NotConfigured(NoStateDirectory)`; one +/// installed against a fresh, empty state dir (exactly what `serve_with_shutdown` produces, since +/// dig_ecosystem#3291's writer does not exist yet) reads `NotConfigured(NoRecordWritten)` -- two +/// DIFFERENT `NotConfiguredReason` variants, genuinely distinguishable at the type level (see +/// `install_funded_distributor_registry_installs_a_tempdir_backed_registry_once`'s assertion on +/// exactly this), but the RPC response for both is byte-for-byte identical +/// `{"funded":{"status":"not_consulted","observed_at":..},"claimable":{..}}`. So today, "installed +/// and empty" and "never installed" are NOT distinguishable from any observer this crate's tests +/// can reach -- not the RPC surface, not the filesystem (the registry writes nothing at +/// construction or install; only a future recorded funding act would), and not a public accessor. +/// Closing that gap needs either a `#[cfg(test)]`-only accessor on `Node`/`AppState` or a `reason` +/// field on the wire response -- both are production-surface changes, out of this ticket's scope +/// (see this file's brief: "do not change production behaviour... stop and ask me first"). +/// +/// So this test proves reachability-without-panic, and +/// `install_call_precedes_the_enable_chain_sync_gate_in_production_code` (above) supplies the +/// actual mutation-(b) RED signal, via source position rather than a runtime read. +#[tokio::test] +async fn production_startup_reaches_the_install_call_site_without_panicking() { + // Serializes with every other test in this crate/process that reads the process-global + // DIG_NODE_CACHE / DIG_NODE_STATE_DIR env vars live (mirrors `tests/server.rs`'s `env_guard`). + static ENV_LOCK: std::sync::OnceLock>> = + std::sync::OnceLock::new(); + let lock = ENV_LOCK + .get_or_init(|| std::sync::Arc::new(tokio::sync::Mutex::new(()))) + .clone(); + let _hold = lock.lock_owned().await; + + let base = tempfile::Builder::new() + .prefix("dig-node-3292-startup-") + .tempdir() + .expect("a scratch dir"); + let cache = base.path().join("cache"); + std::fs::create_dir_all(&cache).expect("create test cache dir"); + std::env::set_var("DIG_NODE_CACHE", &cache); + std::env::set_var("DIG_NODE_CACHE_CAP", "67108864"); + // Isolates the #501 control-token/paired-token state dir -- also where this ticket's + // registry persists (`state.state_dir`) -- so this test never touches a real machine's + // state and no concurrent test shares it. + std::env::set_var("DIG_NODE_STATE_DIR", base.path()); + // Opts out of the §14 peer network bring-up so this stays hermetic (no gossip/DHT/relay + // reach), mirroring `tests/server.rs`'s dual-listener test. + std::env::set_var("DIG_PEER_NETWORK", "off"); + + let free = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind an ephemeral loopback port to learn a free one"); + let port = free.local_addr().expect("local_addr").port(); + drop(free); // release it so serve_with_shutdown can bind the same port + + let config = dig_node_service::Config { + port, + dig_local: false, // skip the privileged :80 attempt entirely + enable_chain_sync: false, // never dial mainnet from this harness (#2501) + ..dig_node_service::Config::default() + }; + + let stop = std::sync::Arc::new(tokio::sync::Notify::new()); + let stop_for_server = stop.clone(); + let server = tokio::spawn(async move { + dig_node_service::server::serve_with_shutdown(config, async move { + stop_for_server.notified().await; + }) + .await + }); + + // Poll /health until the real startup path (including this ticket's install call, which + // runs before any listener binds) has completed and the server is actually serving. + let url = format!("http://127.0.0.1:{port}/health"); + let client = reqwest::Client::new(); + let mut served = false; + for _ in 0..50 { + if let Ok(resp) = client.get(&url).send().await { + if resp.status().is_success() { + served = true; + break; + } + } + tokio::time::sleep(std::time::Duration::from_millis(40)).await; + } + assert!( + served, + "serve_with_shutdown never reached a serving state -- production startup (which includes \ + this ticket's install_funded_distributor_registry call) did not complete cleanly" + ); + + stop.notify_waiters(); + let outcome = tokio::time::timeout(std::time::Duration::from_secs(5), server).await; + let join_result = + outcome.expect("serve_with_shutdown must stop within 5s of the shutdown signal"); + let io_result = join_result.expect("the server task must not panic"); + assert!( + io_result.is_ok(), + "serve_with_shutdown returned an error: {io_result:?}" + ); +}