-
Notifications
You must be signed in to change notification settings - Fork 251
UPnP: Periodic External IP Refresh & Outbound Peer Reconnect on Change #723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,10 @@ use thiserror::Error; | |
|
|
||
| pub use stores::NetAddress; | ||
|
|
||
| pub trait ExternalIpChangeSink: Send + Sync { | ||
| fn on_external_ip_changed(&self, new_ip: std::net::IpAddr, old_ip: Option<std::net::IpAddr>); | ||
| } | ||
|
|
||
| const MAX_ADDRESSES: usize = 4096; | ||
| const MAX_CONNECTION_FAILED_COUNT: u64 = 3; | ||
|
|
||
|
|
@@ -56,35 +60,47 @@ pub struct AddressManager { | |
| address_store: address_store_with_cache::Store, | ||
| config: Arc<Config>, | ||
| local_net_addresses: Vec<NetAddress>, | ||
| external_ip_change_sinks: Vec<Arc<dyn ExternalIpChangeSink>>, | ||
| } | ||
|
|
||
| impl AddressManager { | ||
| pub fn new(config: Arc<Config>, db: Arc<DB>, tick_service: Arc<TickService>) -> (Arc<Mutex<Self>>, Option<Extender>) { | ||
| let mut instance = Self { | ||
| let instance = Self { | ||
| banned_address_store: DbBannedAddressesStore::new(db.clone(), CachePolicy::Count(MAX_ADDRESSES)), | ||
| address_store: address_store_with_cache::new(db), | ||
| local_net_addresses: Vec::new(), | ||
| external_ip_change_sinks: Vec::new(), | ||
| config, | ||
| }; | ||
|
|
||
| let extender = instance.init_local_addresses(tick_service); | ||
| let am = Arc::new(Mutex::new(instance)); | ||
| let extender = Self::init_local_addresses(&am, tick_service); | ||
|
|
||
| (Arc::new(Mutex::new(instance)), extender) | ||
| (am, extender) | ||
| } | ||
|
|
||
| fn init_local_addresses(&mut self, tick_service: Arc<TickService>) -> Option<Extender> { | ||
| self.local_net_addresses = self.local_addresses().collect(); | ||
| pub fn register_external_ip_change_sink(&mut self, sink: Arc<dyn ExternalIpChangeSink>) { | ||
| self.external_ip_change_sinks.push(sink); | ||
| } | ||
|
|
||
| let extender = if self.local_net_addresses.is_empty() && !self.config.disable_upnp { | ||
| let (net_address, ExtendHelper { gateway, local_addr, external_port }) = match self.upnp() { | ||
| pub fn clone_external_ip_change_sinks(&self) -> Vec<Arc<dyn ExternalIpChangeSink>> { | ||
| self.external_ip_change_sinks.clone() | ||
| } | ||
|
|
||
| fn init_local_addresses(this: &Arc<Mutex<Self>>, tick_service: Arc<TickService>) -> Option<Extender> { | ||
| let mut me = this.lock(); | ||
| me.local_net_addresses = me.local_addresses().collect(); | ||
|
|
||
| let extender = if me.local_net_addresses.is_empty() && !me.config.disable_upnp { | ||
| let (net_address, ExtendHelper { gateway, local_addr, external_port }) = match me.upnp() { | ||
| Err(err) => { | ||
| warn!("[UPnP] Error adding port mapping: {err}"); | ||
| return None; | ||
| } | ||
| Ok(None) => return None, | ||
| Ok(Some((net_address, extend_helper))) => (net_address, extend_helper), | ||
| }; | ||
| self.local_net_addresses.push(net_address); | ||
| me.local_net_addresses.push(net_address); | ||
|
|
||
| let gateway: igd_next::aio::Gateway<Tokio> = igd_next::aio::Gateway { | ||
| addr: gateway.addr, | ||
|
|
@@ -101,12 +117,14 @@ impl AddressManager { | |
| gateway, | ||
| external_port, | ||
| local_addr, | ||
| Arc::clone(this), | ||
| Some(net_address.ip.into()), | ||
| )) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| self.local_net_addresses.iter().for_each(|net_addr| { | ||
| me.local_net_addresses.iter().for_each(|net_addr| { | ||
| info!("Publicly routable local address {} added to store", net_addr); | ||
| }); | ||
| extender | ||
|
|
@@ -142,7 +160,19 @@ impl AddressManager { | |
| return Left(Right(iter::empty())); | ||
| }; | ||
| // TODO: Add Check IPv4 or IPv6 match from Go code | ||
| Right(network_interfaces.into_iter().map(|(_, ip)| IpAddress::from(ip)).filter(|&ip| ip.is_publicly_routable()).map( | ||
| Right(network_interfaces | ||
| .into_iter() | ||
| .map(|(_, ip)| IpAddress::from(ip)) | ||
| .filter(|ip| { | ||
| if self.config.disable_ipv6_interface_discovery { | ||
| // Skip IPv6 during automatic discovery if the flag is set | ||
| !matches!(**ip, std::net::IpAddr::V6(_)) | ||
| } else { | ||
| true | ||
| } | ||
| }) | ||
| .filter(|&ip| ip.is_publicly_routable()) | ||
| .map( | ||
| |ip| { | ||
| info!("Publicly routable local address found: {}", ip); | ||
| NetAddress::new(ip, self.config.default_p2p_port()) | ||
|
|
@@ -251,6 +281,14 @@ impl AddressManager { | |
| } | ||
| } | ||
|
|
||
| pub fn set_best_local_address(&mut self, address: NetAddress) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it's better to handle the |
||
| if self.local_net_addresses.is_empty() { | ||
| self.local_net_addresses.push(address); | ||
| } else { | ||
| self.local_net_addresses[0] = address; | ||
| } | ||
| } | ||
|
|
||
| pub fn add_address(&mut self, address: NetAddress) { | ||
| if address.ip.is_loopback() || address.ip.is_unspecified() { | ||
| debug!("[Address manager] skipping local address {}", address.ip); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ use std::{ | |
| use duration_string::DurationString; | ||
| use futures_util::future::{join_all, try_join_all}; | ||
| use itertools::Itertools; | ||
| use kaspa_addressmanager::{AddressManager, NetAddress}; | ||
| use kaspa_addressmanager::{AddressManager, NetAddress, ExternalIpChangeSink}; | ||
| use kaspa_core::{debug, info, warn}; | ||
| use kaspa_p2p_lib::{common::ProtocolError, ConnectionError, Peer}; | ||
| use kaspa_utils::triggers::SingleTrigger; | ||
|
|
@@ -93,6 +93,46 @@ impl ConnectionManager { | |
| }); | ||
| } | ||
|
|
||
| /// Synchronously trigger a staggered outbound reconnect (terminates peers one by one with 30s delays) | ||
| pub fn trigger_outbound_reconnect(&self) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? When your external IP changes, wouldn't a disconnection already have happened (you disconnect from all your peers) and when you try to reconnect to your peers they would then know your new IP? |
||
| let outbound_peers: Vec<_> = self.p2p_adaptor.active_peers() | ||
| .into_iter() | ||
| .filter(|p| p.is_outbound()) | ||
| .collect(); | ||
|
|
||
| if outbound_peers.is_empty() { | ||
| info!("No outbound peers to reconnect"); | ||
| return; | ||
| } | ||
|
|
||
| let peer_count = outbound_peers.len(); | ||
| info!("Starting staggered outbound reconnect: {} peers will be renewed with 30s delays", peer_count); | ||
|
|
||
| // Spawn async task for staggered renewal | ||
| let p2p_adaptor = self.p2p_adaptor.clone(); | ||
| let force_sender = self.force_next_iteration.clone(); | ||
|
|
||
| tokio::spawn(async move { | ||
| for (i, peer) in outbound_peers.into_iter().enumerate() { | ||
| // Terminate peer | ||
| p2p_adaptor.terminate(peer.key()).await; | ||
| info!("Terminated outbound peer {} ({}/{})", peer.net_address(), i+1, peer_count); | ||
|
|
||
| // Trigger reconnection (except for the last peer) | ||
| if i < peer_count - 1 { | ||
| force_sender.send(()).unwrap(); | ||
|
|
||
| // Wait 30 seconds | ||
| tokio::time::sleep(Duration::from_secs(30)).await; | ||
| } | ||
| } | ||
|
|
||
| // Final trigger for the last renewal | ||
| force_sender.send(()).unwrap(); | ||
| info!("Staggered outbound reconnect completed"); | ||
| }); | ||
| } | ||
|
|
||
| async fn handle_event(self: Arc<Self>) { | ||
| debug!("Starting connection loop iteration"); | ||
| let peers = self.p2p_adaptor.active_peers(); | ||
|
|
@@ -338,3 +378,9 @@ impl ConnectionManager { | |
| self.connection_requests.lock().await.iter().any(|(address, request)| request.is_permanent && address.ip() == ip) | ||
| } | ||
| } | ||
|
|
||
| impl ExternalIpChangeSink for ConnectionManager { | ||
| fn on_external_ip_changed(&self, _new_ip: std::net::IpAddr, _old_ip: Option<std::net::IpAddr>) { | ||
| self.trigger_outbound_reconnect(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -611,7 +611,7 @@ do you confirm? (answer y/n or pass --yes to the Kaspad command line to confirm | |
| notify_service.notifier(), | ||
| index_service.as_ref().map(|x| x.notifier()), | ||
| mining_manager, | ||
| flow_context, | ||
| flow_context.clone(), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this clone necessary? |
||
| subscription_context, | ||
| index_service.as_ref().map(|x| x.utxoindex().unwrap()), | ||
| config.clone(), | ||
|
|
@@ -646,13 +646,14 @@ do you confirm? (answer y/n or pass --yes to the Kaspad command line to confirm | |
| if let Some(index_service) = index_service { | ||
| async_runtime.register(index_service) | ||
| }; | ||
|
|
||
| if let Some(port_mapping_extender_svc) = port_mapping_extender_svc { | ||
| async_runtime.register(Arc::new(port_mapping_extender_svc)) | ||
| }; | ||
| async_runtime.register(rpc_core_service.clone()); | ||
| if let Some(grpc_service) = grpc_service { | ||
| async_runtime.register(grpc_service) | ||
| } | ||
| }; | ||
| async_runtime.register(p2p_service); | ||
| async_runtime.register(consensus_monitor); | ||
| async_runtime.register(mining_monitor); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this called
Sink?