diff --git a/Cargo.lock b/Cargo.lock index 39fc31eb..bbee5a4a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.252.92" +version = "0.252.97" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index 388a7a6d..ae90e3d5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # 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.252.92" +version = "0.252.97" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over diff --git a/SPEC.md b/SPEC.md index 7c92b464..96493d85 100644 --- a/SPEC.md +++ b/SPEC.md @@ -8674,24 +8674,24 @@ itself (SYSTEM.md §4.1). > by name BEFORE any chain read (dig-node#426). **RECLAIMS are implemented** and are supported at `fee = 0` with > no fee coins, which is §25.4.4 — and are never gated on any funding read, including the > committed-coin read. -> * **§25.10's verification of OTHER peers' claims is BUILT BUT INERT — no claim is verified on a -> running node today.** The mechanism is present and wired: `dig-node-core`'s `mirror_bond` (the +> * **§25.10's verification of OTHER peers' claims is LIVE.** `dig-node-core`'s `mirror_bond` (the > three verdicts and the ranking locator, installed inside `NodeContent::new`) and -> `dig-node-service`'s `mirror/bond_verify.rs` (the chain read, installed on the running node by -> `spawn_bond_verifier_install`). But this node has no sound source for the coin-to-peer binding -> §25.6a requires, so `bonded` is unreachable for every input, the chain read is short-circuited -> before it is paid, and every holder receives the same verdict — the ranking is a no-op on every -> slate. **A reader must not take this bullet as saying collateral is enforced; it is not.** The -> binding is tracked as , and promotion -> becomes reachable the moment it lands, with no further change here. What is verified once it does -> is a peer's claim; this node still attaches no pointer of its own, per the next bullet. -> * **§25.6's DHT pointer is not attached.** `ProviderRecord::unverified_mirror_coin_id` lives in -> dig-dht 0.15, and `dig-download` 0.21.0 and `dig-peer-selector` 0.10.0 both require -> `dig-dht ^0.13` — semver-incompatible on a `0.x` line, so taking 0.15 here would resolve two -> dig-dht lines. Tracked as . +> `dig-node-service`'s `mirror/bond_verify.rs` (the chain read, installed by +> `spawn_bond_verifier_install`) are wired, and `peer_declaration` now reads `dig-mirror-coin` +> 0.8.0's typed `dig-peer:` accessor, so `bonded` is reachable and a promoted holder has proven both +> halves §25.6a requires. **What a reader MUST NOT infer is that any coin on chain carries a +> declaration yet.** The format is new, and a coin acquires one only when its owner recreates it at +> an epoch boundary; until an owner does, that holder is `unverified` and sits at baseline. That is a +> stated degradation and not a lie about enforcement: no honest holder is ranked below where no +> verifier at all would put it. +> * **§25.6's DHT pointer IS attached.** This bullet previously said it was not, blocked on a +> `dig-dht ^0.13` / `0.15` semver split; that split is resolved, `dig-node-core` takes dig-dht 0.15, +> and the announce passes a coin id (`seams/dig_peer/dht.rs:493`, +> `announce_provider_with_collateral`). The production pointer source is +> `mirror/pointers.rs`'s `SnapshotMirrorPointers`, installed at `server.rs:2169`. A reader MUST still +> treat the field as the untrusted claim its name says it is. > -> So a reader MUST NOT infer that any coin is CREATED at this head, and MUST NOT infer that a mirror -> coin id reaches the DHT. +> So a reader MUST NOT infer that any coin is CREATED at this head. > > **A clause not named in the list above MUST be read as pending, whatever its grammatical voice**, > and the list is to be read NARROWLY: where an entry names a file or a function, it satisfies the @@ -9110,18 +9110,74 @@ to an address, and a provider record's `peer_id`-to-address association is unaut that no chain read can settle. Such a record satisfies the declaration check and would be promoted on the strength of somebody else's bond. -Closing that requires a separate restriction, which a node performing promotion MUST apply: promote -only from a record whose `peer_id`-to-address association is itself authoritative — a first-hand -announce from the peer being ranked, not a slate forwarded by a third party — or defer the credit -until the dialled identity has been checked against the claimed `peer_id`. A dialler is not by itself -a backstop, because peer ids are derived from the presented certificate rather than pinned against -the dialled identity; the residual an unrestricted implementation carries is traffic redirection, not -a stolen bond. +Closing that requires the promoted address to be checked against the promoted identity. **A node +performing promotion MUST dial with the claimed `peer_id` PINNED**, refusing the connection when the +certificate the far end presents hashes to anything else. This is the operative remedy here and it +holds: the download path makes a record's own `provider_peer_id` the dial target's pinned id, the +NAT dialler passes it into the TLS client config, and the verifier fails the handshake with +`peer_id mismatch`. So the attacker in the paragraph above buys a refused connection rather than a +redirected reader, and the content a reader does accept is merkle-verified against the root it asked +for regardless. + +**An earlier version of this clause said a dialler is not by itself a backstop, "because peer ids are +derived from the presented certificate rather than pinned against the dialled identity". That was +false**, and is recorded here because a reader reasoning from it would over-trust the alternative. It +generalised one true fact — `dig-gossip`'s legacy rustls outbound does not pin, and every +`expected_peer_id` in that crate is test-only — into a claim about every dial. A node MUST NOT rank on +a declaration if its own transport lacks the pin; a node whose transport has it MUST NOT be required +to do more. + +The alternative that clause offered — promote only from a record whose `peer_id`-to-address +association is authoritative, a first-hand announce rather than a forwarded slate — is **NOT +available at the layer that ranks, and a specification MUST NOT require it as though it were.** A DHT +keeps attributed and hearsay records apart, but flattens them into one untagged list when answering a +lookup, and the records a reader holds for content it actually wants are overwhelmingly the hearsay +ones: the attributed store is populated for the keys a node is closest to, not for what it fetches. A +locator restricted to attributed records would return almost nothing, which degrades discovery +without improving safety the pin already provides. + +What the ranking layer MUST do instead is BOUND the credit: **at most one record is promoted per +claimed `peer_id` in one locate.** The declaration binds a coin to a peer id and cannot distinguish an +honest holder's record from a stranger's copy of it, so without this a single stolen identity could +occupy every promoted slot on the strength of one bond. A record refused promotion by this bound +takes the baseline tier it would have had with no verifier at all, never below it — the credit-only +rule above is not weakened by it. **One locate is bounded work.** The size of a located set is chosen by whoever answered the lookup, so a node MUST bound the number of bonds it reads against a chain per locate, verifying in source order and leaving the remainder at baseline. +**A per-locate bound is not by itself sufficient, and a node MUST NOT rely on one.** The gate that +admits a locate is per-requestor over self-minted identities, so it bounds nothing in aggregate: an +adversary spreads its locates across fresh identities and multiplies the per-locate ceiling by as +many as it cares to mint. A node performing promotion therefore MUST additionally hold an +AGGREGATE bound on the chain reads promotion causes, and that bound MUST be consulted BEFORE the +chain is touched, so that a refused claim costs nothing. + +Two limits are required, because they answer two different attacks: + +- a **process-wide** budget on verifications, which bounds this node's total outbound read volume + however many identities the traffic is spread across. It is process-wide rather than per-verifier + because the resource being protected — the node's shared chain client — is a property of the + process, so any narrower scope would be several budgets against one resource; +- a **per-claimant** limit on DISTINCT coin ids the claimant has caused reads for without ever + proving a bond, which bounds the fabricated-coin-id case. A coin id the claimant has already been + read about MUST NOT count again: that is a cache entry expiring and being refreshed, not a new + question. A claimant that proves a bond MUST have this ledger forgiven; the process-wide budget + MUST NOT be refunded, since a proven bond says the claimant is honest and says nothing about this + node's chain access. + +Exhausting either limit yields `unverified` having read nothing. **This is a degradation and never a +refusal of service**: `unverified` and `unbonded` share a rank, the sort is stable, and the located +slate is returned unchanged — precisely the behaviour of a node with no verifier installed. The read +path is never blocked and no holder is ever ranked below where it started, so an adversary who +exhausts the budget denies promotion, not content. + +Both limits are keyed on the claiming peer id LOWERCASED before use. A peer id is fixed-length hex, +so its two spellings denote one identity, and every check that GRANTS promotion already treats them +as one — the coin's declaration compares decoded bytes and the TLS pin compares certificate-hash +bytes. A limit keyed on the raw wire text would hand a stranger a fresh allowance per spelling. + **A `bonded` verdict MUST rest on AGREEMENT across independently drawn, concurrently-held untrusted peers -- never on one source.** The §25.6 checks establish that a coin and its creating spend are internally consistent; none of them establishes that the coin was ever on chain. A coin currying the @@ -9168,8 +9224,18 @@ to be remembered. Only DEFINITE verdicts are cached: `unverified` records this node's own momentary inability to look, and holding it would keep an outage in force after it had ended. The cache is keyed partly on attacker-chosen input, so it MUST -be bounded, and overflow MUST evict rather than clear — clearing would let a stranger discard every -verdict a node has earned by rotating coin ids. +be bounded, and overflow MUST NOT clear — clearing would let a stranger discard every verdict a node +has earned by rotating coin ids. + +**Nor may overflow evict indiscriminately.** Reclaiming already-expired entries first is always +correct. But when the cache is full of LIVE entries, admitting a new one costs an honest verdict, and +only `bonded` is worth that trade: `bonded` is expensive to obtain — it requires a coin that exists, +is fully collateralised, and declares its claimant — whereas `unbonded` is the verdict a stranger +elicits for FREE by naming coin ids that do not exist. A policy of evicting one arbitrary entry per +insert reads as conservative but is paced by the attacker, who at a chosen insert rate turns the +whole cache over in well under a minute and discards every earned verdict. An `unbonded` MUST +therefore be refused admission to a cache full of live entries rather than allowed to displace a +`bonded`. ### 25.7. Consent, the switch, and revocation diff --git a/crates/dig-node-core/src/lib.rs b/crates/dig-node-core/src/lib.rs index 827bdb0b..6194f15f 100644 --- a/crates/dig-node-core/src/lib.rs +++ b/crates/dig-node-core/src/lib.rs @@ -4947,6 +4947,15 @@ impl Node { ) -> Option> { self.mirror_pointers.get().cloned() } + + /// This node's own `peer_id`, once its peer network has started. + /// + /// The mirror lifecycle needs it to write the peer declaration into a coin it creates: a coin + /// that names no peer bonds content for nobody in particular and can never be promoted, so a + /// node creating one is paying collateral for a claim no reader can credit to it. + pub fn own_peer_id(&self) -> Option { + self.peer_status.peer_id() + } } /// The COMPOSITION-ROOT upcasts (#1285 W1c — the locked "Option A" shape). `Node` stays ONE diff --git a/crates/dig-node-core/src/mirror_bond.rs b/crates/dig-node-core/src/mirror_bond.rs index 1741474d..ed059cef 100644 --- a/crates/dig-node-core/src/mirror_bond.rs +++ b/crates/dig-node-core/src/mirror_bond.rs @@ -5,19 +5,36 @@ //! at no cost, bonding nothing. Until this module existed nothing anywhere read that field against a //! chain, so the collateral economy's one economic guarantee was unenforced end to end. //! -//! # This layer is INERT today, and that is the safe posture +//! # This layer is LIVE, and what makes promotion sound //! -//! No [`MirrorBondVerifier`] this node can build returns `Bonded` yet, because a coin proves that -//! *a* bond exists and never that the *claimant* holds it, and nothing here can yet bind a mirror -//! coin's owner to a DHT peer id. So every holder receives the same verdict and the ranking below is -//! a no-op on every slate: no reordering, and the host's implementation declines the chain read -//! rather than paying for an answer this module would discard. +//! Promotion requires TWO independent bindings, and neither is sufficient alone. //! -//! Promoting on the chain half alone is the alternative, and it is an attack: coin ids travel the -//! DHT in cleartext by design, so a stranger republishing an honest holder's id would rank first at -//! zero collateral. Withholding credit from everyone is the only posture that is neither exploitable -//! nor expensive. The binding is ; when it lands -//! this module needs no change. +//! 1. **coin -> content**, from `MirrorCoin::advertises`: the coin declares exactly this +//! `(store, root, epoch)`, with the hint recomputed from the coin's own lineage proof. +//! 2. **coin -> peer id**, from the coin's owner-written `dig-peer:` declaration +//! (`dig-mirror-coin` 0.8.0, its `SPEC.md` §5.1). Memos are written by the spend that creates the +//! coin and only the owner's key can produce that spend, so the term is an owner attestation +//! carried by executed on-chain code. +//! +//! Without (2), promotion is itself an attack: coin ids travel the DHT in cleartext by design, so a +//! stranger republishing an honest holder's id would rank first at zero collateral. That is why this +//! module shipped inert until dig-node#473 supplied the declaration. +//! +//! # The binding this module does NOT make, and who does +//! +//! (2) binds a coin to a `peer_id`. It does **not** bind that `peer_id` to the addresses beside it +//! in the provider record, and no chain read can: a provider record is unsigned, and dig-dht says so +//! outright. A record carrying an honest holder's peer id, that holder's real coin id, and an +//! ATTACKER's addresses therefore satisfies everything above and IS promoted here. +//! +//! It is refused one layer down, by the transport, which pins the claimed `peer_id` against the +//! certificate the far end actually presents — `dig-download`'s `provider_peer_id` becomes the +//! `PeerTarget` pin, enforced in `dig-tls`'s verifier as `peer_id mismatch: expected …, got …`, with +//! `dig-peer` re-checking it after connect. So the attacker buys a failed handshake, not a served +//! byte, and the content is merkle-verified against the caller's own requested root regardless. +//! +//! What this layer owes in return is a BOUND: at most one record is promoted per claimed peer id, so +//! a single stolen identity cannot occupy every promoted slot. See `SPEC.md` §25.6a. //! //! # What lives here, and what deliberately does not //! @@ -146,6 +163,30 @@ pub type BondVerifierSlot = Arc>>; pub const MAX_VERIFIED_PER_LOCATE: usize = 8; /// Promotion tier for a verdict — `Bonded` first, everything else in one baseline tier. +/// A peer id from an untrusted provider record, in a shape that is safe to put in a log field. +/// +/// `provider_peer_id` is a bare `String` carried off an unsigned provider record with no +/// wire-boundary normalisation, so it may be any length and hold any UTF-8 — newlines and control +/// characters included, which is what makes an unbounded log field a log-INJECTION surface rather +/// than merely a noisy one: a peer that spells its id with an embedded newline writes a second line +/// into this node's log, and an operator reading that log cannot tell it from one this node wrote. +/// +/// An honest peer id is exactly 64 hex characters, so keeping only hex digits, lowercasing them and +/// taking the first [`PEER_ID_HEX_LEN`] discards nothing real. The lowercasing matches the +/// normalisation the promotion bound above applies for the same reason: two spellings are one +/// identity, so two log lines about them should read as one peer. +fn peer_id_for_log(provider_peer_id: &str) -> String { + provider_peer_id + .chars() + .filter(char::is_ascii_hexdigit) + .map(|c| c.to_ascii_lowercase()) + .take(PEER_ID_HEX_LEN) + .collect() +} + +/// The length of a peer id in hex characters — 32 bytes, so 64. +const PEER_ID_HEX_LEN: usize = 64; + fn credit_rank(verdict: BondVerdict) -> u8 { match verdict { BondVerdict::Bonded => 0, @@ -193,6 +234,16 @@ impl ProviderLocator for BondRankingLocator { // not to: a comparison-driven lookup would read the same coin O(n log n) times. let mut ranked: Vec<(u8, ProviderRecord)> = Vec::with_capacity(found.len()); let mut verified = 0usize; + // Keyed on the ASCII-LOWERCASED peer id, never the raw wire string. A peer id is fixed-length + // hex, so its two spellings denote one identity -- and every check that GRANTS the promotion + // already knows that: the coin's declaration compares 32 decoded bytes, and the TLS pin + // compares 32 bytes of certificate hash. A set keyed on the text would therefore admit the + // same peer once per hex spelling, and one stolen identity could fill every promoted slot at + // zero collateral by varying case alone. dig-dht applies exactly this normalisation to the + // neighbouring `unverified_mirror_coin_id` field, for the reason its own doc gives: without + // it "dedup and equality would split on presentation". + let mut promoted_peers: std::collections::HashSet = + std::collections::HashSet::new(); for record in found { let claimed = record.unverified_mirror_coin_id_bytes(); // A holder that claims nothing, and every record past the budget, keeps its place with @@ -205,16 +256,33 @@ impl ProviderLocator for BondRankingLocator { let verdict = verifier .verify(content, &record.provider_peer_id, claimed) .await; + let mut rank = credit_rank(verdict); + // At most ONE record is promoted per claimed peer id. A coin declares one peer and a + // peer needs its own collateralised coin, so promotion is meant to cost collateral -- + // but nothing stops a stranger republishing one honest holder's peer id and coin id + // across the whole slate with addresses of its choosing. Each copy satisfies the + // declaration check on the strength of the same single bond, and without this the + // budget's worth of promoted slots could all be spent on one stolen identity. + // + // This is a BOUND, not a punishment: a duplicate falls back to the baseline tier it + // would have occupied with no verifier at all, never below it, so the lattice stays + // credit-only. It also costs an honest holder nothing -- a peer that legitimately + // announces twice keeps its first record promoted. + if rank == credit_rank(BondVerdict::Bonded) + && !promoted_peers.insert(record.provider_peer_id.to_ascii_lowercase()) + { + rank = credit_rank(BondVerdict::Unverified); + } if verdict == BondVerdict::Unbonded { // Worth an operator's attention and nobody's ban list: this record's own pointer // disproves its own claim. Logged and NOT demoted — the record may be a stranger's // lie ABOUT an honest holder, and demoting on it is what would make that lie pay. tracing::debug!( - peer = %record.provider_peer_id, + peer = %peer_id_for_log(&record.provider_peer_id), "located holder's claimed mirror coin does not bond this content; no promotion" ); } - ranked.push((credit_rank(verdict), record)); + ranked.push((rank, record)); } // STABLE, so holders sharing a tier keep the order their source gave them. The download @@ -233,6 +301,47 @@ mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Mutex; + /// **Proves (dig-node#501, security round 1, LOW 5):** peer-supplied text reaching a log field + /// is bounded and normalised. + /// + /// **Catches:** log injection through an unsigned field. `provider_peer_id` is a bare `String` + /// with no wire-boundary normalisation, so a peer that spells its id with an embedded newline + /// writes a whole second line into this node's log, and an operator reading it cannot tell that + /// line from one the node wrote itself. The fixture carries a newline, a carriage return, an + /// ANSI escape and 200 characters of padding — each a separate thing the old field would have + /// emitted verbatim — and the honest row is the control, so a function that simply returned the + /// empty string would not pass. + #[test] + fn a_peer_id_in_a_log_field_is_bounded_and_normalised() { + let honest = "aa".repeat(32); + assert_eq!( + peer_id_for_log(&honest), + honest, + "control: an honest 64-hex id must survive unchanged, or the field says nothing" + ); + assert_eq!( + peer_id_for_log(&honest.to_uppercase()), + honest, + "two spellings are one identity, exactly as the promotion bound treats them" + ); + + let hostile = format!("dead\nbeef\r\u{1b}[31m peer=trusted {}", "f".repeat(200)); + let logged = peer_id_for_log(&hostile); + assert!( + !logged.chars().any(char::is_control), + "a control character would forge a log line: {logged:?}" + ); + assert!( + logged.chars().all(|c| c.is_ascii_hexdigit()), + "only hex digits can be part of a peer id: {logged:?}" + ); + assert!( + logged.len() <= PEER_ID_HEX_LEN, + "the field is bounded at one peer id's worth, got {} characters", + logged.len() + ); + } + const STORE: [u8; 32] = [0x11; 32]; const ROOT: [u8; 32] = [0x22; 32]; diff --git a/crates/dig-node-core/src/peer.rs b/crates/dig-node-core/src/peer.rs index cd9e88af..1892d967 100644 --- a/crates/dig-node-core/src/peer.rs +++ b/crates/dig-node-core/src/peer.rs @@ -229,6 +229,16 @@ impl PeerStatus { Arc::new(PeerStatus::default()) } + /// This node's own `peer_id`, once the peer network has started. + /// + /// `None` before start-up. A caller that must NAME this node — writing the peer declaration into + /// a mirror coin, say — has to tolerate that: the identity exists on disk from the first boot, + /// but this status only learns it when the network comes up, so an early caller MUST treat + /// `None` as "not yet" rather than as "this node has no identity". + pub fn peer_id(&self) -> Option { + self.peer_id.lock().unwrap().clone() + } + /// Mark the peer network running under `peer_id` (clears the last error). pub fn set_running(&self, peer_id: String) { self.running.store(true, Ordering::Relaxed); @@ -860,12 +870,19 @@ pub use crate::shared::identity::{install_crypto_provider, load_or_generate_node /// peer-RPC server, the DHT dials, and the gossip pool). Without this the pool would present a cert /// hashing to a different peer_id than the one this node advertises (#1532). /// -/// **A dialler is not guaranteed to notice.** `dig-gossip` DERIVES a peer id from the presented SPKI -/// and does not compare it against the one it dialled — every `expected_peer_id` occurrence in that -/// crate is inside `#[cfg(test)]` (DIG-Network/dig-gossip#85). So a split identity is not caught by a -/// fail-closed handshake in the general case; where pinning does happen it is the dialler's own -/// (`dig-tls::pin_and_bind`, used by the ping path). Do not rely on a mismatch being refused. dig-gossip only READS these files (`dig_peer_protocol::load_ssl_cert`), so pointing at the -/// canonical identity files can never clobber them. +/// **A dialler DOES notice, on every path this node is dialled over.** An earlier version of this +/// note said the opposite, and it was wrong in a way worth stating: it generalised one true fact +/// about `dig-gossip` into a claim about every dial. The true fact is narrow — `dig-gossip`'s legacy +/// rustls outbound derives a peer id from the presented SPKI without comparing it to the one it +/// dialled, and every `expected_peer_id` in that crate is inside `#[cfg(test)]` +/// (DIG-Network/dig-gossip#85). But dig-node never dials on that path. Its dials go through +/// `dig-nat`, which passes the expected id to `dig-tls`; the verifier compares it against the leaf +/// the far end presents and fails the handshake with `peer_id mismatch: expected <..>, got <..>`, +/// and `dig-peer` re-checks it after connect. The download path pins the same way, from a provider +/// record's own `provider_peer_id`. So a split identity IS caught fail-closed here, which is exactly +/// why keeping these files canonical matters. dig-gossip only READS them +/// (`dig_peer_protocol::load_ssl_cert`), so pointing at the canonical identity files can never +/// clobber them. /// /// [`node_cert_dir`]: crate::seams::key_mgmt::key_manager::KeyManager::node_cert_dir fn gossip_identity_paths(node_cert_dir: &std::path::Path) -> (String, String) { @@ -2528,8 +2545,11 @@ async fn run_peer_network(node: Arc) -> Result<(), String> { // pool's cert/key at the persisted `NodeCert` files themselves (dig-gossip only READS them); // letting the GossipService mint its OWN throwaway cert would hash to a DIFFERENT peer_id and // a dial to this node can reach a DIFFERENT identity than the one advertised (#1532 — the - // identity split). Not every dialler refuses that: dig-gossip derives the peer id and does not - // pin it (DIG-Network/dig-gossip#85), so this must be right at the source rather than caught. + // identity split). A pinning dialler DOES refuse that -- dig-gossip's production dials go + // through dig-nat, which pins the expected id in dig-tls -- but getting it right at the source + // is still what this does, because a caught mismatch is a refused connection rather than a + // working one. (dig-gossip's own legacy rustls outbound does not pin, DIG-Network/dig-gossip#85, + // but nothing here dials on it.) // `cfg.peer_id` is set to that same identity so the pool's self-dial guard + handshake agree. // The address book (`peers.json`) stays under `peer-net/`; only the identity is shared. let gossip_dir = node.peer_cert_dir(); diff --git a/crates/dig-node-service/src/continuation_guard.rs b/crates/dig-node-service/src/continuation_guard.rs index 67a8eb19..2e40b9f1 100644 --- a/crates/dig-node-service/src/continuation_guard.rs +++ b/crates/dig-node-service/src/continuation_guard.rs @@ -11,11 +11,10 @@ use std::path::Path; -/// Files under `mirror/` are owned by dig-node#501 (a separate open PR repairs six -/// sites of this same class there). Scanning them here would either duplicate that -/// fix or fail this branch on work this PR does not own. Delete this exclusion once -/// #501 merges. -const EXCLUDED_DIRS: &[&str] = &["mirror"]; +/// No directory under `src` is exempt from the crate-wide scan (dig-node#501 closed +/// the exclusion this list used to carry for `mirror/`, once that directory's own +/// six lost-continuation sites were fixed). +const EXCLUDED_DIRS: &[&str] = &[]; /// `service.rs`'s `sc qc` parser test pins a byte-identical copy of real `sc.exe` /// output; its fixed-width `LABEL : value` columns are deliberate alignment the diff --git a/crates/dig-node-service/src/mirror/advertise.rs b/crates/dig-node-service/src/mirror/advertise.rs index 9e409a7e..05fcb222 100644 --- a/crates/dig-node-service/src/mirror/advertise.rs +++ b/crates/dig-node-service/src/mirror/advertise.rs @@ -50,6 +50,22 @@ pub enum Rejection { ThisMachineOnly, } +impl Rejection { + /// Every variant, so the operator-message guard can walk the set rather than a chosen example. + /// + /// Declared beside the enum, not spelled inside the test. A walk built from an array literal in + /// the test would still compile, still be the same length, and still pass after a variant was + /// added — so the new variant's message would ship unguarded by the very test written to guard + /// it, which is the shape of gap this module already shipped once. + /// + /// **What is and is not enforced, stated precisely because the looser claim was wrong.** The + /// `match` in [`rejection_reason`] is exhaustive, so a new variant CANNOT compile without a + /// developer editing this module and giving it a message. Nothing in Rust then forces that + /// variant into this array — but the guard asserts its walk against `ALL.len()`, so the walk and + /// this list cannot drift apart, and this is the one place to add it. + pub const ALL: [Rejection; 2] = [Rejection::NotAbsolute, Rejection::ThisMachineOnly]; +} + /// What [`parse_advertised_urls`] made of the operator's value. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct Advertised { @@ -75,6 +91,49 @@ pub fn advertised_urls_from_env() -> Advertised { parse_advertised_urls(&std::env::var(ADVERTISE_URLS_ENV).unwrap_or_default()) } +/// Why one configured entry is not advertised, in the words an operator reads. +/// +/// Split out of [`configured_urls`] so a test can walk EVERY variant and assert over the rendered +/// text. That is not tidiness: all three of this module's operator-facing lines shipped with 14- to +/// 18-space runs baked into the middle of a sentence, left behind when a `\` string continuation +/// lost its backslash. Such a literal compiles, no caller inspects it, and the mangled and correct +/// forms are indistinguishable in a diff — so only a test over the VALUE can see it, and it has to +/// reach these literals rather than a neighbouring module's. +fn rejection_reason(why: &Rejection) -> &'static str { + match why { + Rejection::NotAbsolute => { + "it is not an absolute URL with a scheme and a host, so it names no way to reach anything" + } + Rejection::ThisMachineOnly => { + "its host can only mean this machine, so every reader would resolve it to themselves" + } + } +} + +/// The whole line an operator sees when one entry is dropped, the reason included. +/// +/// The wrapper around the reason is operator-facing prose too, so it belongs in the guard's walk. +/// Checking only the reason would leave the sentence it is embedded in unchecked while the guard +/// claims to cover every line this module emits. +fn not_advertised(reason: &str) -> String { + format!("{ADVERTISE_URLS_ENV} entry is not advertised: {reason}") +} + +/// What this node reports when at least one entry survived and will be published. +const ADVERTISING_AT_CONFIGURED_URLS: &str = + "advertising this node's stores at the operator-configured URLs, in the configured order"; + +/// What this node reports when no configured entry may be published. +/// +/// A function rather than a `const` because it names the environment variable, and `concat!` cannot +/// take a `const`. Spelling the variable a second time as a literal would be a second source of +/// truth for the same name — exactly the drift this module's tests exist to catch. +fn nothing_publishable() -> String { + format!( + "no {ADVERTISE_URLS_ENV} entry is publishable, so this node advertises nothing and creates no mirror coin (SPEC.md 25.10)" + ) +} + /// The URLs this node will publish, with every rejected entry reported to the operator. /// /// This is the whole operator surface as the mirror scheduler consumes it: one call, at bring-up, @@ -89,18 +148,11 @@ pub fn configured_urls() -> Vec { let advertised = advertised_urls_from_env(); for (entry, why) in &advertised.rejected { - let reason = match why { - Rejection::NotAbsolute => { - "it is not an absolute URL with a scheme and a host, so it names no way to reach anything" - } - Rejection::ThisMachineOnly => { - "its host can only mean this machine, so every reader would resolve it to themselves" - } - }; tracing::warn!( target: "mirror", entry = %entry, - "{ADVERTISE_URLS_ENV} entry is not advertised: {reason}" + "{}", + not_advertised(rejection_reason(why)) ); } @@ -108,13 +160,10 @@ pub fn configured_urls() -> Vec { tracing::info!( target: "mirror", urls = ?advertised.accepted, - "advertising this node's stores at the operator-configured URLs, in the configured order" + "{ADVERTISING_AT_CONFIGURED_URLS}" ); } else { - tracing::info!( - target: "mirror", - "no {ADVERTISE_URLS_ENV} entry is publishable, so this node advertises nothing and creates no mirror coin (SPEC.md 25.10)" - ); + tracing::info!(target: "mirror", "{}", nothing_publishable()); } advertised.accepted @@ -217,6 +266,75 @@ fn is_this_machine_only_v4(ip: Ipv4Addr) -> bool { mod tests { use super::*; + /// **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 + /// 14 to 18 literal spaces into the middle of each sentence. Nothing else can catch that. The + /// compiler is happy, no caller inspects the text, the two forms are indistinguishable in a + /// diff, and the only witness is an operator reading a line that looks broken at the moment + /// they are trying to work out why nothing is being advertised. + /// + /// The guard walks the whole SET rather than a chosen example, and asserts the count, so + /// deleting a message from the walk fails here instead of silently shrinking the sweep. A + /// sibling guard over `lifecycle.rs`'s two refusals exists and is NOT a substitute: it cannot + /// reach these literals, which is how three corrupted runtime lines sat behind a green test + /// that appeared to cover them. + #[test] + fn every_operator_facing_line_reads_as_a_sentence() { + // Driven from `Rejection::ALL`, and the expected count is DERIVED from it rather than + // written as a literal. A hard-coded `4` would actively cement a subset: add a variant, + // give it a message, leave it out of `ALL`, and a literal count still matches while the new + // line ships unchecked. The `match` below only forces the variant to be NAMED — that is + // what makes it visible, not what makes it walked — so the derived length is what actually + // ties the two together. + let lines: Vec<(&str, String)> = Rejection::ALL + .iter() + .map(|why| { + let name = match why { + Rejection::NotAbsolute => "Rejection::NotAbsolute", + Rejection::ThisMachineOnly => "Rejection::ThisMachineOnly", + }; + // The wrapper, not the bare reason: that is the line an operator actually reads, + // and it contains the reason, so this covers both. + (name, not_advertised(rejection_reason(why))) + }) + .chain([ + ( + "ADVERTISING_AT_CONFIGURED_URLS", + ADVERTISING_AT_CONFIGURED_URLS.to_string(), + ), + ("nothing_publishable", nothing_publishable()), + ]) + .collect(); + + assert_eq!( + lines.len(), + Rejection::ALL.len() + 2, + "every line this module emits must be walked, never a subset: {lines:?}" + ); + for (name, line) in &lines { + assert!( + !line.contains(" "), + "{name}: a run of two spaces is an eaten line continuation, not prose: {line:?}" + ); + assert!( + !line.chars().any(char::is_control), + "{name}: no control character belongs in an operator-facing line: {line:?}" + ); + // Control: a fixture too empty to exhibit the property must not pass. Both assertions + // above are satisfied by "" and by a single word. + assert!( + line.split_whitespace().count() >= 8, + "{name}: control -- each line must still be a sentence saying what happened: {line:?}" + ); + } + assert!( + lines[3].1.contains(ADVERTISE_URLS_ENV), + "control: the no-entry line must name the variable an operator has to set: {:?}", + lines[3].1 + ); + } + /// The refusal that exists today survives an unset value: no URL means no advertisement, which /// is what makes `create` decline rather than publish somewhere unreachable. #[test] diff --git a/crates/dig-node-service/src/mirror/bond_verify.rs b/crates/dig-node-service/src/mirror/bond_verify.rs index 1b891ee2..76f74650 100644 --- a/crates/dig-node-service/src/mirror/bond_verify.rs +++ b/crates/dig-node-service/src/mirror/bond_verify.rs @@ -59,6 +59,61 @@ const VERDICT_TTL: Duration = Duration::from_secs(600); /// earned simply by rotating coin ids, which converts a memoisation into an amplifier. const MAX_CACHED_VERDICTS: usize = 1024; +/// The most locate-triggered verifications this PROCESS will pay for in one burst. +/// +/// Each one costs up to two blocking HTTPS reads through the node's ONE shared `ChiaQuery` client — +/// the same client the wallet, the collateral census and the mirror spends read through. So this +/// ceiling is not about fairness between requestors; it is about this node keeping its own chain +/// access when a stranger directs traffic at it. +/// +/// The inbound admission gate (`allow_miss_lookup`, burst 16 at 4/sec) is PER REQUESTOR over up to +/// 4,096 self-minted identities and has no aggregate cap, so it cannot bound this. Sixteen is +/// therefore chosen against the *outbound* cost, not the inbound one: two locates' worth of a full +/// slate, after which verification degrades to `Unverified` — which is where every record already +/// sits when no verifier is installed at all. +const VERIFICATION_BURST: u32 = 16; + +/// How fast [`VERIFICATION_BURST`] refills, in verifications per second. +/// +/// One per second, deliberately slower than the inbound refill it cannot control: an attacker who +/// sustains the inbound rate gets a *constant* trickle of outbound reads rather than a multiple of +/// its own request rate. An honest node's own reads — a capsule it is downloading — are answered +/// from the burst and then from the verdict cache. +const VERIFICATION_REFILL_PER_SEC: f64 = 1.0; + +/// The most verifications in flight at once, so a burst cannot fan out CONCURRENTLY onto the shared +/// client even while it is within the rate ceiling. +/// +/// A rate bound alone still permits sixteen simultaneous blocking reads, which is what a connection +/// pool experiences as an outage rather than as load. +const MAX_CONCURRENT_VERIFICATIONS: usize = 4; + +/// The most DISTINCT claimed coin ids one claiming peer may spend chain reads on, per +/// [`CLAIMANT_LEDGER_WINDOW`], without ever proving a bond. +/// +/// This is the bound that answers the fabricated-coin-id case specifically. [`VerdictKey`] includes +/// the coin id — it must, or a stranger republishing a public coin id would inherit its holder's +/// verdict — so eight records carrying eight *invented* coin ids miss the cache eight times by +/// construction, and no cache design can fix that. What CAN be bounded is how many invented ids one +/// claimed identity is allowed to be wrong about before this node stops asking on its behalf. +/// +/// Four, because an honest holder needs ONE: a peer publishes the coin it created. A rollover can +/// make it briefly two (the old coin and the new one). Anything beyond that is a peer that does not +/// know which coin it holds, and its records are worth no more chain reads than a stranger's. +const MAX_UNPROVEN_COINS_PER_CLAIMANT: usize = 4; + +/// How long a claimant's unproven-coin ledger is remembered. Matched to [`VERDICT_TTL`] so a peer +/// that genuinely rotates coins is forgiven on the same clock a cached verdict expires on. +const CLAIMANT_LEDGER_WINDOW: Duration = VERDICT_TTL; + +/// The most claimants tracked in that ledger at once. +/// +/// A claiming peer id is attacker-chosen and unbounded in supply, so the ledger MUST be bounded. +/// Once it is full of live entries an UNKNOWN claimant is refused rather than admitted, which is +/// the fail-closed direction: the cost of being wrong is a holder sitting at the baseline tier it +/// would occupy with no verifier at all, never a wrong promotion. +const MAX_TRACKED_CLAIMANTS: usize = 512; + /// A verdict is only ever cached for the exact question it answered. /// /// The coin id alone is not the key: one coin bonds one `(store, root, epoch)`, so caching by coin @@ -91,8 +146,13 @@ impl VerdictKey { epoch: u64, claiming_peer_id: &str, ) -> Self { + // ASCII-lowercased before hashing. A peer id is fixed-length hex, so its two spellings are + // one identity, and `peer_declaration` already treats them as one because it compares + // decoded bytes. Hashing the raw text would give each spelling its own cache entry, so a + // stranger could multiply the chain reads one claim costs simply by varying case -- turning + // a memo into an amplifier against `api.coinset.org`. let mut hasher = chia_sha2::Sha256::new(); - hasher.update(claiming_peer_id.as_bytes()); + hasher.update(claiming_peer_id.to_ascii_lowercase().as_bytes()); VerdictKey { coin_id, store_launcher_id: store.to_bytes(), @@ -105,11 +165,9 @@ impl VerdictKey { /// What a mirror coin's advertised terms say about the peer claiming it. /// -/// `DeclaresThisPeer` and `Silent` are matched but not yet constructed: nothing can construct them -/// until [`peer_declaration`] has a typed source for the binding, which is exactly the promotion gate -/// described there. They are written now so the shape of the answer is fixed before the source -/// arrives, and so the call site reads as the full decision rather than a placeholder. -#[allow(dead_code)] +/// Two answers, not three. Until `dig-mirror-coin` 0.8.0 there was a `NotReadable` state for "this +/// node has no way to read a declaration at all"; the typed accessor removed the situation, so the +/// variant was removed with it rather than left as a case nothing constructs. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum PeerDeclaration { /// The coin's owner declared this exact peer, in code the chain executed. @@ -117,36 +175,30 @@ pub(crate) enum PeerDeclaration { /// The coin declares some other peer, or none. Credit is withheld — never subtracted, because /// the record naming this coin may be a stranger's lie ABOUT the coin's real holder. Silent, - /// This node cannot read the declaration at all, so it knows nothing either way. - NotReadable, } /// Whether `advertised_terms` — the free tail of a mirror coin's memo — declares `claiming_peer_id`. /// -/// **This is deliberately unreadable today, and that is the promotion gate.** The tail is arbitrary -/// UTF-8 and `MirrorCoin::urls()` already hands it over, so a `dig-peer:<64-hex>` term COULD be -/// parsed here — and must not be. `dig-mirror-coin` 0.8.0 is about to own that format with a typed -/// accessor; parsing it here would create a second parser for a security-critical format, in the -/// consumer, where a divergence between the two would be a silent authorization difference rather -/// than a compile error (CLAUDE.md 2.0, centralize rival implementations). -/// -/// So promotion is switched OFF until that accessor exists: with no sound source for the -/// coin -> peer binding, `Bonded` cannot be established, and the layer withholds credit from -/// everyone rather than granting it on a check it cannot make. Nothing is demoted by this -/// (mirror_bond's lattice is credit-only), so the interim behaviour is exactly the behaviour of a -/// node with no verifier at all. -/// -/// Replacing the body with the 0.8.0 accessor turns promotion on, and three things MUST land in -/// that same change, never after: the authoritative-record restriction on dig-node#466, the -/// claiming peer id staying part of [`VerdictKey`] (a peer-agnostic key would serve one peer's -/// earned `Bonded` to a stranger republishing the same public coin id), and the cost analysis on -/// [`declaration_source_is_readable`], whose short-circuit lifts itself the moment this function -/// can answer. +/// The answer comes from `dig-mirror-coin`'s typed accessor and is NOT parsed here. A second parser +/// for this format, living in the consumer, would make a divergence between the two a silent +/// authorization difference rather than a compile error (CLAUDE.md §2.0, centralize rival +/// implementations). Everything the format means — exact prefix matching, byte-wise peer-id +/// comparison, and the rule that a coin carrying two declarations declares NOBODY — is stated once, +/// in that crate's `SPEC.md` §5.1. +/// +/// **What a `DeclaresThisPeer` establishes.** Memos are written by the spend that creates the coin +/// and only the owner's key can produce that spend, so the term is an owner attestation carried by +/// executed on-chain code. It binds the coin to a `peer_id`. It does NOT bind that `peer_id` to the +/// addresses travelling beside it in the provider record — see `SPEC.md` §25.6a for why that gap is +/// closed by the transport rather than here. pub(crate) fn peer_declaration( - _advertised_terms: &[String], - _claiming_peer_id: &str, + advertised_terms: &[String], + claiming_peer_id: &str, ) -> PeerDeclaration { - PeerDeclaration::NotReadable + match dig_mirror_coin::declared_peer(advertised_terms) { + Some(declared) if declared.names(claiming_peer_id) => PeerDeclaration::DeclaresThisPeer, + _ => PeerDeclaration::Silent, + } } /// Whether [`peer_declaration`] can bind a coin to a peer AT ALL — probed through the real @@ -281,17 +333,19 @@ fn chain_bond_verdict_and_coin( /// The full verdict: the chain half, then **whose bond it is**. /// -/// A valid, fully-collateralised coin bonding exactly this content still says nothing about the -/// peer offering the record — every field of that record, the coin id included, was chosen by -/// whoever answered the lookup. Only the coin's own owner-written declaration of a peer closes that, -/// and this node cannot read one yet (see [`peer_declaration`]), so nothing is promoted today. +/// A valid, fully-collateralised coin bonding exactly this content still says nothing about the peer +/// offering the record — every field of that record, the coin id included, was chosen by whoever +/// answered the lookup. Only the coin's own owner-written declaration of a peer closes that, and +/// [`peer_declaration`] reads it, so a `Bonded` here means BOTH halves held: the coin bonds this +/// content, and the coin's owner named this claimant. /// /// Credit is withheld, never subtracted: a record naming this coin may be a stranger's lie ABOUT the /// coin's real holder, and demoting on it is what would make that lie pay. /// -/// **No chain is read at all while the ownership half has no source** (see -/// [`declaration_source_is_readable`]): with `Bonded` unreachable, the reads would be paid for an -/// answer this function is about to discard. +/// **A node with no censused requirement for the epoch cannot promote anyone**, because it cannot +/// price a bond — `required_collateral` is then `None` and the verdict degrades to `Unverified` +/// rather than to `Bonded`. Detection of a false claim still works there (the binding is checked +/// first, deliberately), but certification does not. pub fn verdict_for( source: &S, store_launcher_id: Bytes32, @@ -320,8 +374,184 @@ pub fn verdict_for( }; match peer_declaration(mirror.urls(), claiming_peer_id) { PeerDeclaration::DeclaresThisPeer => BondVerdict::Bonded, - PeerDeclaration::Silent | PeerDeclaration::NotReadable => BondVerdict::Unverified, + PeerDeclaration::Silent => BondVerdict::Unverified, + } +} + +/// Whether a chain read may be spent on one claim, right now. +/// +/// **Why this exists.** Before promotion went live, `verdict_for` returned at +/// `declaration_source_is_readable()` and a locate cost zero chain reads. Activating it turned one +/// cheap inbound token into up to `MAX_VERIFIED_PER_LOCATE` verifications, each two outbound HTTPS +/// reads, on a client shared with the wallet — and the inbound gate that admits the locate is +/// per-requestor over self-minted identities, so it bounds nothing in aggregate. This type is the +/// aggregate bound (dig-node#501, security round 1, HIGH). +/// +/// Two independent limits, because they answer two different attacks: +/// +/// * a **process-wide token bucket** ([`VERIFICATION_BURST`] at [`VERIFICATION_REFILL_PER_SEC`]), +/// which bounds total outbound egress however many identities the traffic is spread across; +/// * a **per-claimant distinct-coin ledger** ([`MAX_UNPROVEN_COINS_PER_CLAIMANT`]), which bounds +/// the fabricated-coin-id case the verdict cache structurally cannot absorb. +/// +/// Exhaustion of either returns [`BondVerdict::Unverified`] having read NOTHING. That is a +/// degradation and not a refusal of service: `Unverified` and `Unbonded` share a rank, the sort is +/// stable, and the located slate is returned unchanged — the exact behaviour of a node with no +/// verifier installed. **The read path is never blocked and no holder is ever ranked below where it +/// started**, so an attacker who exhausts the budget denies promotion, not content. +struct ReadAdmission { + state: Mutex, +} + +/// [`ReadAdmission`]'s interior, held under one lock so the two limits are decided atomically — +/// a token spent on a claim the ledger was about to refuse would be a leak. +struct AdmissionState { + /// Whole verifications available now. Fractional so a sub-second refill is not rounded away. + tokens: f64, + /// When `tokens` was last brought up to date. + refilled_at: Instant, + burst: f64, + refill_per_sec: f64, + /// Per claimant: when its window opened, and the distinct coin ids it has spent reads on + /// without proving a bond. Keyed on the SHA-256 of the lowercased peer id, for the reason + /// [`VerdictKey`] hashes it — the key must be fixed-size against an attacker-chosen string, and + /// two hex spellings must be one identity. + unproven: HashMap<[u8; 32], (Instant, std::collections::HashSet<[u8; 32]>)>, +} + +impl ReadAdmission { + /// A budget of the given size. Parameterised so a test can exhaust one in a few calls rather + /// than by replicating production's arithmetic. + fn new(burst: u32, refill_per_sec: f64) -> Self { + ReadAdmission { + state: Mutex::new(AdmissionState { + tokens: f64::from(burst), + refilled_at: Instant::now(), + burst: f64::from(burst), + refill_per_sec, + unproven: HashMap::new(), + }), + } + } + + /// The one budget every locate on this node draws from. + /// + /// Process-wide rather than per verifier: the constraint being protected is the single shared + /// `ChiaQuery` client, which is a property of the process, so a budget scoped to anything + /// narrower would be several budgets against one resource. + fn shared() -> &'static ReadAdmission { + static SHARED: std::sync::OnceLock = std::sync::OnceLock::new(); + SHARED.get_or_init(|| ReadAdmission::new(VERIFICATION_BURST, VERIFICATION_REFILL_PER_SEC)) } + + /// Whether a chain read may be spent on `claimed_coin_id` for `claiming_peer_id`. + /// + /// Returning `false` costs the network nothing and this node nothing; returning `true` spends a + /// token, so it is called exactly once per verification and never as a peek. + fn admit(&self, claiming_peer_id: &str, claimed_coin_id: [u8; 32]) -> bool { + let Ok(mut state) = self.state.lock() else { + // A poisoned budget cannot be shown to have capacity, so it has none. + return false; + }; + + let now = Instant::now(); + let elapsed = now + .saturating_duration_since(state.refilled_at) + .as_secs_f64(); + state.tokens = (state.tokens + elapsed * state.refill_per_sec).min(state.burst); + state.refilled_at = now; + + let claimant = claimant_key(claiming_peer_id); + state + .unproven + .retain(|_, (opened, _)| opened.elapsed() < CLAIMANT_LEDGER_WINDOW); + let known = state.unproven.contains_key(&claimant); + if !known && state.unproven.len() >= MAX_TRACKED_CLAIMANTS { + return false; + } + if let Some((_, coins)) = state.unproven.get(&claimant) { + // A coin id this claimant has already been read about is not a new question: the cache + // entry for it has simply expired, and re-asking is what keeps a verdict fresh. Only + // DISTINCT unproven ids count against the ledger. + if !coins.contains(&claimed_coin_id) && coins.len() >= MAX_UNPROVEN_COINS_PER_CLAIMANT { + return false; + } + } + + if state.tokens < 1.0 { + return false; + } + state.tokens -= 1.0; + state + .unproven + .entry(claimant) + .or_insert_with(|| (now, std::collections::HashSet::new())) + .1 + .insert(claimed_coin_id); + true + } + + /// Forgive a claimant's ledger: it has proven a bond, so it is not a peer guessing at coin ids. + /// + /// The process-wide bucket is deliberately NOT refunded. A proven bond says the claimant is + /// honest; it says nothing about this node's chain access, which is the thing the bucket exists + /// to protect. + fn record_proven(&self, claiming_peer_id: &str) { + if let Ok(mut state) = self.state.lock() { + state.unproven.remove(&claimant_key(claiming_peer_id)); + } + } +} + +/// A claiming peer id as a fixed-size ledger key: lowercased, then hashed. +/// +/// The same normalisation [`VerdictKey`] applies, for the same two reasons — the key must be +/// fixed-size against an attacker-chosen string, and a peer id's two hex spellings denote one +/// identity, so a ledger keyed on the raw text would give a stranger a fresh allowance per spelling. +fn claimant_key(claiming_peer_id: &str) -> [u8; 32] { + let mut hasher = chia_sha2::Sha256::new(); + hasher.update(claiming_peer_id.to_ascii_lowercase().as_bytes()); + hasher.finalize() +} + +/// [`verdict_for`], but only if the budget allows the reads it would perform. +/// +/// **This is the composition production uses**, so a test that drives it is exercising the real +/// ordering rather than re-deriving it: the budget is consulted BEFORE the source is touched, so a +/// refused claim reads nothing, and a proven bond forgives its claimant's ledger. +/// +/// The parameter list is `verdict_for`'s, in `verdict_for`'s order, with the budget in front. That +/// is deliberate and is why the lint is allowed here rather than satisfied by grouping: four of the +/// arguments are opaque 32-byte values, so the one mistake this wrapper could make is transposing +/// two of them, and a signature that mirrors the wrapped function exactly makes such a transposition +/// visible at the call site below instead of hiding it inside a re-packing struct. +#[allow(clippy::too_many_arguments)] +fn admitted_verdict_for( + admission: &ReadAdmission, + source: &S, + store_launcher_id: Bytes32, + root_hash: Bytes32, + epoch: &BigInt, + required_collateral: Option, + claiming_peer_id: &str, + claimed_coin_id: Bytes32, +) -> BondVerdict { + if !admission.admit(claiming_peer_id, claimed_coin_id.to_bytes()) { + return BondVerdict::Unverified; + } + let verdict = verdict_for( + source, + store_launcher_id, + root_hash, + epoch, + required_collateral, + claiming_peer_id, + claimed_coin_id, + ); + if verdict == BondVerdict::Bonded { + admission.record_proven(claiming_peer_id); + } + verdict } /// The memo of definite verdicts, keyed on the exact question each one answered. @@ -354,9 +584,26 @@ impl VerdictCache { return; }; if entries.len() >= MAX_CACHED_VERDICTS { - // Evict one arbitrary entry, not the map. `HashMap` iteration order is unspecified, so - // the victim is not attacker-selectable either; the cost of a wrong guess is one chain - // read, never a wrong answer. + // Expired first. Eviction should reclaim what is already worthless before it touches + // anything live, and a full map is the only moment worth paying the scan for. + entries.retain(|_, (taken, _)| taken.elapsed() < VERDICT_TTL); + } + if entries.len() >= MAX_CACHED_VERDICTS { + // Still full of LIVE entries, so admitting this one costs an honest verdict. Only a + // `Bonded` is worth that trade, and only a `Bonded` is expensive to obtain: it requires + // a coin that exists, is fully collateralised, and declares its claimant. + // + // **An `Unbonded` is refused admission rather than allowed to evict.** The previous + // version evicted one arbitrary entry per insert, which read as conservative but is + // paced by the attacker: `Unbonded` is the verdict a stranger elicits for FREE by + // naming coin ids that do not exist, so at an attacker-chosen insert rate the map turns + // over entirely in well under a minute and every honest `Bonded` this node earned is + // discarded. Refusing the cheap insert instead means a flood of invented coin ids + // cannot displace a single earned verdict — it only re-reads its own claims, which + // `ReadAdmission` is separately bounding. + if verdict != BondVerdict::Bonded { + return; + } if let Some(victim) = entries.keys().next().copied() { entries.remove(&victim); } @@ -406,9 +653,30 @@ impl ChainBondVerifier { // // Re-read per call rather than held from bring-up, matching the mirror pass: a transport // built once would make a node that started offline one that never verifies again. + // + // At the BOND floor, not the sync one (dig-node#513). `CORROBORATION_FLOOR` is two + // because the sync path writes the wallet's replica and a refused round stalls it; this + // path writes nothing, so a refusal costs `Unverified` -- the tier every record occupies + // with no verifier installed -- while believing a two-voice round sells a promotion for + // the price of two peers. let Ok(source) = self .chain .corroborated_chain_source(tokio::runtime::Handle::current()) + .map(|source| { + source.requiring_corroboration(dig_wallet::sage::quorum::BOND_CORROBORATION_FLOOR) + }) + else { + return BondVerdict::Unverified; + }; + + // The concurrency ceiling, and `try_acquire` rather than `acquire`: a queue of tasks + // waiting for a permit is an unbounded backlog of attacker-directed work held on this + // node's heap, and the honest answer when the budget is saturated is available immediately + // -- `Unverified`, the tier every record occupies with no verifier installed. + static IN_FLIGHT: std::sync::OnceLock = std::sync::OnceLock::new(); + let Ok(_permit) = IN_FLIGHT + .get_or_init(|| tokio::sync::Semaphore::new(MAX_CONCURRENT_VERIFICATIONS)) + .try_acquire() else { return BondVerdict::Unverified; }; @@ -416,7 +684,8 @@ impl ChainBondVerifier { // `ChainSource` is blocking, so the read leaves the async worker rather than parking it. let epoch_big = BigInt::from(epoch); let verdict = tokio::task::block_in_place(|| { - verdict_for( + admitted_verdict_for( + ReadAdmission::shared(), &source, store, root, @@ -576,11 +845,11 @@ mod tests { }; use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}; - /// A chain that counts every read reaching it and answers nothing. + /// A chain that counts every read reaching it and answers "no such coin". /// - /// Answering nothing is deliberate: the property under test is that the source is not consulted - /// AT ALL, so a double that could satisfy a read would let a short-circuit that merely fails - /// fast look identical to one that never asks. + /// Answering nothing is deliberate. It makes the read COUNT the only variable: a claim against + /// this source is disproven at the first step, so any count above one is a retry loop or a + /// second question, both of which this module owes the network not to perform. struct CountingChain { reads: Arc, } @@ -688,31 +957,35 @@ mod tests { let terms = vec![format!("dig-peer:{claiming_peer_id}")]; match peer_declaration(&terms, claiming_peer_id) { PeerDeclaration::DeclaresThisPeer => BondVerdict::Bonded, - PeerDeclaration::Silent | PeerDeclaration::NotReadable => BondVerdict::Unverified, + PeerDeclaration::Silent => BondVerdict::Unverified, } } } - /// **Proves (dig-node#466, HIGH finding 2 — the residual the credit-only lattice does NOT - /// close):** a hearsay record naming an honest holder's peer id AND its real coin id, but - /// carrying the ATTACKER's addresses, is not promoted — so the address a redirected reader - /// dials is unchanged by it. + /// **Proves (dig-node#473):** a record carrying an honest holder's peer id and real coin id but + /// the ATTACKER's addresses IS promoted here — and the promotion is bounded to ONE slot however + /// many copies of it a slate contains. /// - /// **Catches:** the hole neither other test can see. Every field of this record is honest - /// except the one that matters, so a coin-id check passes, a peer-id check passes, and the peer - /// id is IDENTICAL in the passing and failing versions of the code — which is why the assertion - /// is on the addresses. `stop_on_providers` means one answer can be the whole slate, so a - /// promotion here puts the attacker's host first for every reader that trusts the ranking, and - /// the dial does not pin the peer id (DIG-Network/dig-gossip#85) to catch it afterwards. + /// **This test previously asserted the opposite, and it was wrong for a measured reason.** Its + /// premise was that "the dial does not pin the peer id (dig-gossip#85) to catch it afterwards". + /// That is true only of `dig-gossip`'s legacy rustls outbound, which dig-node never dials on: + /// every dial it makes passes the expected id through `dig-nat` to `dig-tls`, whose verifier + /// fails the handshake with `peer_id mismatch`. So the attacker's addresses buy a refused + /// connection, not a redirected reader, and the honest promotion is not a hole. /// - /// The verifier is driven through the REAL [`peer_declaration`] gate rather than a hand-written - /// verdict, so this test measures production's answer: with no sound coin -> peer binding - /// available, nothing is promoted at all. + /// **Catches:** the bound going missing. The coin binds coin -> peer id and never peer id -> + /// address, so nothing in this layer can tell the honest holder's record from the attacker's + /// copy of it — both name the same peer and the same real coin. What this layer CAN do is refuse + /// to spend more than one promoted slot on one claimed peer id, which is what keeps a single + /// stolen identity from filling the whole verified budget. The slate below carries three copies + /// and one ordinary holder, so a missing dedup shows up as three promotions instead of one. #[tokio::test] - async fn an_honest_peer_id_with_attacker_addresses_is_not_promoted() { + async fn one_stolen_identity_cannot_occupy_more_than_one_promoted_slot() { let slate = Slate(vec![ - holder_at(0xCC, None, "honest.example"), // an ordinary holder, no pointer - holder_at(0xAA, Some([0x01; 32]), "attacker.example"), + holder_at(0xCC, None, "honest-no-pointer.example"), + holder_at(0xAA, Some([0x01; 32]), "attacker-1.example"), + holder_at(0xAA, Some([0x01; 32]), "attacker-2.example"), + holder_at(0xAA, Some([0x01; 32]), "attacker-3.example"), ]); let slot = bond_verifier_slot(); let _ = slot.set(Arc::new(EveryChainCheckPasses)); @@ -723,9 +996,13 @@ mod tests { assert_eq!( hosts, - vec!["honest.example", "attacker.example"], - "a record whose peer id and coin id are both honest must still not promote the \ - addresses it chose for itself" + vec![ + "attacker-1.example", + "honest-no-pointer.example", + "attacker-2.example", + "attacker-3.example", + ], + "exactly one copy is promoted; the rest fall back to baseline in source order, never below it" ); } @@ -766,19 +1043,67 @@ mod tests { ); } - /// **Proves (dig-node#466, security F1):** no chain is read at all while nothing can bind a - /// coin to a peer. + /// **Proves (dig-node#473, adversarial gate):** the promotion bound is keyed on the peer's + /// IDENTITY, not on the text a record happened to spell it with. + /// + /// **Catches:** the bound being defeated at zero cost. Everything that GRANTS a promotion + /// compares bytes — the coin's declaration decodes the hex, and the TLS pin compares certificate + /// hashes — so a peer id in upper case and the same id in lower case are one peer to every check + /// that matters. A bound keyed on the raw string is not: a stranger returning one honest + /// holder's peer id in eight different hex spellings, each with its own addresses, would have + /// eight distinct keys, eight promotions, and eight chain reads, and would occupy the whole + /// verified budget on the strength of a single bond it does not hold. /// - /// **Catches:** the amplifier. The production `ChainSource` reaches `api.coinset.org`, and a - /// `Bonded` that degrades to `Unverified` is the one verdict the cache refuses to hold — so - /// each read is re-paid on every locate, up to the locate budget, for an answer the stable - /// credit-only sort provably discards. The counting source makes the absence of that egress an - /// assertion rather than a claim. + /// The two records below differ ONLY in the case of that one field, so a set keyed on the text + /// promotes both and a set keyed on the identity promotes one. + #[tokio::test] + async fn the_promotion_bound_is_not_defeated_by_respelling_one_peer_id() { + let mut shouted = holder_at(0xAA, Some([0x01; 32]), "attacker.example"); + shouted.provider_peer_id = shouted.provider_peer_id.to_uppercase(); + assert_ne!( + shouted.provider_peer_id, + holder_at(0xAA, None, "x").provider_peer_id, + "the fixture must actually differ as TEXT, or it proves nothing" + ); + + // The honest holder sits BETWEEN the two spellings, and that placement is the whole test. + // With it last, a promoted respelling and a baseline one land in the same position under a + // stable sort and the two behaviours are indistinguishable -- the test would pass either + // way. Here, promoting the respelling moves it AHEAD of the honest record; bounding it + // leaves the honest record in front. + let slate = Slate(vec![ + holder_at(0xAA, Some([0x01; 32]), "attacker-lower.example"), + holder_at(0xCC, None, "honest.example"), + shouted, + ]); + let slot = bond_verifier_slot(); + let _ = slot.set(Arc::new(EveryChainCheckPasses)); + let locator = BondRankingLocator::new(Arc::new(slate), slot); + + let got = locator.find_providers(&capsule()).await.expect("located"); + let hosts: Vec = got.iter().map(|r| r.addresses[0].host.clone()).collect(); + + assert_eq!( + hosts, + vec![ + "attacker-lower.example", + "honest.example", + "attacker.example", + ], + "one identity earns one promoted slot however it is spelled; the respelling stays at baseline, BEHIND the honest holder it would otherwise have jumped" + ); + } + + /// **Proves (dig-node#466 / #473):** a wrong pointer costs the verifier exactly ONE chain read, + /// with no retry loop — the cost the ticket requires be borne by the publisher, not the reader. /// - /// The count is asserted at zero AND the verdict at `Unverified`, so a short-circuit that - /// changed the answer would fail here rather than pass quietly. + /// **Catches:** the amplifier. The production `ChainSource` reaches `api.coinset.org`, the coin + /// id is chosen by whoever wrote the provider record, and `Unbonded` is a verdict a stranger can + /// elicit for free. A retry, or a second question asked of a claim already disproven, multiplies + /// attacker-directed egress by the locate budget. The count is asserted alongside the verdict so + /// a change that stopped reading altogether would fail here rather than pass quietly. #[test] - fn nothing_is_read_from_the_chain_while_the_declaration_has_no_source() { + fn a_disproven_pointer_costs_exactly_one_chain_read() { let reads = Arc::new(AtomicUsize::new(0)); let source = CountingChain { reads: Arc::clone(&reads), @@ -796,34 +1121,260 @@ mod tests { assert_eq!( verdict, - BondVerdict::Unverified, - "withholding credit is the answer the short-circuit must preserve" + BondVerdict::Unbonded, + "the chain answered and there is no such coin, which disproves the claim" ); assert_eq!( reads.load(AtomicOrdering::Relaxed), - 0, - "a verdict that cannot be `Bonded` must cost no chain read" + 1, + "one read settles it; anything more is a retry loop paid for by the reader" + ); + } + + /// **Proves (dig-node#501, security round 1, HIGH):** locate-triggered verification spends a + /// PROCESS-WIDE budget of chain reads, so spreading the traffic across identities does not + /// multiply this node's outbound egress. + /// + /// **Catches:** the aggregate cap going missing — the defect this PR introduced by lifting the + /// pre-read short-circuit. The inbound gate that admits a locate is per-requestor (burst 16 at + /// 4/sec) over up to 4,096 self-minted identities, so it bounds nothing in aggregate; every + /// admitted locate then verified up to eight records at two HTTPS reads each, through the ONE + /// `ChiaQuery` client the node's own wallet, census and spends read through. + /// + /// Each row here uses a DISTINCT claimant, so the per-claimant ledger cannot be what bites and + /// the number asserted is the aggregate bucket's. The count is asserted from the chain double + /// itself rather than from the admission's own bookkeeping — an admission that returned `false` + /// while still reading would pass a test that only counted refusals. + #[test] + fn locate_triggered_chain_reads_are_capped_process_wide() { + const BURST: u32 = 4; + const CLAIMS: u32 = 20; + + let reads = Arc::new(AtomicUsize::new(0)); + let source = CountingChain { + reads: Arc::clone(&reads), + }; + // No refill during the test: a rate the test cannot outrun would make the cap unobservable. + let admission = ReadAdmission::new(BURST, 0.0); + + let mut verdicts = Vec::new(); + for claim in 0..CLAIMS { + verdicts.push(admitted_verdict_for( + &admission, + &source, + Bytes32::new(STORE), + Bytes32::new(ROOT), + &BigInt::from(7u64), + Some(1), + // A fresh claimant per row, each a well-formed 64-hex id. + &format!("{claim:064x}"), + Bytes32::new([0x33; 32]), + )); + } + + assert_eq!( + reads.load(AtomicOrdering::Relaxed), + BURST as usize, + "the budget is the ceiling on chain reads, whatever the claim rate" + ); + assert_eq!( + verdicts + .iter() + .filter(|v| **v == BondVerdict::Unbonded) + .count(), + BURST as usize, + "control: every admitted claim was genuinely answered, so the cap is not hiding a \ + function that stopped reading altogether" ); assert!( - !declaration_source_is_readable(), - "control: the short-circuit is active precisely because the source is unreadable — \ - when 0.8.0's accessor lands this flips and the reads resume" + verdicts[BURST as usize..] + .iter() + .all(|v| *v == BondVerdict::Unverified), + "past the budget a record stays at the tier it would occupy with no verifier at all -- \ + withheld credit, never a demotion, and never a blocked read" ); } + /// **Proves (dig-node#501, security round 1, HIGH):** one claimant cannot spend an unbounded + /// number of chain reads on coin ids it invents. + /// + /// **Catches:** the case the verdict cache structurally cannot absorb. [`VerdictKey`] includes + /// the coin id and MUST — without it a stranger republishing a public coin id would inherit its + /// holder's `Bonded` — so a slate of records carrying invented coin ids misses the cache once + /// per invented id, by construction. No cache design fixes that; only a bound on how many + /// unproven ids one claimed identity is allowed does. + /// + /// The budget here is deliberately large, so the number asserted can only be the per-claimant + /// ledger's. A single-claimant test against the aggregate bucket alone would pass with no + /// ledger at all. #[test] - fn no_visible_term_promotes_a_claim_before_the_typed_accessor_exists() { + fn fabricated_coin_ids_from_one_claimant_stop_costing_chain_reads() { + let reads = Arc::new(AtomicUsize::new(0)); + let source = CountingChain { + reads: Arc::clone(&reads), + }; + let admission = ReadAdmission::new(1_000, 0.0); + let claimant = "aa".repeat(32); + + for invented in 0..32u8 { + admitted_verdict_for( + &admission, + &source, + Bytes32::new(STORE), + Bytes32::new(ROOT), + &BigInt::from(7u64), + Some(1), + &claimant, + Bytes32::new([invented; 32]), + ); + } + + assert_eq!( + reads.load(AtomicOrdering::Relaxed), + MAX_UNPROVEN_COINS_PER_CLAIMANT, + "a claimant that never proves a bond gets a fixed allowance of invented coin ids" + ); + + // The control, and the reason the bound is not simply a per-peer denial: a DIFFERENT + // claimant is unaffected by this one's exhaustion. + admitted_verdict_for( + &admission, + &source, + Bytes32::new(STORE), + Bytes32::new(ROOT), + &BigInt::from(7u64), + Some(1), + &"bb".repeat(32), + Bytes32::new([0x01; 32]), + ); + assert_eq!( + reads.load(AtomicOrdering::Relaxed), + MAX_UNPROVEN_COINS_PER_CLAIMANT + 1, + "the ledger is per claimant; one peer exhausting its allowance must not silence another" + ); + } + + /// **Proves (dig-node#501, security round 1, HIGH, second half):** a flood of cheap negative + /// verdicts cannot displace a verdict this node earned. + /// + /// **Catches:** attacker-paced eviction. The previous `remember` evicted one arbitrary entry per + /// insert past [`MAX_CACHED_VERDICTS`], which reads as conservative and is not: `Unbonded` is + /// the verdict a stranger elicits for FREE by naming coin ids that do not exist, so at an + /// attacker-chosen insert rate the whole map turns over in under a minute and every earned + /// `Bonded` is discarded — the "memoisation into an amplifier" the constant's own doc warns + /// about, reached one entry at a time instead of all at once. + /// + /// The honest entries are counted rather than sampled, so an implementation that dropped one + /// per cheap insert would fail here rather than pass on the entry the test happened to check. + #[test] + fn a_flood_of_cheap_negatives_cannot_evict_an_earned_verdict() { + let store = Bytes32::new(STORE); + let root = Bytes32::new(ROOT); + let cache = VerdictCache::default(); + + let earned: Vec = (0..MAX_CACHED_VERDICTS) + .map(|n| VerdictKey::new([n as u8; 32], store, root, 7, &format!("{n:064x}"))) + .collect(); + for key in &earned { + cache.remember(*key, BondVerdict::Bonded); + } + let held = earned.iter().filter(|k| cache.get(k).is_some()).count(); + assert_eq!( + held, MAX_CACHED_VERDICTS, + "control: the map really is full of live earned verdicts before the flood" + ); + + let liar = "ff".repeat(32); + for invented in 0..64u8 { + cache.remember( + VerdictKey::new([invented; 32], store, root, 7, &liar), + BondVerdict::Unbonded, + ); + } + + assert_eq!( + earned.iter().filter(|k| cache.get(k).is_some()).count(), + MAX_CACHED_VERDICTS, + "not one earned verdict may be traded for a negative that cost the publisher nothing" + ); + assert_eq!( + cache.get(&VerdictKey::new([0x07; 32], store, root, 7, &liar)), + None, + "the cheap negative is refused admission rather than admitted at an honest entry's cost" + ); + } + + /// **Proves (dig-node#473):** the declaration source is LIVE, so the pre-read short-circuit no + /// longer withholds every verdict — and the probe that lifts it is a genuine fail-closed + /// self-test rather than a switch someone must remember to flip. + /// + /// **Catches:** a silent regression in the format agreement. The probe asks the production + /// [`peer_declaration`] for the one term a coin owned by `probe_peer` would carry. If a future + /// `dig-mirror-coin` changed the declaration format, or the accessor stopped answering, this + /// goes false and `verdict_for` returns to withholding credit from everyone — the safe + /// direction — instead of promoting on a check it can no longer make. Asserting it TRUE here is + /// what makes that failure visible as a red test rather than as silently inert ranking. + #[test] + fn the_declaration_source_is_live_and_its_probe_is_a_fail_closed_self_test() { + assert!( + declaration_source_is_readable(), + "the typed accessor must answer, or promotion is unreachable for every input" + ); + let peer = "aa".repeat(32); - for terms in [ - vec![], + assert_eq!( + peer_declaration(&[format!("dig-peer:{peer}")], &peer), + PeerDeclaration::DeclaresThisPeer, + "control: the probe and the production path are the same function" + ); + } + + /// **Proves (dig-node#473):** only the coin's own declaration of THIS claimant promotes it. + /// + /// **Catches:** the binding degrading to "some coin bonds this content", which is the weaker + /// question a stranger republishing a public coin id passes. Every row shares one claimant and + /// varies only what the coin says, so a check that ignored the terms would answer identically + /// for all of them. + #[test] + fn only_the_coins_own_declaration_of_this_claimant_promotes_it() { + let peer = "aa".repeat(32); + let other = "bb".repeat(32); + + let promotes = [ vec![format!("dig-peer:{peer}")], - vec![format!("dig-peer:{}", "bb".repeat(32))], - vec!["https://mirror.example/store".to_string()], - ] { - assert_ne!( + vec![ + "https://mirror.example/store".to_string(), + format!("dig-peer:{peer}"), + ], + // The owner wrote the id in the other case; it denotes the same SHA-256. + vec![format!("dig-peer:{}", peer.to_uppercase())], + ]; + for terms in promotes { + assert_eq!( peer_declaration(&terms, &peer), PeerDeclaration::DeclaresThisPeer, - "promotion must stay unreachable until the binding has a typed source; terms {terms:?}" + "terms {terms:?}" + ); + } + + let withholds = [ + vec![], + vec!["https://mirror.example/store".to_string()], + // Someone else's coin, republished by this claimant. + vec![format!("dig-peer:{other}")], + // Two declarations name nobody -- one coin's collateral must not back two peers. + vec![format!("dig-peer:{peer}"), format!("dig-peer:{other}")], + // Prefix lookalikes are ordinary advertised strings. + vec![format!("xdig-peer:{peer}")], + vec![format!("dig-peers:{peer}")], + // A payload that is not a peer id. + vec!["dig-peer:nope".to_string()], + ]; + for terms in withholds { + assert_eq!( + peer_declaration(&terms, &peer), + PeerDeclaration::Silent, + "terms {terms:?}" ); } } diff --git a/crates/dig-node-service/src/mirror/lifecycle.rs b/crates/dig-node-service/src/mirror/lifecycle.rs index 102a1884..2519744f 100644 --- a/crates/dig-node-service/src/mirror/lifecycle.rs +++ b/crates/dig-node-service/src/mirror/lifecycle.rs @@ -147,6 +147,9 @@ pub struct NodeMirrorEffects<'a, S: ChainSource> { committed_coin_ids: Result>, PassError>, /// Where this node advertises its stores can be fetched from. Empty means it cannot advertise. advertised_urls: Vec, + /// This node's own `peer_id`, written into every coin it creates so a reader can credit the bond + /// to it. `None` before the peer network has started. + own_peer_id: Option, /// The chain, for the owned-coin scan and for the reclaim spends. source: &'a S, /// This node's operator puzzle hash — a public value, derived once at bring-up. @@ -169,7 +172,46 @@ pub struct NodeMirrorEffects<'a, S: ChainSource> { resolved: std::cell::RefCell>, } +/// The declaration a create must carry, or the reason this pass must not create at all. +/// +/// **A create with no declaration is REFUSED, not degraded (dig-node#501, security round 1).** A +/// coin created without one locks real collateral for a whole epoch and can never be credited to +/// this node by any reader — `verdict_for` promotes on the coin's own declaration of the claimant, +/// so an undeclared coin buys discovery weight it will never receive. The money is spent either +/// way; only the benefit is lost. +/// +/// Deferring costs a pass. The scheduler runs again, and a node whose peer network has not yet +/// reported an identity almost always has one by the next pass. An epoch of locked $DIG against one +/// deferred pass is not a close call, which is why this fails closed — and why +/// [`super::spends::build_create`] takes a declaration rather than an `Option`, so the refusal is +/// structural instead of a rule a later caller could forget. +fn declaration_for_create( + own_peer_id: Option<&str>, +) -> Result { + let Some(peer_id) = own_peer_id else { + return Err(PassError::Identity( + "the peer network has not reported an identity yet, so a coin created now would name \ + no peer and no reader could credit its bond to this node" + .into(), + )); + }; + dig_mirror_coin::PeerDeclaration::from_hex(peer_id).map_err(|error| { + PassError::Identity(format!( + "this node reported a peer id that is not a well-formed peer id ({error}), so a coin \ + created now would name no peer" + )) + }) +} + impl<'a, S: ChainSource> NodeMirrorEffects<'a, S> { + /// The peer declaration to write into a coin this node creates, or the reason there is none. + /// + /// Delegates to the free [`declaration_for_create`] so the decision can be driven directly by a + /// test without assembling a whole effects struct around it. + fn declaration_for_create(&self) -> Result { + declaration_for_create(self.own_peer_id.as_deref()) + } + /// Assemble the effects for one pass from readings the scheduler has already taken. #[allow(clippy::too_many_arguments)] pub fn new( @@ -177,6 +219,7 @@ impl<'a, S: ChainSource> NodeMirrorEffects<'a, S> { dig_balance: Result, committed_coin_ids: Result, PassError>, advertised_urls: Vec, + own_peer_id: Option, source: &'a S, owner_puzzle_hash: Bytes32, signer: Option<&'a MirrorSigner>, @@ -191,6 +234,7 @@ impl<'a, S: ChainSource> NodeMirrorEffects<'a, S> { // of the audit record, and within-pass accumulation is this type's business. committed_coin_ids: committed_coin_ids.map(std::cell::RefCell::new), advertised_urls, + own_peer_id, source, owner_puzzle_hash, signer, @@ -449,6 +493,18 @@ impl MirrorEffects for NodeMirrorEffects<'_, S> { // `sign_and_broadcast` takes the set mutably to record what this bundle consumed, and a // borrow still live across that call is a runtime panic on the money path rather than a // compile error — so the scope is the guarantee, and it is deliberately narrow. + // BEFORE any coin is selected: a coin created with no declaration would lock this + // collateral for an epoch uncreditably, so the pass defers rather than spends (see + // `declaration_for_create`). + let declaration = self.declaration_for_create().inspect_err(|error| { + tracing::warn!( + target: "mirror", + %error, + store_id = %bond.store_id, + "not creating mirror collateral this pass" + ); + })?; + let dig_coins = { let committed = committed.borrow(); // `_detailed`, so the skips are CONSUMED rather than dropped on the floor. The plain @@ -490,6 +546,7 @@ impl MirrorEffects for NodeMirrorEffects<'_, S> { root_hash, num_bigint::BigInt::from(epoch), self.advertised_urls.clone(), + declaration, amount_dig_base_units, dig_coins, signer.synthetic_key(), @@ -846,6 +903,77 @@ pub fn journal() -> SpendJournal { mod tests { use super::*; + /// **Proves (dig-node#501, security round 1, MEDIUM 3):** a node that cannot name ITSELF does + /// not create a mirror coin. + /// + /// **Catches:** the create-side guard failing OPEN on money. Before this, both reasons a + /// declaration could be absent produced a `None` that `build_create` cheerfully turned into a + /// coin — so a host whose peer network never reported an identity locked a full epoch's + /// collateral, every pass, in a bond no reader could ever credit to it. The rustdoc said so and + /// the code did the opposite. + /// + /// All three rows are asserted, and the `Ok` row is the control: a test that only asserted the + /// refusals would pass against a function that refused unconditionally, which would stop this + /// node bonding anything at all. + #[test] + fn a_create_is_refused_when_this_node_cannot_name_itself() { + let honest = "aa".repeat(32); + let declaration = declaration_for_create(Some(&honest)) + .expect("control: a well-formed peer id yields a declaration and the create proceeds"); + assert!( + declaration.names(&honest), + "control: the declaration written into the coin must name THIS node" + ); + + for absent in [None, Some("not-a-peer-id"), Some("")] { + let refusal = declaration_for_create(absent) + .expect_err("a create with no declaration locks uncreditable collateral"); + assert!( + matches!(refusal, PassError::Identity(_)), + "the refusal is its own cause, not a wallet failure an operator would go debug: \ + got {refusal}" + ); + } + } + + /// **Proves (dig-node#501, review round 2, finding 2):** this module's operator-facing refusal + /// text is not corrupted by a lost line continuation. + /// + /// **Catches:** exactly what shipped at `59a0331`. The two `warn!` literals these messages + /// replace carried an 18- and a 22-space run from an eaten `\`, so the emitted line read + /// "the coin will  … name no peer". It compiles, no assertion looked at the text, and + /// the mangled and correct forms are indistinguishable in a normal diff view — so nothing but a + /// test over the VALUE can see it. Asserted over the rendered message rather than over the + /// source, because that is what an operator actually reads, and it also covers the `Display` + /// prefix the message is glued to. + #[test] + fn the_refusal_messages_read_as_sentences() { + let messages: Vec = [None, Some("not-a-peer-id")] + .into_iter() + .map(|peer| { + declaration_for_create(peer) + .expect_err("both rows refuse") + .to_string() + }) + .collect(); + + assert_eq!(messages.len(), 2, "both refusal literals must be exercised"); + for message in &messages { + assert!( + !message.contains(" "), + "a run of two spaces is an eaten line continuation, not prose: {message:?}" + ); + assert!( + !message.chars().any(char::is_control), + "no control character belongs in an operator-facing line: {message:?}" + ); + assert!( + message.contains("no peer"), + "control: the message must still say what went wrong: {message:?}" + ); + } + } + /// Bring-up with no operator wallet yields NO signer, and says which of the two reasons it was. /// /// The two branches are asserted separately because they collapse to the same `None` signer and diff --git a/crates/dig-node-service/src/mirror/pass.rs b/crates/dig-node-service/src/mirror/pass.rs index 5e82b666..4a08e8c6 100644 --- a/crates/dig-node-service/src/mirror/pass.rs +++ b/crates/dig-node-service/src/mirror/pass.rs @@ -791,7 +791,7 @@ mod tests { epoch: NOW_EPOCH, amount_dig_base_units: REQUIRED, }, - "the coin is still on chain and still locking money, switch or no switch -- and the reclaim carrying it home is the more precise thing to say than `Bonded`" + "the coin is still on chain and still locking money, switch or no switch -- and the reclaim carrying it home is the more precise thing to say than `Bonded`" ); assert_eq!( state("bb", "22"), @@ -1064,7 +1064,7 @@ mod tests { assert_eq!( d.states, vec![(bond("aa", "11"), BondState::Pending)], - "not `Bonded` -- nothing is advertising it yet -- and not `Reclaiming`, which would describe last epoch's money while the question is about this epoch's capsule" + "not `Bonded` -- nothing is advertising it yet -- and not `Reclaiming`, which would describe last epoch's money while the question is about this epoch's capsule" ); } } diff --git a/crates/dig-node-service/src/mirror/runner.rs b/crates/dig-node-service/src/mirror/runner.rs index 7a6b528a..5a4d7950 100644 --- a/crates/dig-node-service/src/mirror/runner.rs +++ b/crates/dig-node-service/src/mirror/runner.rs @@ -88,6 +88,13 @@ pub enum PassError { /// `Display` delegates, so every existing consumer that only renders a `PassError` is /// unaffected: the message an operator sees is the `FundingError`'s own. Funding(super::funding::FundingError), + /// This node cannot yet name ITSELF, so a create would lock uncreditable collateral. + /// + /// Its own variant rather than a [`PassError::Wallet`] because the wallet is fine and the + /// operator has nothing to fix: the peer network has simply not reported an identity yet, and + /// the next pass usually has one. Rendering it as a wallet failure would send an operator to + /// debug a wallet that is working (dig-node#501, security round 1). + Identity(String), } impl std::fmt::Display for PassError { @@ -97,6 +104,9 @@ impl std::fmt::Display for PassError { PassError::Chain(cause) => write!(f, "the chain source could not be read: {cause}"), PassError::Wallet(cause) => write!(f, "the operator wallet could not act: {cause}"), PassError::Funding(cause) => write!(f, "{cause}"), + PassError::Identity(cause) => { + write!(f, "this node cannot declare its own peer identity: {cause}") + } } } } @@ -1371,7 +1381,7 @@ mod tests { assert_eq!( report.states, vec![(bond("aa", "11"), BondState::FundsUnknown)], - "not `Unfunded` -- this pass has no evidence the wallet is short, only that it could not be read" + "not `Unfunded` -- this pass has no evidence the wallet is short, only that it could not be read" ); } diff --git a/crates/dig-node-service/src/mirror/spends.rs b/crates/dig-node-service/src/mirror/spends.rs index f1bef2db..94582405 100644 --- a/crates/dig-node-service/src/mirror/spends.rs +++ b/crates/dig-node-service/src/mirror/spends.rs @@ -170,6 +170,7 @@ pub fn build_create( root_hash: Bytes32, epoch: BigInt, urls: Vec, + declared_peer: dig_mirror_coin::PeerDeclaration, collateral_dig_base_units: u64, dig_coins: Vec, synthetic_key: PublicKey, @@ -178,17 +179,18 @@ pub fn build_create( ) -> Result { let spends = dig_mirror_coin::create( MirrorAdvertisement { + // The peer this collateral stands behind, and NOT an `Option`. A coin that names + // nobody bonds content for no one in particular: no reader can credit it to this node, + // so the collateral is locked for an epoch and buys discovery weight it will never + // receive. Taking a declaration by value is what makes that unspendable rather than + // merely discouraged -- `dig_mirror_coin::create` accepts `None`, so a signature that + // passed the `Option` through would leave the money decision to whichever caller + // remembered it. `lifecycle::declaration_for_create` is where the refusal is decided. + declared_peer: Some(declared_peer), store_launcher_id, root_hash, epoch: epoch.clone(), urls, - // No peer declaration: this reproduces exactly what dig-mirror-coin 0.7 wrote, which - // had no declared_peer concept at all, so the bump changes nothing about the coins this - // node creates. It is a deliberate choice rather than a default -- the crate made the - // field required precisely so a consumer cannot inherit one silently. Binding this - // collateral to the node's own DIG peer id is dig-node#473, which owns that decision and - // the signature change it needs. - declared_peer: None, collateral: collateral_dig_base_units, }, dig_coins, diff --git a/crates/dig-node-service/src/server.rs b/crates/dig-node-service/src/server.rs index 8abf44c1..c1ffe0fb 100644 --- a/crates/dig-node-service/src/server.rs +++ b/crates/dig-node-service/src/server.rs @@ -2906,6 +2906,11 @@ fn spawn_mirror_passes( // any chain read rather than staking collateral on an advertisement // nobody can act on. advertised_urls.clone(), + // Re-read per pass rather than captured at spawn: this task starts + // beside the peer network rather than after it, so a value read once + // could be `None` for the life of the node and every coin it created + // would name nobody. + node.own_peer_id(), &source, owner_puzzle_hash, signer_ref, diff --git a/crates/dig-node-service/tests/mirror_advertised_urls.rs b/crates/dig-node-service/tests/mirror_advertised_urls.rs index 22358baf..f6f8268c 100644 --- a/crates/dig-node-service/tests/mirror_advertised_urls.rs +++ b/crates/dig-node-service/tests/mirror_advertised_urls.rs @@ -246,6 +246,11 @@ fn the_configured_urls_reach_the_coin_in_the_operators_order() { Ok(PER_COIN), Ok(HashSet::new()), urls, + // A well-formed peer id, because `create` now refuses without one: a coin naming no peer + // locks collateral no reader could credit. This fixture is about URL ORDER, so it must + // reach the advertisement rather than stop at the identity guard. `repeat` rather than a + // 64-character literal so the length is right by construction instead of by counting. + Some("a1".repeat(32)), &chain, signer.owner_puzzle_hash(), Some(&signer), @@ -311,6 +316,11 @@ fn an_all_rejected_value_refuses_and_spends_nothing() { Ok(PER_COIN), Ok(HashSet::new()), urls, + // Deliberately absent, and deliberately UNREACHED: the advertisement guard returns before + // `declaration_for_create` is consulted, so this row still refuses for the URL reason. It + // is left as `None` so that a future reordering putting the identity guard first changes + // the REASON, which the assertion below now names explicitly. + None, &chain, signer.owner_puzzle_hash(), Some(&signer), @@ -319,10 +329,18 @@ fn an_all_rejected_value_refuses_and_spends_nothing() { runtime.handle().clone(), ); - let refused = effects.create(&bond(0xB2, 0xD4), EPOCH, PER_COIN); + let reason = effects + .create(&bond(0xB2, 0xD4), EPOCH, PER_COIN) + .expect_err( + "a mirror with nowhere to fetch from is not a mirror, so the create must refuse", + ); + // WHICH refusal, not merely that one happened. `create` now has a second early return -- the + // peer-identity guard -- and this fixture carries no declaration, so a bare `is_err()` would + // pass just as happily if the identity guard were reordered ahead of the advertisement one, + // leaving the URL guard this test exists for completely unexercised. assert!( - refused.is_err(), - "a mirror with nowhere to fetch from is not a mirror, so the create must refuse" + reason.to_string().contains("at least one URL"), + "the refusal must name the missing advertisement rather than any other cause: {reason}" ); assert!( broadcast_bytes(&broadcaster).is_empty(), diff --git a/crates/dig-node-service/tests/mirror_bond_corroboration.rs b/crates/dig-node-service/tests/mirror_bond_corroboration.rs index 521d60b3..0a06c7e0 100644 --- a/crates/dig-node-service/tests/mirror_bond_corroboration.rs +++ b/crates/dig-node-service/tests/mirror_bond_corroboration.rs @@ -177,6 +177,13 @@ async fn source_over(views: Vec>) -> CorroboratedChainSource { CorroboratedChainSource::new(reads, tokio::runtime::Handle::current()) } +/// The same source, asked at the BOND floor -- what `ChainBondVerifier` actually uses. +async fn bond_floor_source_over(views: Vec>) -> CorroboratedChainSource { + source_over(views) + .await + .requiring_corroboration(dig_wallet::sage::quorum::BOND_CORROBORATION_FLOOR) +} + // --------------------------------------------------------------------------- // The fixtures // --------------------------------------------------------------------------- @@ -470,3 +477,95 @@ fn a_single_source_bonds_the_fabricated_coin() { "the fabricated coin passes every internal-consistency check; only corroboration catches it" ); } + +// --------------------------------------------------------------------------- +// dig-node#513 item 1 -- the bond path's own corroboration floor +// --------------------------------------------------------------------------- + +/// **Proves (dig-node#513 item 1):** a round only TWO peers answered does not bond, because the +/// bond path demands `BOND_CORROBORATION_FLOOR` and not the sync path's +/// `CORROBORATION_FLOOR = 2`. +/// +/// **Catches:** the floor being inherited rather than chosen. `CORROBORATION_FLOOR` is two for a +/// liveness reason that belongs to the SYNC path -- a refused round stalls the wallet's replica -- +/// and at two a pair of colluding peers is a full quorum, so a promotion costs an attacker two +/// voices. On this path a refusal costs nothing, so the floor can be higher and must be. +/// +/// **Why the fixture is shaped this way:** four peers are drawn and two are silent, so the round +/// is *thin* rather than *dishonest*; the two that speak hold the GENUINE bond and agree with each +/// other perfectly. Nothing but the floor can refuse it. The control below is the identical +/// fixture at the default floor, differing in exactly one dimension -- the floor -- so this pair +/// cannot be satisfied by a harness that simply never bonds. +#[tokio::test(flavor = "multi_thread")] +async fn a_two_peer_round_does_not_bond_at_the_bond_floor() { + let honest = honest_bond(); + let view = ChainView::holding(std::slice::from_ref(&honest)); + + let source = bond_floor_source_over(vec![Some(view.clone()), Some(view), None, None]).await; + + assert_eq!( + verdict(&source, honest.1.coin_id()), + BondVerdict::Unverified, + "two agreeing peers bonded a coin on the path where refusing is free" + ); +} + +/// **Proves:** the control for the case above -- the SAME two-peer round DOES bond at the default +/// floor, so the refusal there is the bond floor and not the thinness of the fixture. +/// +/// It also pins the sync path's floor from the other side: `CORROBORATION_FLOOR` must stay at two, +/// because raising it is what froze a user's replica, and a change that raised the shared constant +/// to satisfy the case above would turn this control red. +#[tokio::test(flavor = "multi_thread")] +async fn the_same_two_peer_round_still_bonds_at_the_default_floor() { + let honest = honest_bond(); + let view = ChainView::holding(std::slice::from_ref(&honest)); + + let source = source_over(vec![Some(view.clone()), Some(view), None, None]).await; + + assert_eq!( + verdict(&source, honest.1.coin_id()), + BondVerdict::Bonded, + "fixture: a two-peer round must bond at the default floor, or the case above proves nothing" + ); +} + +/// **Proves:** the bond floor is not bought by DIALLING wider. Three peers answer, but only two of +/// them agree, and the round is refused. +/// +/// **Catches:** a floor enforced only on how many peers ANSWERED. Under that reading a round of +/// twelve in which two voices agree clears a floor of three, which is the exact shape -- two +/// colluding peers plus noise -- the floor exists to refuse. `tally_with_floor` therefore applies +/// it to the AGREEING count too. +#[tokio::test(flavor = "multi_thread")] +async fn a_wide_round_in_which_only_two_peers_agree_does_not_bond() { + let honest = honest_bond(); + let forged = fabricated_bond(); + let view = ChainView::holding(std::slice::from_ref(&honest)); + let dissent = ChainView::holding(std::slice::from_ref(&forged)); + + let source = bond_floor_source_over(vec![Some(view.clone()), Some(view), Some(dissent)]).await; + + assert_eq!( + verdict(&source, honest.1.coin_id()), + BondVerdict::Unverified, + "two agreeing voices among three answers cleared a floor of three" + ); +} + +/// **Proves:** the control for the whole floor -- at the bond floor, a round of THREE agreeing +/// peers still bonds a genuine coin. The floor is a floor, not a wall. +#[tokio::test(flavor = "multi_thread")] +async fn three_agreeing_peers_do_bond_at_the_bond_floor() { + let honest = honest_bond(); + let view = ChainView::holding(std::slice::from_ref(&honest)); + + let source = + bond_floor_source_over(vec![Some(view.clone()), Some(view.clone()), Some(view)]).await; + + assert_eq!( + verdict(&source, honest.1.coin_id()), + BondVerdict::Bonded, + "the bond floor refused a round that met it exactly" + ); +} diff --git a/crates/dig-node-service/tests/mirror_bond_verify.rs b/crates/dig-node-service/tests/mirror_bond_verify.rs index 8e422ab5..510de556 100644 --- a/crates/dig-node-service/tests/mirror_bond_verify.rs +++ b/crates/dig-node-service/tests/mirror_bond_verify.rs @@ -411,3 +411,148 @@ fn a_coin_that_passes_every_chain_check_is_still_not_promoted_to_a_claimant() { "a bond proven on chain must not promote a peer the coin does not name" ); } + +/// The world for the promotion rows: two honestly published, fully collateralised mirror coins of +/// `store_a()` at `root_1()`, differing in **which peer each one declares** — plus one that declares +/// the holder but bonds `root_2()`. +/// +/// **Two different wallets rather than two coins of one wallet, and that is a fixture requirement +/// with a reason.** `creating_spend` derives a coin's parent from `(owner, asset, amount)`, so two +/// same-amount advertisements by one wallet are the SAME coin and the second silently overwrites +/// the first's creating spend. Using distinct owners keeps the collateral, the store, the root and +/// the epoch identical across the two rows, so the only thing the promotion decision can see +/// differing between them is the declared peer id. Each coin's hint is recomputed from its own +/// lineage owner, so both are honest publications. +fn declaring_bonds(holder: &str, stranger: &str) -> (Chain, Bytes32, Bytes32, Bytes32) { + let declares_holder = wallet(3); + let declares_stranger = wallet(4); + let wrong_root = wallet(5); + + let (spend_holder, coin_holder) = creating_spend( + &declares_holder, + &mirror_memos( + &declares_holder, + store_a(), + root_1(), + &["https://holder.example", &format!("dig-peer:{holder}")], + ), + ); + let (spend_stranger, coin_stranger) = creating_spend( + &declares_stranger, + &mirror_memos( + &declares_stranger, + store_a(), + root_1(), + &["https://holder.example", &format!("dig-peer:{stranger}")], + ), + ); + let (spend_wrong, coin_wrong) = creating_spend( + &wrong_root, + &mirror_memos( + &wrong_root, + store_a(), + root_2(), + &["https://holder.example", &format!("dig-peer:{holder}")], + ), + ); + + let chain = Chain::holding(&[ + (spend_holder, coin_holder), + (spend_stranger, coin_stranger), + (spend_wrong, coin_wrong), + ]); + ( + chain, + coin_holder.coin_id(), + coin_stranger.coin_id(), + coin_wrong.coin_id(), + ) +} + +/// **Proves (dig-node#466 / #473, review round 2, finding 3):** promotion through `verdict_for` +/// requires BOTH bindings — the coin bonds the requested content, and the coin's own owner-written +/// declaration names the peer claiming it — against a chain holding real coins. +/// +/// **Catches:** exactly what the suite could not see at `59a0331`. Every `Bonded` in the unit tests +/// arrives through a double that builds the declaration term FROM the claiming peer id, so it +/// returns `Bonded` for every claimant by construction and cannot represent a coin that declares +/// someone else. A `verdict_for` that promoted on "some coin bonds this content" — the weaker +/// question a stranger republishing a public coin id passes — would have satisfied that suite in +/// full. +/// +/// The three rows share store, root, epoch, collateral and required collateral, and are asked with +/// the same claimant. Row 2 differs from row 1 in the declared peer id, and row 3 in the generation +/// the coin bonds. Row 3 also exercises `advertises` against a coin that EXISTS, which no prior +/// test did: the existing negative names a coin the chain does not hold, which returns before that +/// step. +/// +/// **Each negative carries a control, because a negative verdict alone is not evidence.** +/// `verdict_for` reaches `Unverified` from two disjoint places — the chain half producing no coin, +/// and `PeerDeclaration::Silent` at the final match — and reaches `Unbonded` from a coin that is +/// merely unverifiable as easily as from one that genuinely bonds elsewhere. So rows 2 and 3 are +/// each followed by a row proving that the SAME coin verifies completely when asked the question it +/// should answer positively. Without those, either negative would be satisfied by a fixture too +/// broken to verify, and the test would measure nothing while appearing to measure the decision. +/// Note that rows 1 and 2 are necessarily minted by different wallets: `creating_spend` derives a +/// coin's parent from `(owner, asset, amount)`, so one wallet cannot publish two same-amount +/// advertisements. "Differs only in the declared peer id" is therefore a statement about what the +/// DECISION can see, which row 2's control is what actually establishes. +#[test] +fn only_a_coin_that_declares_the_claimant_promotes_it() { + let holder = "aa".repeat(32); + let stranger = "bb".repeat(32); + let (chain, declares_holder, declares_stranger, bonds_another_root) = + declaring_bonds(&holder, &stranger); + + let promote_as = |coin, root, claimant: &str| { + verdict_for( + &chain, + store_a(), + root, + &epoch(), + Some(COLLATERAL), + claimant, + coin, + ) + }; + let promote = |coin, root| promote_as(coin, root, &holder); + + assert_eq!( + promote(declares_holder, root_1()), + BondVerdict::Bonded, + "the coin bonds this content and its owner named this claimant, so both halves hold" + ); + assert_eq!( + promote(declares_stranger, root_1()), + BondVerdict::Unverified, + "an equally valid bond that names a DIFFERENT peer promotes nobody: credit is withheld, \ + never subtracted, because this record may be a lie about that coin's real holder" + ); + assert_eq!( + promote(bonds_another_root, root_1()), + BondVerdict::Unbonded, + "a real coin that declares the claimant but advertises another generation disproves the \ + claim -- the step that catches it is `advertises`, on a coin that exists" + ); + assert_eq!( + promote(bonds_another_root, root_2()), + BondVerdict::Bonded, + "control: that same coin promotes this claimant for the generation it actually bonds, so \ + the row above is attributable to the triple and not to a fixture too broken to verify" + ); + // Control for row 2, and it is load-bearing for the same reason row 4 is. `verdict_for` + // reaches `Unverified` from TWO disjoint places: the chain half failing to produce a coin + // (unreadable source, hint or lineage mismatch, admission exhausted), and `PeerDeclaration:: + // Silent` at the final match. Row 2's coin is minted by a DIFFERENT wallet from row 1's -- + // unavoidably, since `creating_spend` derives the parent from `(owner, asset, amount)` and one + // wallet cannot publish two same-amount advertisements -- so "differs only in the declared + // peer id" is a claim about the DECISION's inputs, not about the fixture's construction. + // Without this row, a `declares_stranger` coin broken anywhere in the chain half would satisfy + // row 2 while proving nothing whatever about the declaration. + assert_eq!( + promote_as(declares_stranger, root_1(), &stranger), + BondVerdict::Bonded, + "control: row 2's coin passes the ENTIRE chain half and promotes the peer it actually \ + names, so row 2's Unverified is attributable to the declaration and to nothing else" + ); +} diff --git a/crates/dig-node-service/tests/mirror_intra_pass_reservation.rs b/crates/dig-node-service/tests/mirror_intra_pass_reservation.rs index d08d4fab..02a2140d 100644 --- a/crates/dig-node-service/tests/mirror_intra_pass_reservation.rs +++ b/crates/dig-node-service/tests/mirror_intra_pass_reservation.rs @@ -232,6 +232,10 @@ fn two_creates_in_one_pass_select_disjoint_coins() { // Non-empty: `create` refuses before any chain read without one, and a probe that tripped // that refusal would assert nothing about coin selection. vec!["https://mirror.example/dig".to_string()], + // A well-formed peer id, for the same reason the URL list is non-empty: `create` refuses + // before selecting any coin without one, and a probe that stopped at the identity guard + // would assert nothing about DISJOINTNESS, which is the property under test. + Some("a1".repeat(32)), &chain, signer.owner_puzzle_hash(), Some(&signer), @@ -285,6 +289,10 @@ fn the_only_coin_funds_one_create_and_the_second_refuses() { Ok(PER_COIN), Ok(HashSet::new()), vec!["https://mirror.example/dig".to_string()], + // A well-formed peer id: `create` refuses before selecting any coin without one, and this + // fixture needs the FIRST create to genuinely reach coin selection so that the second one + // has something already reserved to collide with. + Some("a1".repeat(32)), &chain, signer.owner_puzzle_hash(), Some(&signer), diff --git a/crates/dig-wallet/src/sage/corroborated_source.rs b/crates/dig-wallet/src/sage/corroborated_source.rs index 8e0dd02a..2e274514 100644 --- a/crates/dig-wallet/src/sage/corroborated_source.rs +++ b/crates/dig-wallet/src/sage/corroborated_source.rs @@ -60,13 +60,35 @@ use super::peer_reads::PeerCorroboratedReads; pub struct CorroboratedChainSource { reads: Arc, handle: tokio::runtime::Handle, + /// How many peers must corroborate each read. [`super::quorum::CORROBORATION_FLOOR`] unless a + /// caller asked for more via [`CorroboratedChainSource::requiring_corroboration`]. + floor: usize, } impl CorroboratedChainSource { /// A source over `reads`, bridging each blocking call onto `handle`. #[must_use] pub fn new(reads: Arc, handle: tokio::runtime::Handle) -> Self { - Self { reads, handle } + Self { + reads, + handle, + floor: super::quorum::CORROBORATION_FLOOR, + } + } + + /// The same source, refusing any read fewer than `floor` peers corroborate. + /// + /// The seam a caller whose verdict RANKS a peer uses -- dig-node's mirror bond path passes + /// [`super::quorum::BOND_CORROBORATION_FLOOR`] here. It is a caller's choice rather than this + /// type's default because the two callers pay opposite prices for a refusal: the sync path + /// stalls a replica, the bond path merely declines a promotion. + /// + /// A `floor` below [`super::quorum::CORROBORATION_FLOOR`] is not honoured; this can only + /// tighten. + #[must_use] + pub fn requiring_corroboration(mut self, floor: usize) -> Self { + self.floor = floor.max(super::quorum::CORROBORATION_FLOOR); + self } /// Drives one corroborated read to completion from a synchronous caller. @@ -86,12 +108,33 @@ impl CorroboratedChainSource { .to_string(), )) } - Ok(_) => Ok(tokio::task::block_in_place(|| self.handle.block_on(fut))), - Err(_) => Ok(self.handle.block_on(fut)), + Ok(_) => guard_panics(|| tokio::task::block_in_place(|| self.handle.block_on(fut))), + Err(_) => guard_panics(|| self.handle.block_on(fut)), } } } +/// Runs `f`, converting a panic into a [`ChainSourceError::Transport`] so the synchronous +/// [`ChainSource`] boundary never unwinds (dig-node#513). +/// +/// `chia-query`'s own bridge carries this backstop (`provider_registry::bridge::guard_panics`) and +/// this adapter, which replaces that bridge on the bond path, must not be weaker than the thing it +/// replaces. The runtime-flavour check above catches the misuse we can NAME; this catches the ones +/// we cannot. A panic crossing a `ChainSource` method has no defined behaviour for the caller -- +/// on the bond path it would unwind out of a `block_in_place` inside a locate, taking a read-path +/// task with it, which turns a chain hiccup into a denial of the read the verdict was decorating. +/// +/// `AssertUnwindSafe` because the future is consumed exactly once and a panic leaves no observable +/// half-mutated state behind this facade -- the same reasoning `chia-query` records for its own. +fn guard_panics(f: impl FnOnce() -> T) -> Result { + std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)).map_err(|_| { + ChainSourceError::Transport( + "corroborated chain source caught a panic while blocking on the async runtime" + .to_string(), + ) + }) +} + /// The one spelling of a coin id [`PeerCorroboratedReads`] keys its cache on: lowercase hex, no /// `0x`. `hex::encode` of the 32 bytes produces exactly that, so the key can never drift from the /// bytes the caller asked about. @@ -188,7 +231,11 @@ impl ChainSource for CorroboratedChainSource { fn coin_record(&self, coin_id: Bytes32) -> Result, Self::Error> { let key = key_for(coin_id); let answer = self - .block_on(async { self.reads.coin_record_by_id(&key).await })? + .block_on(async { + self.reads + .coin_record_by_id_at_floor(&key, self.floor) + .await + })? .map_err(|e| ChainSourceError::Transport(e.to_string()))?; answer .as_ref() @@ -201,7 +248,7 @@ impl ChainSource for CorroboratedChainSource { fn coin_spend(&self, coin_id: Bytes32) -> Result, Self::Error> { let key = key_for(coin_id); let answer = self - .block_on(async { self.reads.coin_spend(&key).await })? + .block_on(async { self.reads.coin_spend_at_floor(&key, self.floor).await })? .map_err(|e| ChainSourceError::Transport(e.to_string()))?; answer .as_ref() @@ -268,3 +315,82 @@ impl ChainSource for CorroboratedChainSource { )) } } + +#[cfg(test)] +mod tests { + //! dig-node#513 item 5 -- this adapter replaces `chia-query`'s bridge on the bond path, and + //! must not be weaker than the thing it replaces. + + use std::sync::Arc; + + use super::*; + use crate::sage::db::WalletDb; + use crate::sage::peer_reads::{CoinPeer, PeerSample}; + + /// A sample that holds no peers -- enough to build a source, since these cases never read. + struct NoPeers; + + #[async_trait::async_trait] + impl PeerSample for NoPeers { + async fn draw(&self) -> Vec> { + Vec::new() + } + } + + async fn source() -> CorroboratedChainSource { + let db = WalletDb::open_in_memory() + .await + .expect("in-memory wallet db"); + let reads = Arc::new(PeerCorroboratedReads::new(Arc::new(NoPeers), db)); + CorroboratedChainSource::new(reads, tokio::runtime::Handle::current()) + } + + /// PROPERTY: a panic raised while blocking becomes a `ChainSourceError`, and does NOT unwind + /// out of the synchronous trait boundary. + /// + /// NEAREST WRONG IMPLEMENTATION: the runtime-flavour check alone, which is what this adapter + /// shipped with. It catches the misuse we can NAME and nothing else, while the bridge it + /// replaces (`chia_query::provider_registry::bridge::guard_panics`) catches the rest. The + /// assertion is on the RETURN, not on `catch_unwind` being present: a test that merely called + /// `guard_panics` directly would pass with the backstop deleted from `block_on`, so this drives + /// it through the real path. + #[tokio::test(flavor = "multi_thread")] + async fn a_panic_while_blocking_becomes_an_error_rather_than_unwinding() { + let source = source().await; + let previous = std::panic::take_hook(); + std::panic::set_hook(Box::new(|_| {})); + + let outcome: Result<(), ChainSourceError> = + source.block_on(async { panic!("a read panicked") }); + + std::panic::set_hook(previous); + assert!( + matches!(outcome, Err(ChainSourceError::Transport(_))), + "a panic crossed the ChainSource boundary instead of becoming an error" + ); + } + + /// PROPERTY: the control -- the guarded path still RETURNS an ordinary value, so the case above + /// is not satisfied by a `block_on` that errs unconditionally. + #[tokio::test(flavor = "multi_thread")] + async fn an_ordinary_read_still_returns_its_value_through_the_guard() { + let source = source().await; + assert_eq!(source.block_on(async { 7u32 }).ok(), Some(7)); + } + + /// PROPERTY: the bond floor is a caller's choice and can only TIGHTEN. + #[tokio::test(flavor = "multi_thread")] + async fn the_corroboration_floor_can_only_tighten() { + let strict = source() + .await + .requiring_corroboration(super::super::quorum::BOND_CORROBORATION_FLOOR); + assert_eq!(strict.floor, super::super::quorum::BOND_CORROBORATION_FLOOR); + + let relaxed = source().await.requiring_corroboration(1); + assert_eq!( + relaxed.floor, + super::super::quorum::CORROBORATION_FLOOR, + "a caller talked the source down to a single source" + ); + } +} diff --git a/crates/dig-wallet/src/sage/peer_reads.rs b/crates/dig-wallet/src/sage/peer_reads.rs index 92e081c7..232c59f8 100644 --- a/crates/dig-wallet/src/sage/peer_reads.rs +++ b/crates/dig-wallet/src/sage/peer_reads.rs @@ -228,12 +228,34 @@ impl PeerCorroboratedReads { /// `Ok(None)` means the peers AGREED there is no such coin. Failing to assemble agreement is /// an `Err`. pub async fn coin_record_by_id(&self, coin_id: &str) -> Result> { + self.coin_record_by_id_at_floor(coin_id, quorum::CORROBORATION_FLOOR) + .await + } + + /// [`Self::coin_record_by_id`], demanding `floor` corroborating peers instead of the default + /// [`quorum::CORROBORATION_FLOOR`]. + /// + /// # The cache is not consulted above the default floor + /// + /// A cached row records the ANSWER a past round settled on and not how many peers settled it, + /// so serving one to a caller that asked for stronger corroboration would answer a question + /// the cache cannot answer -- and would make the stricter floor vacuous, since the ordinary + /// sync path fills that cache at the default floor. The stricter caller therefore always asks + /// the peers. It still WRITES what it learns, because a row corroborated by more peers is not + /// less true for the readers that wanted fewer. + pub async fn coin_record_by_id_at_floor( + &self, + coin_id: &str, + floor: usize, + ) -> Result> { let id = normalized(coin_id); let parsed = parse_coin_id(&id)?; let now = self.clock.now_unix(); - if let Some(coin) = self.cached_coin_record_by_id(coin_id).await? { - return Ok(Some(coin)); + if floor <= quorum::CORROBORATION_FLOOR { + if let Some(coin) = self.cached_coin_record_by_id(coin_id).await? { + return Ok(Some(coin)); + } } let peers = self.sample.draw().await; @@ -249,7 +271,7 @@ impl PeerCorroboratedReads { } } - let verdict = quorum::tally(&responses); + let verdict = quorum::tally_with_floor(&responses, floor); let Some(answer) = verdict.corroborated() else { return Err(no_corroboration("coin record", &id, &verdict)); }; @@ -333,12 +355,28 @@ impl PeerCorroboratedReads { /// `Ok(None)` means the peers AGREED the coin is unspent or unknown. Failing to assemble /// agreement is an `Err`, because a lineage walk reads absence as *this is the tip*. pub async fn coin_spend(&self, coin_id: &str) -> Result> { + self.coin_spend_at_floor(coin_id, quorum::CORROBORATION_FLOOR) + .await + } + + /// [`Self::coin_spend`], demanding `floor` corroborating peers. + /// + /// The cache is bypassed above the default floor for the reason + /// [`Self::coin_record_by_id_at_floor`] states, and it applies with more force here: a spend + /// row has no TTL, so one written by a thin round would answer every stricter caller forever. + pub async fn coin_spend_at_floor( + &self, + coin_id: &str, + floor: usize, + ) -> Result> { let id = normalized(coin_id); let parsed = parse_coin_id(&id)?; let now = self.clock.now_unix(); - if let Some(spend) = self.cached_coin_spend(coin_id).await? { - return Ok(Some(spend)); + if floor <= quorum::CORROBORATION_FLOOR { + if let Some(spend) = self.cached_coin_spend(coin_id).await? { + return Ok(Some(spend)); + } } let peers = self.sample.draw().await; @@ -352,7 +390,7 @@ impl PeerCorroboratedReads { } } - let verdict = quorum::tally(&responses); + let verdict = quorum::tally_with_floor(&responses, floor); let Some(answer) = verdict.corroborated() else { return Err(no_corroboration("coin spend", &id, &verdict)); }; diff --git a/crates/dig-wallet/src/sage/quorum.rs b/crates/dig-wallet/src/sage/quorum.rs index 5a5146e0..2d97d8a6 100644 --- a/crates/dig-wallet/src/sage/quorum.rs +++ b/crates/dig-wallet/src/sage/quorum.rs @@ -171,6 +171,25 @@ const _: () = assert!( /// of this node may reasonably overturn it in either direction. pub const CORROBORATION_FLOOR: usize = 2; +/// The corroboration floor a **bond verdict** demands, above [`CORROBORATION_FLOOR`]. +/// +/// Three, and the reason it may be higher here is that the two paths pay opposite prices for +/// refusing. [`CORROBORATION_FLOOR`] is pinned at two because the sync path writes the wallet's +/// replica: a round it refuses is a round the replica does not advance, and demanding more peers +/// than a thin network offers is how a user's node froze for hours. **The bond path writes +/// nothing.** A refused round there yields `unverified`, which shares a rank with `unbonded` and +/// leaves the located slate exactly as a node with no verifier installed would return it, so the +/// cost of refusing is zero and the cost of believing a thin round is a promotion an attacker +/// bought for the price of two voices. +/// +/// It is deliberately a SEPARATE constant rather than a raised [`CORROBORATION_FLOOR`]: raising +/// the shared one would re-create the frozen replica, which is the incident that fixed it at two. +/// +/// Applied through [`tally_with_floor`], which enforces it in BOTH dimensions — at least this many +/// peers must answer, and at least this many must give the same answer — so it cannot be satisfied +/// by a wide round in which only two voices agree. +pub const BOND_CORROBORATION_FLOOR: usize = 3; + /// How many distinct peers one round DIALS before narrowing (dig_ecosystem#2827). /// /// Over-subscribing then pulling back is what makes a round resilient to the ordinary case that @@ -760,14 +779,32 @@ pub struct Response { /// cheaper for an attacker to capture, and [`sybil_success_probability`] will tell you by how /// much. pub fn tally(responses: &[Response]) -> Verdict { + tally_with_floor(responses, CORROBORATION_FLOOR) +} + +/// [`tally`], against a caller-chosen corroboration floor. +/// +/// `floor` is applied in BOTH dimensions: fewer than `floor` answers is +/// [`Verdict::Insufficient`], and an answer given by fewer than `floor` peers is +/// [`Verdict::Split`] however wide the round was. Enforcing only the first would let a +/// twelve-peer round in which two voices agree pass a floor of three, which is the shape the +/// stricter floor exists to refuse. +/// +/// A floor BELOW [`CORROBORATION_FLOOR`] is not honoured -- the never-one-source rule is not a +/// caller's to relax -- so this can only ever be stricter than [`tally`], never weaker. +pub fn tally_with_floor( + responses: &[Response], + floor: usize, +) -> Verdict { + let floor = floor.max(CORROBORATION_FLOOR); let answered = responses.len(); - if answered < CORROBORATION_FLOOR { + if answered < floor { return Verdict::Insufficient { answered, - required: CORROBORATION_FLOOR, + required: floor, }; } - let required = required_agreement(answered); + let required = required_agreement(answered).max(floor); let mut counts: HashMap<&A, usize> = HashMap::new(); for r in responses { diff --git a/crates/dig-wallet/src/sage/quorum/tests.rs b/crates/dig-wallet/src/sage/quorum/tests.rs index e7d719c9..88a7005a 100644 --- a/crates/dig-wallet/src/sage/quorum/tests.rs +++ b/crates/dig-wallet/src/sage/quorum/tests.rs @@ -926,3 +926,65 @@ fn a_thinner_round_is_measurably_easier_to_capture() { floor={at_floor} sample={at_sample} hold={at_hold}" ); } + +// --------------------------------------------------------------------------- +// dig-node#513 item 1 -- a caller-chosen, strictly tighter corroboration floor +// --------------------------------------------------------------------------- + +/// PROPERTY: `tally_with_floor` refuses a round of two at a floor of three, and accepts the round +/// of three -- the bound pinned from BOTH sides, so it cannot pass by refusing everything. +#[test] +fn the_bond_floor_is_pinned_from_both_sides() { + let two = responses(&[("a", 1), ("b", 1)]); + let three = responses(&[("a", 1), ("b", 1), ("c", 1)]); + + assert_eq!( + tally_with_floor(&two, BOND_CORROBORATION_FLOOR), + Verdict::Insufficient { + answered: 2, + required: BOND_CORROBORATION_FLOOR, + }, + "one under the bond floor must not corroborate" + ); + assert!( + tally_with_floor(&three, BOND_CORROBORATION_FLOOR) + .corroborated() + .is_some(), + "at the bond floor exactly, a unanimous round must still corroborate" + ); +} + +/// PROPERTY: the floor binds the AGREEING count, not merely how many peers answered. +/// +/// NEAREST WRONG IMPLEMENTATION: enforcing `floor` only on `responses.len()`. Under that, twelve +/// answers of which two agree clears a floor of three -- two colluding peers plus noise, which is +/// precisely the round the floor exists to refuse. `required_agreement(3)` is 3 already, so the +/// fixture is widened to a size where the ratio alone would permit two. +#[test] +fn a_wide_round_does_not_buy_its_way_past_the_bond_floor() { + let scattered = responses(&[("a", 1), ("b", 1), ("c", 2), ("d", 3), ("e", 4), ("f", 5)]); + + assert!( + matches!( + tally_with_floor(&scattered, BOND_CORROBORATION_FLOOR), + Verdict::Split { .. } + ), + "two agreeing voices in a six-peer round cleared a floor of three" + ); +} + +/// PROPERTY: the floor can only TIGHTEN. A caller asking for one source is given +/// `CORROBORATION_FLOOR` anyway -- the never-one-source rule is not a caller's to relax. +#[test] +fn a_floor_below_the_shared_one_is_not_honoured() { + let one = responses(&[("a", 1)]); + + assert_eq!( + tally_with_floor(&one, 1), + Verdict::Insufficient { + answered: 1, + required: CORROBORATION_FLOOR, + }, + "a caller talked the tally down to a single source" + ); +}