Skip to content

Commit b77f03e

Browse files
valentinewallacevalentinewallace
authored
Merge pull request #2002 from valentinewallace/2023-01-keysend-retries
Support auto-retrying keysend payments in `ChannelManager`
2 parents a7293fd + 9f01092 commit b77f03e

File tree

3 files changed

+80
-23
lines changed

3 files changed

+80
-23
lines changed

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2558,7 +2558,21 @@ where
25582558
/// [`send_payment`]: Self::send_payment
25592559
pub fn send_spontaneous_payment(&self, route: &Route, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId) -> Result<PaymentHash, PaymentSendFailure> {
25602560
let best_block_height = self.best_block.read().unwrap().height();
2561-
self.pending_outbound_payments.send_spontaneous_payment(route, payment_preimage, payment_id, &self.entropy_source, &self.node_signer, best_block_height,
2561+
self.pending_outbound_payments.send_spontaneous_payment_with_route(
2562+
route, payment_preimage, payment_id, &self.entropy_source, &self.node_signer,
2563+
best_block_height,
2564+
|path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
2565+
self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
2566+
}
2567+
2568+
/// Similar to [`ChannelManager::send_spontaneous_payment`], but will automatically find a route
2569+
/// based on `route_params` and retry failed payment paths based on `retry_strategy`.
2570+
pub fn send_spontaneous_payment_with_retry(&self, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<PaymentHash, PaymentSendFailure> {
2571+
let best_block_height = self.best_block.read().unwrap().height();
2572+
self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, payment_id,
2573+
retry_strategy, route_params, &self.router, self.list_usable_channels(),
2574+
self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height,
2575+
&self.logger,
25622576
|path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
25632577
self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
25642578
}
@@ -7373,6 +7387,7 @@ where
73737387
session_privs: [session_priv_bytes].iter().map(|a| *a).collect(),
73747388
payment_hash: htlc.payment_hash,
73757389
payment_secret,
7390+
keysend_preimage: None, // only used for retries, and we'll never retry on startup
73767391
pending_amt_msat: path_amt,
73777392
pending_fee_msat: Some(path_fee),
73787393
total_msat: path_amt,

‎lightning/src/ln/outbound_payment.rs‎

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ pub(crate) enum PendingOutboundPayment {
4848
session_privs: HashSet<[u8; 32]>,
4949
payment_hash: PaymentHash,
5050
payment_secret: Option<PaymentSecret>,
51+
keysend_preimage: Option<PaymentPreimage>,
5152
pending_amt_msat: u64,
5253
/// Used to track the fee paid. Only present if the payment was serialized on 0.0.103+.
5354
pending_fee_msat: Option<u64>,
@@ -415,7 +416,7 @@ impl OutboundPayments {
415416
F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
416417
u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
417418
{
418-
self.pay_internal(payment_id, Some((payment_hash, payment_secret, retry_strategy)),
419+
self.pay_internal(payment_id, Some((payment_hash, payment_secret, None, retry_strategy)),
419420
route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
420421
best_block_height, logger, &send_payment_along_path)
421422
.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
@@ -432,13 +433,37 @@ impl OutboundPayments {
432433
F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
433434
u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
434435
{
435-
let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, None, None, entropy_source, best_block_height)?;
436+
let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, None, route, None, None, entropy_source, best_block_height)?;
436437
self.pay_route_internal(route, payment_hash, payment_secret, None, payment_id, None,
437438
onion_session_privs, node_signer, best_block_height, &send_payment_along_path)
438439
.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
439440
}
440441

441-
pub(super) fn send_spontaneous_payment<ES: Deref, NS: Deref, F>(
442+
pub(super) fn send_spontaneous_payment<R: Deref, ES: Deref, NS: Deref, F, L: Deref>(
443+
&self, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId,
444+
retry_strategy: Retry, route_params: RouteParameters, router: &R,
445+
first_hops: Vec<ChannelDetails>, inflight_htlcs: InFlightHtlcs, entropy_source: &ES,
446+
node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: F
447+
) -> Result<PaymentHash, PaymentSendFailure>
448+
where
449+
R::Target: Router,
450+
ES::Target: EntropySource,
451+
NS::Target: NodeSigner,
452+
L::Target: Logger,
453+
F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
454+
u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
455+
{
456+
let preimage = payment_preimage
457+
.unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes()));
458+
let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
459+
self.pay_internal(payment_id, Some((payment_hash, &None, Some(preimage), retry_strategy)),
460+
route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
461+
best_block_height, logger, &send_payment_along_path)
462+
.map(|()| payment_hash)
463+
.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
464+
}
465+
466+
pub(super) fn send_spontaneous_payment_with_route<ES: Deref, NS: Deref, F>(
442467
&self, route: &Route, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId,
443468
entropy_source: &ES, node_signer: &NS, best_block_height: u32, send_payment_along_path: F
444469
) -> Result<PaymentHash, PaymentSendFailure>
@@ -448,12 +473,10 @@ impl OutboundPayments {
448473
F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
449474
u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
450475
{
451-
let preimage = match payment_preimage {
452-
Some(p) => p,
453-
None => PaymentPreimage(entropy_source.get_secure_random_bytes()),
454-
};
476+
let preimage = payment_preimage
477+
.unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes()));
455478
let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
456-
let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, None, None, entropy_source, best_block_height)?;
479+
let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, Some(preimage), &route, None, None, entropy_source, best_block_height)?;
457480

458481
match self.pay_route_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) {
459482
Ok(()) => Ok(payment_hash),
@@ -511,7 +534,7 @@ impl OutboundPayments {
511534

512535
fn pay_internal<R: Deref, NS: Deref, ES: Deref, F, L: Deref>(
513536
&self, payment_id: PaymentId,
514-
initial_send_info: Option<(PaymentHash, &Option<PaymentSecret>, Retry)>,
537+
initial_send_info: Option<(PaymentHash, &Option<PaymentSecret>, Option<PaymentPreimage>, Retry)>,
515538
route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>,
516539
inflight_htlcs: InFlightHtlcs, entropy_source: &ES, node_signer: &NS, best_block_height: u32,
517540
logger: &L, send_payment_along_path: &F,
@@ -539,8 +562,8 @@ impl OutboundPayments {
539562
err: format!("Failed to find a route for payment {}: {:?}", log_bytes!(payment_id.0), e), // TODO: add APIError::RouteNotFound
540563
}))?;
541564

542-
let res = if let Some((payment_hash, payment_secret, retry_strategy)) = initial_send_info {
543-
let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, Some(retry_strategy), Some(route_params.payment_params.clone()), entropy_source, best_block_height)?;
565+
let res = if let Some((payment_hash, payment_secret, keysend_preimage, retry_strategy)) = initial_send_info {
566+
let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, keysend_preimage, &route, Some(retry_strategy), Some(route_params.payment_params.clone()), entropy_source, best_block_height)?;
544567
self.pay_route_internal(&route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path)
545568
} else {
546569
self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path)
@@ -596,21 +619,21 @@ impl OutboundPayments {
596619
onion_session_privs.push(entropy_source.get_secure_random_bytes());
597620
}
598621

599-
let (total_msat, payment_hash, payment_secret) = {
622+
let (total_msat, payment_hash, payment_secret, keysend_preimage) = {
600623
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
601624
match outbounds.get_mut(&payment_id) {
602625
Some(payment) => {
603626
let res = match payment {
604627
PendingOutboundPayment::Retryable {
605-
total_msat, payment_hash, payment_secret, pending_amt_msat, ..
628+
total_msat, payment_hash, keysend_preimage, payment_secret, pending_amt_msat, ..
606629
} => {
607630
let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum();
608631
if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
609632
return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
610633
err: format!("retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat).to_string()
611634
}))
612635
}
613-
(*total_msat, *payment_hash, *payment_secret)
636+
(*total_msat, *payment_hash, *payment_secret, *keysend_preimage)
614637
},
615638
PendingOutboundPayment::Legacy { .. } => {
616639
return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
@@ -645,7 +668,7 @@ impl OutboundPayments {
645668
})),
646669
}
647670
};
648-
self.pay_route_internal(route, payment_hash, &payment_secret, None, payment_id, Some(total_msat), onion_session_privs, node_signer, best_block_height, &send_payment_along_path)
671+
self.pay_route_internal(route, payment_hash, &payment_secret, keysend_preimage, payment_id, Some(total_msat), onion_session_privs, node_signer, best_block_height, &send_payment_along_path)
649672
}
650673

