From 491012c87c98326275f97415016ebe1068f5c95d Mon Sep 17 00:00:00 2001
From: Franz Heinzmann <frando@unbiskant.org>
Date: Thu, 16 May 2024 11:44:37 +0200
Subject: [PATCH] fix: do not bind a mainline DHT socket (#2296)

## 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.
---
 iroh-net/src/discovery/pkarr_publish.rs | 34 ++++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/iroh-net/src/discovery/pkarr_publish.rs b/iroh-net/src/discovery/pkarr_publish.rs
index c733d95db6..41bc676bbb 100644
--- a/iroh-net/src/discovery/pkarr_publish.rs
+++ b/iroh-net/src/discovery/pkarr_publish.rs
@@ -8,7 +8,7 @@
 
 use std::sync::Arc;
 
-use anyhow::Result;
+use anyhow::{anyhow, bail, Result};
 use pkarr::SignedPacket;
 use tokio::{
     task::JoinHandle,
@@ -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
@@ -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(())
     }
 }