feat: add DHT nodes from BEP-5 .torrent files to the routing table#583
feat: add DHT nodes from BEP-5 .torrent files to the routing table#583chantra wants to merge 5 commits into
Conversation
|
@ikatson I am not super psyche about the current DhtNode serde, but did not find a good way to have it working using the existing bencode crate. Mostly due to serialize_tuple . Let me know if there is ways you would prefer this written. |
| }; | ||
|
|
||
| // BEP-0005: inject nodes from .torrent as initial peers. | ||
| if !torrent.meta.nodes.is_empty() { |
There was a problem hiding this comment.
If I'm reading the spec correctly, these nodes aren't peers necessarily, but the closest DHT nodes.
Means they need to be queried through DHT to find peers, but not (just) added to initial parts
There was a problem hiding this comment.
yes, those are nodes that can be queried over DHT. Are you asking because then they should be added to a separate variable?
There was a problem hiding this comment.
I suppose in some ways, it is similar to #485 . Would you prefer that the initial torrent nodes are added to the Session.dht? I have not found a way to insert a node in the DhtState struct. Anything I am missing?
There was a problem hiding this comment.
@ikatson I have been spending a bit more time on this and I believe my test worked initially because it probably leaked to the DHT_BOOTSTRAP.
As much as I can see, in its current state, there is no way tell the Session to add those nodes to the dht table, right?
There was a problem hiding this comment.
Yes, and I think you need to add support for this to make this PR useful.
There was a problem hiding this comment.
let me put this into draft mode for now. Thanks for the feedback.
There was a problem hiding this comment.
@ikatson ok, finally got to the bottom of this PR.
It is composed of 5 commits.
- feat: add BEP-0005 DHT bootstrap nodes support in .torrent files: this is the existing code from this very PR adding support for .torrent.
- test(dht): add loopback harness and bootstrap routing-table test: Adds a test to make sure dht bootstrapping work after a refactor in next diff
- refactor(dht): hoist resolve + find_node onto DhtState: the refactor
- feat(dht): add DhtState::add_bootstrap_nodes: builds the public
DhtState::add_bootstrap_nodes()using the previous refactor + tests - feat(librqbit): route .torrent BEP-5 nodes into the DHT: inject the nodes discovered through the .torrent file into the DHT.
|
As to custom deserialize/serialize that's totally fine and how it's usually done anyway |
|
Re-requesting review in case this is needed to get it back on your radar. |
bfc4040 to
2baf72d
Compare
|
hi, just an update, still want to work on this, but need to prioritize other stuff right now. |
Add DhtNode type with custom serde that serializes as a 2-element bencode list ["<host>", <port>] per the BEP-5 spec. The Deserialize impl drains the sequence after reading host/port to properly consume the list end-marker, preventing a BytesRemaining(1) error when nodes appear before other fields in the outer torrent dict. - New `dht_node` module in librqbit_core with DhtNode struct - Add `nodes: Vec<DhtNode>` field to TorrentMetaV1 - Wire nodes through CreateTorrentOptions and create_torrent() - Add bencode roundtrip tests (single, vec, nested struct, empty vec) Test Plan: - `cargo test -p librqbit test_create_torrent_with_bootstrap_nodes` passes - `cargo test -p librqbit-core dht_node::tests` passes - `cargo clippy -p librqbit-core` clean
Add an in-crate test module to dht.rs (there was none) with a small two-node loopback harness: `spawn_node` starts an in-process DhtState bound to an ephemeral v4 loopback port, and `wait_for_node_in_table` polls the routing tables for a given node id. `bootstrap_populates_routing_table` seeds node B with a known node C, then bootstraps node A off B and asserts A learns about C via the find_node recursion. This pins down the existing bootstrap behaviour so the upcoming refactor of `bootstrap_hostname` is provably non-regressing. Test Plan: - `cargo test -p librqbit-dht bootstrap_populates_routing_table` passes - `cargo clippy -p librqbit-dht --lib --tests` clean - `cargo fmt --all -- --check` clean
`bootstrap_hostname` lived on DhtWorker but only used `self.dht`, never
the worker socket. Extract its body into two private DhtState helpers:
- `find_nodes_in_routing_table(addrs)`: the v4/v6 find_node split.
- `resolve_and_find_nodes(addr)`: lookup_host + the above. Accepts any
`ToSocketAddrs + Debug` ("host:port" string or (host, port) tuple) and
derives the error label from the Debug repr, so no separate display arg.
`DhtWorker::bootstrap_hostname` now delegates to the DhtState helper. No
behaviour change; this deduplicates the find_node logic so an upcoming
public `add_bootstrap_nodes` can reuse it.
Test Plan:
- `cargo test -p librqbit-dht --lib` passes (incl. bootstrap_populates_routing_table)
- `cargo clippy -p librqbit-dht --lib --tests` clean
- `cargo fmt --all -- --check` clean
Expose a public, best-effort API to inject extra bootstrap nodes into the routing table at runtime. Takes (host, port) pairs (host may be a hostname or IPv4/IPv6 literal), resolves each via the shared `resolve_and_find_nodes` helper, and runs find_node in a background task so the nodes get queried over DHT rather than treated as peers. Intended for BEP-5 `nodes` embedded in .torrent files, which are DHT contacts, not BitTorrent peers. Test Plan: - `cargo test -p librqbit-dht --lib` passes, incl: - add_bootstrap_nodes_populates_table (A learns node C only via the API) - add_bootstrap_nodes_empty_is_noop - `cargo clippy -p librqbit-dht --lib --tests` clean - `cargo fmt --all -- --check` clean
When adding a .torrent that carries a BEP-5 `nodes` list, hand those (host, port) contacts to the session DHT via `add_bootstrap_nodes` instead of injecting them as initial peers. Per the spec these are DHT nodes to be queried for peers, not BitTorrent peers, so this lets the DHT discover peers from a trackerless torrent's embedded contacts. Hostnames as well as IPv4/IPv6 literals are supported (resolution happens in the DHT). If DHT is disabled, the nodes are ignored with a debug log. Test Plan: - `cargo test -p librqbit test_create_torrent_with_bootstrap_nodes` passes - `cargo clippy -p librqbit --all-targets` clean - `cargo fmt --all -- --check` clean
2baf72d to
640f425
Compare
chantra
changed the title
2 participants
Summary
Addresses the review on #583. BEP-5
nodesembedded in a.torrentare the closest DHT contacts, not BitTorrent peers — they must be queried over DHT to discover peers. The previous approach injected them intoinitial_peers, treating them as BT peers. This PR:DhtState::add_bootstrap_nodes(Vec<(String, u16)>)API to inject nodes into the routing table at runtime (best-effort, background), resolving hostnames as well as IPv4/IPv6 literals (per the spec,<host>may be a DNS name likeyour.router.node).bootstrap_hostnamelogic ontoDhtState(find_nodes_in_routing_table+resolve_and_find_nodes) so bootstrap and the new API share one code path..torrentnodesinlibrqbit'sadd_torrentto the DHT via that API instead ofinitial_peers(ignored with a debug log if DHT is disabled).The initial BEP-5 commit was amended to remove the now-obsolete initial-peers wiring, keeping history clean.
Test plan
cargo test -p librqbit-dht --lib— new tests:bootstrap_populates_routing_table(locks in existing bootstrap behavior)add_bootstrap_nodes_populates_table(A learns a node only via the new API)add_bootstrap_nodes_empty_is_noopcargo test -p librqbit test_create_torrent_with_bootstrap_nodescargo test -p librqbit-core dht_node::testscargo clippy -p librqbit-dht --lib --testsandcargo clippy -p librqbit --all-targetscleancargo fmt --all -- --checkcleanAssisted by AI