docs(mirror): correct the hint-scan fallback claim and specify unbondable store records - #524
docs(mirror): correct the hint-scan fallback claim and specify unbondable store records#524MichaelTaylor3d wants to merge 11 commits into
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
Five doc sites and the dig-node-core SPEC claimed a verifier falls back to `dig-mirror-coin`'s hint scan when a provider record carries no mirror-coin pointer. It cannot: `discover` and `list` both require `owner_puzzle_hash` because the hint is morphed from the owner, and a `ProviderRecord` carries no owner and no route to one. `mirror_bond.rs` was the load-bearing case -- the claim was the stated justification for `claimed_coin_id` being an `Option` -- so it is rewritten to give the real reason: absence is ordinary, and a verifier answers `Unverified`, withholding credit rather than demoting. SPEC.md 6.2 now states that a store-granularity record is unbondable by construction, so a measurement of pointer-less records is not a measurement of uncollateralised holders. Matches the wording landing in dig-dht PR #29 so the two crates agree. Docs only: no behaviour, signature or wire change. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
|
IN PROGRESS — not the verdict. Fresh-context gate on head Confirmed so far, against
Still checking: cross-crate agreement with dig-dht PR #29, the two new SPEC §6.2 bullets, and a full concept sweep for missed sites. |
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
GATE VERDICT: CHANGES-REQUIRED
Head reviewed: 9816c789829c33c1e07d7745d09448f10ba32735 (resolved from the remote myself; matches the brief).
The rewritten justification in mirror_bond.rs is true and well-sourced - I verified every clause against dig-mirror-coin 0.9.0 and dig-dht 0.15.0, and it agrees with the already-gated dig-dht PR #29. Two findings, both the class this PR exists to eliminate.
Ranked findings
-
GATING -
crates/dig-node-core/src/seams/dig_peer/dht.rs:435andcrates/dig-node-service/src/mirror/pointers.rs:15: the removed claim survives, six lines above a hunk this PR edited. Both read "one coin to fetch instead of a scan of the mirror puzzle hash". That asserts the same counterfactual the PR deletes elsewhere: that a verifier without a pointer performs a scan. Per this PR's own new text (mirror_bond.rs:120-127) an owner-less walk ofmirror_coin_puzzle_hash()is not a route a verifier may act on, anddig_mirror_coin::list/discoverboth requireowner_puzzle_hash(query.rs:269,query.rs:359), which aProviderRecorddoes not carry. It also contradicts the sibling crate: dig-dht PR #29 deliberately changed the identical phrasing atrecord.rs:394andservice.rs:296from "instead of scanning by hint" to "instead of searching for it", precisely so a reader of both SPECs gets one answer.
Accepted wording: "one coin to fetch instead of searching for it", matching dig-dht #29 verbatim. Do not fix this by asserting a verifier CAN scan, and do not delete the untrusted-pointer framing around it; only the counterfactual clause is wrong. -
crates/dig-node-core/SPEC.md:673- "every held capsule always yields one pointer-less record" is false, and it is the calibration claim the bullet exists to make.inventory_content_ids(dht.rs:376-392) emits the store-granularity id deduped per store (seen_stores.insert(store), documented atdht.rs:354as "deduped per store"), plus one capsule id per capsule. A node holding N capsules of one store publishes one pointer-less record and N pointered ones, not N pointer-less ones. As written the bullet overstates the expected pointer-less share, which matters because its stated purpose is telling a measurement reader how to read that ratio.
Accepted wording, already correct in the sibling: "A holder that announces at both granularities therefore always has a pointer-less record alongside a pointered one."
What I verified, and how (so this is a gate, not an opinion)
mirror_bond.rs:120-121"discoverandlistboth takeowner_puzzle_hashas a REQUIRED parameter" - TRUE:dig-mirror-coin-0.9.0/src/query.rs:269list(source, owner_puzzle_hash: Bytes32),query.rs:359discover(source, store_launcher_id, root_hash, owner_puzzle_hash, epoch). Neither isOption.- "because the hint is morphed from the owner" - TRUE:
namespace.rs:76sums all four terms intomirror_hint; the crate's own doc says there is no store-wide bucket to read without one. - "a
ProviderRecordcarries no owner and no route to one" - TRUE:dig-dht-0.15.0/src/record.rs:379-433carriescontent_key,provider_peer_id,addresses,expires_at,unverified_mirror_coin_id. No owner. - "bounded at
MAX_CANDIDATESover a list anyone may extend for the price of a dust coin" - TRUE:query.rs:257(10_000) and its doc verbatim. "unbounded outbound read" is also fair:listfetches the FULLcoin_records_by_puzzle_hashresult before truncating, and authentication is a chain read per candidate. BondVerdictvariants -Bonded/Unverified/Unbondedall exist (mirror_bond.rs:88-104);Unverifiedis documented as "The holder named no coin...", and the credit-only lattice atmirror_bond.rs:37-48puts absent, unprovable and disproven in ONE baseline tier. So "withholds credit ... does not demote" is exactly right, and agrees with the top-levelSPEC.mdverdict table ("unverified ... baseline, position unchanged").- Does any sentence license an unsound verifier or forbid a sound one? Neither. The text gives reasons (unactionable negative, cost) and explicitly says the owner-less walk "is expressible one level down at the chain source" - it does not ban the sound
advertises+declares_peercomposition, and it does not present the scan as an equal-strength substitute. - SPEC 6.2 bullet 2 (pointer-less implies unverified, never demoted) - TRUE against the lattice above.
- SPEC 6.2 bullet 1 mechanism - TRUE:
mirror/pointers.rs:112matches onlyContentId::capsule(store, root), so aContentId::store(...)can never yield a pointer; asserted by the test atpointers.rs:297. Only the per-capsule COUNT (finding 2) is wrong. - Cross-crate agreement - read
origin/loop/28-pointer-doc-truth(1e38358) in full. dig-dht's SPEC andrecord.rsnow say: no fallback search, withhold credit, store-granularity unbondable. Consistent with this PR except for finding 1. - Sweep -
git grepon the head (rgunavailable).hint scan/scanning by hint/not weaker/degrades discovery: 0 hits, control-tested againstby hintandslower, which both return unrelated hits, so the zeros are real.fully supported state: 1 hit, the corrected line. The concept sweep oninstead of a scanis what surfaced finding 1.
No test finding: correct that none were added - nothing behavioural changed.
…d per store - dht.rs: change "scan of the mirror puzzle hash" to "searching for it" - pointers.rs: same wording fix for consistency - SPEC.md: correct deduplication — stores are deduplicated, so S stores produce S pointer-less records; change "a large share" to "some" provider records Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # Cargo.lock # Cargo.toml
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ng merge 0.252.75 (this PR's prior version) is now BELOW origin/main's tip, which would fail ensure-version-increment.yml. Re-bump above the current tip. Co-Authored-By: Claude <noreply@anthropic.com>
…-file fmt (#240) Three fixes folded into one commit since they were all found by the same gate round on the same rebase: - genesis_bringup.rs and the bring-up integration test it names live only on the unmerged loop/240-genesis-bringup branch -- this crate has no such file, so SPEC.md's "Proven by tests/genesis_bringup.rs" was false the moment it landed. The underlying claim (genesis is real/non-zero, no code path produces an all-zero id) is still true and is verified here by genesis_challenge_from's own unit tests in peer.rs. Restated the still-open proof obligation (full bring-up + pool convergence) as open, pointing at issue #240 item 1 rather than a file this repo does not have. - Reclaimed the version at 0.252.94: 0.252.81 collided with two other open PRs (#518, #524) and 0.252.82-93 were also already claimed by other open branches at the time of this rebase. - cargo fmt on the five .rs files this branch's own commits touch (never --all): a module-declaration ordering in dig_peer/mod.rs and a single-line-fits signature in tests/server.rs. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
PASS
Head SHA reviewed: c0921bd (fetched via gh pr view 524 --json headRefOid and git fetch origin pull/524/head).
Scope: doc/comment/SPEC-only change (5 code-comment sites + SPEC.md §6.2), plus workspace version bump 0.252.80 -> 0.252.93 and a trailing-newline .gitignore tweak. No production logic, signature, or wire-format touched.
Per-sentence verification against the code on this head:
-
mirror_bond.rs:112-133(rewritten doc onMirrorBondVerifier'sOption<Bytes32>param) — TRUE.dig_mirror_coin::discover(query.rs:351) andlist(query.rs:269) both takeowner_puzzle_hash: Bytes32as a required, non-Option parameter — confirmed by readingmodules/crates/10-primitives/dig-mirror-coin/src/query.rs.mirror_hint(namespace.rs) sumsstore + root + owner + epochwith an unbounded, freely-chosenepoch— confirmed; the crate's own existing docs (untouched by this PR) already state the forgeability property, so this PR does not introduce or contradict a security claim about the hint.MAX_CANDIDATES = 10_000confirmed at query.rs:257, applied via.take(MAX_CANDIDATES)in bothdiscoverandlist.BondVerdict::Unverifiedexists inmirror_bond.rsandcredit_rankmaps bothUnverifiedandUnbondedto the same baseline rank (mirror_bond.rs:167) — matches the "withholds credit, does not demote" claim.
-
lib.rs:566-568,peer.rs:3145-3146,seams/dig_peer/dht.rs:441-445— the "fallback is the hint scan" phrase replaced with "withholds credit rather than demoting" — TRUE, consistent with (1); these are pure comment edits, no behavior claim beyond what mirror_bond.rs implements. -
mirror/pointers.rs:123-125(hex32's "answering None...") — TRUE, same reasoning;Nonereturn path confirmed unchanged (function body untouched, only the doc comment above it). -
SPEC.md§6.2 new bullets — TRUE:- "store-granularity record is unbondable by construction" — a mirror coin bonds
(store, root, owner, epoch);ContentId::store(...)(referenced at download.rs:3083, 3040, 2202) carries a store id with no root, so it structurally cannot form the bonded tuple. Consistent with the crate-level design already described elsewhere. - "A pointer-less record is unverified, never demoted" — matches
credit_rank's baseline-collapse behavior confirmed in mirror_bond.rs. dig-dht::ProviderRecord(record.rs:379-396) confirmed to have 5 fields and no owner field — supports the "no owner, no route to one" claim, though this file lives in the separatedig-dhtcrate and was not touched by this PR (advisory note below).
- "store-granularity record is unbondable by construction" — a mirror coin bonds
No BORN-FALSE, INCOMPLETE, or VACUOUS sentences found. No sentence claims the mirror hint is binding/unforgeable — the opposite is stated (unchanged, pre-existing crate docs), so no false security claim is introduced.
closingIssuesReferences confirmed via API (gh pr view 524 --json closingIssuesReferences) to resolve to exactly [520] — the body's Closes #520 is not inside a code span and parses correctly.
Advisory (non-gating): dig-dht/src/record.rs:393-399 in the separate dig-dht crate still carries the old phrase "so a verifier can fetch ONE coin instead of scanning by hint" — the same stale claim this PR is correcting elsewhere, left unswept because it's outside this PR's repo. Worth a follow-up ticket in dig-dht so the phrase doesn't survive as the last holdout of the false claim; does not block this PR.
Verdict: PASS. No inline blocking findings; nothing to resolve.
…tion, alert-storm latch (#481) (#529) * chore(mirror): open the bond/funding audit-residue lane at 0.252.32 Salvage anchor for dig-node#527, #513 and #481. Version claimed above every in-flight branch (highest was 0.252.27 on PR #524). Co-Authored-By: Claude <noreply@anthropic.com> * fix(mirror): latch the funding alert on the condition, not on attacker-movable counts `FundingAlertGate::observe` suppressed a repeat unmeasured alert only when the whole `UnmeasuredFunding` value compared equal. `AuthenticationTruncated` carries `skipped` -- `MAX_AUTHENTICATION_ATTEMPTS` minus however many honest coins the bounded walk reached -- and the walk runs over a PUBLIC puzzle hash, so a stranger moves that number by paying one more coin to the operator's $DIG address. The reason value therefore changed between passes, the gate never latched, and a one-time dust spend bought 144 desktop alerts a day: exactly the stream the gate exists to prevent, and the surest way to train an operator to dismiss the alert that matters. Latching now compares `UnmeasuredFunding::alert_key` -- the discriminant plus only those payload fields no stranger can move. `NoCreateAffordable`'s `need_dig_base_units` is derived from the epoch requirement and the plan rather than from the wallet, so it stays IN the key: a changed collateral requirement is a change in what the operator must do, and swallowing it would be its own money defect. `AuthenticationTruncated`'s counts drop out of the key and remain fully available to the alert body, because an operator who is being told still deserves the real figures. The change is private to `funding.rs`: the field type and the comparison, not the public signature of `observe`. Refs dig-node#481 Co-Authored-By: Claude <noreply@anthropic.com> * fix(mirror): consume the funding skip count in production and report it once `FundingSelection.skipped` was computed on every selection and had ZERO production consumers: the mirror lifecycle funded creates through `select_operator_dig_cats`, whose whole body discards it. The module's own doc claimed "a skip is counted and reported, never swallowed", and that was true of the tests and false of the shipped node. It matters because the same path that passes over a stranger's coin passes over one of this node's OWN coins when lineage handling has a bug. Production then refused with `Insufficient { have_dig_base_units }` -- a confident, understated figure. Unknown is not zero, and an operator reading an unmeasured balance as a measured one tops up money they already hold. Three changes: * The lifecycle funds through `select_operator_dig_cats_detailed` and consumes the skips, naming the store the selector cannot know. It reports on the SUCCESS path too, not only where `CandidatesUnverifiable` already reached the operator -- a funded pass that passed candidates over has still established only a floor. * `skip_report` is the one operator-facing sentence, and it says the total is a FLOOR: "at least that much, not exactly that much", plus what to investigate. A report that quoted the figure flat would restate the lie in a new place. * The per-candidate `tracing::warn!` inside the walk is gone. It was up to `MAX_AUTHENTICATION_ATTEMPTS` lines per selection per create per pass -- about 18,400 a day at one store -- and the count was set by whoever planted the coins, in a module with no rate limit. The walk now reports ONCE, with the id list capped at `SKIP_SAMPLE`, so neither the line count nor the line length is a figure an attacker chooses. The doc quoted above now describes what the code does. Refs dig-node#481 Co-Authored-By: Claude <noreply@anthropic.com> * refactor(wallet): rename wallet_funded::FundingObservation to EverFundedEvidence Two types in this crate were called `FundingObservation`, and they are not rivals to centralize -- they are different concepts that happened to share a name (dig-node#481): | | `wallet_funded` | `mirror::funding` | |---|---|---| | subject | the node-custodied wallet | the operator wallet | | question | has it EVER held money | what is spendable THIS pass | | decides | `autoseed::latch_ever_funded` | the operator alert gate | | lifetime | monotonic, permanent | per-pass | Merging them would collapse the node-wallet/operator-wallet boundary that `mirror::funding`'s module doc exists to protect, which is the money lie that module was written to prevent. So the fix is a rename, and the name now states what the type actually decides: it is evidence the wallet has ever held money, not a measurement of funding. The richer, newer `mirror::funding` type keeps the name, which reads correctly for a per-pass measurement. The two are cross-referenced from the renamed type's doc so the next reader cannot re-derive the confusion. Blast radius: `wallet_funded.rs` (18 references, its own module and tests) and `server.rs` (4, one a doc comment) -- the complete set. The gitnexus index for this repo is 338 commits behind, which returns a false-safe empty impact, so the radius was established by grep with a controlled pattern instead. Refs dig-node#481 Co-Authored-By: Claude <noreply@anthropic.com> * docs(spec): state how an unmeasured funding condition is compared, and bound the skip report Two clauses backfilled for behaviour changed in this branch (dig-node#481), so SPEC stops describing a node that no longer exists. 25.12 required an unmeasured observation to raise "once on entering it" and said nothing about what makes two such observations the same condition. The implementation compared the whole value, including the truncated walk's skipped count -- which a stranger moves by paying one coin into the publicly derivable operator address, so the suppression never applied and the attacker set the notification rate. The clause now requires attacker-movable fields to be excluded from that comparison, and requires a re-raise when a figure the operator must act on changes and no stranger can move it, since suppressing that is under-reporting rather than repeat-suppression. 25.11's "counted and reported" is now specific about the report: one bounded message per selection rather than one per candidate, an id list that is itself bounded, reachable on the funding path the node actually uses including where the selection succeeds, and framing any total as a floor. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Closes #520
Normative TEXT correction. No behaviour, signature, or wire change; no test added, because the diff creates no property a test could assert.
The false claim
Five doc sites and the
dig-node-coreSPEC told a verifier it falls back todig-mirror-coin's hint scan when a provider record carries no mirror-coin pointer. It cannot, and the reason is structural:dig_mirror_coin::discover(query.rs:351) andlist(query.rs:269) both takeowner_puzzle_hashas a required parameter, becausemirror_hintis morphed from the owner (namespace.rs:76-88).discover's own doc says so: "The owner is a parameter because the hint is morphed from it, so there is no store-wide bucket to read without one."dig_dht::ProviderRecordhas five fields and no owner (dig-dht-0.15.0/src/record.rs:379-433), and there is nopeer_id -> owner_puzzle_hashroute:peer_id = SHA-256(TLS SPKI DER).ChainSourcelevel, but is bounded atMAX_CANDIDATES = 10_000(query.rs:257) over a list anyone extends for a dust coin, so its "not found" is not a negative a verifier may act on, and it turns one free inbound record into an unbounded outbound read.The text gives those reasons rather than a bare prohibition, so a future sound composition (owner-less scan plus both
advertises(store, root, epoch)anddeclares_peer(claiming_peer_id)) is not foreclosed.Sites changed — five, plus the SPEC paragraph
crates/dig-node-core/src/lib.rs:566crates/dig-node-core/src/mirror_bond.rs:112crates/dig-node-core/src/peer.rs:3145crates/dig-node-core/src/seams/dig_peer/dht.rs:441crates/dig-node-service/src/mirror/pointers.rs:123crates/dig-node-core/SPEC.md§6.2mirror_bond.rswas not a passing remark — the claim was the stated justification forclaimed_coin_idbeing anOption, i.e. the trait's contract documented a fallback no implementation can provide. TheOptionis still correct; its reason is now the real one: absence is ordinary (unbonded holder, publisher predating its coin, epoch rollover, and every store-granularity record by construction), and an implementation with no pointer to fetch answersBondVerdict::Unverified— withholding credit, leaving the holder where a slate with no pointers would have left it. That is consistent with the existing module-level rule atmirror_bond.rs:45and theUnverifiedvariant's own doc.SPEC §6.2
§6.2 mandates announcing at store AND capsule granularity — two records per held capsule — and was silent that one of the two can never be collateral-verified. It now states that the store-granularity record is unbondable by construction (a coin bonds
(store, root, owner, epoch);ContentId::storenames no root), that this is the specified shape rather than a gap, and that a measurement finding a large share of pointer-less records has measured exactly this and MUST NOT be read as uncollateralised holders.Cross-repo coherence
Wording matches the dig-dht half,
DIG-Network/dig-dhtPR #29 (loop/28-pointer-doc-truth, 0.16.0), which makes the same correction insrc/record.rs,src/service.rsandSPEC.md§6.2. The two crates do not disagree (§4.2 layering).Blast radius
Doc comments and one SPEC section only — no symbol was edited, so there is no call-graph radius to scope. Confirmed by the diff: every hunk is inside
///,//!,//or Markdown.cargo check -p dig-node-core -p dig-node-service --all-targetsfinished with 0 errors;cargo doc -p dig-node-core --no-depsexited 0 with no warning namingmirror_bond,BondVerdictorProviderRecord(the crate's pre-existing intra-doc warnings are untouched).Sweep for a sixth site
rgis not installed here, sogit grepwas used and each pattern was control-tested against a known hit before any zero was trusted:hint scan(5 hits, all fixed, now 0) -scanning by hint-scan by hint-fall back to the hint-falls straight back-slower, not weaker-hint-scan-by hint(13 hits, all unrelated CAT-wallet discovery). A second sweep overuncollateralis|fully supported state|absence is normal|degrades discovery|no pointerfound no further false claim; the surviving occurrences already say "unverified, position unchanged". No sixth site exists in this repo.Deps (§2.4b) — deliberately NOT bumped
dig-node-servicedeclaresdig-mirror-coin = "0.7"while 0.9.0 is published — a semver-incompatible0.xuplift on the collateral path, a code cascade that dwarfs a text-only diff. Reported, not started.dig-dhtis declared0.15and 0.15.0 is still the latest published (0.16.0 is the pending PR #29), so there is nothing to bump there.dig-nat 0.21anddig-download 0.22are current.Version: 0.252.12 (patch,
docstype), read back fromCargo.tomlon disk after mergingorigin/mainat 0.252.11.