651674
pub(super) fn send_probe<ES: Deref, NS: Deref, F>(
@@ -669,7 +692,7 @@ impl OutboundPayments {
669692
}
670693

671694
let route = Route { paths: vec![hops], payment_params: None };
672-
let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, None, None, entropy_source, best_block_height)?;
695+
let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, None, &route, None, None, entropy_source, best_block_height)?;
673696

674697
match self.pay_route_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) {
675698
Ok(()) => Ok((payment_hash, payment_id)),
@@ -685,13 +708,13 @@ impl OutboundPayments {
685708
&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId,
686709
route: &Route, retry_strategy: Option<Retry>, entropy_source: &ES, best_block_height: u32
687710
) -> Result<Vec<[u8; 32]>, PaymentSendFailure> where ES::Target: EntropySource {
688-
self.add_new_pending_payment(payment_hash, payment_secret, payment_id, route, retry_strategy, None, entropy_source, best_block_height)
711+
self.add_new_pending_payment(payment_hash, payment_secret, payment_id, None, route, retry_strategy, None, entropy_source, best_block_height)
689712
}
690713

691714
pub(super) fn add_new_pending_payment<ES: Deref>(
692715
&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId,
693-
route: &Route, retry_strategy: Option<Retry>, payment_params: Option<PaymentParameters>,
694-
entropy_source: &ES, best_block_height: u32
716+
keysend_preimage: Option<PaymentPreimage>, route: &Route, retry_strategy: Option<Retry>,
717+
payment_params: Option<PaymentParameters>, entropy_source: &ES, best_block_height: u32
695718
) -> Result<Vec<[u8; 32]>, PaymentSendFailure> where ES::Target: EntropySource {
696719
let mut onion_session_privs = Vec::with_capacity(route.paths.len());
697720
for _ in 0..route.paths.len() {
@@ -711,6 +734,7 @@ impl OutboundPayments {
711734
pending_fee_msat: Some(0),
712735
payment_hash,
713736
payment_secret,
737+
keysend_preimage,
714738
starting_block_height: best_block_height,
715739
total_msat: route.get_total_amount(),
716740
});
@@ -1153,6 +1177,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
11531177
(2, payment_hash, required),
11541178
(3, payment_params, option),
11551179
(4, payment_secret, option),
1180+
(5, keysend_preimage, option),
11561181
(6, total_msat, required),
11571182
(8, pending_amt_msat, required),
11581183
(10, starting_block_height, required),
@@ -1247,9 +1272,9 @@ mod tests {
12471272
Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }));
12481273

