diff --git a/examples/rendezvous/src/bin/rzv-identify.rs b/examples/rendezvous/src/bin/rzv-identify.rs index 406f15bdcdb..7c326688231 100644 --- a/examples/rendezvous/src/bin/rzv-identify.rs +++ b/examples/rendezvous/src/bin/rzv-identify.rs @@ -78,11 +78,14 @@ async fn main() { SwarmEvent::Behaviour(MyBehaviourEvent::Identify(identify::Event::Received { .. })) => { - swarm.behaviour_mut().rendezvous.register( + if let Err(error) = swarm.behaviour_mut().rendezvous.register( rendezvous::Namespace::from_static("rendezvous"), rendezvous_point, None, - ); + ) { + log::error!("Failed to register: {error}"); + return; + } } SwarmEvent::Behaviour(MyBehaviourEvent::Rendezvous( rendezvous::client::Event::Registered { @@ -99,9 +102,18 @@ async fn main() { ); } SwarmEvent::Behaviour(MyBehaviourEvent::Rendezvous( - rendezvous::client::Event::RegisterFailed(error), + rendezvous::client::Event::RegisterFailed { + rendezvous_node, + namespace, + error, + }, )) => { - log::error!("Failed to register {}", error); + log::error!( + "Failed to register: rendezvous_node={}, namespace={}, error_code={:?}", + rendezvous_node, + namespace, + error + ); return; } SwarmEvent::Behaviour(MyBehaviourEvent::Ping(ping::Event { diff --git a/examples/rendezvous/src/bin/rzv-register.rs b/examples/rendezvous/src/bin/rzv-register.rs index 4cfefe95070..f9fd12b1493 100644 --- a/examples/rendezvous/src/bin/rzv-register.rs +++ b/examples/rendezvous/src/bin/rzv-register.rs @@ -74,11 +74,14 @@ async fn main() { log::error!("Lost connection to rendezvous point {}", error); } SwarmEvent::ConnectionEstablished { peer_id, .. } if peer_id == rendezvous_point => { - swarm.behaviour_mut().rendezvous.register( + if let Err(error) = swarm.behaviour_mut().rendezvous.register( rendezvous::Namespace::from_static("rendezvous"), rendezvous_point, None, - ); + ) { + log::error!("Failed to register: {error}"); + return; + } log::info!("Connection established with rendezvous point {}", peer_id); } // once `/identify` did its job, we know our external address and can register @@ -97,9 +100,18 @@ async fn main() { ); } SwarmEvent::Behaviour(MyBehaviourEvent::Rendezvous( - rendezvous::client::Event::RegisterFailed(error), + rendezvous::client::Event::RegisterFailed { + rendezvous_node, + namespace, + error, + }, )) => { - log::error!("Failed to register {}", error); + log::error!( + "Failed to register: rendezvous_node={}, namespace={}, error_code={:?}", + rendezvous_node, + namespace, + error + ); return; } SwarmEvent::Behaviour(MyBehaviourEvent::Ping(ping::Event { diff --git a/protocols/rendezvous/CHANGELOG.md b/protocols/rendezvous/CHANGELOG.md index 8f2bd968e94..1762f4c62e4 100644 --- a/protocols/rendezvous/CHANGELOG.md +++ b/protocols/rendezvous/CHANGELOG.md @@ -1,8 +1,15 @@ ## 0.13.0 - unreleased +- Changed the signature of the function `client::Behavior::register()`, + it returns `Result<(), RegisterError>` now. + Remove the `Remote` variant from `RegisterError` and instead put the information from `Remote` + directly into the variant from the `Event` enum. + See [PR 4073]. + - Raise MSRV to 1.65. See [PR 3715]. +[PR 4073]: https://github.com/libp2p/rust-libp2p/pull/4073 [PR 3715]: https://github.com/libp2p/rust-libp2p/pull/3715 ## 0.12.1 @@ -25,7 +32,7 @@ - Update to `libp2p-swarm` `v0.41.0`. -- Replace `Client` and `Server`'s `NetworkBehaviour` implemention `inject_*` methods with the new `on_*` methods. +- Replace `Client` and `Server`'s `NetworkBehaviour` implementation `inject_*` methods with the new `on_*` methods. See [PR 3011]. - Update `rust-version` to reflect the actual MSRV: 1.62.0. See [PR 3090]. diff --git a/protocols/rendezvous/src/client.rs b/protocols/rendezvous/src/client.rs index 363e31965eb..505635efda8 100644 --- a/protocols/rendezvous/src/client.rs +++ b/protocols/rendezvous/src/client.rs @@ -31,7 +31,7 @@ use libp2p_swarm::{ ConnectionDenied, ConnectionId, ExternalAddresses, FromSwarm, NetworkBehaviour, PollParameters, THandler, THandlerInEvent, THandlerOutEvent, ToSwarm, }; -use std::collections::{HashMap, VecDeque}; +use std::collections::HashMap; use std::iter; use std::task::{Context, Poll}; use std::time::Duration; @@ -41,8 +41,6 @@ pub struct Behaviour { keypair: Keypair, - error_events: VecDeque, - waiting_for_register: HashMap, waiting_for_discovery: HashMap)>, @@ -66,7 +64,6 @@ impl Behaviour { iter::once((crate::PROTOCOL_IDENT, ProtocolSupport::Outbound)), libp2p_request_response::Config::default(), ), - error_events: Default::default(), keypair, waiting_for_register: Default::default(), waiting_for_discovery: Default::default(), @@ -82,30 +79,26 @@ impl Behaviour { /// /// External addresses are either manually added via [`libp2p_swarm::Swarm::add_external_address`] or reported /// by other [`NetworkBehaviour`]s via [`ToSwarm::ExternalAddrConfirmed`]. - pub fn register(&mut self, namespace: Namespace, rendezvous_node: PeerId, ttl: Option) { + pub fn register( + &mut self, + namespace: Namespace, + rendezvous_node: PeerId, + ttl: Option, + ) -> Result<(), RegisterError> { let external_addresses = self.external_addresses.iter().cloned().collect::>(); if external_addresses.is_empty() { - self.error_events - .push_back(Event::RegisterFailed(RegisterError::NoExternalAddresses)); - - return; + return Err(RegisterError::NoExternalAddresses); } - match PeerRecord::new(&self.keypair, external_addresses) { - Ok(peer_record) => { - let req_id = self.inner.send_request( - &rendezvous_node, - Register(NewRegistration::new(namespace.clone(), peer_record, ttl)), - ); - self.waiting_for_register - .insert(req_id, (rendezvous_node, namespace)); - } - Err(signing_error) => { - self.error_events.push_back(Event::RegisterFailed( - RegisterError::FailedToMakeRecord(signing_error), - )); - } - }; + let peer_record = PeerRecord::new(&self.keypair, external_addresses)?; + let req_id = self.inner.send_request( + &rendezvous_node, + Register(NewRegistration::new(namespace.clone(), peer_record, ttl)), + ); + self.waiting_for_register + .insert(req_id, (rendezvous_node, namespace)); + + Ok(()) } /// Unregister ourselves from the given namespace with the given rendezvous peer. @@ -148,12 +141,6 @@ pub enum RegisterError { NoExternalAddresses, #[error("Failed to make a new PeerRecord")] FailedToMakeRecord(#[from] SigningError), - #[error("Failed to register with Rendezvous node")] - Remote { - rendezvous_node: PeerId, - namespace: Namespace, - error: ErrorCode, - }, } #[derive(Debug)] @@ -178,7 +165,11 @@ pub enum Event { namespace: Namespace, }, /// We failed to register with the contained rendezvous node. - RegisterFailed(RegisterError), + RegisterFailed { + rendezvous_node: PeerId, + namespace: Namespace, + error: ErrorCode, + }, /// The connection details we learned from this node expired. Expired { peer: PeerId }, } @@ -239,10 +230,6 @@ impl NetworkBehaviour for Behaviour { ) -> Poll>> { use libp2p_request_response as req_res; - if let Some(event) = self.error_events.pop_front() { - return Poll::Ready(ToSwarm::GenerateEvent(event)); - } - loop { match self.inner.poll(cx, params) { Poll::Ready(ToSwarm::GenerateEvent(req_res::Event::Message { @@ -337,11 +324,11 @@ impl NetworkBehaviour for Behaviour { impl Behaviour { fn event_for_outbound_failure(&mut self, req_id: &RequestId) -> Option { if let Some((rendezvous_node, namespace)) = self.waiting_for_register.remove(req_id) { - return Some(Event::RegisterFailed(RegisterError::Remote { + return Some(Event::RegisterFailed { rendezvous_node, namespace, error: ErrorCode::Unavailable, - })); + }); }; if let Some((rendezvous_node, namespace)) = self.waiting_for_discovery.remove(req_id) { @@ -374,11 +361,11 @@ impl Behaviour { if let Some((rendezvous_node, namespace)) = self.waiting_for_register.remove(request_id) { - return Some(Event::RegisterFailed(RegisterError::Remote { + return Some(Event::RegisterFailed { rendezvous_node, namespace, error: error_code, - })); + }); } None diff --git a/protocols/rendezvous/tests/rendezvous.rs b/protocols/rendezvous/tests/rendezvous.rs index 494f56551d8..992876d1971 100644 --- a/protocols/rendezvous/tests/rendezvous.rs +++ b/protocols/rendezvous/tests/rendezvous.rs @@ -22,6 +22,7 @@ use futures::stream::FuturesUnordered; use futures::StreamExt; use libp2p_identity as identity; use libp2p_rendezvous as rendezvous; +use libp2p_rendezvous::client::RegisterError; use libp2p_swarm::{DialError, Swarm, SwarmEvent}; use libp2p_swarm_test::SwarmExt; use std::convert::TryInto; @@ -36,7 +37,8 @@ async fn given_successful_registration_then_successful_discovery() { alice .behaviour_mut() - .register(namespace.clone(), *robert.local_peer_id(), None); + .register(namespace.clone(), *robert.local_peer_id(), None) + .unwrap(); match libp2p_swarm_test::drive(&mut alice, &mut robert).await { ( @@ -79,6 +81,21 @@ async fn given_successful_registration_then_successful_discovery() { } } +#[tokio::test] +async fn should_return_error_when_no_external_addresses() { + let _ = env_logger::try_init(); + let namespace = rendezvous::Namespace::from_static("some-namespace"); + let server = new_server(rendezvous::server::Config::default()).await; + let mut client = Swarm::new_ephemeral(rendezvous::client::Behaviour::new); + + let actual = client + .behaviour_mut() + .register(namespace.clone(), *server.local_peer_id(), None) + .unwrap_err(); + + assert!(matches!(actual, RegisterError::NoExternalAddresses)) +} + #[tokio::test] async fn given_successful_registration_then_refresh_ttl() { let _ = env_logger::try_init(); @@ -91,7 +108,8 @@ async fn given_successful_registration_then_refresh_ttl() { alice .behaviour_mut() - .register(namespace.clone(), roberts_peer_id, None); + .register(namespace.clone(), roberts_peer_id, None) + .unwrap(); match libp2p_swarm_test::drive(&mut alice, &mut robert).await { ( @@ -114,7 +132,8 @@ async fn given_successful_registration_then_refresh_ttl() { alice .behaviour_mut() - .register(namespace.clone(), roberts_peer_id, Some(refresh_ttl)); + .register(namespace.clone(), roberts_peer_id, Some(refresh_ttl)) + .unwrap(); match libp2p_swarm_test::drive(&mut alice, &mut robert).await { ( @@ -150,18 +169,18 @@ async fn given_invalid_ttl_then_unsuccessful_registration() { let ([mut alice], mut robert) = new_server_with_connected_clients(rendezvous::server::Config::default()).await; - alice.behaviour_mut().register( - namespace.clone(), - *robert.local_peer_id(), - Some(100_000_000), - ); + alice + .behaviour_mut() + .register( + namespace.clone(), + *robert.local_peer_id(), + Some(100_000_000), + ) + .unwrap(); match libp2p_swarm_test::drive(&mut alice, &mut robert).await { ( - [rendezvous::client::Event::RegisterFailed(rendezvous::client::RegisterError::Remote { - error, - .. - })], + [rendezvous::client::Event::RegisterFailed { error, .. }], [rendezvous::server::Event::PeerNotRegistered { .. }], ) => { assert_eq!(error, rendezvous::ErrorCode::InvalidTtl); @@ -182,7 +201,8 @@ async fn discover_allows_for_dial_by_peer_id() { alice .behaviour_mut() - .register(namespace.clone(), roberts_peer_id, None); + .register(namespace.clone(), roberts_peer_id, None) + .unwrap(); match alice.next_behaviour_event().await { rendezvous::client::Event::Registered { .. } => {} event => panic!("Unexpected event: {event:?}"), @@ -233,14 +253,14 @@ async fn eve_cannot_register() { eve.connect(&mut robert).await; eve.behaviour_mut() - .register(namespace.clone(), *robert.local_peer_id(), None); + .register(namespace.clone(), *robert.local_peer_id(), None) + .unwrap(); match libp2p_swarm_test::drive(&mut eve, &mut robert).await { ( - [rendezvous::client::Event::RegisterFailed(rendezvous::client::RegisterError::Remote { - error: err_code, - .. - })], + [rendezvous::client::Event::RegisterFailed { + error: err_code, .. + }], [rendezvous::server::Event::PeerNotRegistered { .. }], ) => { assert_eq!(err_code, rendezvous::ErrorCode::NotAuthorized); @@ -263,7 +283,8 @@ async fn can_combine_client_and_server() { charlie .behaviour_mut() .client - .register(namespace.clone(), *robert.local_peer_id(), None); + .register(namespace.clone(), *robert.local_peer_id(), None) + .unwrap(); match libp2p_swarm_test::drive(&mut charlie, &mut robert).await { ( [CombinedEvent::Client(rendezvous::client::Event::Registered { .. })], @@ -274,7 +295,8 @@ async fn can_combine_client_and_server() { alice .behaviour_mut() - .register(namespace, *charlie.local_peer_id(), None); + .register(namespace, *charlie.local_peer_id(), None) + .unwrap(); match libp2p_swarm_test::drive(&mut charlie, &mut alice).await { ( [CombinedEvent::Server(rendezvous::server::Event::PeerRegistered { .. })], @@ -299,7 +321,8 @@ async fn registration_on_clients_expire() { alice .behaviour_mut() - .register(namespace.clone(), roberts_peer_id, Some(registration_ttl)); + .register(namespace.clone(), roberts_peer_id, Some(registration_ttl)) + .unwrap(); match alice.next_behaviour_event().await { rendezvous::client::Event::Registered { .. } => {} event => panic!("Unexpected event: {event:?}"),