Skip to content

Commit 80f904c

Browse files
valentinewallacevalentinewallace
committed
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.
1 parent 15b1c9b commit 80f904c

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};
@@ -2992,10 +2992,17 @@ where
29922992
#[cfg(test)]
29932993
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> {
29942994
let _lck = self.total_consistency_lock.read().unwrap();
2995-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv_bytes)
2995+
self.send_payment_along_path(SendAlongPathArgs {
2996+
path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage,
2997+
session_priv_bytes
2998+
})
29962999
}
29973000

2998-
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> {
3001+
fn send_payment_along_path(&self, args: SendAlongPathArgs) -> Result<(), APIError> {
3002+
let SendAlongPathArgs {
3003+
path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage,
3004+
session_priv_bytes
3005+
} = args;
29993006
// The top-level caller should hold the total_consistency_lock read lock.
30003007
debug_assert!(self.total_consistency_lock.try_write().is_err());
30013008

@@ -3125,9 +3132,9 @@ where
31253132
let best_block_height = self.best_block.read().unwrap().height();
31263133
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
31273134
self.pending_outbound_payments
3128-
.send_payment_with_route(route, payment_hash, recipient_onion, payment_id, &self.entropy_source, &self.node_signer, best_block_height,
3129-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3130-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3135+
.send_payment_with_route(route, payment_hash, recipient_onion, payment_id,
3136+
&self.entropy_source, &self.node_signer, best_block_height,
3137+
|args| self.send_payment_along_path(args))
31313138
}
31323139

31333140
/// Similar to [`ChannelManager::send_payment_with_route`], but will automatically find a route based on
@@ -3139,18 +3146,16 @@ where
31393146
.send_payment(payment_hash, recipient_onion, payment_id, retry_strategy, route_params,
31403147
&self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
31413148
&self.entropy_source, &self.node_signer, best_block_height, &self.logger,
3142-
&self.pending_events,
3143-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3144-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3149+
&self.pending_events, |args| self.send_payment_along_path(args))
31453150
}
31463151

31473152
#[cfg(test)]
31483153
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> {
31493154
let best_block_height = self.best_block.read().unwrap().height();
31503155
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
3151-
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,
3152-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3153-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3156+
self.pending_outbound_payments.test_send_payment_internal(route, payment_hash, recipient_onion,
3157+
keysend_preimage, payment_id, recv_value_msat, onion_session_privs, &self.node_signer,
3158+
best_block_height, |args| self.send_payment_along_path(args))
31543159
}
31553160

31563161
#[cfg(test)]
@@ -3204,9 +3209,7 @@ where
32043209
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
32053210
self.pending_outbound_payments.send_spontaneous_payment_with_route(
32063211
route, payment_preimage, recipient_onion, payment_id, &self.entropy_source,
3207-
&self.node_signer, best_block_height,
3208-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3209-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3212+
&self.node_signer, best_block_height, |args| self.send_payment_along_path(args))
32103213
}
32113214

32123215
/// Similar to [`ChannelManager::send_spontaneous_payment`], but will automatically find a route
@@ -3222,9 +3225,7 @@ where
32223225
self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, recipient_onion,
32233226
payment_id, retry_strategy, route_params, &self.router, self.list_usable_channels(),
32243227
|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
3225-
&self.logger, &self.pending_events,
3226-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3227-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3228+
&self.logger, &self.pending_events, |args| self.send_payment_along_path(args))
32283229
}
32293230

32303231
/// Send a payment that is probing the given route for liquidity. We calculate the
@@ -3233,9 +3234,9 @@ where
32333234
pub fn send_probe(&self, path: Path) -> Result<(PaymentHash, PaymentId), PaymentSendFailure> {
32343235
let best_block_height = self.best_block.read().unwrap().height();
32353236
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
3236-
self.pending_outbound_payments.send_probe(path, self.probing_cookie_secret, &self.entropy_source, &self.node_signer, best_block_height,
3237-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
3238-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv))
3237+
self.pending_outbound_payments.send_probe(path, self.probing_cookie_secret,
3238+
&self.entropy_source, &self.node_signer, best_block_height,
3239+
|args| self.send_payment_along_path(args))
32393240
}
32403241

32413242
/// Returns whether a payment with the given [`PaymentHash`] and [`PaymentId`] is, in fact, a
@@ -4039,9 +4040,7 @@ where
40394040
let best_block_height = self.best_block.read().unwrap().height();
40404041
self.pending_outbound_payments.check_retry_payments(&self.router, || self.list_usable_channels(),
40414042
|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
4042-
&self.pending_events, &self.logger,
4043-
|path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv|
4044-
self.send_payment_along_path(path, payment_hash, recipient_onion, total_value, cur_height, payment_id, keysend_preimage, session_priv));
4043+
&self.pending_events, &self.logger, |args| self.send_payment_along_path(args));
40454044

40464045
for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) {
40474046
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)