Skip to content

fix(read): bypass getAvailability confirm for connected-pool holders (#836) - #100

Merged
MichaelTaylor3d merged 2 commits into
mainfrom
fix/836-resource-handoff
Jul 25, 2026
Merged

fix(read): bypass getAvailability confirm for connected-pool holders (#836)#100
MichaelTaylor3d merged 2 commits into
mainfrom
fix/836-resource-handoff

Conversation

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor

Root cause (#836 read-leg DATA miss — the confirm-gate sub-cause)

The #97 fix corrected the DIAL ADDRESS (best_address() picks the reachable pool :9444), but the same 404 survives one layer deeper. dig-download's Job::locate_and_confirm (orchestrator.rs) keeps ONLY providers whose dig.getAvailability answer is available, dropping every other provider before any dig.fetchRange. On a relayed/isolated net a holder the reader is already CONNECTED to (offered by PoolProviderLocator) can answer availability=NOT-available as a false negative (its capsule not in the cache_list_cached inventory availability_presence walks, a resource-vs-capsule granularity quirk, or a transient probe failure). The confirm drops it → providers.is_empty()DownloadError::NotFound ("no providers located for ContentId::Resource {…}") → ZERO dig.fetchRange → §21 upstream backfill → 404. Exactly the e2e symptom: the availability probe dials :9444, then no fetchRange.

Ground truth (ends the "key mismatch" misdiagnosis)

  • fetch_resource logs content=%download_key(content); download_key = ContentId::to_key().to_hex(). For ContentId::Resource{rk}, to_key() hashes store+root+rk, so the logged ea12da62… IS the RESOURCE key, NOT the capsule key (capsule.to_key() != resource.to_key(), dig-dht content.rs).
  • The befbbaf9… in the ContentId::Resource Debug is that resource's raw retrieval_key. Same resource — no key mismatch.
  • The locate CHAIN (CapsuleFallback bridge + pool union + self-exclusion) resolves the holder in-process (PR#98's unit tests are green). The drop is downstream, in the CONFIRM.

Fix (reader-side, dig-node)

PoolConfirmTransport (download.rs) wraps the real range transport and short-circuits query_availability to available=true for any provider whose peer_id is in the connected pool — a live, connection-verified holder is confirmed by the connection itself; the whole-resource merkle verify (not the self-reported availability flag) is the real integrity gate. A DHT-only provider still goes through the real confirm. A connected non-holder simply fails its ranges and is dropped there (bounded, safe). Preserves #96/#97/#1584/#1580 + verify-then-decrypt fail-closed; the discovery/redirect leg is untouched.

Test (RED → GREEN, faithful)

connected_pool_holder_is_fetched_even_when_it_answers_availability_false: a CONNECTED, REACHABLE holder that answers availability=false but WOULD serve bytes. RED pre-fix (no providers located for ContentId::Resource {…}, zero fetchRange — the exact e2e symptom); GREEN post-fix (a fetchRange reaches the holder, bytes served). Every prior read-leg mock answered availability=true, so none could reproduce the confirm-gate drop. All 23 download tests pass.

Blast radius (gitnexus disabled per §2.0 override — via ripgrep + read)

  • New symbol PoolConfirmTransport — no external callers; wired only inside NodeContent::new, so it composes into every NodeContent construction (prod for_dht + tests). query_availability/fetch_range semantics unchanged for DHT-only providers.
  • NodeContent::new — the injection point; upstream callers (for_dht, tests) unaffected in signature.
  • No dig-download change (confirm-gate over-strictness worked around reader-side; a future dig-download-side "trust a connected source" option would be release-first — flagged, not required here).

Versions

fix → patch: root workspace 0.58.4 → 0.58.5, dig-node-core 0.18.2 → 0.18.3.

Do NOT merge (investigation lane). Closes nothing yet — leaves #836/#1586/#1572/#1062/#1590 open.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SaxTyBhLj2Ry1eVQBLLotN

…(#836)
The #836 read-leg DATA miss survives the #97 address fix at the next layer:
dig-download's `locate_and_confirm` keeps ONLY providers whose
`dig.getAvailability` answer is `available`, dropping every other provider
BEFORE any `dig.fetchRange`. On a relayed/isolated net a holder the reader is
already CONNECTED to (offered by `PoolProviderLocator`) can answer
availability=not-available as a FALSE NEGATIVE (cache-inventory lag, a
resource-vs-capsule granularity quirk, or a transient probe failure), so the
confirm drops it -> `providers.is_empty()` -> `DownloadError::NotFound`
("no providers located for ContentId::Resource {...}") -> ZERO fetchRange ->
§21 upstream backfill -> 404. This is the exact e2e symptom: the availability
probe dials :9444, then no fetchRange.
Ground truth (ends the "key mismatch" misdiagnosis): the logged `ea12da62` IS
the RESOURCE key (`ContentId::to_key()` hashes store+root+rk), and `befbbaf9`
is that resource's raw retrieval_key -- same resource, no key mismatch. The
locate chain (CapsuleFallback bridge + pool union + self-exclusion) resolves
the holder in-process; the drop is downstream in the CONFIRM.
Fix: `PoolConfirmTransport` wraps the range transport and short-circuits
`query_availability` to available=true for any provider whose peer_id is in the
connected pool -- a live, connection-verified holder is confirmed by the
connection itself, and the whole-resource merkle verify (not the self-reported
availability flag) is the real integrity gate. A DHT-only provider still goes
through the real confirm; a connected non-holder fails its ranges and is
dropped there (bounded, safe). Preserves #96/#97/#1584/#1580 +
verify-then-decrypt fail-closed; the discovery/redirect leg is untouched.
Faithful regression test `connected_pool_holder_is_fetched_even_when_it_answers_availability_false`:
a connected, reachable holder that answers availability=false but WOULD serve
bytes -- RED pre-fix (no fetchRange, "no providers located"), GREEN post-fix.
Every prior read-leg mock answered availability=true, so none could reproduce
the confirm-gate drop.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SaxTyBhLj2Ry1eVQBLLotN

@MichaelTaylor3dMichaelTaylor3d left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

CHANGES-REQUIRED (correctness review, fresh context).

The read-leg fix itself is CORRECT — I verified all five claims against the actual code:

  1. Bypass fires ONLY for connected-pool peer_ids: PoolConfirmTransport.is_connected() checks the SAME Arc<Mutex> connected_pool the PoolProviderLocator uses (download.rs:41 type, shared via .clone() of the Arc). DHT-only providers fall through to inner.query_availability. Confirmed.
  2. Fail-closed: the availability answer's roots/total_length/chunk_count are NOT consumed by locate_and_confirm (orchestrator.rs:870 only reads .available); the commitment is re-derived from the first real fetch frame (establish_commitment, orchestrator.rs:914-953) and each range is merkle-verified. A connected non-holder forced to available=true simply fails its fetch_range/meta-probe -> continue -> NotFound. Bypassing availability lets NO unverified bytes through. Confirmed.
  3. Self-exclusion intact: SelfExcludingLocator wraps the whole download UnionLocator (#1584/#96); discovery/redirect leg (self.locator) untouched. Confirmed.
  4. The regression test is non-vacuous: AvailabilityFalseButServesTransport answers available=false but serves bytes; with an empty DHT the pool is the only source. Pre-fix the confirm drops it -> NotFound -> zero fetchRange (RED, the exact e2e symptom); post-fix the bypass -> establish_commitment -> served (GREEN). Confirmed.
  5. Version bump correct: 0.58.4->0.58.5 + core 0.18.2->0.18.3, Cargo.toml + Cargo.lock consistent, version-increment gate green; readable code with strong WHY-comments.

BLOCKER: the Rustfmt required check is RED (run 30161420013). The task stated 'clippy/fmt/tests green' — fmt is NOT green. cargo fmt --check flags download.rs (the PoolConfirmTransport::new construction must collapse to a single wrapped line). A red required check bars merge (§2.4a/§3.6b). Run cargo fmt and push. Non-gating on logic; gating on the green-gate precondition.

Will re-review + resolve on the fmt fix.

Comment threadcrates/dig-node-core/src/download.rs Outdated
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review July 25, 2026 14:54
@MichaelTaylor3d
MichaelTaylor3d merged commit 77c1422 into mainJul 25, 2026
13 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the fix/836-resource-handoff branch July 25, 2026 14:54
Sign up for freeto 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

@MichaelTaylor3d