Skip to content

feat: add XOR-only self-inclusive local lookup for closeness verification - #122

Merged
jacderida merged 2 commits into
rc-2026.5.4from
fix/xor-only-local-verification-with-self
May 26, 2026
Merged

jacderida merged 2 commits into
rc-2026.5.4from
fix/xor-only-local-verification-with-self

Conversation

@jacderida

@jacderida jacderida commented May 26, 2026 •

Copy link
Copy Markdown
Member

Summary

find_closest_nodes_local_with_self orders by (reachability_tier, xor_distance) — directly-reachable peers first. That's the right ranking for storage selection (prefer a reachable peer over an XOR-equal unreachable one, per #121), but it's wrong for closeness verification.

A verifier that reuses _with_self compares the uploader's pure-XOR-distance peer set against a reachability-reranked local view. On a network with a meaningful NAT fraction, the re-rank demotes XOR-close relay-only / NAT'd peers out of the compared window, so honest payments that legitimately quoted those peers get falsely rejected. #121 already shipped the XOR-only find_closest_nodes_local_by_distance for exactly this reason — but only the non-self-inclusive variant.

This adds the missing self-inclusive counterpart:

  • find_closest_nodes_local_by_distance_with_self — same pure in-memory routing-table read as find_closest_nodes_local_with_self, still appends the local node so callers can compute IsResponsible(self, K), but sorts purely by XOR distance (compare_node_distance) with no reachability re-rank.

Fetching the XOR-closest count peers and adding only self is sufficient: any peer beyond the closest count is farther than all of them, so self cannot pull it into the top-count.

Consumer

ant-node's single-node payment close-group verification switches onto this method (companion PR against ant-node rc-2026.5.4), mirroring the Merkle closeness path that already uses the XOR-only lookup.

Test plan

  • cargo check --lib — clean
  • Verified downstream ant-node compiles against this method via a local path patch

🤖 Generated with Claude Code

Greptile Summary

This PR adds find_closest_nodes_local_by_distance_with_self, the self-inclusive, XOR-only counterpart to the existing find_closest_nodes_local_by_distance. It fixes a correctness gap in closeness verification: the previous self-inclusive variant (find_closest_nodes_local_with_self) applies a reachability re-rank that can demote NAT'd peers out of the comparison window, causing honest payments to be falsely rejected.

  • New method (find_closest_nodes_local_by_distance_with_self): fetches the count XOR-closest remote peers, appends the local node, re-sorts by pure XOR distance (compare_node_distance), and truncates to count — matching the pattern of the existing _with_self method but without reachability re-ranking.
  • No new tests: both sibling methods have dedicated integration tests; the new method has none, leaving the self-inclusion contract and XOR-only ordering unverified at the integration level.

Confidence Score: 4/5

The change is a clean, minimal addition that correctly mirrors an already-reviewed pattern; the only gap is missing integration test coverage for the new public method.

The implementation faithfully follows the established _with_self pattern, the count-sufficiency argument holds under all routing-table configurations, and the XOR-only sort comparator is the correct choice for closeness verification. The sole concern is the absence of an integration test: every sibling method has a dedicated test asserting its self-inclusion and ordering contracts, but this new method ships without one.

src/dht_network_manager.rs — specifically the new find_closest_nodes_local_by_distance_with_self method, which lacks a companion integration test in tests/.

Important Files Changed

Filename Overview
src/dht_network_manager.rs Adds find_closest_nodes_local_by_distance_with_self: a XOR-only, self-inclusive local lookup that mirrors the existing find_closest_nodes_local_with_self pattern but uses pure XOR ordering instead of reachability re-ranking. Logic and argument for count-sufficiency are sound; no integration test covers the new method.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["find_closest_nodes_local_by_distance_with_self(key, count)"] --> B["find_closest_nodes_local_by_distance(key, count)\n(XOR-only, excludes self)"]
    B --> C["Routing table\nfind_nodes_with_publish_seq(key, count)\n→ XOR-sorted, self filtered"]
    C --> D["Vec≤ count remote peers"]
    D --> E["push(local_dht_node())"]
    E --> F["sort_by compare_node_distance\n(pure XOR, no reachability tier)"]
    F --> G["truncate(count)"]
    G --> H["Vec — XOR-closest count nodes\n(including self if competitive)"]
Loading
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:2145-2162
**Missing integration test for the new public API**

