Skip to content

Commit 6b49af1

Browse files
valentinewallacevalentinewallace
committed
Support spontaneous payment retries in ChannelManager
1 parent c863350 commit 6b49af1

File tree

3 files changed

+67
-10
lines changed

3 files changed

+67
-10
lines changed

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 15 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
}

‎lightning/src/ln/outbound_payment.rs‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ impl OutboundPayments {
416416
F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
417417
u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
418418
{
419-
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)),
420420
route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
421421
best_block_height, logger, &send_payment_along_path)
422422
.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
@@ -439,7 +439,33 @@ impl OutboundPayments {
439439
.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
440440
}
441441

442-
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 = match payment_preimage {
457+
Some(p) => p,
458+
None => PaymentPreimage(entropy_source.get_secure_random_bytes()),
459+
};
460+
let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
461+
self.pay_internal(payment_id, Some((payment_hash, &None, Some(preimage), retry_strategy)),
462+
route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
463+
best_block_height, logger, &send_payment_along_path)
464+
.map(|()| payment_hash)
465+
.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
466+
}
467+
468+
pub(super) fn send_spontaneous_payment_with_route<ES: Deref, NS: Deref, F>(
443469
&self, route: &Route, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId,
444470
entropy_source: &ES, node_signer: &NS, best_block_height: u32, send_payment_along_path: F
445471
) -> Result<PaymentHash, PaymentSendFailure>
@@ -512,7 +538,7 @@ impl OutboundPayments {
512538

513539
fn pay_internal<R: Deref, NS: Deref, ES: Deref, F, L: Deref>(
514540
&self, payment_id: PaymentId,
515-
initial_send_info: Option<(PaymentHash, &Option<PaymentSecret>, Retry)>,
541+
initial_send_info: Option<(PaymentHash, &Option<PaymentSecret>, Option<PaymentPreimage>, Retry)>,
516542
route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>,
517543
inflight_htlcs: InFlightHtlcs, entropy_source: &ES, node_signer: &NS, best_block_height: u32,
518544
logger: &L, send_payment_along_path: &F,
@@ -540,8 +566,8 @@ impl OutboundPayments {
540566
err: format!("Failed to find a route for payment {}: {:?}", log_bytes!(payment_id.0), e), // TODO: add APIError::RouteNotFound
541567
}))?;
542568

543-
let res = if let Some((payment_hash, payment_secret, retry_strategy)) = initial_send_info {
544-
let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, None, &route, Some(retry_strategy), Some(route_params.payment_params.clone()), entropy_source, best_block_height)?;
569+
let res = if let Some((payment_hash, payment_secret, keysend_preimage, retry_strategy)) = initial_send_info {
570+
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)?;
545571
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)
546572
} else {
547573
self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path)
@@ -597,21 +623,21 @@ impl OutboundPayments {
597623
onion_session_privs.push(entropy_source.get_secure_random_bytes());
598624
}
599625

600-
let (total_msat, payment_hash, payment_secret) = {
626+
let (total_msat, payment_hash, payment_secret, keysend_preimage) = {
601627
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
602628
match outbounds.get_mut(&payment_id) {
603629
Some(payment) => {
604630
let res = match payment {
605631
PendingOutboundPayment::Retryable {
606-
total_msat, payment_hash, payment_secret, pending_amt_msat, ..
632+
total_msat, payment_hash, keysend_preimage, payment_secret, pending_amt_msat, ..
607633
} => {
608634
let retry_amt_msat: u64 = route.paths.iter().map(|path| path.last().unwrap().fee_msat).sum();
609635
if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
610636
return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
611637
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()
612638
}))
613639
}
614-
(*total_msat, *payment_hash, *payment_secret)
640+
(*total_msat, *payment_hash, *payment_secret, *keysend_preimage)
615641
},
616642
PendingOutboundPayment::Legacy { .. } => {
617643
return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
@@ -646,7 +672,7 @@ impl OutboundPayments {
646672
})),
647673
}
648674
};
649-
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)
675+
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)
650676
}
651677

652678
pub(super) fn send_probe<ES: Deref, NS: Deref, F>(

‎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)