Skip to content

feat(holdings): real-time opcode-222 announce/retract with verified inbound ingest (#1429) - #115

Merged
MichaelTaylor3d merged 8 commits into
mainfrom
feat/1429-holdings-flywheel
Jul 28, 2026
Merged

feat(holdings): real-time opcode-222 announce/retract with verified inbound ingest (#1429)#115
MichaelTaylor3d merged 8 commits into
mainfrom
feat/1429-holdings-flywheel

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Closes DIG-Network/dig_ecosystem#1429. Refs #1394, #1423, #1424, #1428, #1425.

What this is, and what it is NOT

The flywheel's durable half already shipped and I did not rebuild it: seams/dig_peer/dht.rs already reconciles DHT provider records on every inventory change (#1586) and module_reshare.rs already pulls a whole capsule after a remote read and announces (#1576), so read -> cache -> announce -> find_providers mechanically closes today. What it lacks is speed and authentication: a new holder is only as discoverable as the last PUT reached, and a departed holder lingers for its record's TTL.

This PR adds the real-time, authenticated half — opcode-222 HoldingsAnnounce egress/ingress plus an ACTIVE retract.

Ticket corrections

  • #1429 was labelled status:blocked; it was not. Both blockers had landed. dig-dht 0.7 (already the pinned dep) carries all four #1424 APIs (ingest_verified_provider, remove_provider_record, retract_own_provider, holders_of), and the pinned dig-gossip rev b065e70 (v0.16.0) already contains src/service/holdings_announce.rs byte-identical to its origin/main. No lower-level crate needed to ship and no dep was bumped — no release-first cascade.
  • Scope item 5 ("a list-my-holdings surface") already exists as dig.listInventory (peer.rs::list_inventory). Not rebuilt.
  • Capsule LRU eviction does not exist yet. Node::evict_if_needed is the response-window cache, not the capsule cache; capsule loss today is cache_remove_cached or an inventory shrink. Retract is therefore hooked to the real capsule-loss signal (the single inventory-reconcile chokepoint), so it picks up LRU eviction for free when #1423 lands. Restart-durability of the seq counter is deliberately left to #1477 (noted in SPEC and in the code); the loop closes without it.

Blast radius checked

gitnexus is disabled per the standing CLAUDE.md override, so this was done with ripgrep plus direct code reads.

Symbol Change Callers Risk
dht::sync_inventory returns InventoryDelta instead of (usize, usize); loss uses retract_own_provider not withdraw_provider exactly one: DhtHandle::refresh_inventory. No test referenced it. LOW
DhtHandle::refresh_inventory kept, now a thin projection over the new reconcile_inventory peer.rs DhtInventoryAnnouncer plus the inventory-refresher hook (both migrated) LOW
bring_up_dht returns (handle, Option<HoldingsBroadcaster>) one call site, migrated LOW
DhtInventoryAnnouncer gains a holdings field; body routes through the shared reconcile_and_flood constructed once LOW

One behaviour change worth reviewer attention (MEDIUM, intentional): capsule loss now ACTIVELY retracts the local provider record instead of letting it lapse via TTL. Rationale: a passively-withdrawn record keeps answering find_providers with this node for content it can no longer serve, costing every reader a wasted dial. This is the local half of #1423's atomic evict-and-retract. Whole dig-node-core suite green (448 unit plus all integration), workspace cargo check --all-targets clean.

Security answers

What does an announce cost the announcer vs. a victim? The announcer pays an ECDSA-P256 signature and a flood. A receiver pays verify_holdings_announce (one SHA-256 plus one P-256 verify) plus bounded local map work. The ingress performs ZERO egress — it never re-broadcasts, dials, probes or fetches, so there is no path by which one cheap inbound message makes other peers do more work than the sender did. Dissemination is dig-gossip's existing Plumtree flood with its own seen-set dedup, not a per-message re-send added here. This is deliberately unlike the #1670 shape that turned one unauthenticated record into 3x egress.

Can a peer announce holdings it does not have, and what does a reader waste? Yes, and this is bounded by cost asymmetry rather than prevented — the honest design. A false claim buys at most one failed dial or a dig.getAvailability answering "no", after which the peer-selector deranks the liar. No merkle- or chain-verification decision ever rests on an announcement; the record is a hint, and the read path's anchor gate is unchanged. The liar paid a signature plus a flood for one wasted dial.

Can retract de-list an honest holder? No, and it holds on the wire, not just in a struct: HoldingsDelta::Remove carries only a content_key — there is no per-delta peer field to abuse. The only provider id the receive path can produce is announce.provider_peer_id, which verify_holdings_announce proves equals SHA-256(provider_spki) and which signed the batch. HoldingsIngress::accept takes no caller-supplied provider id at all, so this is enforced by the type signature rather than by a check that could be moved or dropped. Pinned by a_retract_cannot_delist_another_peers_record, whose fixture keeps a truthful CONTROL holder of the same content key that must survive — a single-holder fixture would pass under a "wipe the whole key" implementation and prove nothing — and by the real-wire a_signed_retract_crosses_the_real_wire_and_spares_the_other_holder.

Rate/quantity bounds, as numbers, at one chokepoint (IngressState::admit, the only place an announcement is charged):

Bound Value Keyed by Why
Announcements per window 10 provider dig-dht SPEC 14's delegated number; semantically "how often a holder may revise its holdings"
Deltas per window 1,024 (4 x HOLDINGS_MAX_CHANGES) transport sender derived from the protocol's own frame cap
Window 60 s
Tracked senders 1,024 live key space is the connected pool
Tracked providers 8,192 key space is attacker-minted, so it must be capped

The two buckets are keyed differently on purpose, and each covers the other's weakness. The provider bucket is semantically right but its key space is attacker-minted, so its map must be capacity-bounded — and an attacker who overflows it can evict its own bucket and refill, making it bypassable alone. The sender bucket is coarse but its key space is the connected pool, which cannot be inflated from off-network, so it is the unbypassable backstop. A rejected announcement charges neither bucket, or the limiter itself becomes the denial of service (one bad signature per honest announce would silence a neighbour).

I caught and fixed one vacuous bound during design: my first draft set the delta cap to 10 x MAX_CHANGES, which is exactly what the announce bucket already permits — the guard would have executed and constrained nothing.

Guard-deletion probe — every guard proved load-bearing

Each guard was deleted individually (committed first, restored from a file copy) and the suite re-run:

Guard Tests that went RED
G1 verify_holdings_announce 3 (a_forged_signature..., an_announce_claiming_another_peers_id..., rejected_announcements_do_not_consume_the_budget)
G2 self-attribution 1 (our_own_announce_replayed_back_at_us_is_ignored)
G3 replay seq 1 (a_replayed_older_seq_cannot_resurrect_a_retracted_record)
G4 per-provider announce bucket 1 (the_per_provider_announce_budget_binds_at_its_stated_bound)
G5 per-sender delta bucket 1 (the_per_sender_delta_budget_binds_independently_of_the_provider_budget)
G6 attribution uses the verified signer (swapped for another in-scope peer id) 2 (a_retract_cannot_delist_another_peers_record, a_replayed_older_seq...)
G7 verify-before-charge ordering 3 (rejected_announcements_do_not_consume_the_budget plus 2)

Also probed the real-wire test itself: signing with a cert other than the one the gossip handshake presents makes a_signed_announcement_crosses_the_real_wire_and_becomes_discoverable fail on the peer_id assertion — so it genuinely binds announce identity to handshake identity rather than checking self-consistency. cargo-mutants was not run (not installed).

Two real bugs the real-wire test caught that the unit tests could not

  1. Fixture time vs wall-clock. DhtService reads the SYSTEM clock to clamp and expire provider records. The first draft threaded a pinned past NOW through it, so every ingested record was already expired and find_providers returned []. A pinned-NOW wire test would have "proved" the flywheel closed while exercising only the expired path. The wire test now uses real wall-clock with the reason documented at the fixture; the pure-policy suite keeps its pinned NOW because its API is clock-injected.
  2. A swallowed PutOutcome. HoldingsSink::ingest discarded dig-dht's admission outcome, so Applied::ingested counted attempts. It now returns bool and counts only PutOutcome::Accepted — which is what surfaced bug 1 at all.

Tests

  • tests/holdings_ingress.rs — 16 adversarial tests. Fixtures built against the nearest WRONG implementation, with a truthful control actor in every censorship/flood test, an explicit pinned NOW, sizes taken from the protocol limits (HOLDINGS_MAX_CHANGES, 2x+1), and every numeric bound pinned from both sides (at-bound passes, one-over fails).
  • tests/holdings_wire.rs — 2 REAL-WIRE tests: two live dig-gossip services over loopback mTLS, each presenting its own persisted dig_tls::NodeCert; real signature by the TLS leaf; real frame_holdings_announce -> transmit -> holdings_announce_payload -> verify -> real DhtService -> real find_providers/holders_of.
  • Coverage on the new module: 86.26% lines / 83.91% regions (from these tests alone), above the 80% floor. Full dig-node-core suite: 448 unit plus all integration green. cargo fmt --check clean, cargo clippy zero warnings.

Version bump — MINOR

dig-node 0.63.0 -> 0.64.0, dig-node-core 0.21.0 -> 0.22.0. New backwards-compatible capability; no wire break (opcode 222 is additive, and a peer that does not send it is simply not real-time-discoverable, falling back to the durable DHT records). dig-node-core's minor covers the changed pub signature of sync_inventory — the crate is an internal library shared by the two host shells, and both shells are in this workspace and migrated here.

SPEC.md 19.3a records the normative contract (egress MUSTs, the five ingress gates, the attribution invariant, the no-amplification property, and the bounded false-claim cost).

@MichaelTaylor3d
MichaelTaylor3d force-pushed the feat/1429-holdings-flywheel branch 2 times, most recently from 29550fb to 8b6db9b Compare July 27, 2026 18:01
…nbound ingest

Closes #1429.

Adds the real-time, authenticated half of the content-replication flywheel. The
durable half already existed (DHT provider PUT on capsule gain, find_providers on
read, #1576/#1586); it converges only as fast as the last PUT reached, and a
departed holder lingers for its record's TTL. This closes that gap.

Egress: every inventory reconcile now derives BOTH effects from one delta -- the
DHT records (with an ACTIVE retract_own_provider on loss, replacing the passive
withdraw_provider that left this node answering find_providers for content it can
no longer serve) and a signed opcode-222 HoldingsAnnounce for exactly the ids
that changed. Signed by the node's own NodeCert leaf, since the wire derives
provider_peer_id from SHA-256(provider_spki). Oversized reconciles split across
frames with advancing seq rather than truncate, so a bulk cache-clear cannot drop
retracts.

Ingress: verify -> reject self-attributed -> reject non-advancing seq -> two
token buckets -> ingest_verified_provider / remove_provider_record. The provider
identity is always the verified signer, structurally: accept() takes no
caller-supplied provider id, so a retract cannot de-list an honest holder. The
receive path performs zero egress, so an inbound announce cannot be amplified.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d force-pushed the feat/1429-holdings-flywheel branch from 8b6db9b to eb4d89d Compare July 27, 2026 18:16
@MichaelTaylor3d MichaelTaylor3d changed the title feat(dht): real-time holdings announce/retract + verified inbound ingest (#1429) feat(holdings): real-time opcode-222 announce/retract with verified inbound ingest (#1429) Jul 27, 2026

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recorded as a COMMENT review: GitHub returns 422 on REQUEST_CHANGES for a self-authored PR (the loop commits under the same identity). The verdict is nonetheless CHANGES-REQUIRED - the four blocking inline threads below must be addressed and resolved before merge.

CHANGES-REQUIRED

Strong PR: the SPEC section is genuinely normative, the real-wire tests are real (live GossipService pair over loopback mTLS, real broadcast/decode, real DhtService, real find_providers — not a shared helper or a sink()), the truthful-control discipline is held, and the two bugs you credit to the wire test are genuinely fixed. Suite reproduced green on my side: 16 + 2 holdings tests, 448 lib tests, cargo fmt --check clean.

Your central claim: VERIFIED

The durable flywheel does already close on 0.63.0. I traced the whole chain on origin/main independently:

download.rs:1008 spawn_capsule_resharemodule_reshare.rs:454 spawn_capsule_warmwarm()module_reshare.rs:398 promote_into_cachemodule_reshare.rs:401 announce_inventory()peer.rs:849 DhtInventoryAnnouncerdht.rs:466 refresh_inventorysync_inventoryannounce_provider, and dht.rs:472 locate_providersfind_providers. Production wiring confirmed at peer.rs:2274 wire_capsule_reshare (not test-only). The second hook (capsule_store.rs:230 refresh_dht_inventory) covers the gap-fill / explicit-cache paths. So read → cache → announce → find_providers mechanically closes today, gated on ReadOrigin::Local + backfill_on_miss_enabled(). Your reframe of #1429 as speed and authentication is correct, and I confirm the two smaller corrections too: peer.rs:568 list_inventory already serves dig.listInventory, and lib.rs:1145 evict_if_needed operates on responses_dir() — the response-window cache — so capsule LRU genuinely does not exist and hooking retract to the inventory-reconcile chokepoint is the right seam.

But three of your seven guards do not hold over their class

The guard-deletion probe was run against inputs that are all same-case, so it proved the guards load-bearing without proving them sound. verify_holdings_announce decodes provider_peer_id with hex::decode (dig-gossip holdings_announce.rs:353), which is case-insensitive, and the signing message is computed over the decoded bytes — so an attacker can uppercase the hex of a captured announce and it still verifies with the original signature, no re-signing. Both G2 and G3 then compare/key on the raw wire string.

I confirmed all three empirically with a throwaway probe against this head (since deleted, not pushed):

PROBE A  uppercase provider_peer_id STILL VERIFIES                    -> pass (the enabler)
PROBE B  replayed older ADD, hex uppercased -> Ok(ingested: 1)
         retracted record RESURRECTED as 10e5e64c...  (canonical lowercase!)
PROBE C  our OWN announce, hex uppercased  -> Ok(ingested: 1)         G2 bypassed
PROBE D  50 REJECTED announces -> providers tracked = 51              unbounded

PROBE B is the sharp one: ProviderRecord::new (dig-dht record.rs:303) canonicalises to lowercase, so the resurrected record lands under the victim's real peer id — the exact outcome a_replayed_older_seq_cannot_resurrect_a_retracted_record exists to forbid. That test passes only because its fixture never varies the case.

Details inline. Four blocking findings, four non-gating (which I will resolve myself so they cannot bar merge).

Not blocking / verified

  • SemVer MINOR is defensible — dig-node-core is not on crates.io (confirmed via the API), both consumers are in-workspace and migrated, opcode 222 is additive. Both manifests agree.
  • PutOutcome fix confirmed at holdings.rs:224-229; the wall-clock / pinned-NOW split is real and documented at the fixture (holdings_wire.rs:37-51) rather than accidental.
  • Fixture sizes are taken from HOLDINGS_MAX_CHANGES and 2x+1, not round numbers. Good.
  • §2.5: holdings.rs reads cleanly and is genuinely cohesive (egress / ingress / composition), guard clauses throughout, doc-comments on every public item, WHY-comments that earn their place. No complaint.
  • No regression to 0.63.0's framing fix — full 448-test lib suite green.

What I did not examine

I did not run cargo llvm-cov (the 86.26% / 83.91% / 44.69% figures are unverified by me), did not review the Cargo.lock delta beyond the version lines, and did not exercise run_holdings_ingest or the degraded bring-up branches (see finding 4).

Comment thread crates/dig-node-core/src/seams/dig_peer/holdings.rs
Comment thread crates/dig-node-core/src/seams/dig_peer/holdings.rs Outdated
Comment thread crates/dig-node-core/tests/holdings_ingress.rs Outdated
Comment thread crates/dig-node-core/src/seams/dig_peer/dht.rs
Comment thread crates/dig-node-core/src/seams/dig_peer/holdings.rs Outdated
Comment thread crates/dig-node-core/src/seams/dig_peer/holdings.rs
Comment thread SPEC.md
Michael Taylor and others added 6 commits July 27, 2026 12:35
…ation-free rejections

Co-Authored-By: Claude <noreply@anthropic.com>
…verified

Co-Authored-By: Claude <noreply@anthropic.com>
…ed record

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…test additions

Co-Authored-By: Claude <noreply@anthropic.com>
…bility

A node that cannot mint a P-256 holdings signer takes the `holdings: None`
branch, which the module documents as staying discoverable through the durable
DHT provider records alone. Both halves sit behind ONE `if let (Some(..),
false)` condition, so that promise was compile-checked only: a mistake letting
the missing signer short-circuit the reconcile too would make an unsigning node
invisible to find_providers - a discovery outage, not a freshness one.

Asserts the SURVIVING half positively (the record really is in the provider
store) in both the gain and loss directions, because asserting only that
nothing was flooded passes identically whether or not the reconcile ran.

Refs #1429

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probe

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-gate (correctness + test-vacuity leg) — VERDICT: CHANGES-REQUIRED (one blocking finding; posted as a comment review because request-changes on the loop identity is a 422)

I re-executed the lane's probe table in a throwaway worktree rather than reading the diff. All four claimed rows reproduce, and I added a fifth probe the lane did not claim (wrong-victim eviction), which also reds — so the_provider_map_evicts_the_least_recently_seen_at_its_capacity pins placement AND victim, not just a count.

Probe Mutation Observed
1 dht.rs:437 retract_own_provider -> withdraw_provider RED — losing_a_capsule_stops_this_node_being_returned_as_a_provider_at_once at holdings_wire.rs:499 on the stated assertion (record still returned by find_providers), plus a_node_that_cannot_sign_still_reconciles_its_records_without_flooding at :621
2 delete evict_lru(&mut self.providers, ...) RED — the_provider_map_evicts_the_least_recently_seen_at_its_capacity, left: 4 right: 3
2b (mine) evict_lru min_by -> max_by (evict the WRONG victim) RED — at :1192, "the LEAST-recently-seen provider is the one evicted" — the test discriminates victim selection, not only cardinality
3 flood omits the retracted ids (SUBSET) RED — a_reconcile_floods_exactly_the_deltas_it_moved at :573, removes {} vs expected 2
3b lost ids flooded as adds (SUPERSET) RED — same test at :569, adds 4 vs expected 2

Both prior blocking findings are therefore genuinely discharged.

Independent guard probes (not claimed by the lane)

Guard Mutation Observed
Gate 3 self-attribution (holdings.rs) condition -> false RED (2 tests)
Gate 4 freshness abs_diff short-circuited RED (3 tests)
Gate 2 canonicalization provider_hex = raw wire spelling RED (3 tests)
Gate 5 seq watermark seq <= highest -> false && RED (4 tests)
bounded_dht_addresses .take(MAX_ADDRESSES_PER_RECORD) deleted RED (unit test)
evict_lru(&mut self.senders, ...) deleted GREEN — 32/32 still pass

The last row is the one blocking finding: the sibling of the exact defect round 1 was sent back for. Detail inline.

Everything else clears: version bump 0.63.0 -> 0.64.0 / dig-node-core 0.21.0 -> 0.22.0 is correct minor for additive capability; SPEC.md §19.3a is normative and matches the code; the reject path IS now exercised (rejected_announcements_allocate_no_tracking_state, and the stale frame embedded mid-fixture in the capacity test); §2.5 readable-code is well above bar — the rustdoc states the WHY for each guard and each doc claim I spot-checked is true of the code. dig-constants: nothing here redefines a cross-repo value locally (HOLDINGS_MAX_CHANGES, MAX_ADDRESSES_PER_RECORD, the opcode-222 framing all come from dig-gossip/dig-dht); the receiver-policy numbers are documented in SPEC.md, which is their right home.

Comment thread crates/dig-node-core/src/seams/dig_peer/holdings.rs
Comment thread crates/dig-node-core/src/seams/dig_peer/holdings.rs
Comment thread crates/dig-node-core/src/seams/dig_peer/holdings.rs
SPEC.md 19.3a states the ingress is "capacity-bounded (1,024 senders,
8,192 providers) with least-recently-seen eviction". The provider half
was proven; the sender half was an untested claim -- deleting
evict_lru(&mut self.senders, ..) left the whole suite green.

The bound is load-bearing: a peer_id is SHA-256(NodeCert SPKI), so it is
self-minted, and no path other than eviction ever removes a sender
entry. Connect -> announce -> disconnect churn therefore offers an
unbounded key space.

The new test mirrors the provider-side fixture disciplines. It drives a
lowered tracked_senders cap, exhausts each sender's delta budget, and
reads eviction through its CONSEQUENCE -- a sender entry IS that
sender's budget, so losing the entry lets an exhausted sender relay
again. The same relay is asserted refused before the bound is crossed
(making the later admission attributable), and a RETAINED sender is
probed before the evicted one. It fails under both nearest-wrong
implementations: deleting the eviction call, and evicting the wrong
victim (min_by -> max_by).

Two doc corrections alongside: MAX_TRACKED_SENDERS is restated as a hard
bound on unbounded growth rather than "headroom" over a connected pool,
and ADVERTISED_TTL_SECS now names dig_dht::DhtConfig::provider_ttl as
the symbol it must track.

Refs #1429

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review July 28, 2026 05:09
@MichaelTaylor3d
MichaelTaylor3d merged commit ae75767 into main Jul 28, 2026
13 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the feat/1429-holdings-flywheel branch July 28, 2026 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant