Skip to content

Commit c383f06

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #2370 from valentinewallace/2023-06-send-along-path-args
Update `send_payment_along_path` to take its args as struct
2 parents e13ff10 + 80f904c commit c383f06

File tree

2 files changed

+61
-63
lines changed

2 files changed

+61
-63
lines changed

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ use crate::ln::onion_utils::HTLCFailReason;
5353
use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
5454
#[cfg(test)]
5555
use crate::ln::outbound_payment;
56-
use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment};
56+
use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment, SendAlongPathArgs};
5757
use crate::ln::wire::Encode;
5858
use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, ChannelSigner, WriteableEcdsaChannelSigner};
5959
use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate};
@@ -3097,10 +3097,17 @@ where
30973097
#[cfg(test)]
30983098
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<PaymentPreimage>, session_priv_bytes: [u8; 32]) -> Result<(), APIError> {
30993099
let _lck = self.total_consistency_lock.read().unwrap();
3100-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv_bytes)
3100+
self.send_payment_along_path(SendAlongPathArgs {
3101+
path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage,
3102+
session_priv_bytes
3103+
})
31013104
}
31023105

3103-
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<PaymentPreimage>, session_priv_bytes: [u8; 32]) -> Result<(), APIError> {
3106+
fn send_payment_along_path(&self, args: SendAlongPathArgs) -> Result<(), APIError> {
3107+
let SendAlongPathArgs {
3108+
path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage,
3109+
session_priv_bytes
3110+
} = args;
31043111
// The top-level caller should hold the total_consistency_lock read lock.
31053112
debug_assert!(self.total_consistency_lock.try_write().is_err());
31063113

@@ -3230,9 +3237,9 @@ where
32303237
let best_block_height = self.best_block.read().unwrap().height();
32313238
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
32323239
self.pending_outbound_payments
3233-
.send_payment_with_route(route, payment_hash, recipient_onion, payment_id, &self.entropy_source, &self.node_signer, best_block_height,
3234-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3235-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3240+
.send_payment_with_route(route, payment_hash, recipient_onion, payment_id,
3241+
&self.entropy_source, &self.node_signer, best_block_height,
3242+
|args| self.send_payment_along_path(args))
32363243
}
32373244

32383245
/// Similar to [`ChannelManager::send_payment_with_route`], but will automatically find a route based on
@@ -3244,18 +3251,16 @@ where
32443251
.send_payment(payment_hash, recipient_onion, payment_id, retry_strategy, route_params,
32453252
&self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
32463253
&self.entropy_source, &self.node_signer, best_block_height, &self.logger,
3247-
&self.pending_events,
3248-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3249-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3254+
&self.pending_events, |args| self.send_payment_along_path(args))
32503255
}
32513256

32523257
#[cfg(test)]
32533258
pub(super) fn test_send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, keysend_preimage: Option<PaymentPreimage>, payment_id: PaymentId, recv_value_msat: Option<u64>, onion_session_privs: Vec<[u8; 32]>) -> Result<(), PaymentSendFailure> {
32543259
let best_block_height = self.best_block.read().unwrap().height();
32553260
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
3256-
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,
3257-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3258-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3261+
self.pending_outbound_payments.test_send_payment_internal(route, payment_hash, recipient_onion,
3262+
keysend_preimage, payment_id, recv_value_msat, onion_session_privs, &self.node_signer,
3263+
best_block_height, |args| self.send_payment_along_path(args))
32593264
}
32603265

32613266
#[cfg(test)]
@@ -3309,9 +3314,7 @@ where
33093314
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
33103315
self.pending_outbound_payments.send_spontaneous_payment_with_route(
33113316
route, payment_preimage, recipient_onion, payment_id, &self.entropy_source,
3312-
&self.node_signer, best_block_height,
3313-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3314-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3317+
&self.node_signer, best_block_height, |args| self.send_payment_along_path(args))
33153318
}
33163319

33173320
/// Similar to [`ChannelManager::send_spontaneous_payment`], but will automatically find a route
@@ -3327,9 +3330,7 @@ where
33273330
self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, recipient_onion,
33283331
payment_id, retry_strategy, route_params, &self.router, self.list_usable_channels(),
33293332
|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
3330-
&self.logger, &self.pending_events,
3331-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3332-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3333+
&self.logger, &self.pending_events, |args| self.send_payment_along_path(args))
33333334
}
33343335

