Skip to content

Commit

Permalink
fix: do not bind a mainline DHT socket (#2296)
Browse files Browse the repository at this point in the history
## Description

By merging #2188 we unknowingly made `iroh` bind a socket for a mainline
DHT.
Pkarr includes code for querying records from the bittorent mainline DHT
behind a `dht` feature flag, which is disabled in `iroh-net` but with
#2188 is enabled in `iroh-dns-server`, and due to feature unification in
cargo workspaces this silently also enabled the feature in `iroh-net`.


The DHT is not used because we only use the `relay_` methods of the
pkarr client in iroh-net. It is binding a socket, but not doing anything
else because it has no bootstrap node configured.

Still, this is unfortunate - we are shipping code and binding a socket
for a feature we don't (yet) use.

From a cursory skim of the source code, this would still be the same
with the recently released `pkarr` v2.

We should check the docs on feature unification if there is a clean way
around that.

For now, this PR makes iroh not bind a socket for an unused mainline DHT
client. Instead, we just use reqwest directly, which is the same as what
`PkarrClient::relay_put` would do here.


## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
- [x] All breaking changes documented.
  • Loading branch information
Frando committed May 16, 2024
1 parent 9d71fd8 commit 491012c
Showing 1 changed file with 25 additions and 9 deletions.
34 changes: 25 additions & 9 deletions iroh-net/src/discovery/pkarr_publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

use std::sync::Arc;

use anyhow::Result;
use anyhow::{anyhow, bail, Result};
use pkarr::SignedPacket;
use tokio::{
task::JoinHandle,
Expand Down Expand Up @@ -135,7 +135,7 @@ impl PublisherService {
loop {
if let Some(info) = self.watcher.get() {
if let Err(err) = self.publish_current(info).await {
warn!(?err, url = %self.pkarr_client.pkarr_relay , "Failed to publish to pkarr");
warn!(?err, url = %self.pkarr_client.pkarr_relay_url , "Failed to publish to pkarr");
failed_attemps += 1;
// Retry after increasing timeout
republish
Expand Down Expand Up @@ -177,24 +177,40 @@ impl PublisherService {
/// A pkarr client to publish [`pkarr::SignedPacket`]s to a pkarr relay.
#[derive(Debug, Clone)]
pub struct PkarrRelayClient {
inner: pkarr::PkarrClient,
pkarr_relay: Url,
http_client: reqwest::Client,
pkarr_relay_url: Url,
}

impl PkarrRelayClient {
/// Create a new client.
pub fn new(pkarr_relay: Url) -> Self {
pub fn new(pkarr_relay_url: Url) -> Self {
Self {
inner: pkarr::PkarrClient::builder().build(),
pkarr_relay,
http_client: reqwest::Client::new(),
pkarr_relay_url,
}
}

/// Publish a [`SignedPacket`]
pub async fn publish(&self, signed_packet: &SignedPacket) -> anyhow::Result<()> {
self.inner
.relay_put(&self.pkarr_relay, signed_packet)
let mut url = self.pkarr_relay_url.clone();
url.path_segments_mut()
.map_err(|_| anyhow!("Failed to publish: Invalid relay URL"))?
.push(&signed_packet.public_key().to_z32());

let response = self
.http_client
.put(url)
.body(signed_packet.as_relay_request())
.send()
.await?;

if !response.status().is_success() {
bail!(format!(
"Publish request failed with status {}",
response.status()
))
}

Ok(())
}
}

0 comments on commit 491012c

Please sign in to comment.