From d7e3cd50ad76a708702fe4bb1f8c2467258b030e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 20 Dec 2023 05:17:43 +0000 Subject: [PATCH 01/23] [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 92ea8ffed55..d1c768dc7fe 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 54ee8b4c5f2577451672d04487c8454521d231dc Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 18 Jan 2024 00:42:55 +0000 Subject: [PATCH 02/23] [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 d1c768dc7fe..e48cefaa044 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 10434988642f33d9a44c44a5ffcdcb1fbfe58361 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 01:16:46 +0000 Subject: [PATCH 03/23] [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 1dbd1ffc655..e0774894cb7 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 da620285eec3978841edafa061d03783c751a6fc Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 19:49:36 +0000 Subject: [PATCH 04/23] 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. --- lightning-background-processor/src/lib.rs | 7 ++- lightning/src/ln/channelmanager.rs | 8 +-- .../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 | 10 ++-- 6 files changed, 62 insertions(+), 63 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 0f2c67538d6..4bbebf46fe1 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 0506a81c519..0f881637ff1 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/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index 23d27b18943..3947e1531cc 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/lightning/src/onion_message/functional_tests.rs @@ -30,6 +30,8 @@ use crate::sync::{Arc, Mutex}; use crate::prelude::*; +use core::ops::Deref; + struct MessengerNode { node_id: PublicKey, entropy_source: Arc, @@ -59,10 +61,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 5dc3bb422e6..c8f74b67eda 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -97,8 +97,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!() /// # } @@ -286,34 +286,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 @@ -344,10 +347,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; @@ -367,7 +369,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::, _>>(); @@ -375,7 +377,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(()) @@ -1059,7 +1061,7 @@ pub type SimpleArcOnionMessenger = OnionMessenger< Arc, Arc, Arc, - Arc>>, Arc>>, + Arc>>, Arc, Arc>>, Arc>, IgnoringMessageHandler >; @@ -1078,7 +1080,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 e0774894cb7..67164bf8b74 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 1c45c4a4c5d..94aec91a5b5 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -66,6 +66,7 @@ use crate::io; use crate::prelude::*; use core::cell::RefCell; use core::time::Duration; +use core::ops::Deref; use crate::sync::{Mutex, Arc}; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use core::mem; @@ -194,10 +195,10 @@ 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, ()> { unreachable!() } @@ -211,10 +212,9 @@ 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, ()> { unreachable!() } From b1367839784ffa548df5f9432b5b0ddd703cb329 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 18 Jan 2024 20:53:20 +0000 Subject: [PATCH 05/23] 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 ffd6760f8a9..00000000000 --- 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 0561975f821..1a8727a7cd8 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 d54f1329798..9023b9ba38c 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 2855c74c65fa1adfa1099b586de8c21177b01218 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 19:20:05 +0000 Subject: [PATCH 06/23] 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 1a8727a7cd8..68c451a6372 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 76bdd0020f9660e63b83bff86a200dbd61dabd17 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 06:19:24 +0000 Subject: [PATCH 07/23] [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 94aec91a5b5..0621c23733c 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -151,7 +151,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; } } @@ -163,7 +163,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 { @@ -178,7 +178,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; } From a9db4db5254c1a8455cd130faecfae4c3f6de49e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Jan 2024 19:55:24 +0000 Subject: [PATCH 08/23] Stop relying on a `Clone`able `NetworkGraph` ref in `DefaultRouter` While there's not really much harm in requiring a `Clone`able reference (they almost always are), it does make our bindings struggle a bit as they don't support multi-trait bounds (as it would require synthesizing a new C trait, which the bindings don't do automatically). Luckily, there's really no reason for it, and we can just call the `DefaultMessageRouter` directly when we want to route a message. --- lightning/src/onion_message/messenger.rs | 39 ++++++++++++++++++------ lightning/src/routing/router.rs | 16 +++++----- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index c8f74b67eda..f56f425061f 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -313,13 +313,13 @@ where } } -impl>, L: Deref, ES: Deref> MessageRouter for DefaultMessageRouter +impl>, L: Deref, ES: Deref> DefaultMessageRouter where L::Target: Logger, ES::Target: EntropySource, { - fn find_path( - &self, _sender: PublicKey, peers: Vec, destination: Destination + pub(crate) fn find_path( + network_graph: &G, _sender: PublicKey, peers: Vec, destination: Destination ) -> Result { let first_node = destination.first_node(); if peers.contains(&first_node) { @@ -327,7 +327,7 @@ where intermediate_nodes: vec![], destination, first_node_addresses: None }) } else { - let network_graph = self.network_graph.deref().read_only(); + let network_graph = network_graph.deref().read_only(); let node_announcement = network_graph .node(&NodeId::from_pubkey(&first_node)) .and_then(|node_info| node_info.announcement_info.as_ref()) @@ -346,10 +346,11 @@ where } } - fn create_blinded_paths< + pub(crate) fn create_blinded_paths< T: secp256k1::Signing + secp256k1::Verification >( - &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + network_graph: &G, recipient: PublicKey, peers: Vec, entropy_source: &ES, + secp_ctx: &Secp256k1, ) -> Result, ()> { // Limit the number of blinded paths that are computed. const MAX_PATHS: usize = 3; @@ -358,7 +359,7 @@ where // recipient's node_id. const MIN_PEER_CHANNELS: usize = 3; - let network_graph = self.network_graph.deref().read_only(); + let network_graph = network_graph.deref().read_only(); let paths = peers.iter() // Limit to peers with announced channels .filter(|pubkey| @@ -369,7 +370,7 @@ where .unwrap_or(false) ) .map(|pubkey| vec![*pubkey, recipient]) - .map(|node_pks| BlindedPath::new_for_message(&node_pks, &*self.entropy_source, secp_ctx)) + .map(|node_pks| BlindedPath::new_for_message(&node_pks, &**entropy_source, secp_ctx)) .take(MAX_PATHS) .collect::, _>>(); @@ -377,7 +378,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, &*self.entropy_source, secp_ctx) + BlindedPath::one_hop_for_message(recipient, &**entropy_source, secp_ctx) .map(|path| vec![path]) } else { Err(()) @@ -387,6 +388,26 @@ where } } +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 + ) -> Result { + Self::find_path(&self.network_graph, sender, peers, destination) + } + + fn create_blinded_paths< + T: secp256k1::Signing + secp256k1::Verification + >( + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + ) -> Result, ()> { + Self::create_blinded_paths(&self.network_graph, recipient, peers, &self.entropy_source, secp_ctx) + } +} + /// A path for sending an [`OnionMessage`]. #[derive(Clone)] pub struct OnionMessagePath { diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 67164bf8b74..c2f2287f779 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -33,7 +33,7 @@ use core::{cmp, fmt}; use core::ops::Deref; /// A [`Router`] implemented using [`find_route`]. -pub struct DefaultRouter> + Clone, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> where +pub struct DefaultRouter>, 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, @@ -43,22 +43,20 @@ pub struct DefaultRouter> + Clone, L: Deref, E entropy_source: ES, scorer: S, score_params: SP, - message_router: DefaultMessageRouter, } -impl> + Clone, L: Deref, ES: Deref + Clone, S: Deref, SP: Sized, Sc: ScoreLookUp> DefaultRouter where +impl>, L: Deref, ES: Deref, 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, 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 } + Self { network_graph, logger, entropy_source, scorer, score_params } } } -impl> + Clone, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> Router for DefaultRouter where +impl>, 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, @@ -154,7 +152,7 @@ impl> + Clone, L: Deref, ES: Deref, S: Deref, } } -impl< G: Deref> + Clone, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> MessageRouter for DefaultRouter where +impl< G: Deref>, 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, @@ -162,7 +160,7 @@ impl< G: Deref> + Clone, L: Deref, ES: Deref, S: Deref, fn find_path( &self, sender: PublicKey, peers: Vec, destination: Destination ) -> Result { - self.message_router.find_path(sender, peers, destination) + DefaultMessageRouter::<_, _, ES>::find_path(&self.network_graph, sender, peers, destination) } fn create_blinded_paths< @@ -170,7 +168,7 @@ impl< G: Deref> + Clone, L: Deref, ES: Deref, S: Deref, > ( &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - self.message_router.create_blinded_paths(recipient, peers, secp_ctx) + DefaultMessageRouter::create_blinded_paths(&self.network_graph, recipient, peers, &self.entropy_source, secp_ctx) } } From e79d53a134107e3cd717191b71f940b87fee4b7b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 1 Mar 2022 03:46:52 +0000 Subject: [PATCH 09/23] Make `as_directed_to` non-public ...as the bindings generation does not currently have the ability to map a reference to a `NodeId` inside a tuple. --- lightning/src/routing/gossip.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 9b4e41ae174..faf48cdecc4 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -869,7 +869,7 @@ pub struct ChannelInfo { impl ChannelInfo { /// Returns a [`DirectedChannelInfo`] for the channel directed to the given `target` from a /// returned `source`, or `None` if `target` is not one of the channel's counterparties. - pub fn as_directed_to(&self, target: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> { + pub(crate) fn as_directed_to(&self, target: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> { let (direction, source, outbound) = { if target == &self.node_one { (self.two_to_one.as_ref(), &self.node_two, false) From a55935ce507f568f66234f54409e33405357ae2e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 29 Mar 2022 10:20:39 -0500 Subject: [PATCH 10/23] Restrict ChannelInfo::as_directed_from visibility Bindings can't handle references in return types, so reduce the visibility to pub(crate). --- lightning/src/routing/gossip.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index faf48cdecc4..5959874c956 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -884,7 +884,7 @@ impl ChannelInfo { /// Returns a [`DirectedChannelInfo`] for the channel directed from the given `source` to a /// returned `target`, or `None` if `source` is not one of the channel's counterparties. - pub fn as_directed_from(&self, source: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> { + pub(crate) fn as_directed_from(&self, source: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> { let (direction, target, outbound) = { if source == &self.node_one { (self.one_to_two.as_ref(), &self.node_two, true) From 6096a4f6ddcd38a6a56c8d7f2394b33712c999b6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 24 Dec 2022 04:16:48 +0000 Subject: [PATCH 11/23] Use an explicit `Sign` type on the `ChannelMonitor` read tuple The bindings currently get confused by the implicit `Sign` type, so we temporarily remove it on the `impl` here. --- lightning/src/chain/channelmonitor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index c81a48b78ac..6e1841281b7 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -4338,8 +4338,8 @@ where const MAX_ALLOC_SIZE: usize = 64*1024; -impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP)> - for (BlockHash, ChannelMonitor) { +impl<'a, 'b, ES: EntropySource, Signer: WriteableEcdsaChannelSigner, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP)> + for (BlockHash, ChannelMonitor) { fn read(reader: &mut R, args: (&'a ES, &'b SP)) -> Result { macro_rules! unwrap_obj { ($key: expr) => { From cb8cd4e2f143cab4d4bd44d9281bd2ad2718cbbe Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 28 Feb 2023 21:45:14 +0000 Subject: [PATCH 12/23] Export `outbound_payment` structs in their respective modules Re-exports in Rust make `use` statements a little shorter, but for otherwise don't materially change a crate's API. Sadly, the C bindings generator currently can't figure out re-exports, but it also exports everything into one global namespace, so it doesn't matter much anyway. --- fuzz/src/chanmon_consistency.rs | 3 ++- fuzz/src/full_stack.rs | 3 ++- lightning-invoice/src/payment.rs | 5 +++-- lightning-invoice/src/utils.rs | 3 ++- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/mod.rs | 2 +- 6 files changed, 11 insertions(+), 7 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 89ff07fe698..f44218a4426 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -41,7 +41,8 @@ use lightning::sign::{KeyMaterial, InMemorySigner, Recipient, EntropySource, Nod use lightning::events; use lightning::events::MessageSendEventsProvider; use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; -use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs, PaymentId, RecipientOnionFields}; +use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, ChannelManagerReadArgs, PaymentId}; +use lightning::ln::outbound_payment::{RecipientOnionFields, PaymentSendFailure}; use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; use lightning::ln::msgs::{self, CommitmentUpdate, ChannelMessageHandler, DecodeError, UpdateAddHTLC, Init}; use lightning::ln::script::ShutdownScript; diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 1f5ceb21235..2dd8e9440d5 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -38,7 +38,8 @@ use lightning::chain::transaction::OutPoint; use lightning::sign::{InMemorySigner, Recipient, KeyMaterial, EntropySource, NodeSigner, SignerProvider}; use lightning::events::Event; use lightning::ln::{ChannelId, PaymentHash, PaymentPreimage, PaymentSecret}; -use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentId, RecipientOnionFields, Retry}; +use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentId}; +use lightning::ln::outbound_payment::{RecipientOnionFields, Retry}; use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor,IgnoringMessageHandler}; use lightning::ln::msgs::{self, DecodeError}; use lightning::ln::script::ShutdownScript; diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index 8196fa9eb89..152b9d34832 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -13,7 +13,7 @@ use crate::Bolt11Invoice; use bitcoin::hashes::Hash; use lightning::ln::PaymentHash; -use lightning::ln::channelmanager::RecipientOnionFields; +use lightning::ln::outbound_payment::RecipientOnionFields; use lightning::routing::router::{PaymentParameters, RouteParameters}; /// Builds the necessary parameters to pay or pre-flight probe the given zero-amount @@ -170,7 +170,8 @@ mod tests { #[cfg(feature = "std")] fn payment_metadata_end_to_end() { use lightning::events::Event; - use lightning::ln::channelmanager::{Retry, PaymentId}; + use lightning::ln::channelmanager::PaymentId; + use lightning::ln::outbound_payment::Retry; use lightning::ln::msgs::ChannelMessageHandler; use lightning::ln::functional_test_utils::*; // Test that a payment metadata read from an invoice passed to `pay_invoice` makes it all diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 5e8b72467e5..bfd3bb0192d 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -825,7 +825,8 @@ mod test { use lightning::ln::PaymentHash; #[cfg(feature = "std")] use lightning::ln::PaymentPreimage; - use lightning::ln::channelmanager::{PhantomRouteHints, MIN_FINAL_CLTV_EXPIRY_DELTA, PaymentId, RecipientOnionFields, Retry}; + use lightning::ln::channelmanager::{PhantomRouteHints, MIN_FINAL_CLTV_EXPIRY_DELTA, PaymentId}; + use lightning::ln::outbound_payment::{RecipientOnionFields, Retry}; use lightning::ln::functional_test_utils::*; use lightning::ln::msgs::ChannelMessageHandler; use lightning::routing::router::{PaymentParameters, RouteParameters}; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0f881637ff1..de1145cc7ce 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -95,7 +95,7 @@ use core::time::Duration; use core::ops::Deref; // Re-export this for use in the public API. -pub use crate::ln::outbound_payment::{PaymentSendFailure, ProbeSendFailure, Retry, RetryableSendFailure, RecipientOnionFields}; +pub(crate) use crate::ln::outbound_payment::{PaymentSendFailure, ProbeSendFailure, Retry, RetryableSendFailure, RecipientOnionFields}; use crate::ln::script::ShutdownScript; // We hold various information about HTLC relay in the HTLC objects in Channel itself: diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs index 43ec34eaf61..5692f23e088 100644 --- a/lightning/src/ln/mod.rs +++ b/lightning/src/ln/mod.rs @@ -38,7 +38,7 @@ pub(crate) mod channel; pub use channel_id::ChannelId; pub(crate) mod onion_utils; -mod outbound_payment; +pub mod outbound_payment; pub mod wire; pub use onion_utils::create_payment_onion; From b64dc67896b51c9c5ec19de38c5a553a48a620e6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 5 Mar 2023 20:38:42 +0000 Subject: [PATCH 13/23] Avoid enums containing references with lifetimes Having struct fields with references to other structs is tough in our bindings logic, but even worse if the fields are in an enum. Its simplest to just take the clone penalty here. --- lightning/src/ln/channel.rs | 4 ++-- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/msgs.rs | 11 ++++++----- lightning/src/ln/peer_handler.rs | 2 +- lightning/src/ln/priv_short_conf_tests.rs | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ec4f26664a7..ffac09b758f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5604,7 +5604,7 @@ impl Channel where return None; } }; - let our_node_sig = match node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelAnnouncement(&announcement)) { + let our_node_sig = match node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelAnnouncement(announcement.clone())) { Err(_) => { log_error!(logger, "Failed to generate node signature for channel_announcement. Channel will not be announced!"); return None; @@ -5650,7 +5650,7 @@ impl Channel where .map_err(|_| ChannelError::Ignore("Signer failed to retrieve own public key".to_owned()))?); let were_node_one = announcement.node_id_1 == our_node_key; - let our_node_sig = node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelAnnouncement(&announcement)) + let our_node_sig = node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelAnnouncement(announcement.clone())) .map_err(|_| ChannelError::Ignore("Failed to generate node signature for channel_announcement".to_owned()))?; match &self.context.holder_signer { ChannelSignerType::Ecdsa(ecdsa) => { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index de1145cc7ce..cf718c94470 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3332,7 +3332,7 @@ where // If we returned an error and the `node_signer` cannot provide a signature for whatever // reason`, we wouldn't be able to receive inbound payments through the corresponding // channel. - let sig = self.node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelUpdate(&unsigned)).unwrap(); + let sig = self.node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelUpdate(unsigned.clone())).unwrap(); Ok(msgs::ChannelUpdate { signature: sig, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index d39f1a60842..e833e3ebf04 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1132,16 +1132,17 @@ impl FromStr for SocketAddress { } /// Represents the set of gossip messages that require a signature from a node's identity key. -pub enum UnsignedGossipMessage<'a> { +#[derive(Clone)] +pub enum UnsignedGossipMessage { /// An unsigned channel announcement. - ChannelAnnouncement(&'a UnsignedChannelAnnouncement), + ChannelAnnouncement(UnsignedChannelAnnouncement), /// An unsigned channel update. - ChannelUpdate(&'a UnsignedChannelUpdate), + ChannelUpdate(UnsignedChannelUpdate), /// An unsigned node announcement. - NodeAnnouncement(&'a UnsignedNodeAnnouncement) + NodeAnnouncement(UnsignedNodeAnnouncement) } -impl<'a> Writeable for UnsignedGossipMessage<'a> { +impl Writeable for UnsignedGossipMessage { fn write(&self, writer: &mut W) -> Result<(), io::Error> { match self { UnsignedGossipMessage::ChannelAnnouncement(ref msg) => msg.write(writer), diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index c3a50fd2490..77208921cc1 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -2542,7 +2542,7 @@ impl sig, Err(_) => { diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 85e757204fe..beb9ba49e3e 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -516,7 +516,7 @@ fn test_scid_alias_returned() { fee_proportional_millionths: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_proportional_millionths, excess_data: Vec::new(), }; - let signature = nodes[1].keys_manager.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelUpdate(&contents)).unwrap(); + let signature = nodes[1].keys_manager.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelUpdate(contents.clone())).unwrap(); let msg = msgs::ChannelUpdate { signature, contents }; let mut err_data = Vec::new(); From b7a63e3d97f829849c3f1c523b9e5c6e812d04b6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 25 Apr 2023 17:40:40 +0000 Subject: [PATCH 14/23] Add some no-exporting of more offers code These really could be handled in the bindings, but for lack of immediate users there's not a strong reason to, so instead we just disable them for now. --- lightning/src/offers/invoice.rs | 4 ++++ lightning/src/offers/offer.rs | 8 ++++++-- lightning/src/offers/parse.rs | 3 ++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index bb29c76164e..48dcfd9f57e 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -314,6 +314,8 @@ impl<'a, S: SigningPubkeyStrategy> InvoiceBuilder<'a, S> { /// /// Successive calls to this method will add another address. Caller is responsible for not /// adding duplicate addresses and only calling if capable of receiving to P2TR addresses. + /// + /// This is not exported to bindings users as TweakedPublicKey isn't yet mapped. pub fn fallback_v1_p2tr_tweaked(mut self, output_key: &TweakedPublicKey) -> Self { let address = FallbackAddress { version: WitnessVersion::V1.to_num(), @@ -680,6 +682,8 @@ macro_rules! invoice_accessors { ($self: ident, $contents: expr) => { /// Fallback addresses for paying the invoice on-chain, in order of most-preferred to /// least-preferred. + /// + /// This is not exported to bindings users as Address is not yet mapped pub fn fallbacks(&$self) -> Vec
{ $contents.fallbacks() } diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index ab7fe62cb50..490c6ad5699 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -772,6 +772,9 @@ pub enum Amount { /// An amount of currency specified using ISO 4712. Currency { /// The currency that the amount is denominated in. + /// + /// This is not exported to bindings users as bindings have troubles with type aliases to + /// byte arrays. iso4217_code: CurrencyCode, /// The amount in the currency unit adjusted by the ISO 4712 exponent (e.g., USD cents). amount: u64, @@ -779,7 +782,7 @@ pub enum Amount { } /// An ISO 4712 three-letter currency code (e.g., USD). -pub type CurrencyCode = [u8; 3]; +pub(crate) type CurrencyCode = [u8; 3]; /// Quantity of items supported by an [`Offer`]. #[derive(Clone, Copy, Debug, PartialEq)] @@ -789,7 +792,8 @@ pub enum Quantity { /// /// May be used with `NonZeroU64::new(1)` but prefer to use [`Quantity::One`] if only one item /// is supported. - Bounded(NonZeroU64), + Bounded(/// This is not exported to bindings users as builder patterns don't map outside of move semantics. + NonZeroU64), /// One or more items. Use when more than one item can be requested without any limit. Unbounded, /// Only one item. Use when only a single item can be requested. diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index 400cf51a294..e536d25d501 100644 --- a/lightning/src/offers/parse.rs +++ b/lightning/src/offers/parse.rs @@ -125,7 +125,8 @@ pub enum Bolt12ParseError { /// being parsed. InvalidBech32Hrp, /// The string could not be bech32 decoded. - Bech32(bech32::Error), + Bech32(/// This is not exported to bindings users as the details don't matter much + bech32::Error), /// The bech32 decoded string could not be decoded as the expected message type. Decode(DecodeError), /// The parsed message has invalid semantics. From eee4138e5c9b0fb00c5a904a651a2636069c4873 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 21 Oct 2023 02:28:53 +0000 Subject: [PATCH 15/23] Hard-code scorer parameters to `ProbabilisticScoringFeeParameters` The scorer currently relies on an associated type for the fee parameters. This isn't supportable at all in bindings, and for lack of a better option we simply hard-code the parameter for all scorers to `ProbabilisticScoringFeeParameters`. --- lightning-background-processor/src/lib.rs | 7 ++-- lightning/src/ln/channelmanager.rs | 4 --- lightning/src/routing/router.rs | 40 +++++++++++++---------- lightning/src/routing/scoring.rs | 16 ++++++--- lightning/src/util/test_utils.rs | 3 +- 5 files changed, 39 insertions(+), 31 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 4bbebf46fe1..ddedc9dacce 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -985,9 +985,7 @@ mod tests { Arc>>, Arc, Arc, - Arc>, - (), - TestScorer> + Arc>> >, Arc>; @@ -1144,9 +1142,10 @@ mod tests { } impl ScoreLookUp for TestScorer { + #[cfg(not(c_bindings))] type ScoreParams = (); fn channel_penalty_msat( - &self, _candidate: &CandidateRouteHop, _usage: ChannelUsage, _score_params: &Self::ScoreParams + &self, _candidate: &CandidateRouteHop, _usage: ChannelUsage, _score_params: &lightning::routing::scoring::ProbabilisticScoringFeeParameters ) -> u64 { unimplemented!(); } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index cf718c94470..b5eaf5aad0a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -966,8 +966,6 @@ pub type SimpleArcChannelManager = ChannelManager< Arc, Arc, Arc>>, Arc>>>, - ProbabilisticScoringFeeParameters, - ProbabilisticScorer>>, Arc>, >>, Arc >; @@ -997,8 +995,6 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = &'g L, &'c KeysManager, &'h RwLock, &'g L>>, - ProbabilisticScoringFeeParameters, - ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L> >, &'g L >; diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index c2f2287f779..0abb90e1f74 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -33,32 +33,32 @@ use core::{cmp, fmt}; use core::ops::Deref; /// A [`Router`] implemented using [`find_route`]. -pub struct DefaultRouter>, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> where +pub struct DefaultRouter>, L: Deref, ES: Deref, S: Deref> where L::Target: Logger, - S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>, + S::Target: for <'a> LockableScore<'a>, ES::Target: EntropySource, { network_graph: G, logger: L, entropy_source: ES, scorer: S, - score_params: SP, + score_params: crate::routing::scoring::ProbabilisticScoringFeeParameters, } -impl>, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> DefaultRouter where +impl>, L: Deref, ES: Deref, S: Deref> DefaultRouter where L::Target: Logger, - S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>, + S::Target: for <'a> LockableScore<'a>, ES::Target: EntropySource, { /// Creates a new router. - pub fn new(network_graph: G, logger: L, entropy_source: ES, scorer: S, score_params: SP) -> Self { + pub fn new(network_graph: G, logger: L, entropy_source: ES, scorer: S, score_params: crate::routing::scoring::ProbabilisticScoringFeeParameters) -> Self { Self { network_graph, logger, entropy_source, scorer, score_params } } } -impl>, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> Router for DefaultRouter where +impl>, L: Deref, ES: Deref, S: Deref> Router for DefaultRouter where L::Target: Logger, - S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>, + S::Target: for <'a> LockableScore<'a>, ES::Target: EntropySource, { fn find_route( @@ -152,9 +152,9 @@ impl>, L: Deref, ES: Deref, S: Deref, SP: Size } } -impl< G: Deref>, L: Deref, ES: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp> MessageRouter for DefaultRouter where +impl< G: Deref>, L: Deref, ES: Deref, S: Deref> MessageRouter for DefaultRouter where L::Target: Logger, - S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>, + S::Target: for <'a> LockableScore<'a>, ES::Target: EntropySource, { fn find_path( @@ -231,8 +231,9 @@ impl<'a, S: Deref> ScorerAccountingForInFlightHtlcs<'a, S> where S::Target: Scor } impl<'a, S: Deref> ScoreLookUp for ScorerAccountingForInFlightHtlcs<'a, S> where S::Target: ScoreLookUp { + #[cfg(not(c_bindings))] type ScoreParams = ::ScoreParams; - fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 { + fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters) -> u64 { let target = match candidate.target() { Some(target) => target, None => return self.scorer.channel_penalty_msat(candidate, usage, score_params), @@ -1805,7 +1806,7 @@ fn sort_first_hop_channels( pub fn find_route( our_node_pubkey: &PublicKey, route_params: &RouteParameters, network_graph: &NetworkGraph, first_hops: Option<&[&ChannelDetails]>, logger: L, - scorer: &S, score_params: &S::ScoreParams, random_seed_bytes: &[u8; 32] + scorer: &S, score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters, random_seed_bytes: &[u8; 32] ) -> Result where L::Target: Logger, GL::Target: Logger { let graph_lock = network_graph.read_only(); @@ -1817,7 +1818,7 @@ where L::Target: Logger, GL::Target: Logger { pub(crate) fn get_route( our_node_pubkey: &PublicKey, route_params: &RouteParameters, network_graph: &ReadOnlyNetworkGraph, - first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S, score_params: &S::ScoreParams, + first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S, score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters, _random_seed_bytes: &[u8; 32] ) -> Result where L::Target: Logger { @@ -3176,9 +3177,10 @@ fn build_route_from_hops_internal( } impl ScoreLookUp for HopScorer { + #[cfg(not(c_bindings))] type ScoreParams = (); fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, - _usage: ChannelUsage, _score_params: &Self::ScoreParams) -> u64 + _usage: ChannelUsage, _score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters) -> u64 { let mut cur_id = self.our_node_id; for i in 0..self.hop_ids.len() { @@ -6528,8 +6530,9 @@ mod tests { fn write(&self, _w: &mut W) -> Result<(), crate::io::Error> { unimplemented!() } } impl ScoreLookUp for BadChannelScorer { + #[cfg(not(c_bindings))] type ScoreParams = (); - fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 { + fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters) -> u64 { if candidate.short_channel_id() == Some(self.short_channel_id) { u64::max_value() } else { 0 } } } @@ -6544,8 +6547,9 @@ mod tests { } impl ScoreLookUp for BadNodeScorer { + #[cfg(not(c_bindings))] type ScoreParams = (); - fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 { + fn channel_penalty_msat(&self, candidate: &CandidateRouteHop, _: ChannelUsage, _score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters) -> u64 { if candidate.target() == Some(self.node_id) { u64::max_value() } else { 0 } } } @@ -8437,7 +8441,7 @@ pub(crate) mod bench_utils { } pub(crate) fn generate_test_routes(graph: &NetworkGraph<&TestLogger>, scorer: &mut S, - score_params: &S::ScoreParams, features: Bolt11InvoiceFeatures, mut seed: u64, + score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters, features: Bolt11InvoiceFeatures, mut seed: u64, starting_amount: u64, route_count: usize, ) -> Vec<(ChannelDetails, PaymentParameters, u64)> { let payer = payer_pubkey(); @@ -8622,7 +8626,7 @@ pub mod benches { fn generate_routes( bench: &mut Criterion, graph: &NetworkGraph<&TestLogger>, mut scorer: S, - score_params: &S::ScoreParams, features: Bolt11InvoiceFeatures, starting_amount: u64, + score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters, features: Bolt11InvoiceFeatures, starting_amount: u64, bench_name: &'static str, ) { let payer = bench_utils::payer_pubkey(); diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index 646405c6287..83de89c64d2 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -93,9 +93,13 @@ macro_rules! define_score { ($($supertrait: path)*) => { /// /// Scoring is in terms of fees willing to be paid in order to avoid routing through a channel. pub trait ScoreLookUp { + #[cfg(not(c_bindings))] /// A configurable type which should contain various passed-in parameters for configuring the scorer, /// on a per-routefinding-call basis through to the scorer methods, /// which are used to determine the parameters for the suitability of channels for use. + /// + /// Note that due to limitations in many other languages' generics features, language bindings + /// use [`ProbabilisticScoringFeeParameters`] for the parameters on all scorers. type ScoreParams; /// Returns the fee in msats willing to be paid to avoid routing `send_amt_msat` through the /// given channel in the direction from `source` to `target`. @@ -106,7 +110,7 @@ pub trait ScoreLookUp { /// [`u64::max_value`] is given to indicate sufficient capacity for the invoice's full amount. /// Thus, implementations should be overflow-safe. fn channel_penalty_msat( - &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams + &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters ) -> u64; } @@ -144,9 +148,10 @@ impl Score for T {} #[cfg(not(c_bindings))] impl> ScoreLookUp for T { + #[cfg(not(c_bindings))] type ScoreParams = S::ScoreParams; fn channel_penalty_msat( - &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams + &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters ) -> u64 { self.deref().channel_penalty_msat(candidate, usage, score_params) } @@ -327,8 +332,9 @@ impl<'a, T: 'a + Score> Deref for MultiThreadedScoreLockRead<'a, T> { #[cfg(c_bindings)] impl<'a, T: Score> ScoreLookUp for MultiThreadedScoreLockRead<'a, T> { + #[cfg(not(c_bindings))] type ScoreParams = T::ScoreParams; - fn channel_penalty_msat(&self, candidate:&CandidateRouteHop, usage: ChannelUsage, score_params: &Self::ScoreParams + fn channel_penalty_msat(&self, candidate:&CandidateRouteHop, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters ) -> u64 { self.0.channel_penalty_msat(candidate, usage, score_params) } @@ -409,8 +415,9 @@ impl FixedPenaltyScorer { } impl ScoreLookUp for FixedPenaltyScorer { + #[cfg(not(c_bindings))] type ScoreParams = (); - fn channel_penalty_msat(&self, _: &CandidateRouteHop, _: ChannelUsage, _score_params: &Self::ScoreParams) -> u64 { + fn channel_penalty_msat(&self, _: &CandidateRouteHop, _: ChannelUsage, _score_params: &ProbabilisticScoringFeeParameters) -> u64 { self.penalty_msat } } @@ -1319,6 +1326,7 @@ DirectedChannelLiquidity { } impl>, L: Deref> ScoreLookUp for ProbabilisticScorer where L::Target: Logger { + #[cfg(not(c_bindings))] type ScoreParams = ProbabilisticScoringFeeParameters; fn channel_penalty_msat( &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 0621c23733c..3e0090f8d17 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -1357,9 +1357,10 @@ impl crate::util::ser::Writeable for TestScorer { } impl ScoreLookUp for TestScorer { + #[cfg(not(c_bindings))] type ScoreParams = (); fn channel_penalty_msat( - &self, candidate: &CandidateRouteHop, usage: ChannelUsage, _score_params: &Self::ScoreParams + &self, candidate: &CandidateRouteHop, usage: ChannelUsage, _score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters ) -> u64 { let short_channel_id = match candidate.globally_unique_short_channel_id() { Some(scid) => scid, From efc148b61c4817a70af44c3e789799e3e4087b89 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 19 Jul 2023 20:09:23 +0000 Subject: [PATCH 16/23] Mark several types no-export which should be exported eventually --- lightning/src/events/bump_transaction.rs | 4 ++++ lightning/src/ln/features.rs | 2 ++ 2 files changed, 6 insertions(+) diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 1b019ba349e..3787f540878 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -265,6 +265,8 @@ impl Utxo { } /// Returns a `Utxo` with the `satisfaction_weight` estimate for a P2WPKH nested in P2SH output. + /// + /// This is not exported to bindings users as WPubkeyHash is not yet exported pub fn new_nested_p2wpkh(outpoint: OutPoint, value: u64, pubkey_hash: &WPubkeyHash) -> Self { let script_sig_size = 1 /* script_sig length */ + 1 /* OP_0 */ + @@ -281,6 +283,8 @@ impl Utxo { } /// Returns a `Utxo` with the `satisfaction_weight` estimate for a SegWit v0 P2WPKH output. + /// + /// This is not exported to bindings users as WPubkeyHash is not yet exported pub fn new_v0_p2wpkh(outpoint: OutPoint, value: u64, pubkey_hash: &WPubkeyHash) -> Self { Self { outpoint, diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 2e732b17aa8..1707e0f8b29 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -440,6 +440,7 @@ pub struct Features { mark: PhantomData, } +/// This is not exported to bindings users but probably should be. impl> core::ops::BitOrAssign for Features { fn bitor_assign(&mut self, rhs: Rhs) { let total_feature_len = cmp::max(self.flags.len(), rhs.borrow().flags.len()); @@ -450,6 +451,7 @@ impl> core::ops::BitOrAssign for Feat } } +/// This is not exported to bindings users but probably should be. impl core::ops::BitOr for Features { type Output = Self; From 77aafd0b13639fee0c9ba3b0d1047ae68c18d505 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 Sep 2023 03:03:38 +0000 Subject: [PATCH 17/23] `crate`-only several BOLT12 methods that require unbounded generics These are not expressible in C/most languages, and thus must be hidden. --- lightning/src/offers/invoice.rs | 2 +- lightning/src/offers/invoice_request.rs | 2 +- lightning/src/offers/merkle.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 48dcfd9f57e..c462e1ff7c3 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -424,7 +424,7 @@ impl UnsignedBolt12Invoice { /// Note: The hash computation may have included unknown, odd TLV records. /// /// This is not exported to bindings users as functions aren't currently mapped. - pub fn sign(mut self, sign: F) -> Result> + pub(crate) fn sign(mut self, sign: F) -> Result> where F: FnOnce(&Self) -> Result { diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 4dd85b352f7..80770ccf233 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -395,7 +395,7 @@ impl UnsignedInvoiceRequest { /// Note: The hash computation may have included unknown, odd TLV records. /// /// This is not exported to bindings users as functions are not yet mapped. - pub fn sign(mut self, sign: F) -> Result> + pub(crate) fn sign(mut self, sign: F) -> Result> where F: FnOnce(&Self) -> Result { diff --git a/lightning/src/offers/merkle.rs b/lightning/src/offers/merkle.rs index ced3b08b7c2..0c5441cf5f5 100644 --- a/lightning/src/offers/merkle.rs +++ b/lightning/src/offers/merkle.rs @@ -76,7 +76,7 @@ impl AsRef for TaggedHash { /// Error when signing messages. #[derive(Debug, PartialEq)] -pub enum SignError { +pub(crate) enum SignError { /// User-defined error when signing the message. Signing(E), /// Error when verifying the produced signature using the given pubkey. From b1236d22579fdbfbedadbdbe6f86b6442e17597f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 31 Jan 2021 20:12:50 -0500 Subject: [PATCH 18/23] Make ChannelMonitor always clonable Rather than `ChannelMonitor` only being clonable when the signer is clonable, we require all signers to be clonable and then make all `ChannelMonitor`s clonable. --- lightning/src/chain/channelmonitor.rs | 2 +- lightning/src/sign/ecdsa.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 6e1841281b7..1284553e74c 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -753,7 +753,7 @@ pub struct ChannelMonitor { pub(super) inner: Mutex>, } -impl Clone for ChannelMonitor where Signer: Clone { +impl Clone for ChannelMonitor { fn clone(&self) -> Self { let inner = self.inner.lock().unwrap().clone(); ChannelMonitor::from_impl(inner) diff --git a/lightning/src/sign/ecdsa.rs b/lightning/src/sign/ecdsa.rs index 2e98213c182..d00800e6232 100644 --- a/lightning/src/sign/ecdsa.rs +++ b/lightning/src/sign/ecdsa.rs @@ -167,4 +167,4 @@ pub trait EcdsaChannelSigner: ChannelSigner { /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor -pub trait WriteableEcdsaChannelSigner: EcdsaChannelSigner + Writeable {} +pub trait WriteableEcdsaChannelSigner: EcdsaChannelSigner + Writeable + Clone {} From a19a6d95d879c2b3d13992f145a3801640bc252d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 24 Sep 2021 18:44:32 +0000 Subject: [PATCH 19/23] Make the custom message traits cloneable as they're deep in nested structs --- lightning/src/ln/wire.rs | 14 +++++++------- lightning/src/onion_message/functional_tests.rs | 2 +- lightning/src/onion_message/messenger.rs | 4 ++-- lightning/src/onion_message/packet.rs | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/wire.rs b/lightning/src/ln/wire.rs index 5087df33b83..855fbfe53d2 100644 --- a/lightning/src/ln/wire.rs +++ b/lightning/src/ln/wire.rs @@ -45,9 +45,9 @@ impl TestEq for T {} /// A Lightning message returned by [`read`] when decoding bytes received over the wire. Each /// variant contains a message from [`msgs`] or otherwise the message type if unknown. #[allow(missing_docs)] -#[derive(Debug)] +#[derive(Clone, Debug)] #[cfg_attr(test, derive(PartialEq))] -pub(crate) enum Message where T: core::fmt::Debug + Type + TestEq { +pub(crate) enum Message where T: Clone + core::fmt::Debug + Type + TestEq { Init(msgs::Init), Error(msgs::ErrorMessage), Warning(msgs::WarningMessage), @@ -407,13 +407,13 @@ pub(crate) use self::encode::Encode; /// Defines a type identifier for sending messages over the wire. /// /// Messages implementing this trait specify a type and must be [`Writeable`]. -pub trait Type: core::fmt::Debug + Writeable { +pub trait Type: core::fmt::Debug + Writeable + Clone { /// Returns the type identifying the message payload. fn type_id(&self) -> u16; } #[cfg(test)] -pub trait Type: core::fmt::Debug + Writeable + PartialEq { +pub trait Type: core::fmt::Debug + Writeable + Clone + PartialEq { fn type_id(&self) -> u16; } @@ -423,12 +423,12 @@ impl Type for () { } #[cfg(test)] -impl Type for T where T: Encode { +impl Type for T where T: Encode { fn type_id(&self) -> u16 { T::TYPE } } #[cfg(not(test))] -impl Type for T where T: Encode { +impl Type for T where T: Encode { fn type_id(&self) -> u16 { T::TYPE } } @@ -775,7 +775,7 @@ mod tests { } } - #[derive(Eq, PartialEq, Debug)] + #[derive(Clone, Eq, PartialEq, Debug)] struct TestCustomMessage {} const CUSTOM_MESSAGE_TYPE : u16 = 9000; diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index 3947e1531cc..f1246531ab0 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/lightning/src/onion_message/functional_tests.rs @@ -427,7 +427,7 @@ fn reply_path() { fn invalid_custom_message_type() { let nodes = create_nodes(2); - #[derive(Debug)] + #[derive(Debug, Clone)] struct InvalidCustomMessage{} impl OnionMessageContents for InvalidCustomMessage { fn tlv_type(&self) -> u64 { diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index f56f425061f..5554497a4b7 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -121,8 +121,8 @@ pub(super) const MAX_TIMER_TICKS: usize = 2; /// &keys_manager, &keys_manager, logger, message_router, &offers_message_handler, /// &custom_message_handler /// ); - -/// # #[derive(Debug)] +/// +/// # #[derive(Debug, Clone)] /// # struct YourCustomMessage {} /// impl Writeable for YourCustomMessage { /// fn write(&self, w: &mut W) -> Result<(), io::Error> { diff --git a/lightning/src/onion_message/packet.rs b/lightning/src/onion_message/packet.rs index d9349fdadbf..e38dbd08931 100644 --- a/lightning/src/onion_message/packet.rs +++ b/lightning/src/onion_message/packet.rs @@ -117,7 +117,7 @@ pub(super) enum Payload { /// The contents of an [`OnionMessage`] as read from the wire. /// /// [`OnionMessage`]: crate::ln::msgs::OnionMessage -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum ParsedOnionMessageContents { /// A message related to BOLT 12 Offers. Offers(OffersMessage), @@ -147,7 +147,7 @@ impl Writeable for ParsedOnionMessageContents { } /// The contents of an onion message. -pub trait OnionMessageContents: Writeable + core::fmt::Debug { +pub trait OnionMessageContents: Writeable + core::fmt::Debug + Clone { /// Returns the TLV type identifying the message contents. MUST be >= 64. fn tlv_type(&self) -> u64; } From 06066446620c7f9d70614adfce5c88891c1042f6 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 17 Dec 2023 19:43:28 +0000 Subject: [PATCH 20/23] Replace `EventsProvider` on `OnionMessageHandler` with a single fn `OnionMessageHandler`s are expected to regularly release a set of nodes which we need to directly connect to to deliver onion messages. In order to do so, they currently extend `EventsProvider`, returning that set as `Event::ConnectionNeeded`s. While this works fine in Rust, the `EventsProvider` interface doesn't map well in bindings due to it taking a flexible trait impl as a method argument. Instead, here, we convert `OnionMessageHandler` to include a single function which returns nodes in the form of a `node_id` and `Vec`. This is a bit simpler, if less flexible, API, and while largely equivalent, is easier to map in bindings. --- lightning-background-processor/src/lib.rs | 31 +++++++++-------------- lightning/src/ln/msgs.rs | 11 +++++++- lightning/src/ln/peer_handler.rs | 1 + lightning/src/onion_message/messenger.rs | 19 ++++---------- 4 files changed, 28 insertions(+), 34 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index ddedc9dacce..dd4eea96040 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -694,7 +694,10 @@ where persister, chain_monitor, chain_monitor.process_pending_events_async(async_event_handler).await, channel_manager, channel_manager.process_pending_events_async(async_event_handler).await, - peer_manager, process_onion_message_handler_events_async(&peer_manager, async_event_handler).await, + peer_manager, + for event in onion_message_handler_events(peer_manager) { + handler(event).await + }, gossip_sync, logger, scorer, should_break, { let fut = Selector { a: channel_manager.get_event_or_persistence_needed_future(), @@ -719,23 +722,11 @@ where ) } -#[cfg(feature = "futures")] -async fn process_onion_message_handler_events_async< - EventHandlerFuture: core::future::Future, - EventHandler: Fn(Event) -> EventHandlerFuture, - PM: 'static + Deref + Send + Sync, ->( - peer_manager: &PM, handler: EventHandler -) -where - PM::Target: APeerManager + Send + Sync, -{ - let events = core::cell::RefCell::new(Vec::new()); - peer_manager.onion_message_handler().process_pending_events(&|e| events.borrow_mut().push(e)); - - for event in events.into_inner() { - handler(event).await - } +fn onion_message_handler_events( + peer_manager: &PM +) -> impl Iterator where PM::Target: APeerManager + Send + Sync { + peer_manager.onion_message_handler().get_and_clear_connections_needed() + .into_iter().map(|(node_id, addresses)| Event::ConnectionNeeded { node_id, addresses }) } #[cfg(feature = "std")] @@ -851,7 +842,9 @@ impl BackgroundProcessor { persister, chain_monitor, chain_monitor.process_pending_events(&event_handler), channel_manager, channel_manager.process_pending_events(&event_handler), peer_manager, - peer_manager.onion_message_handler().process_pending_events(&event_handler), + for event in onion_message_handler_events(&peer_manager) { + event_handler.handle_event(event); + }, gossip_sync, logger, scorer, stop_thread.load(Ordering::Acquire), { Sleeper::from_two_futures( channel_manager.get_event_or_persistence_needed_future(), diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index e833e3ebf04..c5763d08d67 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1632,7 +1632,16 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider { } /// A handler for received [`OnionMessage`]s and for providing generated ones to send. -pub trait OnionMessageHandler: EventsProvider { +pub trait OnionMessageHandler { + /// Because much of the lightning network does not yet support forwarding onion messages, we + /// may need to directly connect to a node which will forward a message for us. In such a case, + /// this method will return the set of nodes which need connection by node_id and the + /// corresponding socket addresses where they may accept incoming connections. + /// + /// Thus, this method should be polled regularly to detect messages await such a direct + /// connection. + fn get_and_clear_connections_needed(&self) -> Vec<(PublicKey, Vec)>; + /// Handle an incoming `onion_message` message from the given peer. fn handle_onion_message(&self, peer_node_id: &PublicKey, msg: &OnionMessage); diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 77208921cc1..3ad75cccefb 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -123,6 +123,7 @@ impl RoutingMessageHandler for IgnoringMessageHandler { fn processing_queue_high(&self) -> bool { false } } impl OnionMessageHandler for IgnoringMessageHandler { + fn get_and_clear_connections_needed(&self) -> Vec<(PublicKey, Vec)> { Vec::new() } fn handle_onion_message(&self, _their_node_id: &PublicKey, _msg: &msgs::OnionMessage) {} fn next_onion_message_for_peer(&self, _peer_node_id: PublicKey) -> Option { None } fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init, _inbound: bool) -> Result<(), ()> { Ok(()) } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 5554497a4b7..a0e674fa45d 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -890,7 +890,7 @@ fn outbound_buffer_full(peer_node_id: &PublicKey, buffer: &HashMap EventsProvider +impl OnionMessageHandler for OnionMessenger where ES::Target: EntropySource, @@ -900,27 +900,18 @@ where OMH::Target: OffersMessageHandler, CMH::Target: CustomOnionMessageHandler, { - fn process_pending_events(&self, handler: H) where H::Target: EventHandler { + fn get_and_clear_connections_needed(&self) -> Vec<(PublicKey, Vec)> { + let mut res = Vec::new(); for (node_id, recipient) in self.message_recipients.lock().unwrap().iter_mut() { if let OnionMessageRecipient::PendingConnection(_, addresses, _) = recipient { if let Some(addresses) = addresses.take() { - handler.handle_event(Event::ConnectionNeeded { node_id: *node_id, addresses }); + res.push((*node_id, addresses)); } } } + res } -} -impl OnionMessageHandler -for OnionMessenger -where - ES::Target: EntropySource, - NS::Target: NodeSigner, - L::Target: Logger, - MR::Target: MessageRouter, - OMH::Target: OffersMessageHandler, - CMH::Target: CustomOnionMessageHandler, -{ fn handle_onion_message(&self, _peer_node_id: &PublicKey, msg: &OnionMessage) { match peel_onion_message( msg, &self.secp_ctx, &*self.node_signer, &*self.logger, &*self.custom_handler From 5d0a963e0808e50d7e948cd043b1d81e916cf13a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 Sep 2023 03:05:09 +0000 Subject: [PATCH 21/23] Avoid slices without inner references As we cannot express slices without inner references in bindings wrappers. --- lightning/src/blinded_path/mod.rs | 8 ++++---- lightning/src/ln/channelmanager.rs | 8 ++++---- lightning/src/routing/gossip.rs | 2 +- lightning/src/routing/router.rs | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lightning/src/blinded_path/mod.rs b/lightning/src/blinded_path/mod.rs index b1fc9e1c0a4..90d926ed125 100644 --- a/lightning/src/blinded_path/mod.rs +++ b/lightning/src/blinded_path/mod.rs @@ -92,7 +92,7 @@ impl BlindedPath { // be in relation to a specific channel. let htlc_maximum_msat = u64::max_value(); Self::new_for_payment( - &[], payee_node_id, payee_tlvs, htlc_maximum_msat, entropy_source, secp_ctx + Vec::new(), payee_node_id, payee_tlvs, htlc_maximum_msat, entropy_source, secp_ctx ) } @@ -106,19 +106,19 @@ impl BlindedPath { /// [`ForwardTlvs`]: crate::blinded_path::payment::ForwardTlvs // TODO: make all payloads the same size with padding + add dummy hops pub fn new_for_payment( - intermediate_nodes: &[payment::ForwardNode], payee_node_id: PublicKey, + intermediate_nodes: Vec, payee_node_id: PublicKey, payee_tlvs: payment::ReceiveTlvs, htlc_maximum_msat: u64, entropy_source: &ES, secp_ctx: &Secp256k1 ) -> Result<(BlindedPayInfo, Self), ()> { let blinding_secret_bytes = entropy_source.get_secure_random_bytes(); let blinding_secret = SecretKey::from_slice(&blinding_secret_bytes[..]).expect("RNG is busted"); - let blinded_payinfo = payment::compute_payinfo(intermediate_nodes, &payee_tlvs, htlc_maximum_msat)?; + let blinded_payinfo = payment::compute_payinfo(&intermediate_nodes, &payee_tlvs, htlc_maximum_msat)?; Ok((blinded_payinfo, BlindedPath { introduction_node_id: intermediate_nodes.first().map_or(payee_node_id, |n| n.node_id), blinding_point: PublicKey::from_secret_key(secp_ctx, &blinding_secret), blinded_hops: payment::blinded_hops( - secp_ctx, intermediate_nodes, payee_node_id, payee_tlvs, &blinding_secret + secp_ctx, &intermediate_nodes, payee_node_id, payee_tlvs, &blinding_secret ).map_err(|_| ())?, })) } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b5eaf5aad0a..2728240c719 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4013,7 +4013,7 @@ where /// [`ChannelUnavailable`]: APIError::ChannelUnavailable /// [`APIMisuseError`]: APIError::APIMisuseError pub fn update_partial_channel_config( - &self, counterparty_node_id: &PublicKey, channel_ids: &[ChannelId], config_update: &ChannelConfigUpdate, + &self, counterparty_node_id: &PublicKey, channel_ids: Vec, config_update: &ChannelConfigUpdate, ) -> Result<(), APIError> { if config_update.cltv_expiry_delta.map(|delta| delta < MIN_CLTV_EXPIRY_DELTA).unwrap_or(false) { return Err(APIError::APIMisuseError { @@ -4027,14 +4027,14 @@ where .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?; let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; - for channel_id in channel_ids { + for channel_id in channel_ids.iter() { if !peer_state.has_channel(channel_id) { return Err(APIError::ChannelUnavailable { err: format!("Channel with id {} not found for the passed counterparty node_id {}", channel_id, counterparty_node_id), }); }; } - for channel_id in channel_ids { + for channel_id in channel_ids.iter() { if let Some(channel_phase) = peer_state.channel_by_id.get_mut(channel_id) { let mut config = channel_phase.context().config(); config.apply(config_update); @@ -4088,7 +4088,7 @@ where /// [`ChannelUnavailable`]: APIError::ChannelUnavailable /// [`APIMisuseError`]: APIError::APIMisuseError pub fn update_channel_config( - &self, counterparty_node_id: &PublicKey, channel_ids: &[ChannelId], config: &ChannelConfig, + &self, counterparty_node_id: &PublicKey, channel_ids: Vec, config: &ChannelConfig, ) -> Result<(), APIError> { return self.update_partial_channel_config(counterparty_node_id, channel_ids, &(*config).into()); } diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 5959874c956..7e3232d5536 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -80,7 +80,7 @@ impl NodeId { } /// Get the public key as an array from this NodeId - pub fn as_array(&self) -> &[u8; PUBLIC_KEY_SIZE] { + pub fn as_array(&self) -> &[u8; 33] { &self.0 } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 0abb90e1f74..a4a32d1cbca 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -132,7 +132,7 @@ impl>, L: Deref, ES: Deref, S: Deref> Router f }) .map(|forward_node| { BlindedPath::new_for_payment( - &[forward_node], recipient, tlvs.clone(), u64::MAX, &*self.entropy_source, secp_ctx + vec![forward_node], recipient, tlvs.clone(), u64::MAX, &*self.entropy_source, secp_ctx ) }) .take(MAX_PAYMENT_PATHS) From 6d26a0c265a62450bb990f7354e92ff6332e451d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 23 Apr 2024 00:04:52 +0000 Subject: [PATCH 22/23] Mark `io_extras` module as no-export (matching its `doc(hidden)`) --- lightning/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lightning/src/lib.rs b/lightning/src/lib.rs index ad51dbe18b7..b8e25db0b50 100644 --- a/lightning/src/lib.rs +++ b/lightning/src/lib.rs @@ -95,7 +95,9 @@ pub use core2::io; #[cfg(not(feature = "std"))] #[doc(hidden)] -/// IO utilities public only for use by in-crate macros. These should not be used externally +/// IO utilities public only for use by in-crate macros. These should not be used externally. +/// +/// This is not exported to bindings users as its not intended for public consumption. pub mod io_extras { use core2::io::{self, Read, Write}; From 6b45237f46590eda6412402fb40c1fb0831ed828 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 25 Apr 2024 14:30:05 +0000 Subject: [PATCH 23/23] Drop completed blocked `ChannelMonitorUpdate`s on startup If a user receives a payment preimage for an outbound payment, the `PaymentSent` event will block any eventual RAA `ChannelMonitorUpdate` from the same channel, assuming it comes in before the event can be processed. If this blocking kicks in, but the flow eventually completes with the RAA `ChannelMonitorUpdate` being persisted, but the `ChannelManager` is only persisted prior to the event being handled, on startup we'll have a fully up-to-date `ChannelMonitor` but a pending, blocked `ChannelMonitorUpdate`. When the `PaymentSent` event is replayed we'll end up trying to apply a redundant `ChannelMonitorUpdate` which will panic. See the test added in this commit for an implementation of this situation. In this commit we fix this issue by simply dropping blocked `ChannelMonitorUpdate`s the same as we do pending ones. --- lightning/src/ln/channel.rs | 20 ++++++++++++++++ lightning/src/ln/channelmanager.rs | 5 ++-- lightning/src/ln/monitor_tests.rs | 37 ++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ffac09b758f..dea2ae760cd 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5179,6 +5179,26 @@ impl Channel where } } + /// On startup, its possible we detect some monitor updates have actually completed (and the + /// ChannelManager was simply stale). In that case, we should simply drop them, which we do + /// here after logging them. + pub fn on_startup_drop_completed_blocked_mon_updates_through(&mut self, logger: &L, loaded_mon_update_id: u64) { + let channel_id = self.context.channel_id(); + self.context.blocked_monitor_updates.retain(|update| { + if update.update.update_id <= loaded_mon_update_id { + log_info!( + logger, + "Dropping completed ChannelMonitorUpdate id {} on channel {} due to a stale ChannelManager", + update.update.update_id, + channel_id, + ); + false + } else { + true + } + }); + } + pub fn blocked_monitor_updates_pending(&self) -> usize { self.context.blocked_monitor_updates.len() } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2728240c719..d65ee589e64 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10365,9 +10365,10 @@ where } } } else { - log_info!(logger, "Successfully loaded channel {} at update_id {} against monitor at update id {}", + channel.on_startup_drop_completed_blocked_mon_updates_through(&logger, monitor.get_latest_update_id()); + log_info!(logger, "Successfully loaded channel {} at update_id {} against monitor at update id {} with {} blocked updates", &channel.context.channel_id(), channel.context.get_latest_monitor_update_id(), - monitor.get_latest_update_id()); + monitor.get_latest_update_id(), channel.blocked_monitor_updates_pending()); if let Some(short_channel_id) = channel.context.get_short_channel_id() { short_to_chan_info.insert(short_channel_id, (channel.context.get_counterparty_node_id(), channel.context.channel_id())); } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index b6270181439..26bec030f13 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -2821,3 +2821,40 @@ fn test_monitor_claims_with_random_signatures() { do_test_monitor_claims_with_random_signatures(true, false); do_test_monitor_claims_with_random_signatures(true, true); } + +#[test] +fn test_event_replay_causing_monitor_replay() { + // In LDK 0.0.121 there was a bug where if a `PaymentSent` event caused an RAA + // `ChannelMonitorUpdate` hold and then the node was restarted after the `PaymentSent` event + // and `ChannelMonitorUpdate` both completed but without persisting the `ChannelManager` we'd + // replay the `ChannelMonitorUpdate` on restart (which is fine, but triggered a safety panic). + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let persister; + let new_chain_monitor; + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let node_deserialized; + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000); + + let payment_preimage = route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0; + + do_claim_payment_along_route(&nodes[0], &[&[&nodes[1]]], false, payment_preimage); + + // At this point the `PaymentSent` event has not been processed but the full commitment signed + // dance has completed. + let serialized_channel_manager = nodes[0].node.encode(); + + // Now process the `PaymentSent` to get the final RAA `ChannelMonitorUpdate`, checking that it + // resulted in a `ChannelManager` persistence request. + nodes[0].node.get_and_clear_needs_persistence(); + expect_payment_sent(&nodes[0], payment_preimage, None, true, true /* expected post-event monitor update*/); + assert!(nodes[0].node.get_and_clear_needs_persistence()); + + let serialized_monitor = get_monitor!(nodes[0], chan.2).encode(); + reload_node!(nodes[0], &serialized_channel_manager, &[&serialized_monitor], persister, new_chain_monitor, node_deserialized); + + // Expect the `PaymentSent` to get replayed, this time without the duplicate monitor update + expect_payment_sent(&nodes[0], payment_preimage, None, false, false /* expected post-event monitor update*/); +}