Skip to content

feat(chunk): add a get-or-closer request - #44

Open
mickvandijke wants to merge 2 commits into
mainfrom
mickvandijke/v2-1358-kad-chunk-get
Open

mickvandijke wants to merge 2 commits into
mainfrom
mickvandijke/v2-1358-kad-chunk-get

Conversation

@mickvandijke

@mickvandijke mickvandijke commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Adds a get-or-closer request, Kademlia's FIND_VALUE for chunks (ADR-0020). A node that holds the chunk answers Found with it; one that does not answers Closer with its closest known peers.

  • ChunkMessageBody::{GetOrCloserRequest, GetOrCloserResponse} carry ChunkGetOrCloserRequest and ChunkGetOrCloserResponse (Found | Closer | Error). They are appended after every existing variant, so older discriminants keep their wire values; a new test pins them.
  • Closer carries an opaque payload. saorsa-core encodes it and checks it as a FIND_NODE answer (feat(dht): expose lookup steps for caller-driven lookups saorsa-core#173), so the peer format is versioned in one place.
  • GET_OR_CLOSER_AGENT_TOKEN (get-or-closer/1) and advertises_get_or_closer: a node built before this request drops it without a reply, so clients send it only to peers whose user agent carries the token.
  • The crate takes the 3.1.0 release as its baseline, so the ant-node and ant-client branches can build against it.

Stack, merge in this order:

  1. feat(dht): expose lookup steps for caller-driven lookups saorsa-core#173
  2. this PR
  3. feat(storage): answer get-or-closer requests (ADR-0020) ant-node#249
  4. feat(read): read chunks through a get-or-closer lookup (ADR-0020) ant-client#221

Linear issue

Closes V2-1358

Risk tier

  • T0 — docs / tooling / CI / pure UX-output. Repo CI only.
  • T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
  • T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
  • T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Compatibility

  • Wire: two new ChunkMessageBody variants, appended at indices 14 and 15. Existing messages are unchanged.
    • A node built before them cannot decode them: natively it sends no reply, and over WebRTC Direct it closes the channel.
    • So nodes announce get-or-closer/1 in their user agent, and clients only send the request to peers that do.
  • Storage: none.
  • API: additive. New types, a constant and a function; ChunkMessageBody is #[non_exhaustive].

Semver impact

  • breaking
  • feature
  • fix

Test evidence

  • cargo test --all-features: 108 passed. New tests cover:
    • request and response round trips
    • discriminant wire values for GetRequest (2), PointerGetRequest (12) and the new variants (14, 15)
    • token detection
  • cargo clippy --all-targets --all-features -- -D warnings: clean.
  • End to end through ant-node and ant-client, as listed in those PRs.
  • Dev testnet comparison against main, matched (2026-10-09). Two 60-node fleets, 30% NAT, 4 uploaders and 2 paced downloaders each, both on the rc-2026.10.1 baseline, run with the whole stack.
    • Downloads: p50 20.5 s → 9.7 s (−53%), p95 28.6 s → 15.5 s (−46%), 0 failures, same read load.
    • Uploads: non-merkle identical. Merkle uploads were slower on that fleet, likely due to its infrastructure (chain droplet CPU steal); not attributed to this change.
    • Details: feat(storage): answer get-or-closer requests (ADR-0020) ant-node#249.

New dependency

none

ADR

ADR-0020: Kademlia-style chunk GET: return the chunk or closer peers. Measurements are in the ant-client investigation.

Mitigation / rollback

Inert until nodes answer and clients send the request; revert the commit.

🤖 Generated with Claude Code

Kademlia's FIND_VALUE for chunks (ant-node ADR-0020). A node that holds
the chunk answers `Found` with it; one that does not answers `Closer`
with its closest known peers. The peers travel as an opaque payload that
saorsa-core encodes and checks as a FIND_NODE answer, so the peer format
stays versioned in one place.

The new variants are appended after every existing one, so older
discriminants keep their wire values; a test now pins them. A node built
before this request drops it without a reply, so nodes that answer it
announce `get-or-closer/1` in their user agent and clients check for it
with `advertises_get_or_closer`.

The crate takes the 3.1.0 release as its baseline so the ant-node and
ant-client branches can build against it.

Part of V2-1358.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Takes the rc-2026.10.1 release baseline (saorsa-core 0.29.0). The
get-or-closer variants are unaffected: 4.0.0 adds no message variants,
so they keep wire values 14 and 15.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code approval at the reviewed head

No introduced blocking defect found in this PR. Appended wire variants, pinned discriminants and exact capability-token matching are consistent with the linked implementations. The peers field is deliberately opaque; its existing 5 MiB outer envelope is not a substitute for the inner decoder bound.

Linked stack is NOT approved to ship: saorsa-core#173 needs its decoder size guard, and downstream pins must follow. Preserve dependency-order publication and revalidate CI before merge.

Reviewed commit: 69a031bacd106100376f1914718bfbd44bb1e955.

Review method: six substantive seats across the linked set, including a completed z-ai/glm-5.2 API review recovered from a worker that timed out after saving its response; coordinator source checks and local execution. Initial functional/security/test seats recommended approval. The coordinator probe, independent adjudicator, API seat and replacement concurrency/release seats corroborate the missing decoder bound. This is not unanimous approval. The API seat recommended client approval with concerns; the coordinator withholds it because the exact client pin still exposes the defect.

Local verification (macOS, rustc 1.96.0): core closer-peers 3 passed and DHT-manager regression 143 passed; protocol all-features 108 passed; node get-or-closer unit tests 3 passed and real-transport E2E 1 passed; client kad_read unit tests 10 passed and real-transport E2E 1 passed. Formatting passed in all four repositories. No production/testnet deployment or merge performed. Draft git pins were verified with cargo metadata --locked; this is not published dependency closure. ADR-0020 remains Proposed.

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-review — no change at reviewed head

Head is unchanged at 69a031ba; the first-round APPROVE still stands. Nothing new in this delta.

Verdict

APPROVE (unchanged). CI all green at this head (format, clippy, tests ubuntu/macos/no-default-features, docs, wasm, security, publish dry-run).

Panel: GLM 5.3 seat billing-failed (402) — this seat is 2/3 reviewed (GLM unavailable), Qwen 3.8 + DS4 concur.

@mickvandijke
mickvandijke marked this pull request as ready for review October 13, 2026 13:00
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.

2 participants