33353336
/// Send a payment that is probing the given route for liquidity. We calculate the
@@ -3338,9 +3339,9 @@ where
33383339
pub fn send_probe(&self, path: Path) -> Result<(PaymentHash, PaymentId), PaymentSendFailure> {
33393340
let best_block_height = self.best_block.read().unwrap().height();
33403341
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
3341-
self.pending_outbound_payments.send_probe(path, self.probing_cookie_secret, &self.entropy_source, &self.node_signer, best_block_height,
3342-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3343-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3342+
self.pending_outbound_payments.send_probe(path, self.probing_cookie_secret,
3343+
&self.entropy_source, &self.node_signer, best_block_height,
3344+
|args| self.send_payment_along_path(args))
33443345
}
33453346

33463347
/// Returns whether a payment with the given [`PaymentHash`] and [`PaymentId`] is, in fact, a
@@ -4166,9 +4167,7 @@ where
41664167
let best_block_height = self.best_block.read().unwrap().height();
41674168
self.pending_outbound_payments.check_retry_payments(&self.router, || self.list_usable_channels(),
41684169
|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
4169-
&self.pending_events, &self.logger,
4170-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
4171-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv));
4170+
&self.pending_events, &self.logger, |args| self.send_payment_along_path(args));
41724171

41734172
for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) {
41744173
self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination);

‎lightning/src/ln/outbound_payment.rs‎

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,18 @@ impl RecipientOnionFields {
473473
}
474474
}
475475

