From 6dc40eed8ee4cdee911060a9942a7afe21e9b042 Mon Sep 17 00:00:00 2001 From: nazeh Date: Sat, 27 Jul 2024 11:53:02 +0300 Subject: [PATCH 1/2] fix(pkarr): non-wasm relay clients resolve not_found correctly --- pkarr/src/lib.rs | 1 + pkarr/src/relay_client.rs | 56 +++++++++++++++++++++++---------- pkarr/src/relay_client_async.rs | 55 ++++++++++++++++++++++---------- pkarr/src/signed_packet.rs | 4 ++- 4 files changed, 81 insertions(+), 35 deletions(-) diff --git a/pkarr/src/lib.rs b/pkarr/src/lib.rs index 0a3bec5..7fffe55 100644 --- a/pkarr/src/lib.rs +++ b/pkarr/src/lib.rs @@ -35,6 +35,7 @@ pub const DEFAULT_RESOLVERS: [&str; 1] = ["resolver.pkarr.org:6881"]; pub use bytes; pub use simple_dns as dns; +#[cfg(not(target_arch = "wasm32"))] macro_rules! if_async { ($($item:item)*) => {$( #[cfg(all(not(target_arch = "wasm32"), feature = "async"))] diff --git a/pkarr/src/relay_client.rs b/pkarr/src/relay_client.rs index c8b1a33..c52ebe3 100644 --- a/pkarr/src/relay_client.rs +++ b/pkarr/src/relay_client.rs @@ -112,27 +112,21 @@ impl PkarrRelayClient { /// /// # Errors /// - /// - Returns [Error::RelayError] if the relay responded with a status >= 400 or something - /// wring with the transport, transparent from [ureq::Error]. + /// - Returns [Error::RelayError] if the relay responded with a status >= 400 + /// (except 404 in which case you should receive Ok(None)) or something wrong + /// with the transport, transparent from [ureq::Error]. /// - Returns [Error::IO] if something went wrong while reading the payload. pub fn resolve(&self, public_key: &PublicKey) -> Result> { - let mut last_result = Ok(None); + if let Some(signed_packet) = self.resolve_inner(public_key).recv()?? { + self.cache + .lock() + .unwrap() + .put(public_key.clone(), signed_packet.clone()); - while let Ok(response) = self.resolve_inner(public_key).recv() { - match response { - Ok(Some(signed_packet)) => { - self.cache - .lock() - .unwrap() - .put(public_key.clone(), signed_packet.clone()); - - return Ok(Some(signed_packet)); - } - result => last_result = result, - } - } + return Ok(Some(signed_packet)); + }; - last_result + Ok(None) } // === Private Methods === @@ -246,6 +240,11 @@ impl PkarrRelayClient { }; } } + Err(ureq::Error::Status(404, _)) => { + dbg!(404); + debug!(?url, "SignedPacket not found"); + let _ = sender.send(Ok(None)); + } Err(error) => { debug!(?url, ?error, "Error response"); let _ = sender.send(Err(Error::RelayError(Box::new(error)))); @@ -308,4 +307,27 @@ mod tests { assert_eq!(resolved.as_bytes(), signed_packet.as_bytes()); } + + #[test] + fn not_found() { + let keypair = Keypair::random(); + + let mut server = mockito::Server::new(); + + let path = format!("/{}", keypair.public_key()); + + server.mock("GET", path.as_str()).with_status(404).create(); + + let relays: Vec = vec![server.url()]; + let settings = RelaySettings { + relays, + ..RelaySettings::default() + }; + + let client = PkarrRelayClient::new(settings.clone()).unwrap(); + + let resolved = client.resolve(&keypair.public_key()).unwrap(); + + assert!(resolved.is_none()); + } } diff --git a/pkarr/src/relay_client_async.rs b/pkarr/src/relay_client_async.rs index 49922be..c9fd38a 100644 --- a/pkarr/src/relay_client_async.rs +++ b/pkarr/src/relay_client_async.rs @@ -49,27 +49,21 @@ impl PkarrRelayClientAsync { /// /// # Errors /// - /// - Returns [Error::RelayError] if the relay responded with a status >= 400 or something - /// wring with the transport, transparent from [ureq::Error]. + /// - Returns [Error::RelayError] if the relay responded with a status >= 400 + /// (except 404 in which case you should receive Ok(None)) or something wrong + /// with the transport, transparent from [ureq::Error]. /// - Returns [Error::IO] if something went wrong while reading the payload. pub async fn resolve(&self, public_key: &PublicKey) -> Result> { - let mut last_result = Ok(None); + if let Some(signed_packet) = self.0.resolve_inner(public_key).recv_async().await?? { + self.cache() + .lock() + .unwrap() + .put(public_key.clone(), signed_packet.clone()); - while let Ok(response) = self.0.resolve_inner(public_key).recv_async().await { - match response { - Ok(Some(signed_packet)) => { - self.cache() - .lock() - .unwrap() - .put(public_key.clone(), signed_packet.clone()); + return Ok(Some(signed_packet)); + }; - return Ok(Some(signed_packet)); - } - result => last_result = result, - } - } - - last_result + Ok(None) } } @@ -127,4 +121,31 @@ mod tests { futures::executor::block_on(test()); } + + #[test] + fn not_found() { + async fn test() { + let keypair = Keypair::random(); + + let mut server = mockito::Server::new(); + + let path = format!("/{}", keypair.public_key()); + + server.mock("GET", path.as_str()).with_status(404).create(); + + let relays: Vec = vec![server.url()]; + let settings = RelaySettings { + relays, + ..RelaySettings::default() + }; + + let client = PkarrRelayClient::new(settings.clone()).unwrap().as_async(); + + let resolved = client.resolve(&keypair.public_key()).await.unwrap(); + + assert!(resolved.is_none()); + } + + futures::executor::block_on(test()); + } } diff --git a/pkarr/src/signed_packet.rs b/pkarr/src/signed_packet.rs index 7c9e3ef..a95a726 100644 --- a/pkarr/src/signed_packet.rs +++ b/pkarr/src/signed_packet.rs @@ -12,9 +12,11 @@ use std::{ char, fmt::{self, Display, Formatter}, net::{Ipv4Addr, Ipv6Addr}, - time::SystemTime, }; +#[cfg(not(target_arch = "wasm32"))] +use std::time::SystemTime; + const DOT: char = '.'; self_cell!( From 465e25acf1a257dc18ed4ad618cd1a7d09516e41 Mon Sep 17 00:00:00 2001 From: nazeh Date: Sat, 27 Jul 2024 12:33:39 +0300 Subject: [PATCH 2/2] fix(pkarr): wasm relay client resolve not_found correctly --- pkarr/src/relay_client.rs | 1 - pkarr/src/relay_client_web.rs | 32 ++++++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/pkarr/src/relay_client.rs b/pkarr/src/relay_client.rs index c52ebe3..e49193a 100644 --- a/pkarr/src/relay_client.rs +++ b/pkarr/src/relay_client.rs @@ -241,7 +241,6 @@ impl PkarrRelayClient { } } Err(ureq::Error::Status(404, _)) => { - dbg!(404); debug!(?url, "SignedPacket not found"); let _ = sender.send(Ok(None)); } diff --git a/pkarr/src/relay_client_web.rs b/pkarr/src/relay_client_web.rs index 54de1b7..ba1aab3 100644 --- a/pkarr/src/relay_client_web.rs +++ b/pkarr/src/relay_client_web.rs @@ -57,6 +57,7 @@ impl PkarrRelayClient { /// # Errors /// /// - Returns [Error::WasmRelayError] For Error responses + /// (except 404, these get converted to Ok(None). /// - Returns [Error::JsError] If an error happened on JS side. pub async fn resolve(&self, public_key: &PublicKey) -> Result> { let futures = self.relays.iter().map(|relay| { @@ -85,7 +86,13 @@ impl PkarrRelayClient { match select_ok(futures).await { Ok((response, _)) => Ok(response), - Err(e) => Err(e), + Err(e) => { + if let Error::WasmRelayError(404, _) = e { + return Ok(None); + } + + Err(e) + } } } } @@ -176,6 +183,8 @@ mod tests { }; } + const TEST_RELAY: &str = "http://localhost:6881"; + #[wasm_bindgen_test] async fn basic() { let keypair = Keypair::random(); @@ -190,11 +199,9 @@ mod tests { let signed_packet = SignedPacket::from_packet(&keypair, &packet).unwrap(); - let client = PkarrRelayClient::new(vec![ - "http://fail.non".to_string(), - "https://relay.pkarr.org".to_string(), - ]) - .unwrap(); + let client = + PkarrRelayClient::new(vec!["http://fail.non".to_string(), TEST_RELAY.to_string()]) + .unwrap(); client.publish(&signed_packet).await.unwrap(); @@ -208,4 +215,17 @@ mod tests { assert_eq!(resolved.as_bytes(), signed_packet.as_bytes()); } + + #[wasm_bindgen_test] + async fn not_found() { + let keypair = Keypair::random(); + + let client = PkarrRelayClient::new(vec![TEST_RELAY.to_string()]).unwrap(); + + let resolved = client.resolve(&keypair.public_key()).await.unwrap(); + + log!("{:?}", resolved); + + assert!(resolved.is_none()); + } }