Skip to content

fix(read): translate gossip→peer-RPC port in selector-registry pool feed (#1590, #836 DATA leg) - #95

Merged
MichaelTaylor3d merged 2 commits into
mainfrom
fix/836-selector-pool-port
Jul 25, 2026
Merged

fix(read): translate gossip→peer-RPC port in selector-registry pool feed (#1590, #836 DATA leg)#95
MichaelTaylor3d merged 2 commits into
mainfrom
fix/836-selector-pool-port

Conversation

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor

What / why

Closes the #836 read-leg DATA gate. Root cause (decider plan #1586): the connected-pool fetch feed (spawn_selector_registry_feed) seeded the download-side ConnectedPool from raw gossip PoolEvent addresses carrying the gossip port (:9445). PoolProviderLocator::find_providers then emitted :9445 candidates, so the Tier-2 dig.fetchRange dialed the gossip listener instead of the peer-RPC listener → InvalidContentType → silent Tier-2 miss → Tier-3 §21 → 404. Same class of bug #1575 fixed for the DHT routing feed, recurring in the selector-registry/pool feed.

Fix

Apply dht_addr_from_gossip_addr() (existing, #1575; gossip 9445 → peer-RPC 9444) at BOTH feed sites in spawn_selector_registry_feed: the snapshot-seed loop AND the live PeerAdded branch (map_gossip_pool_event). on_pool_event stays addr-agnostic — translation happens at the gossip boundary, mirroring #1575 exactly.

Observability (the gap that hid this 6 iterations)

Added INFO/DEBUG tracing to peer_serve_plaintext, fetch_resource, and the pool locator (dial targets incl. port).

TDD (RED→GREEN, through the REAL feed)

  1. selector_pool_feed_candidate_uses_peer_rpc_port_not_gossip_port — drives map_gossip_pool_eventon_pool_eventPoolProviderLocator; asserts the candidate for a resource carries :9444 (RED = :9445).
  2. serve_content_plaintext_reaches_pool_holder_at_peer_rpc_port — full Node tiering: Tier1-miss → Tier2 with a port-gated transport (serves only at :9444), holder fed through the real gossip feed at :9445, unroutable upstream; asserts Served{source: Peer}.

Blast radius (gitnexus disabled — socraticode + ripgrep + read)

  • spawn_selector_registry_feed / map_gossip_pool_event (peer.rs) — sole caller is the feed; map_gossip_pool_event now pub(crate) for tests.
  • ConnectedPool / PoolProviderLocator unchanged in behaviour (still reads stored addrs); only the addr STORED changes.
  • No wire/API/format change → patch (0.58.0 → 0.58.1).

Version

patch 0.58.0 → 0.58.1 (behaviour fix, no API/wire change), Cargo.toml + Cargo.lock in agreement.

DRAFT until CI green.

🤖 Generated with Claude Code

…eed (#1590, #836 DATA leg)
The connected-pool fetch feed (spawn_selector_registry_feed) seeded the
download-side ConnectedPool from raw gossip PoolEvent addrs (:9445), so the
PoolProviderLocator offered :9445 candidates and Tier-2 fetchRange dialed the
gossip listener -> InvalidContentType -> read 404. Mirror #1575: map the gossip
addr down to the peer-RPC addr (:9444) at both feed sites (snapshot seed +
live PeerAdded branch), keeping on_pool_event addr-agnostic. Add Tier-2
tracing + two TDD tests through the real feed translation.
Closes #836 (DATA leg). Refs #1586 #1590 #1425.
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3dforce-pushed the fix/836-selector-pool-port branch from 2ec9a1c to a479417CompareJuly 25, 2026 08:14
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review July 25, 2026 08:14

@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.

Independent CORRECTNESS review (fresh context) — verdict: CHANGES-REQUIRED (recorded as a COMMENT review; self-review cannot REQUEST_CHANGES) (one gating finding on the hot read path; the port-translation fix itself is correct and well-tested).

What is correct (verified against a479417):

  • Both selector-feed sites translate exactly once: snapshot-seed dht_addr_from_gossip_addr(addr) (peer.rs:1289) and the live PeerAdded branch via map_gossip_pool_event (peer.rs:1392). The helper is reused, not duplicated. on_pool_event and PoolProviderLocator stay addr-agnostic (no port math) — no double-translation.
  • The DHT routing feed's translation (peer.rs:1336/1358) is unchanged.
  • Both tests drive the REAL feed (map_gossip_pool_eventon_pool_event→locator / full Node tiering with a :9444-port-gated transport) and gate on the port, so they go RED on the unfixed code (candidate = :9445). They close the exact gap (mocks that ignored the port) that hid this 6 iterations.
  • verify-then-decrypt fail-closed path unchanged (the Err arm still returns None and falls through; only tracing added). New tracing logs public identifiers (store/root/rk hex, byte lengths, dial targets, provider counts) — no keys/salt/plaintext.
  • patch bump 0.58.0→0.58.1 correct (no API/wire change); Cargo.toml + Cargo.lock agree. SPEC §5.3 note + DEVELOPMENT_LOG updated. Comments are WHY-focused and read cleanly.
  • CI: Rustfmt/Clippy/CodeQL/version-increment/commitlint all green; Test+coverage pending (will validate the two new tests).

Gating finding — see inline on download.rs:712.

Comment threadcrates/dig-node-core/src/download.rs Outdated
…r cache hit) (#836)
Move the fetch_resource located-provider-count trace below both
in-memory cache-hit checks and gate the find_providers locate itself
on tracing::enabled!(Level::DEBUG), so a cached re-serve never pays a
duplicate DHT locate and a real cache-miss download only pays it when
DEBUG tracing is actually on.
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d merged commit 8a6be4d into mainJul 25, 2026
13 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the fix/836-selector-pool-port branch July 25, 2026 08:42
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