fix(read): never self-dial on the fetch path; self-exclude the download locator (#836, #92)

[MichaelTaylor3d]

Root cause

The #1590 port fix (v0.58.1, 8a6be4d) let the pool locator correctly offer the connected holder at :9444, but the resource FETCH still dialed the reader's OWN address instead of the holder — so no fetchRange ever reached the holder and DATA 404'd (run e2e-836-arb-20260725-084501, reader 172.31.66.104 / holder 172.31.69.169).

One root, two symptoms. Self-exclusion was enforced only on the DISCOVERY leg (#1584 — the gossip pool + the raw dig-dht locator). The DOWNLOAD locator is a SEPARATE candidate set: NodeContent::new UNIONs the discovery locator with a PoolProviderLocator over the connected pool, and that pool branch was never self-excluded. A relay-introduced self-connection surfaces this node in its own gossip pool (peer_id == local); the pool feed mirrors it into the connected-pool map; the un-excluded PoolProviderLocator then offers SELF as a fetch candidate.

Result: the confirm round dials self (Direct -> own IP -> Connection refused; Relayed -> refusing relayed self-dial (target == local peer_id)), never completes against the reachable holder, and dig-download reports no providers located for the ContentId::Resource -> read falls through to the §21 upstream (400) -> 404. The self-dial (log L118/L120) and the no providers located (L129) are the SAME root — the self entry poisons the confirm.

• Defect 1 root: crates/dig-node-core/src/download.rs — NodeContent::newdownload_locator (the PoolProviderLocator branch was outside SelfExcludingLocator).
• Defect 2 root: same — the self-dial prevents the confirm from completing against the holder, so no resource provider is confirmed. Shared root.

dig-node#92 (the reflexive self-candidate) is RESOLVED by this fix — the leaked self candidate is now dropped both at the feed and at the locator.

Fix (two defenses)

1. on_pool_event drops a self PeerAdded at the pool feed — self never enters the connected pool OR the selector registry.
2. The WHOLE download locator is wrapped in SelfExcludingLocator, so no source (DHT or pool) can offer self on the fetch/dial path (mirrors #1584 on the fetch leg).

verify-then-decrypt fail-closed unchanged.

Test (reproduce-first, target-asserting)

fetch_never_dials_self_and_reaches_the_connected_holder uses a TargetRecordingTransport that records every dial target (availability + fetch) — closing the gap that hid the bug for nine iterations (a target-blind mock serves bytes regardless of target and falsely passes). With self in the connected pool, the fetch reaches the holder and NEVER dials self. Plus on_pool_event_drops_a_self_peer_added.

Verified RED on pre-fix code for the right reason:

• the reader must NEVER dial itself on the fetch path (download.rs:1522)
• a self entry must never enter the connected pool (download.rs:1550)

GREEN after fix; full download:: suite 21 passed; cargo clippy -p dig-node-core --all-targets -D warnings clean; cargo fmt applied.

Blast radius

gitnexus/socraticode are disabled (§2.0 override) — radius via ripgrep + read. Edited symbols: NodeContent::new (download_locator composition) and NodeContent::on_pool_event. Both are internal to dig-node-core; on_pool_event callers (spawn_selector_registry_feed, tests) are behaviour-preserving for non-self peers (only a self PeerAdded — which should never have been a source — is now dropped). No public API/wire/format change. Version: workspace 0.58.1 -> 0.58.2 (patch), dig-node-core 0.18.0 -> 0.18.1.

Resume

Re-run runlegs-read-arbiter.sh on cached 8a6be4db + this fix -> expect a fetchRange to reach the holder :9444 -> DATA_MERKLE_VERIFIED.

Closes-partial: #836 read-leg DATA blocker. Resolves#92. Does NOT close #836/#1586/#1572/#1062/#1590.

🤖 Generated with Claude Code

[MichaelTaylor3d]

Independent correctness review (fresh context) — PASS.

Verified the two defenses and the tests against the branch tip:

• Defense 1 (on_pool_event, download.rs:660-665): a self PeerAdded (self.self_peer_id == peer_id.to_hex()) returns EARLY, before both self.selector.on_pool_event and the connected-pool insertion. So self never enters the selector registry OR the download-side connected pool the PoolProviderLocator reads. PeerRemoved is unaffected (removing self is a harmless no-op). Only peer_id == local is dropped; a distinct peer (holder [1]) is preserved — asserted by on_pool_event_drops_a_self_peer_added.
• Defense 2 (NodeContent::new, download.rs:549-555): the WHOLE download_locator union (discovery locator ∪ PoolProviderLocator) is wrapped in SelfExcludingLocator(.., self_peer_id), which drops any record whose provider_peer_id == me (hex). Mirrors the #1584 discovery-leg exclusion on the fetch path; idempotent over the already-wrapped discovery locator. None identity = transparent pass-through, so no legitimate peer is excluded.
• Tests non-vacuous + target-asserting:TargetRecordingTransport records every dial target (availability + fetch), closing the target-blind-mock gap that hid the bug — a self-dial is caught even though the inner mock serves bytes regardless of target. mock_peer_hex(9) == PeerId::from_bytes([9;32]).to_hex(), so the test's self_id matches the injected self PeerAdded; mock_peer_hex(1) is the holder. Both tests are RED on pre-fix code (self enters pool + is dialed / registry non-empty) and GREEN after.
• verify-then-decrypt untouched; patch bump correct (0.58.1→0.58.2, core 0.18.0→0.18.1, Cargo.lock consistent); SPEC + DEVELOPMENT_LOG updated in-unit; readable-code holds.

Non-gating observation (resolved, no action required): fetch_never_dials_self_and_reaches_the_connected_holder exercises Defense 1 directly, but because Defense 1 removes self before it reaches the pool the PoolProviderLocator reads, Defense 2's wrap is not hit at the composition level in that test. Defense 2's SelfExcludingLocator is independently unit-tested in self_excluding_locator.rs, so coverage of the wrap itself is adequate — belt-and-suspenders is acceptable.

Correctness verdict: PASS. (Merge still gated by the orchestrator on Test+coverage / build checks currently pending.)