From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon, 10 Feb 2020 22:18:48 +0000 (+0000) Subject: Merge pull request #490 from jkczyz/2020-02-initial-routing-sync X-Git-Tag: v0.0.12~140 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=b7f4f764afbe7105d9fa5f70e2c30c71ad90caa2;hp=88b7dcd7e4b10a53a55be9fc73c81228e377fce5;p=rust-lightning Merge pull request #490 from jkczyz/2020-02-initial-routing-sync Refactor logic for setting initial_routing_sync feature bit --- diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 483d69e7..519c5654 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -604,6 +604,8 @@ pub trait RoutingMessageHandler : Send + Sync { /// starting at the node *after* the provided publickey and including batch_amount entries. /// If None is provided for starting_point, we start at the first node. fn get_next_node_announcements(&self, starting_point: Option<&PublicKey>, batch_amount: u8) -> Vec; + /// Returns whether a full sync should be requested from a peer. + fn should_request_full_sync(&self, node_id: &PublicKey) -> bool; } pub(crate) struct OnionRealm0HopData { diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 327664c7..6576da80 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -189,7 +189,6 @@ pub struct PeerManager where CM::Target peer_counter_low: AtomicUsize, peer_counter_high: AtomicUsize, - initial_syncs_sent: AtomicUsize, logger: Arc, } @@ -212,9 +211,6 @@ macro_rules! encode_msg { }} } -//TODO: Really should do something smarter for this -const INITIAL_SYNCS_TO_SEND: usize = 5; - /// Manages and reacts to connection events. You probably want to use file descriptors as PeerIds. /// PeerIds may repeat, but only after disconnect_event() has been called. impl PeerManager where CM::Target: msgs::ChannelMessageHandler { @@ -236,7 +232,6 @@ impl PeerManager where ephemeral_key_midstate, peer_counter_low: AtomicUsize::new(0), peer_counter_high: AtomicUsize::new(0), - initial_syncs_sent: AtomicUsize::new(0), logger, } } @@ -549,8 +544,7 @@ impl PeerManager where peer.their_node_id = Some(their_node_id); insert_node_id!(); let mut features = InitFeatures::supported(); - if self.initial_syncs_sent.load(Ordering::Acquire) < INITIAL_SYNCS_TO_SEND { - self.initial_syncs_sent.fetch_add(1, Ordering::AcqRel); + if self.message_handler.route_handler.should_request_full_sync(&peer.their_node_id.unwrap()) { features.set_initial_routing_sync(); } @@ -648,8 +642,7 @@ impl PeerManager where if !peer.outbound { let mut features = InitFeatures::supported(); - if self.initial_syncs_sent.load(Ordering::Acquire) < INITIAL_SYNCS_TO_SEND { - self.initial_syncs_sent.fetch_add(1, Ordering::AcqRel); + if self.message_handler.route_handler.should_request_full_sync(&peer.their_node_id.unwrap()) { features.set_initial_routing_sync(); } diff --git a/lightning/src/ln/router.rs b/lightning/src/ln/router.rs index 00256560..36e68462 100644 --- a/lightning/src/ln/router.rs +++ b/lightning/src/ln/router.rs @@ -22,6 +22,7 @@ use util::logger::Logger; use std::cmp; use std::sync::{RwLock,Arc}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::collections::{HashMap,BinaryHeap,BTreeMap}; use std::collections::btree_map::Entry as BtreeEntry; use std; @@ -347,6 +348,7 @@ pub struct RouteHint { pub struct Router { secp_ctx: Secp256k1, network_map: RwLock, + full_syncs_requested: AtomicUsize, chain_monitor: Arc, logger: Arc, } @@ -390,6 +392,7 @@ impl ReadableArgs for Router { Ok(Router { secp_ctx: Secp256k1::verification_only(), network_map: RwLock::new(network_map), + full_syncs_requested: AtomicUsize::new(0), chain_monitor: args.chain_monitor, logger: args.logger, }) @@ -406,6 +409,7 @@ macro_rules! secp_verify_sig { } impl RoutingMessageHandler for Router { + fn handle_node_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result { let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]); secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id); @@ -698,6 +702,17 @@ impl RoutingMessageHandler for Router { } result } + + fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool { + //TODO: Determine whether to request a full sync based on the network map. + const FULL_SYNCS_TO_REQUEST: usize = 5; + if self.full_syncs_requested.load(Ordering::Acquire) < FULL_SYNCS_TO_REQUEST { + self.full_syncs_requested.fetch_add(1, Ordering::AcqRel); + true + } else { + false + } + } } #[derive(Eq, PartialEq)] @@ -750,6 +765,7 @@ impl Router { our_node_id: our_pubkey, nodes: nodes, }), + full_syncs_requested: AtomicUsize::new(0), chain_monitor, logger, } @@ -1035,7 +1051,7 @@ mod tests { use ln::channelmanager; use ln::router::{Router,NodeInfo,NetworkMap,ChannelInfo,DirectionalChannelInfo,RouteHint}; use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures}; - use ln::msgs::{LightningError, ErrorAction}; + use ln::msgs::{ErrorAction, LightningError, RoutingMessageHandler}; use util::test_utils; use util::test_utils::TestVecWriter; use util::logger::Logger; @@ -1048,17 +1064,23 @@ mod tests { use hex; use secp256k1::key::{PublicKey,SecretKey}; + use secp256k1::All; use secp256k1::Secp256k1; use std::sync::Arc; - #[test] - fn route_test() { + fn create_router() -> (Secp256k1, PublicKey, Router) { let secp_ctx = Secp256k1::new(); let our_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap()); let logger: Arc = Arc::new(test_utils::TestLogger::new()); let chain_monitor = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&logger))); let router = Router::new(our_id, chain_monitor, Arc::clone(&logger)); + (secp_ctx, our_id, router) + } + + #[test] + fn route_test() { + let (secp_ctx, our_id, router) = create_router(); // Build network from our_id to node8: // @@ -1823,4 +1845,17 @@ mod tests { assert!(::read(&mut ::std::io::Cursor::new(&w.0)).unwrap() == *network); } } + + #[test] + fn request_full_sync_finite_times() { + let (secp_ctx, _, router) = create_router(); + let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode("0202020202020202020202020202020202020202020202020202020202020202").unwrap()[..]).unwrap()); + + assert!(router.should_request_full_sync(&node_id)); + assert!(router.should_request_full_sync(&node_id)); + assert!(router.should_request_full_sync(&node_id)); + assert!(router.should_request_full_sync(&node_id)); + assert!(router.should_request_full_sync(&node_id)); + assert!(!router.should_request_full_sync(&node_id)); + } } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index aa68fdb1..cd2064a4 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -155,6 +155,9 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler { fn get_next_node_announcements(&self, _starting_point: Option<&PublicKey>, _batch_amount: u8) -> Vec { Vec::new() } + fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool { + true + } } pub struct TestLogger {