fix: rank close-group/candidate selection by reachability

[jacderida]

Summary

Close-group (7 XOR-closest) and Merkle candidate-pool (16 XOR-closest) selection ranked purely by XOR distance, ignoring whether a peer is actually reachable. An unreachable / relay-only / NAT-stuck peer that happened to be XOR-close was picked, then failed — costing a store slot and skewing closeness lookups. This ranks by (reachability_tier, xor_distance) so a directly-reachable peer is preferred over an XOR-equal unreachable one.

Re-rank only, never filter: every path over-fetches, stable-sorts, then truncates, so a group can never shrink below count (sparse-network-safe). Tier keys on the absence of a Direct address, so a peer advertising both relay and direct stays tier 0. No wire-format or public-API change.

• E.1 find_closest_nodes_local: over-fetch 2×, re-rank by (reachability_tier, xor_distance), truncate; find_closest_nodes_local_with_self reuses the same comparator.
• E.2 Stamp DHTNode.reliability from trust_engine.score() instead of the hardcoded 1.0 placeholder, so ant-node consumers (replication, payment) see a real signal. Mirrors the existing routing_table_peers pattern.
• E.3 apply_lookup_report_winners (the sole final-ordering chokepoint for the network/candidate-pool path) re-ranks reachability-first; iterative convergence and the dial-failure skip are left intact.

Out of scope (per Fix E plan): saorsa-transport relay/holepunch work (E.4).

Test plan

• cargo fmt clean
• cargo clippy --lib -- -D warnings -D clippy::unwrap_used -D clippy::expect_used clean
• cargo test --lib — 485 passed, 0 failed
• New unit tests: Direct outranks XOR-closer relay-only; sparse (all relay-only) keeps count in XOR order; reachability_tier 0/1
• New 2-node integration test (local_selection_stamps_neutral_trust_not_hardcoded_one) — passed 3/3 runs; verifies a learned peer's reliability is the neutral trust default, not the old hardcoded 1.0

🤖 Generated with Claude Code

Greptile Summary

Re-ranks close-group and Merkle candidate-pool selection by (reachability_tier, xor_distance) instead of pure XOR distance, so a directly-reachable peer beats an XOR-closer relay-only peer without ever filtering peers out. Also replaces the hardcoded reliability: 1.0 placeholder in find_closest_nodes_local with the real trust-engine score.

• E.1/E.3 re-rank: find_closest_nodes_local over-fetches 2× from the routing table, re-ranks, and truncates; find_closest_nodes_local_with_self and apply_lookup_report_winners adopt the same comparator — all three selection chokepoints now consistently prefer reachable peers.
• E.2 trust stamping: find_closest_nodes_local now calls trust_engine.score() (falling back to DEFAULT_NEUTRAL_TRUST) instead of the constant SELF_RELIABILITY_SCORE, mirroring the existing routing_table_peers pattern; a new 2-node integration test verifies the 0.5 default for freshly-learned peers.

Confidence Score: 4/5

The change is a re-rank-only fix with no wire-format or API changes; sparse-network safety is preserved by the over-fetch+truncate pattern.

The logic is straightforward and well-tested: the new comparator is exercised by four targeted unit tests and an integration test. The only gap is a stale doc-comment on find_closest_nodes_local that still describes pure XOR ordering.

src/dht_network_manager.rs — doc-comment on find_closest_nodes_local (line 1963) still describes pure XOR sort order.

Important Files Changed

Filename	Overview
src/dht_network_manager.rs	Adds reachability-aware (reachability_tier, xor_distance) comparator; applies it in find_closest_nodes_local (with 2× over-fetch), find_closest_nodes_local_with_self, and apply_lookup_report_winners; stamps real trust scores instead of hardcoded 1.0. One stale doc-comment in find_closest_nodes_local (still describes pure XOR sort).
tests/two_node_messaging.rs	Adds integration test verifying that a freshly-discovered peer's reliability is stamped as DEFAULT_NEUTRAL_TRUST (0.5) rather than the old hardcoded 1.0.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[find_closest_nodes_local\ncalled with count] --> B[Over-fetch 2×count\nfrom routing table]
    B --> C[Filter out local peer ID]
    C --> D[Stamp reliability:\ntrust_engine.score OR\nDEFAULT_NEUTRAL_TRUST]
    D --> E[sort_by reachability_tier\nthen xor_distance]
    E --> F[truncate to count]
    F --> G[Return ≤count DHTNodes]
    G --> H{find_closest_nodes_local_with_self?}
    H -- Yes --> I[Push local_dht_node\nreliability = 1.0]
    I --> J[sort_by reachability_tier\nthen xor_distance]
    J --> K[truncate to count]
    H -- No --> L[Done]
    K --> L
    M[apply_lookup_report_winners\nnetwork/converged path] --> N[Deduplicate by peer_id\nkeep prefer_lookup_record winner]
    N --> O[sort_by reachability_tier\nthen xor_distance]
    O --> P[truncate to count]
    P --> L
    subgraph reachability_tier
        Q[DHTNode has Direct address?] -- Yes --> R[tier = 0]
        Q -- No --> S[tier = 1]
    end

