Conversation
Two related changes that together restore merkle batch payment uploads
on under-warm networks. Validated experimentally on STG-01 (1k-node
testnet, 30% NAT-simulated peers, 2026-05-01).
A. CLOSENESS_LOOKUP_TIMEOUT: 60s → 240s
The iterative DHT lookup runs MAX_ITERATIONS=20 with ALPHA=3 and a
15s/RPC request_timeout. On a young network iterations average ~10s
each (dial cascades to NAT-simulated and unresponsive peers consume
the full per-RPC budget). Captured trace from STG-01 EWR-3 ant-node-1
immediately before a pre-fix timeout shows the lookup making real
forward progress through 7 of 20 iterations and then hitting the
60s wall:
Iter 0: +0.0s | Iter 1: +0.2s | Iter 2: +6.6s | Iter 3: +13.1s
Iter 4: +20.9s | Iter 5: +39.8s | Iter 6: +50.8s | [60s wall]
240s gives ~1.2× headroom over the ~200s natural worst-case runtime
on a 1k-node testnet. DoS amplification stays bounded by the
per-pool single-flight cache (closeness_pass_cache +
inflight_closeness): one in-flight lookup per pool_hash regardless
of concurrency.
B. CLOSENESS_LOOKUP_WIDTH: new constant = 32 (was implicit 16)
The client's get_merkle_candidate_pool over-queries
2 * CANDIDATES_PER_POOL = 32 peers via find_closest_peers(addr, 32),
then picks the 16 closest *valid responders* by XOR distance. So
legitimate pools routinely include peers from positions 17–32 of
the network's true ranking when the closer peers are slow or
NAT-stuck. The storer was fetching only top-16, so it never even
observed those legitimate candidates and rejected the pool with no
security benefit. Widening to 32 lets the storer's view be a
superset of the client's selection space.
Empirical effect on STG-01: client-side closeness mismatches
dropped from ~115 to ~31 per 5 min (73% reduction) after deploy.
Security argument
CANDIDATE_CLOSENESS_REQUIRED (13/16) is unchanged. The pay-yourself
attack still requires the attacker's fabricated PeerIds to land in
the storer's authoritative top-K. K=32 vs K=16 doubles the window
(≈1 extra bit of grinding cost), but the dominant cost remains
Sybil-grinding midpoint addresses or running real nodes near the
target — same security floor.
Tests
Four new constant-pinning tests + one compile-time const-assert in
the existing closeness test module:
- closeness_lookup_timeout_is_240s
- closeness_lookup_width_is_32
- closeness_required_threshold_unchanged_at_13
- const _: () = assert!(WIDTH >= CANDIDATES_PER_POOL)
Full lib suite: 452 passed; 0 failed. Clippy clean
(--all-features --all-targets -D warnings). Format clean.
…elper Addresses adversarial-review findings on PR #89. Correctness fixes - MAX_LEADER_RETRIES: 4 -> 1. Worst-case waiter wall-clock under a leader-cancellation cascade was (4+1) * CLOSENESS_LOOKUP_TIMEOUT = 5 * 240s = 20min user-visible. Capped at 2 * 240s = 8min by reducing the retry budget. The only realistic trigger is leader future-cancellation which should be extraordinarily rare; adversarial cancellation does not benefit from a higher cap (it just hides the symptom). - Doc note on CLOSENESS_LOOKUP_TIMEOUT: acknowledge that inflight_closeness is an LRU and a flood of unique pool_hash entries can evict an in-flight slot, allowing a second leader to race for the same pool. Previously the doc claimed a hard 'one in-flight per pool_hash' bound; the InflightGuard::drop comment already documented this caveat. Aligns the public-facing doc with reality. Testability - Extract closeness_lookup_count(pool_len) -> usize as a const fn so the WIDTH.max(pool_len) formula can be exercised in unit tests without standing up a DhtNetworkManager. Adds closeness_lookup_count_uses_max_of_width_and_pool_len pinning all three behaviours: standard (16-pool -> 32 lookup), future-proof (64-pool -> 64 lookup), lower-bound (any short pool -> WIDTH). Doc clarifications - Performance note on CLOSENESS_LOOKUP_WIDTH: count does not just truncate the lookup; saorsa-core's stagnation rule keeps iterating until best_nodes.len() >= count. K=32 can extend lookups by a few iterations vs K=16, which reinforces the timeout bump rather than undermining it. - Tighten .max(...) doc to acknowledge that pool.candidate_nodes is a fixed-size array today, so the .max() form is belt-and-braces for a hypothetical Vec-based future protocol bump rather than doing useful work today. Tests - 48/48 verifier tests pass (1 new). Clippy clean, format clean. - The deeper behavioural gaps (proving 80s lookup succeeds at 240s, proving 16/16-at-position-17 pool accepts at K=32) require a trait seam on DhtNetworkManager that doesn't exist yet — flagged for a follow-up PR.
…fixed
Refactor the matched-count check out of verify_merkle_candidate_closeness_inner
into a pure-logic helper check_closeness_match(candidate_ids, network_ids,
pool_address) and add 7 new regression tests against synthetic peer-ID
sets. The helper is a 1:1 extraction — same sparse-DHT short-circuit,
same set-membership check, same error strings. Production call site
goes through the helper; only the network-lookup wiring stays in the
async outer function.
This unlocks unit-test coverage for the original STG-01 failure modes
without standing up a real DHT, which previously required either
integration tests or a saorsa-core trait extraction.
New regression tests (all under #[cfg(test)] in the existing module):
closeness_match_passes_when_all_16_candidates_in_top_16
Happy path: trivial all-match case still works post-refactor.
closeness_match_passes_when_candidates_span_positions_1_to_15_and_17
A pool with 15 candidates at network-true positions 1..=15 plus
one at position 17 verifies cleanly under K=32 — the simplest
'one peer in the 17–32 window' shape.
closeness_match_fails_at_k_16_passes_at_k_32_for_honest_skew
Headline regression test: candidates at positions {1..=12, 17,
19, 21, 23} are REJECTED at K=16 (the original STG-01 bug) and
ACCEPTED at K=32 (the fix). Asserts both directions in one test.
closeness_match_rejects_forged_pool_at_k_32
Security floor: a pool with 16 candidates fully disjoint from the
network's top-32 must still be rejected at K=32. Pins the
pay-yourself defence.
closeness_match_rejects_pool_at_exactly_12_of_16_match
closeness_match_accepts_pool_at_exactly_13_of_16_match
Threshold sanity at the 13/16 boundary, in both directions.
closeness_match_returns_sparse_dht_error_when_lookup_too_small
The sparse-DHT short-circuit fires correctly when network_peers
falls below CANDIDATE_CLOSENESS_REQUIRED.
460/460 lib tests pass (was 452). Clippy clean
(--all-features --all-targets -D warnings) — added clippy::panic to
the test-module allow list alongside the existing expect_used (panic
is appropriate for asserting unexpected match arms in tests). Format
clean.
The refactor preserves all existing behaviour by construction: the
new check_closeness_match is the same code that was inlined in
verify_merkle_candidate_closeness_inner, just reachable from tests.
…R_POOL Per Copilot review on PR #89: defining the constant as a literal 32 duplicates the relationship the pinning test enforces. Define it as '2 * CANDIDATES_PER_POOL' so the constant tracks the protocol's over-query factor automatically. The pinning test still asserts equality with '2 * CANDIDATES_PER_POOL' — same value, just the constant is now derived rather than coincidental. No functional change. Tests still 15/15 pass for closeness, lib full suite still 460/460. Clippy + fmt clean.
fix(payment): widen merkle closeness lookup to K=32, raise timeout to 240s
There was a problem hiding this comment.
Pull request overview
Promotes ant-node to version 0.11.2 and updates payment-verifier “merkle candidate closeness” verification behavior (lookup timeout/width and related logic), along with adding regression/unit tests around that closeness matching.
Changes:
- Bump crate version to
0.11.2and regenerateCargo.lockwith updated dependency resolutions. - Adjust merkle candidate closeness verification parameters (timeout/lookup width) and refactor matching logic into a testable helper.
- Add a suite of unit/regression tests to pin the closeness constants and cover prior mismatch/failure modes.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/payment/verifier.rs |
Increases closeness lookup timeout/width, refactors closeness matching into a helper, and adds multiple regression/constant-pinning tests. |
Cargo.toml |
Bumps ant-node version to 0.11.2. |
Cargo.lock |
Updates lockfile for the release (dependency versions/checksums refreshed). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// flood of unique `pool_hash` entries can evict an in-flight slot, | ||
| /// at which point a second leader can race for the same pool (see | ||
| /// [`InflightGuard::drop`]). At steady state the pool cache and pool | ||
| /// signature verification gate keep this rare in practice. |
Comment on lines
+1028
to
+1035
| // `CANDIDATES_PER_POOL` (= 16), so `.max(...)` always evaluates to | ||
| // `CLOSENESS_LOOKUP_WIDTH` today. The compile-time | ||
| // `const _: () = assert!(WIDTH >= CANDIDATES_PER_POOL)` in the test | ||
| // module pins that invariant. The `.max(...)` form is belt-and-braces | ||
| // for a hypothetical future protocol that grows pool size to a | ||
| // `Vec`-typed candidate set: the storer would scale its lookup with | ||
| // the pool rather than truncating, which would otherwise re-open the | ||
| // K-too-small failure mode. |
Comment on lines
+2850
to
+2859
| // 15 of which are at network-true positions 1..=15, and ONE of | ||
| // which is at position 17 (because the network-true position-16 | ||
| // peer was unresponsive when the client over-queried 32). | ||
| // | ||
| // Pre-fix (K=16 storer): network_peer_ids = 16 entries (positions | ||
| // 1..=16); position 17 is NOT in the network set, so matched = | ||
| // 15 < 13 — wait, 15 ≥ 13, that path actually passes too. The | ||
| // failure mode was a *worse* skew where 4+ of the storer's top-16 | ||
| // were unresponsive at the client side. Let me model that case | ||
| // precisely below. |
Comment on lines
1
to
4
| [package] | ||
| name = "ant-node" | ||
| version = "0.11.1" | ||
| version = "0.11.2" | ||
| edition = "2021" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes
rc-2026.5.1to release version(s): 0.11.2.-rc.*from[package].versionCargo.lockOnce merged, the release tag will be pushed to fire the publish workflow.