From b0645bb96eb712c947b8b9863eba00a66c445291 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 1 Jul 2021 19:14:09 +1000 Subject: [PATCH] WIP - register/re-register logic fixes The logic should be fixed (i.e. initial registration vs. refresh and knowing the connection state to not constantly trigger new connections), but we don't receive the registration on the rendezvous node. --- swap/src/asb/event_loop.rs | 4 +- swap/src/asb/rendezvous.rs | 102 ++++++++++++++++++++++--------------- 2 files changed, 63 insertions(+), 43 deletions(-) diff --git a/swap/src/asb/event_loop.rs b/swap/src/asb/event_loop.rs index ffe639da..6264cd19 100644 --- a/swap/src/asb/event_loop.rs +++ b/swap/src/asb/event_loop.rs @@ -149,9 +149,7 @@ where loop { // rendezvous node re-registration if let Some(rendezvous_behaviour) = self.swarm.behaviour_mut().rendezvous.as_mut() { - if let Err(error) = rendezvous_behaviour.refresh_registration() { - tracing::error!("Failed to register with rendezvous point: {:#}", error); - } + rendezvous_behaviour.refresh(); } tokio::select! { diff --git a/swap/src/asb/rendezvous.rs b/swap/src/asb/rendezvous.rs index 015a1bd2..8705c542 100644 --- a/swap/src/asb/rendezvous.rs +++ b/swap/src/asb/rendezvous.rs @@ -1,5 +1,4 @@ use crate::rendezvous::XmrBtcNamespace; -use anyhow::Result; use libp2p::core::connection::ConnectionId; use libp2p::identity::Keypair; use libp2p::multiaddr::Protocol; @@ -12,6 +11,13 @@ use libp2p::{Multiaddr, PeerId}; use std::task::{Context, Poll}; use std::time::{Duration, Instant}; +#[derive(Debug)] +enum ConnectionState { + Dialed, + Connected, + Disconnected, +} + /// A `NetworkBehaviour` that handles registration of the xmr-btc swap service /// with a rendezvous point pub struct Behaviour { @@ -20,7 +26,8 @@ pub struct Behaviour { rendezvous_point_addr: Multiaddr, rendezvous_namespace: XmrBtcNamespace, rendezvous_reregister_timestamp: Option, - is_connected: bool, + rendezvous_node_connection: ConnectionState, + is_initial_registration: bool, events: Vec>, } @@ -40,46 +47,58 @@ impl Behaviour { rendezvous_point_addr: addr, rendezvous_namespace: namespace, rendezvous_reregister_timestamp: None, - is_connected: false, + rendezvous_node_connection: ConnectionState::Disconnected, + is_initial_registration: true, events: vec![], } } - pub fn refresh_registration(&mut self) -> Result<()> { - if self.is_connected { - if let Some(rendezvous_reregister_timestamp) = self.rendezvous_reregister_timestamp { - if Instant::now() > rendezvous_reregister_timestamp { - self.rendezvous_behaviour.register( - Namespace::new(self.rendezvous_namespace.to_string()) - .expect("our namespace to be a correct string"), - self.rendezvous_point_peer_id, - None, - ); + fn register(&mut self) { + self.rendezvous_behaviour.register( + Namespace::new(self.rendezvous_namespace.to_string()) + .expect("our namespace to be a correct string"), + self.rendezvous_point_peer_id, + None, + ); + } + + pub fn refresh(&mut self) { + match self.rendezvous_node_connection { + ConnectionState::Dialed => {} /* we are waiting for a connection to be established, + * no refresh */ + ConnectionState::Connected => { + if let Some(rendezvous_reregister_timestamp) = self.rendezvous_reregister_timestamp + { + if Instant::now() > rendezvous_reregister_timestamp + && !self.is_initial_registration + { + tracing::debug!("Sending re-registration to rendezvous node"); + self.register(); + } + } else if self.is_initial_registration { + tracing::debug!("Sending initial registration to rendezvous node"); + self.is_initial_registration = false; + self.register(); } } - } else { - // TODO: The refresh has to be tied to a refersh time that is tied to us having - // gotten a Ttl! We should not be able to refresh like this, - // because it may result in constant retry (with new connections) if there is an - // error on the rendezvous side and the connection is closed. - // In such an error scenario the initial dial to the rendezvous node should - // just fail and then we get an error on the ASB and don't try again. (pointless - // to try again anyway if the rendezvous server is broken / not available) - - let p2p_suffix = Protocol::P2p(self.rendezvous_point_peer_id.into()); - let address_with_p2p = if !self - .rendezvous_point_addr - .ends_with(&Multiaddr::empty().with(p2p_suffix.clone())) - { - self.rendezvous_point_addr.clone().with(p2p_suffix) - } else { - self.rendezvous_point_addr.clone() - }; - self.events.push(NetworkBehaviourAction::DialAddress { - address: address_with_p2p, - }) + ConnectionState::Disconnected => { + let p2p_suffix = Protocol::P2p(self.rendezvous_point_peer_id.into()); + let address_with_p2p = if !self + .rendezvous_point_addr + .ends_with(&Multiaddr::empty().with(p2p_suffix.clone())) + { + self.rendezvous_point_addr.clone().with(p2p_suffix) + } else { + self.rendezvous_point_addr.clone() + }; + + self.events.push(NetworkBehaviourAction::DialAddress { + address: address_with_p2p, + }); + + self.rendezvous_node_connection = ConnectionState::Dialed; + } } - Ok(()) } } @@ -100,13 +119,14 @@ impl NetworkBehaviour for Behaviour { fn inject_connected(&mut self, peer_id: &PeerId) { if *peer_id == self.rendezvous_point_peer_id { - self.is_connected = true; + self.rendezvous_node_connection = ConnectionState::Connected; } } fn inject_disconnected(&mut self, peer_id: &PeerId) { if *peer_id == self.rendezvous_point_peer_id { - self.is_connected = false; + self.rendezvous_node_connection = ConnectionState::Disconnected; + self.is_initial_registration = false; } } @@ -134,16 +154,16 @@ impl NetworkBehaviourEventProcess for Behaviour { fn inject_event(&mut self, event: Event) { match event { Event::RegisterFailed(error) => { + self.is_initial_registration = false; tracing::error!(rendezvous_node=%self.rendezvous_point_peer_id, "Registration with rendezvous node failed: {:#}", error); } - Event::RegistrationExpired(registration) => { - tracing::warn!("Registation expired: {:?}", registration) - } Event::Registered { rendezvous_node, ttl, namespace, } => { + self.is_initial_registration = false; + // TODO: this can most likely not happen at all, potentially remove these checks if rendezvous_node != self.rendezvous_point_peer_id { tracing::error!(peer_id=%rendezvous_node, "Ignoring message from unknown rendezvous node"); @@ -157,6 +177,8 @@ impl NetworkBehaviourEventProcess for Behaviour { // record re-registration after half the ttl has expired self.rendezvous_reregister_timestamp = Some(Instant::now() + Duration::from_secs(ttl) / 2); + + tracing::info!("Registration with rendezvous node successfull") } _ => {} }