Loading

Comments Outside Diff (1)

1. src/dht_network_manager.rs, line 1963-1964 (link)

   The doc-comment for find_closest_nodes_local still describes pure XOR-distance ordering, but the new implementation sorts by (reachability_tier, xor_distance). The sibling function find_closest_nodes_local_with_self was correctly updated; this one was missed. A caller reading only the doc-comment would assume XOR order and could be surprised by the reachability-based ranking.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/dht_network_manager.rs
   Line: 1963-1964
   
   Comment:
   The doc-comment for `find_closest_nodes_local` still describes pure XOR-distance ordering, but the new implementation sorts by `(reachability_tier, xor_distance)`. The sibling function `find_closest_nodes_local_with_self` was correctly updated; this one was missed. A caller reading only the doc-comment would assume XOR order and could be surprised by the reachability-based ranking.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/dht_network_manager.rs:1963-1964
The doc-comment for `find_closest_nodes_local` still describes pure XOR-distance ordering, but the new implementation sorts by `(reachability_tier, xor_distance)`. The sibling function `find_closest_nodes_local_with_self` was correctly updated; this one was missed. A caller reading only the doc-comment would assume XOR order and could be surprised by the reachability-based ranking.

```suggestion
    /// Results are sorted by `(reachability_tier, XOR distance)` to the key —
    /// directly-reachable peers first — and truncated to `count`.
    pub async fn find_closest_nodes_local(&self, key: &Key, count: usize) -> Vec<DHTNode> {
```

_{Reviews (1): Last reviewed commit: "fix: rank close-group/candidate selectio..." | Re-trigger Greptile}

[dirvine]

Review Summary

Overall: Sound fix, well-tested, CI all green. The re-rank-only approach (over-fetch → stable-sort → truncate) correctly preserves sparse-network safety. One minor doc fix needed.

Stale doc-comment (minor)

Greptile flagged this correctly — find_closest_nodes_local at line 1963-1964 still documents pure XOR-distance ordering. The sibling function find_closest_nodes_local_with_self was correctly updated. Suggest fixing before merging:

/// Results are sorted by `(reachability_tier, XOR distance)` to the key —
/// directly-reachable peers first — and truncated to `count`.

All CI passes

macOS/Ubuntu/Windows build, lint, security audit, unit tests, Greptile — all green.

👍 No blockers

Approving on the understanding the doc comment is fixed before merge.

[jacderida]

Added: find_closest_nodes_local_by_distance (commit a04f428)

This commit adds a distance-pure counterpart to find_closest_nodes_local, motivated by a coordination issue surfaced in WithAutonomi/ant-node#111.

Why this is needed. After this PR, the local routing table serves two consumers with opposite ordering needs:

• Close-group / candidate selection for storage (the majority of callers) benefits from the (reachability_tier, xor_distance) re-rank introduced here — a directly-reachable peer is a better place to put data than an XOR-equal relay-only one.
• Closeness verification (ant-node's Merkle pay-yourself defence) is the opposite: it must mirror the uploader's view, which is built from a pure XOR-distance network lookup. If the verifier saw the reachability re-rank, the 2× over-fetch + reachability sort + truncate could demote an XOR-close relay-only peer out of the compared window and falsely reject an honest candidate pool that legitimately contains that peer.

What the new function does. find_closest_nodes_local_by_distance returns the routing table's XOR-distance order as-is — no over-fetch, no reachability re-rank — while still excluding self and stamping the real trust score (so the E.2 reliability fix is preserved; only the ordering differs). Every existing selection caller keeps the reachability re-rank via find_closest_nodes_local; nothing else changes. Purely additive, no public-API or wire-format change.

Verification. cargo fmt clean; cargo clippy --lib and --test two_node_messaging (-D warnings -D clippy::unwrap_used -D clippy::expect_used) clean; cargo test --lib 485 passed / 0 failed; new two-node integration test local_by_distance_returns_peer_and_stamps_neutral_trust passes.

ant-node#111 will switch its Merkle closeness check to this method once this PR merges (see the note on that PR).