feat(dht): add bootstrap and iterative peer discovery#69
Conversation
- add DHTNode and DHTRoutingTable (k-bucket, XOR distance) structures - add bootstrap and iterative find_node/get_peers lookups on DHTClient - wire user callback through lookup and guard unmatched responses - add __main__ example and unit tests for the new DHT support Closes #9
|
Warning Review limit reached
More reviews will be available in 14 minutes and 48 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a routing-table-backed iterative DHT client with ChangesDHT Bootstrapping and BEP9 Metadata Exchange
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- query closest unqueried nodes from the routing table on each response - dedup queried nodes by (host, port) instead of host only - avoids a single unresponsive node dead-ending the iterative lookup
- repeat get_peers lookup rounds on a configurable interval (DHT_INTERVAL) - stop the loop once at least one peer has been discovered - reuse the persistent routing table to resume from the closest nodes
- send a valid 20 byte node id (drop the appended contact bytes) - query every node returned by a response, not only the closest 8 - ignore malformed datagrams and non numeric transaction ids - skip unroutable nodes (zero port, unspecified host) - include the responding node contact in the lookup debug line
- add metadata extension support (BEP 9/10) to fetch the info dict from peers - defer piece and file loading until the metadata is available - drive DHT peer discovery from the get_peers lookup and parse peer values - guard block requesting until the metadata has been received - add a DHT based download example to the torrent client
- lookup now seeds from the well known bootstrap nodes when the routing table is empty and no nodes are provided (fixes DHT peer requests that were issued with an empty contact set, eg: torrent get_peers lookups) - fix invalid percent format string in the torrent task status output
- include the file name, info hash, total size and piece size in the info string, with safe defaults while the metadata is not yet available
- skip set_data when the pieces structure is not loaded (avoids a crash on a data block received before the metadata exchange or after unload) - list the actual file names in the task status output (the name field is the directory name for multiple file torrents)
- print the task status at most once every print interval (3 seconds) - include the (resolved) download path in the completion message
- read the download target path from the TORRENT_TARGET_PATH config value (defaulting to the downloads directory)
- catch index and struct errors when parsing malformed DHT datagrams - ignore DHT peer responses received after the task has been unloaded (eg: once the download has completed) to avoid a None owner access
- close the (threaded) DHT client during server cleanup so its loop thread ends, otherwise the process hangs after the download completes
- add the DHT client as a regular (non threaded) container base instead of a separate thread, the shared event loop handles the datagram traffic together with the peer connections without issues - drop the explicit DHT client close on cleanup and the thread safe peer connection delay as they are no longer required without a separate thread
- add a configurable maximum number of peers per task (TORRENT_MAX_PEERS) - show the estimated time of arrival in the task status output - skip malformed (non compact) DHT peer values instead of raising - guard the speed computation against a zero elapsed time
There was a problem hiding this comment.
Pull request overview
Adds higher-level BitTorrent/DHT functionality to Netius by implementing DHT bootstrapping + iterative lookups and enabling torrent downloads from an info-hash via metadata exchange (BEP 9/10), with accompanying tests and changelog updates.
Changes:
- Implement DHT routing-table primitives (
DHTNode,DHTRoutingTable) and iterativebootstrap()/lookup()onDHTClient, plus response node parsing. - Add torrent extension-protocol support to negotiate and fetch metadata, defer piece/file loading until metadata exists, and improve task status output (ETA, throttled printing, peer cap).
- Add/extend unit tests for DHT, torrent client, and torrent server/task behaviors; update
CHANGELOG.md.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/netius/clients/dht.py | Adds routing table + iterative lookup/bootstrap; hardens parsing and request matching; exports node parsing helpers. |
| src/netius/clients/init.py | Re-exports new DHT structures (DHTNode, DHTRoutingTable). |
| src/netius/clients/torrent.py | Implements BEP 10 extended messaging + BEP 9 metadata exchange and adds a CLI example. |
| src/netius/common/torrent.py | Recognizes message type 20 as extended. |
| src/netius/servers/torrent.py | Defers loading until metadata exists, parses DHT peer values, adds ETA/status throttling and max-peers support. |
| src/netius/test/clients/dht.py | Unit tests for DHT request/response parsing, node/routing-table behavior, and client lookup/bootstrap. |
| src/netius/test/clients/torrent.py | Unit tests for extended handshake/metadata request + assembly logic. |
| src/netius/test/servers/torrent.py | Unit tests for torrent task robustness (late DHT responses, malformed peers, metadata, ETA, peer cap, pre-metadata data). |
| CHANGELOG.md | Documents the new DHT + torrent features and the related fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def is_error(self): | ||
| return self.info("y", True) | ||
|
|
||
| def is_response(self): | ||
| return self.info("r", True) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc6e775f4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/netius/servers/torrent.py (1)
774-812: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMissing guard for
self.storedaccess in status string.Lines 803-808 access
self.stored.marked_pieces,self.stored.total_pieces, etc. without checking ifstoredis initialized. While current usage only callsinfo_string()from piece callbacks (wherestoredis guaranteed to exist), calling it before metadata arrives would raiseAttributeError.Consider adding a guard or providing fallback values for when
storedisNone:🛡️ Optional defensive handling
+ + "pieces := %d/%d\n" + % (self.stored.marked_pieces if self.stored else 0, self.stored.total_pieces if self.stored else 0) - + "pieces := %d/%d\n" - % (self.stored.marked_pieces, self.stored.total_pieces)Or guard the entire section with an early check on
self.stored.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/netius/servers/torrent.py` around lines 774 - 812, The info_string() method accesses self.stored attributes (marked_pieces, total_pieces, marked_blocks, total_blocks, missing_pieces, missing_blocks) without checking if self.stored is initialized, which could raise an AttributeError if called before metadata arrives. Add a guard check to verify that self.stored is not None before accessing these attributes on lines 803-808, and provide appropriate fallback values (such as 0 or "N/A") when self.stored is None to ensure the method handles the case gracefully.src/netius/clients/torrent.py (1)
227-252: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueFragile header/payload split relies on "ee" substring.
The parsing at line 231 uses
data.index(b"ee")to find the end of the bencoded header. While this works for well-formed BEP 9 messages (the header dict contains only integer values formsg_typeandpiece), it could fail if a peer sends malformed data containing "ee" within string values.Consider wrapping in a try/except to handle
ValueErrorfromindex()if the pattern is missing:🛡️ Optional defensive handling
def on_metadata(self, data): + try: + index = data.index(b"ee") + 2 + except ValueError: + return - index = data.index(b"ee") + 2 message = netius.common.bdecode(data[:index])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/netius/clients/torrent.py` around lines 227 - 252, In the on_metadata method, the line using data.index(b"ee") to locate the end of the bencoded header can raise a ValueError if the "ee" delimiter is not found in malformed data. Wrap the header parsing logic (the data.index(b"ee") call and the subsequent bdecode operation) in a try/except block to catch ValueError exceptions. When a ValueError occurs, the method should return early without processing, since this indicates malformed metadata that cannot be safely parsed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 17-20: The `### Changed` section in the CHANGELOG contains an
empty bullet point `*` with no description, which violates the changelog
guidelines requiring concrete entries. Either remove the entire empty `###
Changed` section if there are no actual changes to document, or replace the
empty bullet point with a meaningful description of what was changed in this
release.
In `@src/netius/clients/dht.py`:
- Around line 150-154: The `is_error()` and `is_response()` methods are
attempting to call `self.info(...)` as if it were a function, but `self.info` is
a dictionary and cannot be called directly, causing a TypeError. Replace the
calls to `self.info(...)` with proper dictionary access using `.get()` method.
Additionally, correct the logic: `is_error()` should check if the value of key
"y" equals "e", and `is_response()` should check if the key "r" exists in the
dictionary by verifying if `.get("r")` returns a truthy value.
---
Nitpick comments:
In `@src/netius/clients/torrent.py`:
- Around line 227-252: In the on_metadata method, the line using
data.index(b"ee") to locate the end of the bencoded header can raise a
ValueError if the "ee" delimiter is not found in malformed data. Wrap the header
parsing logic (the data.index(b"ee") call and the subsequent bdecode operation)
in a try/except block to catch ValueError exceptions. When a ValueError occurs,
the method should return early without processing, since this indicates
malformed metadata that cannot be safely parsed.
In `@src/netius/servers/torrent.py`:
- Around line 774-812: The info_string() method accesses self.stored attributes
(marked_pieces, total_pieces, marked_blocks, total_blocks, missing_pieces,
missing_blocks) without checking if self.stored is initialized, which could
raise an AttributeError if called before metadata arrives. Add a guard check to
verify that self.stored is not None before accessing these attributes on lines
803-808, and provide appropriate fallback values (such as 0 or "N/A") when
self.stored is None to ensure the method handles the case gracefully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e6081fd-d9da-473e-ad8b-53915fce6561
📒 Files selected for processing (9)
CHANGELOG.mdsrc/netius/clients/__init__.pysrc/netius/clients/dht.pysrc/netius/clients/torrent.pysrc/netius/common/torrent.pysrc/netius/servers/torrent.pysrc/netius/test/clients/dht.pysrc/netius/test/clients/torrent.pysrc/netius/test/servers/torrent.py
- fix DHT is_error/is_response to read the message type instead of calling the info dict (which raised a type error) - guard the extended and metadata message handlers against malformed data so a bad message no longer tears down the connection - validate the info hash in the download example with a clear error - load the pieces and file before the peer discovery so the tracker announce no longer fails for torrent file based downloads - move the download example imports to the top of the module
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/netius/clients/torrent.py (1)
296-300:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
block not ininstead ofnot block inper coding guidelines.The membership test should use the preferred syntax.
Proposed fix
def remove_request(self, block): - if not block in self.requests: + if block not in self.requests: return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/netius/clients/torrent.py` around lines 296 - 300, In the remove_request method, change the membership test syntax from `not block in self.requests` to `block not in self.requests` to follow the preferred coding guidelines for membership testing in Python.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/netius/clients/torrent.py`:
- Around line 296-300: In the remove_request method, change the membership test
syntax from `not block in self.requests` to `block not in self.requests` to
follow the preferred coding guidelines for membership testing in Python.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a5a6885-ff4a-4fb0-b842-bdace7de7b7a
📒 Files selected for processing (5)
src/netius/clients/dht.pysrc/netius/clients/torrent.pysrc/netius/servers/torrent.pysrc/netius/test/clients/dht.pysrc/netius/test/clients/torrent.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/netius/clients/dht.py
- src/netius/servers/torrent.py
Summary
Implements DHT bootstrap and proper peer discovery/mapping for the DHT client, addressing the lack of a routing table and iterative lookups on top of the existing KRPC query primitives.
Closes #9.
Changes
DHTNode— a routing-table entry (id + contact) with compact-infounpackand XORdistance.DHTRoutingTable— a k-bucket table keyed by XOR distance (addwith dedupe/bucket-full guard,closestdistance-ordered).DHTClient.routing/bootstrap/lookup— lazy routing table, bootstrap from well-known nodes, and iterativefind_node/get_peerslookups built on the existingquery. The originalping/find_node/get_peers/query/on_data*primitives are preserved.DHTResponse.get_nodes— parses the compactnodesfield of a response.on_data_dhtno longer crashes on a response with no matching pending request (guardsNone, mirroring the DNS client);lookup/bootstrapnow actually invoke the user-suppliedcallback(it was previously accepted but ignored).__main__— a command line example that runs aget_peerslookup around an info hash (configurable viaDHT_INFO_HASH) and prints discovered nodes/peers.Verification
blackclean.values) from the test environment; this was independently confirmed with aria2c (a mature BitTorrent client), which also gets zero peers for the same hash here — only one DHT router is reachable from this network, so it is an environment limitation, not a code defect.🤖 Generated with Claude Code