fix(read): merge same-peer address hints so a reachable pool address is not shadowed (#1590, #836)

[MichaelTaylor3d]

Root cause (#1590/#836 read-leg DATA blocker)

Provider-source ADDRESS shadowing in the download locator — NOT the resource→capsule granularity gap the ticket hypothesised.

The download locator is SelfExcludingLocator(UnionLocator([raw_dht_discovery, PoolProviderLocator])). Both sources can name the SAME authenticated peer_id with DIFFERENT addresses:

• the DHT carries the holder's advertised provider record — an untrusted, often stale/relayed-net reach hint (172.31.44.121:<ephemeral> in the e2e, refused);
• the PoolProviderLocator carries the holder's live connection address — connection-verified and reachable (172.31.29.67:9444).

UnionLocator (crates/dig-node-core/src/seams/dig_peer/union_locator.rs) deduped by peer_id keeping the FIRST-SEEN record whole and discarding every later same-peer record. So the DHT's unreachable hint (seen first) SHADOWED the pool's reachable address for the same holder. The confirm step (dig.getAvailability) then dialed only the unreachable address, every dial refused, dig-download's locate_and_confirm returned empty → NotFound → no providers located for ContentId::Resource {…}, and the read fell through to §21 upstream rpc.dig.net → 400 → 404. Holder inbound fetchRange = 0.

Evidence (arbiter e2e c095436): fetch_resource: located providers before download … located=1 connected_pool=1, the pool locator logs it is offering the holder at :9444, yet the fetch dials 172.31.44.121:<ephemeral> (the DHT hint, refused) and never 172.31.29.67:9444.

Fix

UnionLocator now MERGES the (untrusted, MAX_ADDRS_PER_PROVIDER-capped) address hints of same-peer_id records across sources instead of dropping the later record. peer_id stays the authenticated identity (SPKI-pinned at connect); addresses are only reach hints, so unioning them is safe, and the reachable pool address survives so a fetchRange reaches the holder.

Why the DISCOVERY locator stays unpolluted: the merge is a no-op on the discovery union [dht, EmptyLocator, EmptyLocator] (no same-peer cross-source dups), and NodeContent::find_providers / redirect-on-miss use self.locator (the raw discovery locator) unchanged — redirect hints stay capsule/announced-holder granularity. #96 self-exclusion (both defenses) and #1584 discovery leg are untouched; verify-then-decrypt fail-closed is unchanged.

Tests (TDD red → green)

• download.rs::resource_fetch_uses_the_reachable_pool_address_when_the_dht_hint_is_unreachable — drives the REAL locator composition (NodeContent::new) with an address-aware transport that fails a dial unless the record carries the reachable address, and asserts a fetchRange reaches the holder over its reachable pool address + merkle-verified bytes. Pre-fix it reproduces the exact content not found: no providers located for ContentId::Resource {…} error.
• union_locator.rs::same_peer_across_sources_merges_address_hints — pins the cross-source address merge.

Verification

• cargo test -p dig-node-core --lib: 341 passed, 0 failed (incl. all fix(read): never self-dial on the fetch path; self-exclude the download locator (#836, #92) #96 self-exclusion + #1590 pool tests).
• cargo clippy -p dig-node-core --all-targets -- -D warnings: clean. cargo fmt: clean.

Blast radius

UnionLocator::find_providers — callers: the download locator (NodeContent::new) and the discovery union (NodeContent::for_dht). The discovery union has no same-peer cross-source dups → behaviour unchanged there. Only the download/fetch path changes (reachable address now survives). Version bumped 0.58.2 → 0.58.3 (patch), dig-node-core 0.18.1 → 0.18.2.

Resume step

Re-run runlegs-read-arbiter.sh on cached c095436 + this fix — it should now issue a fetchRange to the holder at :9444 → DATA_MERKLE_VERIFIED.

Do NOT merge. Does not close #836/#1586/#1572/#1062/#1590.

[MichaelTaylor3d]

Correctness review — PASS (fresh independent context)

Verified the #836 read-leg DATA fix (address-shadowing in UnionLocator). All confirm points hold:

1. Merge is correct (union_locator.rs:76-100): same-peer_id records now UNION their address hints (existing.addresses.extend(record.addresses)) into the first-seen record instead of drop-later. peer_id stays the sole authenticated identity (SPKI-pinned at connect); only untrusted reach hints merge. The reachable pool address survives alongside the DHT hint.
2. Cap respected (sanitize_address_hints re-run after every extend, MAX_ADDRS_PER_PROVIDER = 8): merging can never grow a record past the #1490/#94 dial-amplification bound, even across many sources. No unbounded growth.
3. Discovery/redirect untouched: download.rs:549-556 composes a SEPARATE outer UnionLocator([self.locator, PoolProviderLocator]) for the download path only; the discovery/redirect leg keeps the raw self.locator = SelfExcludingLocator(CapsuleFallbackLocator(UnionLocator([dht, empty, empty]))). The inner discovery union has no cross-source dups, so the merge is a genuine no-op there — redirect hints stay announced-holder granularity, unpolluted. #96 self-exclusion (both defenses), #1584, #1580 CapsuleFallback, verify-then-decrypt all unchanged.
4. Tests non-vacuous + RED/GREEN: AddressAwareTransport (download.rs:1541) fails a dial unless the record carries the reachable addr — an address-only mock, so pre-fix the pool record is dropped → is_reachable false → fetch_resource Err (the exact e2e no providers located for ContentId::Resource path); post-fix both addrs present → fetchRange served + merkle-verified. same_peer_across_sources_merges_address_hints asserts one record, both hints survive. Both drive the real composition; helpers (anchored_mock_content, on_pool_event, mock_peer_hex) are real pre-existing testkit.
5. Meta: patch bump correct (0.58.2→0.58.3 / core 0.18.1→0.18.2, behaviour-fix); DEVELOPMENT_LOG records the address-shadowing gotcha + the merge-not-drop lesson; readable-code clean (intent-revealing match, WHY-comments citing #1590/#836/#1490).

dig-constants: no shared constant introduced or shadowed — MAX_ADDRS_PER_PROVIDER is a local dial-amplification bound (not a cross-repo value); addresses are runtime reach hints, not canon. Nothing belongs in / should consume dig-constants here.

Non-blocking note (resolving myself): the fix's real-world efficacy depends on the NatRangeTransport dialing BEYOND the first address in a record (DHT hint is ordered first, reachable pool addr second) — that behaviour lives in dig-nat, out of this diff, and is validated by the arbiter e2e (c095436). No action needed.

Blast radius: only union_locator.rsfind_providers (merge logic) + additive tests + version/log. detect_changes scope matches. Gates: Rustfmt/Clippy/CodeQL/version/commitlint green; Test+coverage + build jobs pending — merge gated on those going green (coverage ≥80%).