12491274
let err = if on_retry {
1250-
outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]),
1251-
&Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)), Some(route_params.payment_params.clone()),
1252-
&&keys_manager, 0).unwrap();
1275+
outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]), None,
1276+
&Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)),
1277+
Some(route_params.payment_params.clone()), &&keys_manager, 0).unwrap();
12531278
outbound_payments.pay_internal(
12541279
PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(),
12551280
&&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()

‎lightning/src/ln/payment_tests.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,6 +1586,7 @@ fn do_test_intercepted_payment(test: InterceptTest) {
15861586
#[derive(PartialEq)]
15871587
enum AutoRetry {
15881588
Success,
1589+
Spontaneous,
15891590
FailAttempts,
15901591
FailTimeout,
15911592
FailOnRestart,
@@ -1594,6 +1595,7 @@ enum AutoRetry {
15941595
#[test]
15951596
fn automatic_retries() {
15961597
do_automatic_retries(AutoRetry::Success);
1598+
do_automatic_retries(AutoRetry::Spontaneous);
15971599
do_automatic_retries(AutoRetry::FailAttempts);
15981600
do_automatic_retries(AutoRetry::FailTimeout);
15991601
do_automatic_retries(AutoRetry::FailOnRestart);
@@ -1692,6 +1694,21 @@ fn do_automatic_retries(test: AutoRetry) {
16921694
assert_eq!(msg_events.len(), 1);
16931695
pass_along_path(&nodes[0], &[&nodes[1], &nodes[2]], amt_msat, payment_hash, Some(payment_secret), msg_events.pop().unwrap(), true, None);
16941696
claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage);
1697+
} else if test == AutoRetry::Spontaneous {
1698+
nodes[0].node.send_spontaneous_payment_with_retry(Some(payment_preimage), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
1699+
pass_failed_attempt_with_retry_along_path!(channel_id_2, true);
1700+
1701+
// Open a new channel with liquidity on the second hop so we can find a route for the retry
1702+
// attempt, since the initial second hop channel will be excluded from pathfinding
1703+
create_announced_chan_between_nodes(&nodes, 1, 2);
1704+
1705+
// We retry payments in `process_pending_htlc_forwards`
1706+
nodes[0].node.process_pending_htlc_forwards();
1707+
check_added_monitors!(nodes[0], 1);
1708+
let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
1709+
assert_eq!(msg_events.len(), 1);
1710+
pass_along_path(&nodes[0], &[&nodes[1], &nodes[2]], amt_msat, payment_hash, None, msg_events.pop().unwrap(), true, Some(payment_preimage));
1711+
claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage);
16951712
} else if test == AutoRetry::FailAttempts {
16961713
// Ensure ChannelManager will not retry a payment if it has run out of payment attempts.
16971714
nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();

0 commit comments

Comments
 (0)