476+
/// Arguments for [`super::channelmanager::ChannelManager::send_payment_along_path`].
477+
pub(super) struct SendAlongPathArgs<'a> {
478+
pub path: &'a Path,
479+
pub payment_hash: &'a PaymentHash,
480+
pub recipient_onion: RecipientOnionFields,
481+
pub total_value: u64,
482+
pub cur_height: u32,
483+
pub payment_id: PaymentId,
484+
pub keysend_preimage: &'a Option<PaymentPreimage>,
485+
pub session_priv_bytes: [u8; 32],
486+
}
487+
476488
pub(super) struct OutboundPayments {
477489
pub(super) pending_outbound_payments: Mutex<HashMap<PaymentId, PendingOutboundPayment>>,
478490
pub(super) retry_lock: Mutex<()>,
@@ -499,8 +511,7 @@ impl OutboundPayments {
499511
NS::Target: NodeSigner,
500512
L::Target: Logger,
501513
IH: Fn() -> InFlightHtlcs,
502-
SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
503-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
514+
SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
504515
{
505516
self.send_payment_internal(payment_id, payment_hash, recipient_onion, None, retry_strategy,
506517
route_params, router, first_hops, &compute_inflight_htlcs, entropy_source, node_signer,
@@ -515,8 +526,7 @@ impl OutboundPayments {
515526
where
516527
ES::Target: EntropySource,
517528
NS::Target: NodeSigner,
518-
F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
519-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
529+
F: Fn(SendAlongPathArgs) -> Result<(), APIError>
520530
{
521531
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)?;
522532
self.pay_route_internal(route, payment_hash, recipient_onion, None, payment_id, None,
@@ -537,8 +547,7 @@ impl OutboundPayments {
537547
NS::Target: NodeSigner,
538548
L::Target: Logger,
539549
IH: Fn() -> InFlightHtlcs,
540-
SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
541-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
550+
SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
542551
{
543552
let preimage = payment_preimage
544553
.unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes()));
@@ -557,8 +566,7 @@ impl OutboundPayments {
557566
where
558567
ES::Target: EntropySource,
559568
NS::Target: NodeSigner,
560-
F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
561-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
569+
F: Fn(SendAlongPathArgs) -> Result<(), APIError>,
562570
{
563571
let preimage = payment_preimage
564572
.unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes()));
@@ -587,8 +595,7 @@ impl OutboundPayments {
587595
R::Target: Router,
588596
ES::Target: EntropySource,
589597
NS::Target: NodeSigner,
590-
SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
591-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
598+
SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
592599
IH: Fn() -> InFlightHtlcs,
593600
FH: Fn() -> Vec<ChannelDetails>,
594601
L::Target: Logger,
@@ -658,8 +665,7 @@ impl OutboundPayments {
658665
NS::Target: NodeSigner,
659666
L::Target: Logger,
660667
IH: Fn() -> InFlightHtlcs,
661-
SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
662-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
668+
SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
663669
{
664670
#[cfg(feature = "std")] {
665671
if has_expired(&route_params) {
@@ -699,8 +705,7 @@ impl OutboundPayments {
699705
NS::Target: NodeSigner,
700706
L::Target: Logger,
701707
IH: Fn() -> InFlightHtlcs,
702-
SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
703-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
708+
SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
704709
{
705710
#[cfg(feature = "std")] {
706711
if has_expired(&route_params) {
@@ -821,8 +826,7 @@ impl OutboundPayments {
821826
NS::Target: NodeSigner,
822827
L::Target: Logger,
823828
IH: Fn() -> InFlightHtlcs,
824-
SP: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
825-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
829+
SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
826830
{
827831
match err {
828832
PaymentSendFailure::AllFailedResendSafe(errs) => {
@@ -894,8 +898,7 @@ impl OutboundPayments {
894898
where
895899
ES::Target: EntropySource,
896900
NS::Target: NodeSigner,
897-
F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
898-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
901+
F: Fn(SendAlongPathArgs) -> Result<(), APIError>,
899902
{
900903
let payment_id = PaymentId(entropy_source.get_secure_random_bytes());
901904

@@ -989,8 +992,7 @@ impl OutboundPayments {
989992
) -> Result<(), PaymentSendFailure>
990993
where
991994
NS::Target: NodeSigner,
992-
F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
993-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
995+
F: Fn(SendAlongPathArgs) -> Result<(), APIError>,
994996
{
995997
if route.paths.len() < 1 {
996998
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 {
10311033
let cur_height = best_block_height + 1;
10321034
let mut results = Vec::new();
10331035
debug_assert_eq!(route.paths.len(), onion_session_privs.len());
1034-
for (path, session_priv) in route.paths.iter().zip(onion_session_privs.into_iter()) {
1035-
let mut path_res = send_payment_along_path(&path, &payment_hash, recipient_onion.clone(),
1036-
total_value, cur_height, payment_id, &keysend_preimage, session_priv);
1036+
for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.into_iter()) {
1037+
let mut path_res = send_payment_along_path(SendAlongPathArgs {
1038+
path: &path, payment_hash: &payment_hash, recipient_onion: recipient_onion.clone(),
1039+
total_value, cur_height, payment_id, keysend_preimage: &keysend_preimage, session_priv_bytes
1040+
});
10371041
match path_res {
10381042
Ok(_) => {},
10391043
Err(APIError::MonitorUpdateInProgress) => {
@@ -1044,7 +1048,7 @@ impl OutboundPayments {
10441048
Err(_) => {
10451049
let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
10461050
if let Some(payment) = pending_outbounds.get_mut(&payment_id) {
1047-
let removed = payment.remove(&session_priv, Some(path));
1051+
let removed = payment.remove(&session_priv_bytes, Some(path));
10481052
debug_assert!(removed, "This can't happen as the payment has an entry for this path added by callers");
10491053
} else {
10501054
debug_assert!(false, "This can't happen as the payment was added by callers");
@@ -1098,8 +1102,7 @@ impl OutboundPayments {
10981102
) -> Result<(), PaymentSendFailure>
10991103
where
11001104
NS::Target: NodeSigner,
1101-
F: Fn(&Path, &PaymentHash, RecipientOnionFields, u64, u32, PaymentId,
1102-
&Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
1105+
F: Fn(SendAlongPathArgs) -> Result<(), APIError>,
11031106
{
11041107
self.pay_route_internal(route, payment_hash, recipient_onion, keysend_preimage, payment_id,
11051108
recv_value_msat, onion_session_privs, node_signer, best_block_height,
@@ -1480,8 +1483,8 @@ mod tests {
14801483
&&keys_manager, 0).unwrap();
14811484
outbound_payments.retry_payment_internal(
14821485
PaymentHash([0; 32]), PaymentId([0; 32]), expired_route_params, &&router, vec![],
1483-
&|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
1484-
&pending_events, &|_, _, _, _, _, _, _, _| Ok(()));
1486+
&|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events,
1487+
&|_| Ok(()));
14851488
let events = pending_events.lock().unwrap();
14861489
assert_eq!(events.len(), 1);
14871490
if let Event::PaymentFailed { ref reason, .. } = events[0].0 {
@@ -1491,8 +1494,7 @@ mod tests {
14911494
let err = outbound_payments.send_payment(
14921495
PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]),
14931496
Retry::Attempts(0), expired_route_params, &&router, vec![], || InFlightHtlcs::new(),
1494-
&&keys_manager, &&keys_manager, 0, &&logger,
1495-
&pending_events, |_, _, _, _, _, _, _, _| Ok(())).unwrap_err();
1497+
&&keys_manager, &&keys_manager, 0, &&logger, &pending_events, |_| Ok(())).unwrap_err();
14961498
if let RetryableSendFailure::PaymentExpired = err { } else { panic!("Unexpected error"); }
14971499
}
14981500
}
@@ -1528,17 +1530,16 @@ mod tests {
15281530
&&keys_manager, 0).unwrap();
15291531
outbound_payments.retry_payment_internal(
15301532
PaymentHash([0; 32]), PaymentId([0; 32]), route_params, &&router, vec![],
1531-
&|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
1532-
&pending_events, &|_, _, _, _, _, _, _, _| Ok(()));
1533+
&|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events,
1534+
&|_| Ok(()));
15331535
let events = pending_events.lock().unwrap();
15341536
assert_eq!(events.len(), 1);
15351537
if let Event::PaymentFailed { .. } = events[0].0 { } else { panic!("Unexpected event"); }
15361538
} else {
15371539
let err = outbound_payments.send_payment(
15381540
PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]),
15391541
Retry::Attempts(0), route_params, &&router, vec![], || InFlightHtlcs::new(),
1540-
&&keys_manager, &&keys_manager, 0, &&logger,
1541-
&pending_events, |_, _, _, _, _, _, _, _| Ok(())).unwrap_err();
1542+
&&keys_manager, &&keys_manager, 0, &&logger, &pending_events, |_| Ok(())).unwrap_err();
15421543
if let RetryableSendFailure::RouteNotFound = err {
15431544
} else { panic!("Unexpected error"); }
15441545
}
@@ -1587,8 +1588,7 @@ mod tests {
15871588
PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]),
15881589
Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(),
15891590
&&keys_manager, &&keys_manager, 0, &&logger, &pending_events,
1590-
|_, _, _, _, _, _, _, _| Err(APIError::ChannelUnavailable { err: "test".to_owned() }))
1591-
.unwrap();
1591+
|_| Err(APIError::ChannelUnavailable { err: "test".to_owned() })).unwrap();
15921592
let mut events = pending_events.lock().unwrap();
15931593
assert_eq!(events.len(), 2);
15941594
if let Event::PaymentPathFailed {
@@ -1606,16 +1606,15 @@ mod tests {
16061606
PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([0; 32]),
16071607
Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(),
16081608
&&keys_manager, &&keys_manager, 0, &&logger, &pending_events,
1609-
|_, _, _, _, _, _, _, _| Err(APIError::MonitorUpdateInProgress)).unwrap();
1609+
|_| Err(APIError::MonitorUpdateInProgress)).unwrap();
16101610
assert_eq!(pending_events.lock().unwrap().len(), 0);
16111611

16121612
// Ensure that any other error will result in a PaymentPathFailed event but no blamed scid.
16131613
outbound_payments.send_payment(
16141614
PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(), PaymentId([1; 32]),
16151615
Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(),
16161616
&&keys_manager, &&keys_manager, 0, &&logger, &pending_events,
1617-
|_, _, _, _, _, _, _, _| Err(APIError::APIMisuseError { err: "test".to_owned() }))
1618-
.unwrap();
1617+
|_| Err(APIError::APIMisuseError { err: "test".to_owned() })).unwrap();
16191618
let events = pending_events.lock().unwrap();
16201619
assert_eq!(events.len(), 2);
16211620
if let Event::PaymentPathFailed {

0 commit comments

Comments
 (0)