From e5c0c62c178a33957d9dac13c74a37bf97d618cd Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 20 Dec 2023 05:17:43 +0000 Subject: [PATCH 1/8] [bindings] Drop the lifetime bound on `Record` for bindings builds Because log `Record`s are now being passed by ownership to `log`, the bindings get quite annoyed that there's a lifetime hanging around. We already don't use this lifetime in bindings (the `FormatArgs` is converted to a string and stored on the heap), so we can just drop the lifetime, even though it requires some macro'ing of the struct definition to do so. --- lightning/src/util/logger.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lightning/src/util/logger.rs b/lightning/src/util/logger.rs index 92ea8ffed5..d1c768dc7f 100644 --- a/lightning/src/util/logger.rs +++ b/lightning/src/util/logger.rs @@ -91,10 +91,12 @@ impl Level { } } +macro_rules! impl_record { + ($($args: lifetime)?, $($nonstruct_args: lifetime)?) => { /// A Record, unit of logging output with Metadata to enable filtering /// Module_path, file, line to inform on log's source #[derive(Clone, Debug)] -pub struct Record<'a> { +pub struct Record<$($args)?> { /// The verbosity level of the message. pub level: Level, /// The node id of the peer pertaining to the logged record. @@ -118,22 +120,17 @@ pub struct Record<'a> { pub file: &'static str, /// The line containing the message. pub line: u32, - - #[cfg(c_bindings)] - /// We don't actually use the lifetime parameter in C bindings (as there is no good way to - /// communicate a lifetime to a C, or worse, Java user). - _phantom: core::marker::PhantomData<&'a ()>, } -impl<'a> Record<'a> { +impl<$($args)?> Record<$($args)?> { /// Returns a new Record. /// /// This is not exported to bindings users as fmt can't be used in C #[inline] - pub fn new( + pub fn new<$($nonstruct_args)?>( level: Level, peer_id: Option, channel_id: Option, args: fmt::Arguments<'a>, module_path: &'static str, file: &'static str, line: u32 - ) -> Record<'a> { + ) -> Record<$($args)?> { Record { level, peer_id, @@ -145,11 +142,14 @@ impl<'a> Record<'a> { module_path, file, line, - #[cfg(c_bindings)] - _phantom: core::marker::PhantomData, } } } +} } +#[cfg(not(c_bindings))] +impl_record!('a, ); +#[cfg(c_bindings)] +impl_record!(, 'a); /// A trait encapsulating the operations required of a logger. pub trait Logger { From 4764fa3985c436fe820732344ac1a746ca9e540f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 18 Jan 2024 00:42:55 +0000 Subject: [PATCH 2/8] [bindings] Mark `WithContext` log wrapper with no-export There's not a lot of reason for downstream users to use the `WithContext` wrapper, it mostly exists for our own downstream crates anyway, and dealing with lifetimes in bindings isn't super practical, so simply no-export it. --- lightning/src/util/logger.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lightning/src/util/logger.rs b/lightning/src/util/logger.rs index d1c768dc7f..e48cefaa04 100644 --- a/lightning/src/util/logger.rs +++ b/lightning/src/util/logger.rs @@ -158,6 +158,9 @@ pub trait Logger { } /// Adds relevant context to a [`Record`] before passing it to the wrapped [`Logger`]. +/// +/// This is not exported to bindings users as lifetimes are problematic and there's little reason +/// for this to be used downstream anyway. pub struct WithContext<'a, L: Deref> where L::Target: Logger { /// The logger to delegate to after adding context to the record. logger: &'a L, From a5ba3391f8dd61515385bfcf3d946a1c6b731f0a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 01:16:46 +0000 Subject: [PATCH 3/8] [bindings] No-export `RouteHopCandidate` lifetime'd fields The bindings cannot express lifetimes, so exposing any field which is a reference (and not `clone`-able, at least for garbage collected language bindings) is unsafe for those expecting a high-level interface. Thus, we simply no-export the `RouteHopCandidate` inner struct fields which are references (there are relevant accessors for them anyway). --- lightning/src/routing/router.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 1bf90883ec..7ca9d4b7d9 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1146,8 +1146,12 @@ pub struct FirstHopCandidate<'a> { /// has been funded and is able to pay), and accessor methods may panic otherwise. /// /// [`find_route`] validates this prior to constructing a [`CandidateRouteHop`]. + /// + /// This is not exported to bindings users as lifetimes are not expressable in most languages. pub details: &'a ChannelDetails, /// The node id of the payer, which is also the source side of this candidate route hop. + /// + /// This is not exported to bindings users as lifetimes are not expressable in most languages. pub payer_node_id: &'a NodeId, } @@ -1156,6 +1160,8 @@ pub struct FirstHopCandidate<'a> { pub struct PublicHopCandidate<'a> { /// Information about the channel, including potentially its capacity and /// direction-specific information. + /// + /// This is not exported to bindings users as lifetimes are not expressable in most languages. pub info: DirectedChannelInfo<'a>, /// The short channel ID of the channel, i.e. the identifier by which we refer to this /// channel. @@ -1166,8 +1172,12 @@ pub struct PublicHopCandidate<'a> { #[derive(Clone, Debug)] pub struct PrivateHopCandidate<'a> { /// Information about the private hop communicated via BOLT 11. + /// + /// This is not exported to bindings users as lifetimes are not expressable in most languages. pub hint: &'a RouteHintHop, /// Node id of the next hop in BOLT 11 route hint. + /// + /// This is not exported to bindings users as lifetimes are not expressable in most languages. pub target_node_id: &'a NodeId } @@ -1176,6 +1186,8 @@ pub struct PrivateHopCandidate<'a> { pub struct BlindedPathCandidate<'a> { /// Information about the blinded path including the fee, HTLC amount limits, and /// cryptographic material required to build an HTLC through the given path. + /// + /// This is not exported to bindings users as lifetimes are not expressable in most languages. pub hint: &'a (BlindedPayInfo, BlindedPath), /// Index of the hint in the original list of blinded hints. /// @@ -1191,6 +1203,8 @@ pub struct OneHopBlindedPathCandidate<'a> { /// cryptographic material required to build an HTLC terminating with the given path. /// /// Note that the [`BlindedPayInfo`] is ignored here. + /// + /// This is not exported to bindings users as lifetimes are not expressable in most languages. pub hint: &'a (BlindedPayInfo, BlindedPath), /// Index of the hint in the original list of blinded hints. /// From 37162273597c93f33ade58a52ec966762cf00eb6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 30 Jan 2024 23:54:56 +0000 Subject: [PATCH 4/8] Fix `three_hop_blinded_path_success` with different randomization In `three_hop_blinded_path_success`, the nodes in the test ended up at radically different block heights after channel opening. At that point, if the CLTV randomization is done slightly different the test payment may fail, which we fix here by ensuring all nodes are at the same height before we go to send a payment. --- lightning/src/ln/blinded_payment_tests.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 3232cd0d33..04742d6ba4 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -509,6 +509,12 @@ fn three_hop_blinded_path_success() { let chan_upd_2_3 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).0.contents; let chan_upd_3_4 = create_announced_chan_between_nodes_with_value(&nodes, 3, 4, 1_000_000, 0).0.contents; + // Get all our nodes onto the same height so payments don't fail for CLTV violations. + connect_blocks(&nodes[0], nodes[4].best_block_info().1 - nodes[0].best_block_info().1); + connect_blocks(&nodes[1], nodes[4].best_block_info().1 - nodes[1].best_block_info().1); + connect_blocks(&nodes[2], nodes[4].best_block_info().1 - nodes[2].best_block_info().1); + assert_eq!(nodes[4].best_block_info().1, nodes[3].best_block_info().1); + let amt_msat = 5000; let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[4], Some(amt_msat), None); let route_params = get_blinded_route_parameters(amt_msat, payment_secret, From 5d5c8187b40994079036c2a809639e8fd731c867 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 19:49:36 +0000 Subject: [PATCH 5/8] Store `EntropySource` in `DefaultRouter` instead of passing it ...as an arg to `Router`. Passing an `EntropySource` around all the time is a bit strange as the `Router` may or may not actually use it, and the `DefaultRouter` can just as easily store it. --- fuzz/src/chanmon_consistency.rs | 13 ++--- fuzz/src/full_stack.rs | 13 ++--- fuzz/src/onion_message.rs | 7 +-- lightning-background-processor/src/lib.rs | 7 ++- lightning/src/ln/channelmanager.rs | 8 +-- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 2 +- .../src/onion_message/functional_tests.rs | 7 +-- lightning/src/onion_message/messenger.rs | 36 ++++++------ lightning/src/routing/router.rs | 57 +++++++++---------- lightning/src/util/test_utils.rs | 37 ++++++------ 11 files changed, 86 insertions(+), 103 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 89ff07fe69..b1126433be 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -103,11 +103,9 @@ impl Router for FuzzRouter { }) } - fn create_blinded_payment_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( + fn create_blinded_payment_paths( &self, _recipient: PublicKey, _first_hops: Vec, _tlvs: ReceiveTlvs, - _amount_msats: u64, _entropy_source: &ES, _secp_ctx: &Secp256k1 + _amount_msats: u64, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } @@ -120,11 +118,8 @@ impl MessageRouter for FuzzRouter { unreachable!() } - fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( - &self, _recipient: PublicKey, _peers: Vec, _entropy_source: &ES, - _secp_ctx: &Secp256k1 + fn create_blinded_paths( + &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 1f5ceb2123..1a520557a1 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -146,11 +146,9 @@ impl Router for FuzzRouter { }) } - fn create_blinded_payment_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( + fn create_blinded_payment_paths( &self, _recipient: PublicKey, _first_hops: Vec, _tlvs: ReceiveTlvs, - _amount_msats: u64, _entropy_source: &ES, _secp_ctx: &Secp256k1 + _amount_msats: u64, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } @@ -163,11 +161,8 @@ impl MessageRouter for FuzzRouter { unreachable!() } - fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( - &self, _recipient: PublicKey, _peers: Vec, _entropy_source: &ES, - _secp_ctx: &Secp256k1 + fn create_blinded_paths( + &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 4ad770fc20..f2bae246fa 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -86,11 +86,8 @@ impl MessageRouter for TestMessageRouter { }) } - fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( - &self, _recipient: PublicKey, _peers: Vec, _entropy_source: &ES, - _secp_ctx: &Secp256k1 + fn create_blinded_paths( + &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 0f2c67538d..4bbebf46fe 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -984,6 +984,7 @@ mod tests { Arc>>, Arc, + Arc, Arc>, (), TestScorer> @@ -1263,12 +1264,12 @@ mod tests { let genesis_block = genesis_block(network); let network_graph = Arc::new(NetworkGraph::new(network, logger.clone())); let scorer = Arc::new(LockingWrapper::new(TestScorer::new())); + let now = Duration::from_secs(genesis_block.header.time as u64); let seed = [i as u8; 32]; - let router = Arc::new(DefaultRouter::new(network_graph.clone(), logger.clone(), seed, scorer.clone(), Default::default())); + let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos())); + let router = Arc::new(DefaultRouter::new(network_graph.clone(), logger.clone(), Arc::clone(&keys_manager), scorer.clone(), Default::default())); let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Bitcoin)); let kv_store = Arc::new(FilesystemStore::new(format!("{}_persister_{}", &persist_dir, i).into())); - let now = Duration::from_secs(genesis_block.header.time as u64); - let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos())); let chain_monitor = Arc::new(chainmonitor::ChainMonitor::new(Some(chain_source.clone()), tx_broadcaster.clone(), logger.clone(), fee_estimator.clone(), kv_store.clone())); let best_block = BestBlock::from_network(network); let params = ChainParameters { network, best_block }; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ec3e8ef604..5724b13fba 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -964,6 +964,7 @@ pub type SimpleArcChannelManager = ChannelManager< Arc>>, Arc, + Arc, Arc>>, Arc>>>, ProbabilisticScoringFeeParameters, ProbabilisticScorer>>, Arc>, @@ -994,6 +995,7 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = &'e DefaultRouter< &'f NetworkGraph<&'g L>, &'g L, + &'c KeysManager, &'h RwLock, &'g L>>, ProbabilisticScoringFeeParameters, ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L> @@ -7933,7 +7935,6 @@ where /// Errors if the `MessageRouter` errors or returns an empty `Vec`. fn create_blinded_path(&self) -> Result { let recipient = self.get_our_node_id(); - let entropy_source = self.entropy_source.deref(); let secp_ctx = &self.secp_ctx; let peers = self.per_peer_state.read().unwrap() @@ -7943,7 +7944,7 @@ where .collect::>(); self.router - .create_blinded_paths(recipient, peers, entropy_source, secp_ctx) + .create_blinded_paths(recipient, peers, secp_ctx) .and_then(|paths| paths.into_iter().next().ok_or(())) } @@ -7952,7 +7953,6 @@ where fn create_blinded_payment_paths( &self, amount_msats: u64, payment_secret: PaymentSecret ) -> Result, ()> { - let entropy_source = self.entropy_source.deref(); let secp_ctx = &self.secp_ctx; let first_hops = self.list_usable_channels(); @@ -7967,7 +7967,7 @@ where }, }; self.router.create_blinded_payment_paths( - payee_node_id, first_hops, payee_tlvs, amount_msats, entropy_source, secp_ctx + payee_node_id, first_hops, payee_tlvs, amount_msats, secp_ctx ) } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 8df84000c2..2563868e77 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2932,7 +2932,7 @@ pub fn create_node_cfgs_with_persisters<'a>(node_count: usize, chanmon_cfgs: &'a tx_broadcaster: &chanmon_cfgs[i].tx_broadcaster, fee_estimator: &chanmon_cfgs[i].fee_estimator, router: test_utils::TestRouter::new(network_graph.clone(), &chanmon_cfgs[i].logger, &chanmon_cfgs[i].scorer), - message_router: test_utils::TestMessageRouter::new(network_graph.clone()), + message_router: test_utils::TestMessageRouter::new(network_graph.clone(), &chanmon_cfgs[i].keys_manager), chain_monitor, keys_manager: &chanmon_cfgs[i].keys_manager, node_seed: seed, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index be9bfb81f2..2f43056fb0 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -5534,7 +5534,7 @@ fn test_key_derivation_params() { let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &chanmon_cfgs[0].logger)); let scorer = RwLock::new(test_utils::TestScorer::new()); let router = test_utils::TestRouter::new(network_graph.clone(), &chanmon_cfgs[0].logger, &scorer); - let message_router = test_utils::TestMessageRouter::new(network_graph.clone()); + let message_router = test_utils::TestMessageRouter::new(network_graph.clone(), &keys_manager); let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, router, message_router, chain_monitor, keys_manager: &keys_manager, network_graph, node_seed: seed, override_init_features: alloc::rc::Rc::new(core::cell::RefCell::new(None)) }; let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs); node_cfgs.remove(0); diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index 23d27b1894..271b56dc0c 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/lightning/src/onion_message/functional_tests.rs @@ -13,7 +13,7 @@ use crate::blinded_path::BlindedPath; use crate::events::{Event, EventsProvider}; use crate::ln::features::InitFeatures; use crate::ln::msgs::{self, DecodeError, OnionMessageHandler, SocketAddress}; -use crate::sign::{EntropySource, NodeSigner, Recipient}; +use crate::sign::{NodeSigner, Recipient}; use crate::util::ser::{FixedLengthReader, LengthReadable, Writeable, Writer}; use crate::util::test_utils; use super::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage, SendError}; @@ -59,10 +59,9 @@ impl MessageRouter for TestMessageRouter { } fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification + T: secp256k1::Signing + secp256k1::Verification >( - &self, _recipient: PublicKey, _peers: Vec, _entropy_source: &ES, - _secp_ctx: &Secp256k1 + &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 3678dea84d..bad3ae96bb 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -96,8 +96,8 @@ pub(super) const MAX_TIMER_TICKS: usize = 2; /// # first_node_addresses: None, /// # }) /// # } -/// # fn create_blinded_paths( -/// # &self, _recipient: PublicKey, _peers: Vec, _entropy_source: &ES, _secp_ctx: &Secp256k1 +/// # fn create_blinded_paths( +/// # &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1 /// # ) -> Result, ()> { /// # unreachable!() /// # } @@ -285,34 +285,37 @@ pub trait MessageRouter { /// Creates [`BlindedPath`]s to the `recipient` node. The nodes in `peers` are assumed to be /// direct peers with the `recipient`. fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification + T: secp256k1::Signing + secp256k1::Verification >( - &self, recipient: PublicKey, peers: Vec, entropy_source: &ES, - secp_ctx: &Secp256k1 + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()>; } /// A [`MessageRouter`] that can only route to a directly connected [`Destination`]. -pub struct DefaultMessageRouter>, L: Deref> +pub struct DefaultMessageRouter>, L: Deref, ES: Deref> where L::Target: Logger, + ES::Target: EntropySource, { network_graph: G, + entropy_source: ES, } -impl>, L: Deref> DefaultMessageRouter +impl>, L: Deref, ES: Deref> DefaultMessageRouter where L::Target: Logger, + ES::Target: EntropySource, { /// Creates a [`DefaultMessageRouter`] using the given [`NetworkGraph`]. - pub fn new(network_graph: G) -> Self { - Self { network_graph } + pub fn new(network_graph: G, entropy_source: ES) -> Self { + Self { network_graph, entropy_source } } } -impl>, L: Deref> MessageRouter for DefaultMessageRouter +impl>, L: Deref, ES: Deref> MessageRouter for DefaultMessageRouter where L::Target: Logger, + ES::Target: EntropySource, { fn find_path( &self, _sender: PublicKey, peers: Vec, destination: Destination @@ -343,10 +346,9 @@ where } fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification + T: secp256k1::Signing + secp256k1::Verification >( - &self, recipient: PublicKey, peers: Vec, entropy_source: &ES, - secp_ctx: &Secp256k1 + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { // Limit the number of blinded paths that are computed. const MAX_PATHS: usize = 3; @@ -366,7 +368,7 @@ where .unwrap_or(false) ) .map(|pubkey| vec![*pubkey, recipient]) - .map(|node_pks| BlindedPath::new_for_message(&node_pks, entropy_source, secp_ctx)) + .map(|node_pks| BlindedPath::new_for_message(&node_pks, &*self.entropy_source, secp_ctx)) .take(MAX_PATHS) .collect::, _>>(); @@ -374,7 +376,7 @@ where Ok(paths) if !paths.is_empty() => Ok(paths), _ => { if network_graph.nodes().contains_key(&NodeId::from_pubkey(&recipient)) { - BlindedPath::one_hop_for_message(recipient, entropy_source, secp_ctx) + BlindedPath::one_hop_for_message(recipient, &*self.entropy_source, secp_ctx) .map(|path| vec![path]) } else { Err(()) @@ -1069,7 +1071,7 @@ pub type SimpleArcOnionMessenger = OnionMessenger< Arc, Arc, Arc, - Arc>>, Arc>>, + Arc>>, Arc, Arc>>, Arc>, IgnoringMessageHandler >; @@ -1088,7 +1090,7 @@ pub type SimpleRefOnionMessenger< &'a KeysManager, &'a KeysManager, &'b L, - &'i DefaultMessageRouter<&'g NetworkGraph<&'b L>, &'b L>, + &'i DefaultMessageRouter<&'g NetworkGraph<&'b L>, &'b L, &'a KeysManager>, &'j SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L>, IgnoringMessageHandler >; diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 7ca9d4b7d9..d5072f9caa 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -10,8 +10,6 @@ //! The router finds paths within a [`NetworkGraph`] for a payment. use bitcoin::secp256k1::{PublicKey, Secp256k1, self}; -use bitcoin::hashes::Hash; -use bitcoin::hashes::sha256::Hash as Sha256; use crate::blinded_path::{BlindedHop, BlindedPath}; use crate::blinded_path::payment::{ForwardNode, ForwardTlvs, PaymentConstraints, PaymentRelay, ReceiveTlvs}; @@ -30,39 +28,40 @@ use crate::crypto::chacha20::ChaCha20; use crate::io; use crate::prelude::*; -use crate::sync::Mutex; use alloc::collections::BinaryHeap; use core::{cmp, fmt}; use core::ops::Deref; /// A [`Router`] implemented using [`find_route`]. -pub struct DefaultRouter> + Clone, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> where +pub struct DefaultRouter> + Clone, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> where L::Target: Logger, S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>, + ES::Target: EntropySource, { network_graph: G, logger: L, - random_seed_bytes: Mutex<[u8; 32]>, + entropy_source: ES, scorer: S, score_params: SP, - message_router: DefaultMessageRouter, + message_router: DefaultMessageRouter, } -impl> + Clone, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> DefaultRouter where +impl> + Clone, L: Deref, ES: Deref + Clone, S: Deref, SP: Sized, Sc: ScoreLookUp> DefaultRouter where L::Target: Logger, S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>, + ES::Target: EntropySource, { /// Creates a new router. - pub fn new(network_graph: G, logger: L, random_seed_bytes: [u8; 32], scorer: S, score_params: SP) -> Self { - let random_seed_bytes = Mutex::new(random_seed_bytes); - let message_router = DefaultMessageRouter::new(network_graph.clone()); - Self { network_graph, logger, random_seed_bytes, scorer, score_params, message_router } + pub fn new(network_graph: G, logger: L, entropy_source: ES, scorer: S, score_params: SP) -> Self { + let message_router = DefaultMessageRouter::new(network_graph.clone(), entropy_source.clone()); + Self { network_graph, logger, entropy_source, scorer, score_params, message_router } } } -impl> + Clone, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> Router for DefaultRouter where +impl> + Clone, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> Router for DefaultRouter where L::Target: Logger, S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>, + ES::Target: EntropySource, { fn find_route( &self, @@ -71,11 +70,7 @@ impl> + Clone, L: Deref, S: Deref, SP: Sized, first_hops: Option<&[&ChannelDetails]>, inflight_htlcs: InFlightHtlcs ) -> Result { - let random_seed_bytes = { - let mut locked_random_seed_bytes = self.random_seed_bytes.lock().unwrap(); - *locked_random_seed_bytes = Sha256::hash(&*locked_random_seed_bytes).to_byte_array(); - *locked_random_seed_bytes - }; + let random_seed_bytes = self.entropy_source.get_secure_random_bytes(); find_route( payer, params, &self.network_graph, first_hops, &*self.logger, &ScorerAccountingForInFlightHtlcs::new(self.scorer.read_lock(), &inflight_htlcs), @@ -85,10 +80,10 @@ impl> + Clone, L: Deref, S: Deref, SP: Sized, } fn create_blinded_payment_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( + T: secp256k1::Signing + secp256k1::Verification + > ( &self, recipient: PublicKey, first_hops: Vec, tlvs: ReceiveTlvs, - amount_msats: u64, entropy_source: &ES, secp_ctx: &Secp256k1 + amount_msats: u64, secp_ctx: &Secp256k1 ) -> Result, ()> { // Limit the number of blinded paths that are computed. const MAX_PAYMENT_PATHS: usize = 3; @@ -139,7 +134,7 @@ impl> + Clone, L: Deref, S: Deref, SP: Sized, }) .map(|forward_node| { BlindedPath::new_for_payment( - &[forward_node], recipient, tlvs.clone(), u64::MAX, entropy_source, secp_ctx + &[forward_node], recipient, tlvs.clone(), u64::MAX, &*self.entropy_source, secp_ctx ) }) .take(MAX_PAYMENT_PATHS) @@ -149,7 +144,7 @@ impl> + Clone, L: Deref, S: Deref, SP: Sized, Ok(paths) if !paths.is_empty() => Ok(paths), _ => { if network_graph.nodes().contains_key(&NodeId::from_pubkey(&recipient)) { - BlindedPath::one_hop_for_payment(recipient, tlvs, entropy_source, secp_ctx) + BlindedPath::one_hop_for_payment(recipient, tlvs, &*self.entropy_source, secp_ctx) .map(|path| vec![path]) } else { Err(()) @@ -159,9 +154,10 @@ impl> + Clone, L: Deref, S: Deref, SP: Sized, } } -impl< G: Deref> + Clone, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> MessageRouter for DefaultRouter where +impl< G: Deref> + Clone, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> MessageRouter for DefaultRouter where L::Target: Logger, S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>, + ES::Target: EntropySource, { fn find_path( &self, sender: PublicKey, peers: Vec, destination: Destination @@ -170,12 +166,11 @@ impl< G: Deref> + Clone, L: Deref, S: Deref, SP: Sized, } fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( - &self, recipient: PublicKey, peers: Vec, entropy_source: &ES, - secp_ctx: &Secp256k1 + T: secp256k1::Signing + secp256k1::Verification + > ( + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - self.message_router.create_blinded_paths(recipient, peers, entropy_source, secp_ctx) + self.message_router.create_blinded_paths(recipient, peers, secp_ctx) } } @@ -209,10 +204,10 @@ pub trait Router: MessageRouter { /// are assumed to be with the `recipient`'s peers. The payment secret and any constraints are /// given in `tlvs`. fn create_blinded_payment_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( + T: secp256k1::Signing + secp256k1::Verification + > ( &self, recipient: PublicKey, first_hops: Vec, tlvs: ReceiveTlvs, - amount_msats: u64, entropy_source: &ES, secp_ctx: &Secp256k1 + amount_msats: u64, secp_ctx: &Secp256k1 ) -> Result, ()>; } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 02423541fb..6cbe53ab85 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -70,7 +70,7 @@ use crate::sync::{Mutex, Arc}; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use core::mem; use bitcoin::bech32::u5; -use crate::sign::{InMemorySigner, Recipient, EntropySource, NodeSigner, SignerProvider}; +use crate::sign::{InMemorySigner, RandomBytes, Recipient, EntropySource, NodeSigner, SignerProvider}; #[cfg(feature = "std")] use std::time::{SystemTime, UNIX_EPOCH}; @@ -107,10 +107,12 @@ pub struct TestRouter<'a> { pub router: DefaultRouter< Arc>, &'a TestLogger, + Arc, &'a RwLock, (), TestScorer, >, + //pub entropy_source: &'a RandomBytes, pub network_graph: Arc>, pub next_routes: Mutex)>>, pub scorer: &'a RwLock, @@ -119,10 +121,11 @@ pub struct TestRouter<'a> { impl<'a> TestRouter<'a> { pub fn new( network_graph: Arc>, logger: &'a TestLogger, - scorer: &'a RwLock + scorer: &'a RwLock, ) -> Self { + let entropy_source = Arc::new(RandomBytes::new([42; 32])); Self { - router: DefaultRouter::new(network_graph.clone(), logger, [42u8; 32], scorer, ()), + router: DefaultRouter::new(network_graph.clone(), logger, entropy_source, scorer, ()), network_graph, next_routes: Mutex::new(VecDeque::new()), scorer, @@ -205,13 +208,13 @@ impl<'a> Router for TestRouter<'a> { } fn create_blinded_payment_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification + T: secp256k1::Signing + secp256k1::Verification >( &self, recipient: PublicKey, first_hops: Vec, tlvs: ReceiveTlvs, - amount_msats: u64, entropy_source: &ES, secp_ctx: &Secp256k1 + amount_msats: u64, secp_ctx: &Secp256k1, ) -> Result, ()> { self.router.create_blinded_payment_paths( - recipient, first_hops, tlvs, amount_msats, entropy_source, secp_ctx + recipient, first_hops, tlvs, amount_msats, secp_ctx ) } } @@ -224,12 +227,11 @@ impl<'a> MessageRouter for TestRouter<'a> { } fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification + T: secp256k1::Signing + secp256k1::Verification >( - &self, recipient: PublicKey, peers: Vec, entropy_source: &ES, - secp_ctx: &Secp256k1 + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - self.router.create_blinded_paths(recipient, peers, entropy_source, secp_ctx) + self.router.create_blinded_paths(recipient, peers, secp_ctx) } } @@ -245,12 +247,12 @@ impl<'a> Drop for TestRouter<'a> { } pub struct TestMessageRouter<'a> { - inner: DefaultMessageRouter>, &'a TestLogger>, + inner: DefaultMessageRouter>, &'a TestLogger, &'a TestKeysInterface>, } impl<'a> TestMessageRouter<'a> { - pub fn new(network_graph: Arc>) -> Self { - Self { inner: DefaultMessageRouter::new(network_graph) } + pub fn new(network_graph: Arc>, entropy_source: &'a TestKeysInterface) -> Self { + Self { inner: DefaultMessageRouter::new(network_graph, entropy_source) } } } @@ -261,13 +263,10 @@ impl<'a> MessageRouter for TestMessageRouter<'a> { self.inner.find_path(sender, peers, destination) } - fn create_blinded_paths< - ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification - >( - &self, recipient: PublicKey, peers: Vec, entropy_source: &ES, - secp_ctx: &Secp256k1 + fn create_blinded_paths( + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - self.inner.create_blinded_paths(recipient, peers, entropy_source, secp_ctx) + self.inner.create_blinded_paths(recipient, peers, secp_ctx) } } From 8805d0656b6c4a119d9ea49b31924bc44dd66543 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 18 Jan 2024 20:53:20 +0000 Subject: [PATCH 6/8] Move RGS `GraphSyncError` into the top-level module The top-level module is only a few hundred lines, so there's not a lot of reason to hide the `GraphSyncError` in its own module. Instead, we simply move it to the top-level `lib.rs`, which doesn't change the public API as it was previously re-exported at the top level. --- lightning-rapid-gossip-sync/src/error.rs | 40 --------------- lightning-rapid-gossip-sync/src/lib.rs | 49 ++++++++++++++++--- lightning-rapid-gossip-sync/src/processing.rs | 6 +-- 3 files changed, 44 insertions(+), 51 deletions(-) delete mode 100644 lightning-rapid-gossip-sync/src/error.rs diff --git a/lightning-rapid-gossip-sync/src/error.rs b/lightning-rapid-gossip-sync/src/error.rs deleted file mode 100644 index ffd6760f8a..0000000000 --- a/lightning-rapid-gossip-sync/src/error.rs +++ /dev/null @@ -1,40 +0,0 @@ -use core::fmt::Debug; -use core::fmt::Formatter; -use lightning::ln::msgs::{DecodeError, LightningError}; - -/// All-encompassing standard error type that processing can return -pub enum GraphSyncError { - /// Error trying to read the update data, typically due to an erroneous data length indication - /// that is greater than the actual amount of data provided - DecodeError(DecodeError), - /// Error applying the patch to the network graph, usually the result of updates that are too - /// old or missing prerequisite data to the application of updates out of order - LightningError(LightningError), -} - -impl From for GraphSyncError { - fn from(error: lightning::io::Error) -> Self { - Self::DecodeError(DecodeError::Io(error.kind())) - } -} - -impl From for GraphSyncError { - fn from(error: DecodeError) -> Self { - Self::DecodeError(error) - } -} - -impl From for GraphSyncError { - fn from(error: LightningError) -> Self { - Self::LightningError(error) - } -} - -impl Debug for GraphSyncError { - fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { - match self { - GraphSyncError::DecodeError(e) => f.write_fmt(format_args!("DecodeError: {:?}", e)), - GraphSyncError::LightningError(e) => f.write_fmt(format_args!("LightningError: {:?}", e)) - } - } -} diff --git a/lightning-rapid-gossip-sync/src/lib.rs b/lightning-rapid-gossip-sync/src/lib.rs index 0561975f82..1a8727a7cd 100644 --- a/lightning-rapid-gossip-sync/src/lib.rs +++ b/lightning-rapid-gossip-sync/src/lib.rs @@ -72,19 +72,54 @@ extern crate alloc; use std::fs::File; use core::ops::Deref; use core::sync::atomic::{AtomicBool, Ordering}; +use core::fmt::Debug; +use core::fmt::Formatter; use lightning::io; +use lightning::ln::msgs::{DecodeError, LightningError}; use lightning::routing::gossip::NetworkGraph; use lightning::util::logger::Logger; -pub use crate::error::GraphSyncError; - -/// Error types that these functions can return -mod error; - /// Core functionality of this crate mod processing; +/// All-encompassing standard error type that processing can return +pub enum GraphSyncError { + /// Error trying to read the update data, typically due to an erroneous data length indication + /// that is greater than the actual amount of data provided + DecodeError(DecodeError), + /// Error applying the patch to the network graph, usually the result of updates that are too + /// old or missing prerequisite data to the application of updates out of order + LightningError(LightningError), +} + +impl From for GraphSyncError { + fn from(error: lightning::io::Error) -> Self { + Self::DecodeError(DecodeError::Io(error.kind())) + } +} + +impl From for GraphSyncError { + fn from(error: DecodeError) -> Self { + Self::DecodeError(error) + } +} + +impl From for GraphSyncError { + fn from(error: LightningError) -> Self { + Self::LightningError(error) + } +} + +impl Debug for GraphSyncError { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + match self { + GraphSyncError::DecodeError(e) => f.write_fmt(format_args!("DecodeError: {:?}", e)), + GraphSyncError::LightningError(e) => f.write_fmt(format_args!("LightningError: {:?}", e)) + } + } +} + /// The main Rapid Gossip Sync object. /// /// See [crate-level documentation] for usage. @@ -167,7 +202,7 @@ mod tests { use lightning::ln::msgs::DecodeError; use lightning::routing::gossip::NetworkGraph; use lightning::util::test_utils::TestLogger; - use crate::RapidGossipSync; + use crate::{GraphSyncError, RapidGossipSync}; #[test] fn test_sync_from_file() { @@ -265,7 +300,7 @@ mod tests { let start = std::time::Instant::now(); let sync_result = rapid_sync .sync_network_graph_with_file_path("./res/full_graph.lngossip"); - if let Err(crate::error::GraphSyncError::DecodeError(DecodeError::Io(io_error))) = &sync_result { + if let Err(GraphSyncError::DecodeError(DecodeError::Io(io_error))) = &sync_result { let error_string = format!("Input file lightning-rapid-gossip-sync/res/full_graph.lngossip is missing! Download it from https://bitcoin.ninja/ldk-compressed_graph-285cb27df79-2022-07-21.bin\n\n{:?}", io_error); #[cfg(not(require_route_graph_test))] { diff --git a/lightning-rapid-gossip-sync/src/processing.rs b/lightning-rapid-gossip-sync/src/processing.rs index d54f132979..9023b9ba38 100644 --- a/lightning-rapid-gossip-sync/src/processing.rs +++ b/lightning-rapid-gossip-sync/src/processing.rs @@ -14,8 +14,7 @@ use lightning::{log_debug, log_warn, log_trace, log_given_level, log_gossip}; use lightning::util::ser::{BigSize, Readable}; use lightning::io; -use crate::error::GraphSyncError; -use crate::RapidGossipSync; +use crate::{GraphSyncError, RapidGossipSync}; #[cfg(all(feature = "std", not(test)))] use std::time::{SystemTime, UNIX_EPOCH}; @@ -269,9 +268,8 @@ mod tests { use lightning::routing::gossip::NetworkGraph; use lightning::util::test_utils::TestLogger; - use crate::error::GraphSyncError; use crate::processing::STALE_RGS_UPDATE_AGE_LIMIT_SECS; - use crate::RapidGossipSync; + use crate::{GraphSyncError, RapidGossipSync}; const VALID_RGS_BINARY: [u8; 300] = [ 76, 68, 75, 1, 111, 226, 140, 10, 182, 241, 179, 114, 193, 166, 162, 70, 174, 99, 247, From 0a14559297f4d1a7dc63f091e8c477eebc86ef11 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 19:20:05 +0000 Subject: [PATCH 7/8] Drop manual `Debug` impl on RGS' `GraphSyncError` As it does the same thing as a derived `Debug` does anyway. --- lightning-rapid-gossip-sync/src/lib.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/lightning-rapid-gossip-sync/src/lib.rs b/lightning-rapid-gossip-sync/src/lib.rs index 1a8727a7cd..68c451a637 100644 --- a/lightning-rapid-gossip-sync/src/lib.rs +++ b/lightning-rapid-gossip-sync/src/lib.rs @@ -72,8 +72,6 @@ extern crate alloc; use std::fs::File; use core::ops::Deref; use core::sync::atomic::{AtomicBool, Ordering}; -use core::fmt::Debug; -use core::fmt::Formatter; use lightning::io; use lightning::ln::msgs::{DecodeError, LightningError}; @@ -84,6 +82,7 @@ use lightning::util::logger::Logger; mod processing; /// All-encompassing standard error type that processing can return +#[derive(Debug)] pub enum GraphSyncError { /// Error trying to read the update data, typically due to an erroneous data length indication /// that is greater than the actual amount of data provided @@ -111,15 +110,6 @@ impl From for GraphSyncError { } } -impl Debug for GraphSyncError { - fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { - match self { - GraphSyncError::DecodeError(e) => f.write_fmt(format_args!("DecodeError: {:?}", e)), - GraphSyncError::LightningError(e) => f.write_fmt(format_args!("LightningError: {:?}", e)) - } - } -} - /// The main Rapid Gossip Sync object. /// /// See [crate-level documentation] for usage. From fb693ecbf81be0146a81cc7fd95342c403eacaa9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 06:19:24 +0000 Subject: [PATCH 8/8] [bindings] Move additional score params from `&()` to `Default` In 26c1639ab69d6780c97a118f09e42cb42304088a (and later in 50c55dcf32466e3ccf17acea50697fc664950deb) we switched to using `Default::default()` to initialize `()` for scoring parameters in tests. A number of `()`s slipped back in recently, which we replace here. --- lightning/src/util/test_utils.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 6cbe53ab85..e5489ac8a2 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -168,7 +168,7 @@ impl<'a> Router for TestRouter<'a> { details: first_hops[idx], payer_node_id: &node_id, }); - scorer.channel_penalty_msat(&candidate, usage, &()); + scorer.channel_penalty_msat(&candidate, usage, &Default::default()); continue; } } @@ -180,7 +180,7 @@ impl<'a> Router for TestRouter<'a> { info: directed, short_channel_id: hop.short_channel_id, }); - scorer.channel_penalty_msat(&candidate, usage, &()); + scorer.channel_penalty_msat(&candidate, usage, &Default::default()); } else { let target_node_id = NodeId::from_pubkey(&hop.pubkey); let route_hint = RouteHintHop { @@ -195,7 +195,7 @@ impl<'a> Router for TestRouter<'a> { hint: &route_hint, target_node_id: &target_node_id, }); - scorer.channel_penalty_msat(&candidate, usage, &()); + scorer.channel_penalty_msat(&candidate, usage, &Default::default()); } prev_hop_node = &hop.pubkey; }