Both of the method's siblings have dedicated integration tests: `find_closest_nodes_local_with_self` is covered by `dht_self_advertisement.rs`, and `find_closest_nodes_local_by_distance` is covered by `two_node_messaging.rs::local_by_distance_returns_peer_and_stamps_neutral_trust`. The new `find_closest_nodes_local_by_distance_with_self` has no test at all. At minimum, a two-node smoke test should assert that: (a) the local node IS included in the result, and (b) `Self::compare_node_distance` is used rather than `compare_node_reachability_then_distance` — the whole correctness argument for closeness verification rests on that distinction.

Reviews (1): Last reviewed commit: "feat: add XOR-only self-inclusive local ..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

`find_closest_nodes_local_with_self` ranks by `(reachability_tier,
xor_distance)`, which is correct for storage *selection* but wrong for
closeness *verification*: the reachability re-rank (added in #121)
demotes XOR-close relay-only / NAT'd peers out of the compared window,
so a verifier using it falsely rejects honest payments that legitimately
quoted those peers.

Add `find_closest_nodes_local_by_distance_with_self`: the self-inclusive
counterpart of the XOR-only `find_closest_nodes_local_by_distance` (also
added in #121). Same pure in-memory routing-table read, still includes
self so callers can compute `IsResponsible(self, K)`, but orders purely
by XOR distance with no reachability re-rank.

Consumed by ant-node's single-node payment close-group verification.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread src/dht_network_manager.rs
Greptile flagged the new public method as having no test while both its
siblings do. Add the same two layers of coverage:

- Unit test `by_distance_comparator_keeps_xor_closer_relay_only_ahead_of_direct`:
  the complement of `reachability_rerank_prefers_direct_over_xor_closer_relay_only`.
  Proves the method's comparator (`compare_node_distance`) keeps an XOR-closer
  relay-only peer ahead of an XOR-farther direct one — i.e. it does NOT apply
  the reachability re-rank, which is the whole correctness argument for the
  verification path.

- Integration test `local_by_distance_with_self_includes_self_and_connected_peer`
  (sibling of `local_by_distance_returns_peer_and_stamps_neutral_trust`): a
  two-node smoke test asserting the distinguishing contract — the local node IS
  included — plus connected-peer presence and neutral-trust stamping.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jacderida
jacderida merged commit 82bb541 into rc-2026.5.4 May 26, 2026
12 checks passed
jacderida added a commit to WithAutonomi/ant-node that referenced this pull request May 26, 2026
Picks up find_closest_nodes_local_by_distance_with_self
(WithAutonomi/saorsa-core#122, now merged to rc-2026.5.4) that the
single-node close-group verification change depends on. The crate is
pinned to `branch = "rc-2026.5.4"`, so this only advances Cargo.lock
from 1be7352 to 82bb541; no manifest change. ant-node now compiles
against the published branch without a local patch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jacderida added a commit to jacderida/ant-node that referenced this pull request May 28, 2026
… on mismatch

Two upload-breaking regressions on testnets with a meaningful NAT
fraction, both from storer-side closeness verification diverging from
the uploader's network-walked peer selection.

Single-node close-group check (introduced in WithAutonomi#107): switch off the
reachability-reranked find_closest_nodes_local_with_self onto the
XOR-only find_closest_nodes_local_by_distance_with_self. The re-rank
(saorsa-core WithAutonomi#121) demoted XOR-close relay-only / NAT'd peers out of the
local top-CLOSE_GROUP_SIZE, dropping 2-3 of the uploader's 7 quoted
peers and breaching the >=5 threshold. This mirrors the fix already
applied to the Merkle path; it remains a pure local lookup, so no added
network cost.

Merkle candidate-pool check (changed in WithAutonomi#111): WithAutonomi#111 moved the check off
the authoritative network lookup onto the local routing table, with the
fallback gated on local-table *size*, not match *outcome*. On a real
network the local k-bucket sample legitimately diverges from the
uploader's network-walked candidates (which include reachable responders
from positions 17-32), so honest pools were hard-rejected with no
escalation. Keep WithAutonomi#111's local fast path (accept on a local match), but
escalate to the authoritative network lookup on match *failure* too.

Bound the reopened network-fallback path with a new
closeness_fallback_permits semaphore (CLOSENESS_NETWORK_FALLBACK_CONCURRENCY
= 16): inflight_closeness already collapses same-pool concurrency, and
this caps the distinct-pool case so a forged-pool flood cannot spawn
unbounded 240s Kademlia walks -- addressing the DoS rationale WithAutonomi#111 gave
for the size-only gate.

Requires saorsa-core's find_closest_nodes_local_by_distance_with_self
(WithAutonomi/saorsa-core#122) on rc-2026.5.4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mickvandijke mickvandijke mentioned this pull request Jun 14, 2026
Sign up for free to 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