Skip to content

Commit

Permalink
On initial send retries, avoid previously failed scids
Browse files Browse the repository at this point in the history
Previously, we would have tried the same failed channels over and over until
retries are exhausted.
  • Loading branch information
valentinewallace committed Feb 20, 2023
1 parent 8492847 commit 107e6e1
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
19 changes: 11 additions & 8 deletions lightning/src/ln/outbound_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,8 +725,8 @@ impl OutboundPayments {

fn handle_pay_route_err<R: Deref, NS: Deref, ES: Deref, IH, SP, L: Deref>(
&self, err: PaymentSendFailure, payment_id: PaymentId, payment_hash: PaymentHash, route: Route,
route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>, inflight_htlcs: &IH,
entropy_source: &ES, node_signer: &NS, best_block_height: u32, logger: &L,
mut route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>,
inflight_htlcs: &IH, entropy_source: &ES, node_signer: &NS, best_block_height: u32, logger: &L,
pending_events: &Mutex<Vec<events::Event>>, send_payment_along_path: &SP,
)
where
Expand All @@ -740,11 +740,11 @@ impl OutboundPayments {
{
match err {
PaymentSendFailure::AllFailedResendSafe(errs) => {
push_payment_path_failed_evs(payment_id, payment_hash, route.paths, errs.into_iter().map(|e| Err(e)), pending_events);
push_path_failed_evs_and_scids(payment_id, payment_hash, Some(&mut route_params), route.paths, errs.into_iter().map(|e| Err(e)), pending_events);
self.retry_payment_internal(payment_id, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path);
},
PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. } => {
push_payment_path_failed_evs(payment_id, payment_hash, route.paths, results.into_iter(), pending_events);
push_path_failed_evs_and_scids(payment_id, payment_hash, None, route.paths, results.into_iter(), pending_events);
// Some paths were sent, even if we failed to send the full MPP value our recipient may
// misbehave and claim the funds, at which point we have to consider the payment sent, so
// return `Ok()` here, ignoring any retry errors.
Expand All @@ -756,7 +756,7 @@ impl OutboundPayments {
// initial HTLC-Add messages yet.
},
PaymentSendFailure::PathParameterError(results) => {
push_payment_path_failed_evs(payment_id, payment_hash, route.paths, results.into_iter(), pending_events);
push_path_failed_evs_and_scids(payment_id, payment_hash, None, route.paths, results.into_iter(), pending_events);
self.abandon_payment(payment_id, pending_events);
},
PaymentSendFailure::ParameterError(e) => {
Expand Down Expand Up @@ -1261,14 +1261,17 @@ impl OutboundPayments {
}
}

fn push_payment_path_failed_evs<I: Iterator<Item = Result<(), APIError>>>(
payment_id: PaymentId, payment_hash: PaymentHash, paths: Vec<Vec<RouteHop>>, path_results: I,
pending_events: &Mutex<Vec<events::Event>>
fn push_path_failed_evs_and_scids<I: Iterator<Item = Result<(), APIError>>>(
payment_id: PaymentId, payment_hash: PaymentHash, mut route_params: Option<&mut RouteParameters>,
paths: Vec<Vec<RouteHop>>, path_results: I, pending_events: &Mutex<Vec<events::Event>>
) {
let mut events = pending_events.lock().unwrap();
for (path, path_res) in paths.into_iter().zip(path_results) {
if let Err(e) = path_res {
let scid = path[0].short_channel_id;
if let Some(ref mut params) = route_params {
params.payment_params.previously_failed_channels.push(scid);
}
events.push(events::Event::PaymentPathFailed {
payment_id: Some(payment_id),
payment_hash,
Expand Down
6 changes: 4 additions & 2 deletions lightning/src/ln/payment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2199,9 +2199,11 @@ fn immediate_retry_on_failure() {
route.paths[0][0].short_channel_id = chans[1].short_channel_id.unwrap();
route.paths[0][0].fee_msat = 50_000_000;
route.paths[1][0].fee_msat = 50_000_001;
let mut pay_params = route_params.payment_params.clone();
pay_params.previously_failed_channels.push(chans[0].short_channel_id.unwrap());
nodes[0].router.expect_find_route(RouteParameters {
payment_params: route_params.payment_params.clone(),
final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV
payment_params: pay_params, final_value_msat: amt_msat,
final_cltv_expiry_delta: TEST_FINAL_CLTV
}, Ok(route.clone()));

nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
Expand Down

0 comments on commit 107e6e1

Please sign in to comment.