From 80f904c5ebdd74f81eed4f853ed136ef5a751bbe Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 22 Jun 2023 15:40:24 -0400 Subject: [PATCH] Move send_payment_along_path to take args as struct Cleans up and paves the way for additional arguments to be added more easily, specifically an update_add_htlc's blinding point. --- lightning/src/ln/channelmanager.rs | 47 +++++++++-------- lightning/src/ln/outbound_payment.rs | 77 ++++++++++++++-------------- 2 files changed, 61 insertions(+), 63 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index fcc672a86..37483e049 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -53,7 +53,7 @@ use crate::ln::onion_utils::HTLCFailReason; use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError}; #[cfg(test)] use crate::ln::outbound_payment; -use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment}; +use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment, SendAlongPathArgs}; use crate::ln::wire::Encode; use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, ChannelSigner, WriteableEcdsaChannelSigner}; use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate}; @@ -2992,10 +2992,17 @@ where #[cfg(test)] pub(crate) fn test_send_payment_along_path(&self, path: &Path, payment_hash: &PaymentHash, recipient_onion: RecipientOnionFields, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option, session_priv_bytes: [u8; 32]) -> Result<(), APIError> { let _lck = self.total_consistency_lock.read().unwrap(); - self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv_bytes) + self.send_payment_along_path(SendAlongPathArgs { + path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, + session_priv_bytes + }) } - fn send_payment_along_path(&self, path: &Path, payment_hash: &PaymentHash, recipient_onion: RecipientOnionFields, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option, session_priv_bytes: [u8; 32]) -> Result<(), APIError> { + fn send_payment_along_path(&self, args: SendAlongPathArgs) -> Result<(), APIError> { + let SendAlongPathArgs { + path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, + session_priv_bytes + } = args; // The top-level caller should hold the total_consistency_lock read lock. debug_assert!(self.total_consistency_lock.try_write().is_err()); @@ -3125,9 +3132,9 @@ where let best_block_height = self.best_block.read().unwrap().height(); let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); self.pending_outbound_payments - .send_payment_with_route(route, payment_hash, recipient_onion, payment_id, &self.entropy_source, &self.node_signer, best_block_height, - |path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + .send_payment_with_route(route, payment_hash, recipient_onion, payment_id, + &self.entropy_source, &self.node_signer, best_block_height, + |args| self.send_payment_along_path(args)) } /// Similar to [`ChannelManager::send_payment_with_route`], but will automatically find a route based on @@ -3139,18 +3146,16 @@ where .send_payment(payment_hash, recipient_onion, payment_id, retry_strategy, route_params, &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height, &self.logger, - &self.pending_events, - |path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + &self.pending_events, |args| self.send_payment_along_path(args)) } #[cfg(test)] pub(super) fn test_send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, keysend_preimage: Option, payment_id: PaymentId, recv_value_msat: Option, onion_session_privs: Vec<[u8; 32]>) -> Result<(), PaymentSendFailure> { let best_block_height = self.best_block.read().unwrap().height(); let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); - self.pending_outbound_payments.test_send_payment_internal(route, payment_hash, recipient_onion, keysend_preimage, payment_id, recv_value_msat, onion_session_privs, &self.node_signer, best_block_height, - |path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + self.pending_outbound_payments.test_send_payment_internal(route, payment_hash, recipient_onion, + keysend_preimage, payment_id, recv_value_msat, onion_session_privs, &self.node_signer, + best_block_height, |args| self.send_payment_along_path(args)) } #[cfg(test)] @@ -3204,9 +3209,7 @@ where let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); self.pending_outbound_payments.send_spontaneous_payment_with_route( route, payment_preimage, recipient_onion, payment_id, &self.entropy_source, - &self.node_signer, best_block_height, - |path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + &self.node_signer, best_block_height, |args| self.send_payment_along_path(args)) } /// Similar to [`ChannelManager::send_spontaneous_payment`], but will automatically find a route @@ -3222,9 +3225,7 @@ where self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, recipient_onion, payment_id, retry_strategy, route_params, &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height, - &self.logger, &self.pending_events, - |path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + &self.logger, &self.pending_events, |args| self.send_payment_along_path(args)) } /// Send a payment that is probing the given route for liquidity. We calculate the @@ -3233,9 +3234,9 @@ where pub fn send_probe(&self, path: Path) -> Result<(PaymentHash, PaymentId), PaymentSendFailure> { let best_block_height = self.best_block.read().unwrap().height(); let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); - self.pending_outbound_payments.send_probe(path, self.probing_cookie_secret, &self.entropy_source, &self.node_signer, best_block_height, - |path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + self.pending_outbound_payments.send_probe(path, self.probing_cookie_secret, + &self.entropy_source, &self.node_signer, best_block_height, + |args| self.send_payment_along_path(args)) } /// Returns whether a payment with the given [`PaymentHash`] and [`PaymentId`] is, in fact, a @@ -4039,9 +4040,7 @@ where let best_block_height = self.best_block.read().unwrap().height(); self.pending_outbound_payments.check_retry_payments(&self.router, || self.list_usable_channels(), || self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height, - &self.pending_events, &self.logger, - |path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv)); + &self.pending_events, &self.logger, |args| self.send_payment_along_path(args)); for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) { self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination); diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 2ac24baa5..6d5b06168 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -473,6 +473,18 @@ impl RecipientOnionFields { } } +/// Arguments for [`super::channelmanager::ChannelManager::send_payment_along_path`]. +pub(super) struct SendAlongPathArgs<'a> { + pub path: &'a Path, + pub payment_hash: &'a PaymentHash, + pub recipient_onion: RecipientOnionFields, + pub total_value: u64, + pub cur_height: u32, + pub payment_id: PaymentId, + pub keysend_preimage: &'a Option, + pub session_priv_bytes: [u8; 32], +} + pub(super) struct OutboundPayments { pub(super) pending_outbound_payments: Mutex>, pub(super) retry_lock: Mutex<()>, @@ -499,8 +511,7 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError>, + SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { self.send_payment_internal(payment_id, payment_hash, recipient_onion, None, retry_strategy, route_params, router, first_hops, &compute_inflight_htlcs, entropy_source, node_signer, @@ -515,8 +526,7 @@ impl OutboundPayments { where ES::Target: EntropySource, NS::Target: NodeSigner, - F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(SendAlongPathArgs) -> Result<(), APIError> { let onion_session_privs = self.add_new_pending_payment(payment_hash, recipient_onion.clone(), payment_id, None, route, None, None, entropy_source, best_block_height)?; self.pay_route_internal(route, payment_hash, recipient_onion, None, payment_id, None, @@ -537,8 +547,7 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError>, + SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { let preimage = payment_preimage .unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes())); @@ -557,8 +566,7 @@ impl OutboundPayments { where ES::Target: EntropySource, NS::Target: NodeSigner, - F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(SendAlongPathArgs) -> Result<(), APIError>, { let preimage = payment_preimage .unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes())); @@ -587,8 +595,7 @@ impl OutboundPayments { R::Target: Router, ES::Target: EntropySource, NS::Target: NodeSigner, - SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError>, + SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, IH: Fn() -> InFlightHtlcs, FH: Fn() -> Vec, L::Target: Logger, @@ -658,8 +665,7 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError> + SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { #[cfg(feature = "std")] { if has_expired(&route_params) { @@ -699,8 +705,7 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError> + SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { #[cfg(feature = "std")] { if has_expired(&route_params) { @@ -821,8 +826,7 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError> + SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, { match err { PaymentSendFailure::AllFailedResendSafe(errs) => { @@ -894,8 +898,7 @@ impl OutboundPayments { where ES::Target: EntropySource, NS::Target: NodeSigner, - F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(SendAlongPathArgs) -> Result<(), APIError>, { let payment_id = PaymentId(entropy_source.get_secure_random_bytes()); @@ -989,8 +992,7 @@ impl OutboundPayments { ) -> Result<(), PaymentSendFailure> where NS::Target: NodeSigner, - F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(SendAlongPathArgs) -> Result<(), APIError>, { if route.paths.len() < 1 { return Err(PaymentSendFailure::ParameterError(APIError::InvalidRoute{err: "There must be at least one path to send over".to_owned()})); @@ -1031,9 +1033,11 @@ impl OutboundPayments { let cur_height = best_block_height + 1; let mut results = Vec::new(); debug_assert_eq!(route.paths.len(), onion_session_privs.len()); - for (path, session_priv) in route.paths.iter().zip(onion_session_privs.into_iter()) { - let mut path_res = send_payment_along_path(&path, &payment_hash, recipient_onion.clone(), - total_value, cur_height, payment_id, &keysend_preimage, session_priv); + for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.into_iter()) { + let mut path_res = send_payment_along_path(SendAlongPathArgs { + path: &path, payment_hash: &payment_hash, recipient_onion: recipient_onion.clone(), + total_value, cur_height, payment_id, keysend_preimage: &keysend_preimage, session_priv_bytes + }); match path_res { Ok(_) => {}, Err(APIError::MonitorUpdateInProgress) => { @@ -1044,7 +1048,7 @@ impl OutboundPayments { Err(_) => { let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap(); if let Some(payment) = pending_outbounds.get_mut(&payment_id) { - let removed = payment.remove(&session_priv, Some(path)); + let removed = payment.remove(&session_priv_bytes, Some(path)); debug_assert!(removed, "This can't happen as the payment has an entry for this path added by callers"); } else { debug_assert!(false, "This can't happen as the payment was added by callers"); @@ -1098,8 +1102,7 @@ impl OutboundPayments { ) -> Result<(), PaymentSendFailure> where NS::Target: NodeSigner, - F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId, - &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(SendAlongPathArgs) -> Result<(), APIError>, { self.pay_route_internal(route, payment_hash, recipient_onion, keysend_preimage, payment_id, recv_value_msat, onion_session_privs, node_signer, best_block_height, @@ -1480,8 +1483,8 @@ mod tests { &&keys_manager, 0).unwrap(); outbound_payments.retry_payment_internal( PaymentHash([0; 32]), PaymentId([0; 32]), expired_route_params, &&router, vec![], - &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, &|_, _, _, _, _, _, _, _| Ok(())); + &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, + &|_| Ok(())); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 1); if let Event::PaymentFailed { ref reason, .. } = events[0].0 { @@ -1491,8 +1494,7 @@ mod tests { let err = outbound_payments.send_payment( PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), Retry::Attempts(0), expired_route_params, &&router, vec![], || InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, |_, _, _, _, _, _, _, _| Ok(())).unwrap_err(); + &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, |_| Ok(())).unwrap_err(); if let RetryableSendFailure::PaymentExpired = err { } else { panic!("Unexpected error"); } } } @@ -1528,8 +1530,8 @@ mod tests { &&keys_manager, 0).unwrap(); outbound_payments.retry_payment_internal( PaymentHash([0; 32]), PaymentId([0; 32]), route_params, &&router, vec![], - &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, &|_, _, _, _, _, _, _, _| Ok(())); + &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, + &|_| Ok(())); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 1); if let Event::PaymentFailed { .. } = events[0].0 { } else { panic!("Unexpected event"); } @@ -1537,8 +1539,7 @@ mod tests { let err = outbound_payments.send_payment( PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), Retry::Attempts(0), route_params, &&router, vec![], || InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, |_, _, _, _, _, _, _, _| Ok(())).unwrap_err(); + &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, |_| Ok(())).unwrap_err(); if let RetryableSendFailure::RouteNotFound = err { } else { panic!("Unexpected error"); } } @@ -1587,8 +1588,7 @@ mod tests { PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, - |_, _, _, _, _, _, _, _| Err(APIError::ChannelUnavailable { err: "test".to_owned() })) - .unwrap(); + |_| Err(APIError::ChannelUnavailable { err: "test".to_owned() })).unwrap(); let mut events = pending_events.lock().unwrap(); assert_eq!(events.len(), 2); if let Event::PaymentPathFailed { @@ -1606,7 +1606,7 @@ mod tests { PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, - |_, _, _, _, _, _, _, _| Err(APIError::MonitorUpdateInProgress)).unwrap(); + |_| Err(APIError::MonitorUpdateInProgress)).unwrap(); assert_eq!(pending_events.lock().unwrap().len(), 0); // Ensure that any other error will result in a PaymentPathFailed event but no blamed scid. @@ -1614,8 +1614,7 @@ mod tests { PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([1; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, - |_, _, _, _, _, _, _, _| Err(APIError::APIMisuseError { err: "test".to_owned() })) - .unwrap(); + |_| Err(APIError::APIMisuseError { err: "test".to_owned() })).unwrap(); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 2); if let Event::PaymentPathFailed { -- 2.39.5