mirror of
https://github.com/lightningdevkit/rust-lightning.git
synced 2025-02-24 23:08:36 +01:00
On initial send retries, avoid previously failed scids
Previously, we could have tried the same failed channels over and over until retries are exhausted.
This commit is contained in:
parent
b826d1735d
commit
1224dac862
2 changed files with 25 additions and 17 deletions
|
@ -735,8 +735,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
|
||||
|
@ -750,11 +750,11 @@ impl OutboundPayments {
|
|||
{
|
||||
match err {
|
||||
PaymentSendFailure::AllFailedResendSafe(errs) => {
|
||||
Self::push_payment_path_failed_evs(payment_id, payment_hash, route.paths, errs.into_iter().map(|e| Err(e)), pending_events);
|
||||
Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &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, .. } => {
|
||||
Self::push_payment_path_failed_evs(payment_id, payment_hash, route.paths, results.into_iter(), pending_events);
|
||||
PaymentSendFailure::PartialFailure { failed_paths_retry: Some(mut retry), results, .. } => {
|
||||
Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut retry, 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.
|
||||
|
@ -766,7 +766,7 @@ impl OutboundPayments {
|
|||
// initial HTLC-Add messages yet.
|
||||
},
|
||||
PaymentSendFailure::PathParameterError(results) => {
|
||||
Self::push_payment_path_failed_evs(payment_id, payment_hash, route.paths, results.into_iter(), pending_events);
|
||||
Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut route_params, route.paths, results.into_iter(), pending_events);
|
||||
self.abandon_payment(payment_id, pending_events);
|
||||
},
|
||||
PaymentSendFailure::ParameterError(e) => {
|
||||
|
@ -777,9 +777,9 @@ impl OutboundPayments {
|
|||
}
|
||||
}
|
||||
|
||||
fn push_payment_path_failed_evs<I: ExactSizeIterator + 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: ExactSizeIterator + Iterator<Item = Result<(), APIError>>>(
|
||||
payment_id: PaymentId, payment_hash: PaymentHash, route_params: &mut RouteParameters,
|
||||
paths: Vec<Vec<RouteHop>>, path_results: I, pending_events: &Mutex<Vec<events::Event>>
|
||||
) {
|
||||
let mut events = pending_events.lock().unwrap();
|
||||
debug_assert_eq!(paths.len(), path_results.len());
|
||||
|
@ -788,7 +788,9 @@ impl OutboundPayments {
|
|||
let failed_scid = if let APIError::InvalidRoute { .. } = e {
|
||||
None
|
||||
} else {
|
||||
Some(path[0].short_channel_id)
|
||||
let scid = path[0].short_channel_id;
|
||||
route_params.payment_params.previously_failed_channels.push(scid);
|
||||
Some(scid)
|
||||
};
|
||||
events.push(events::Event::PaymentPathFailed {
|
||||
payment_id: Some(payment_id),
|
||||
|
|
|
@ -1857,13 +1857,15 @@ fn auto_retry_partial_failure() {
|
|||
payment_params: Some(route_params.payment_params.clone()),
|
||||
};
|
||||
nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route));
|
||||
let mut payment_params = route_params.payment_params.clone();
|
||||
payment_params.previously_failed_channels.push(chan_2_id);
|
||||
nodes[0].router.expect_find_route(RouteParameters {
|
||||
payment_params: route_params.payment_params.clone(),
|
||||
final_value_msat: amt_msat / 2, final_cltv_expiry_delta: TEST_FINAL_CLTV
|
||||
payment_params, final_value_msat: amt_msat / 2, final_cltv_expiry_delta: TEST_FINAL_CLTV
|
||||
}, Ok(retry_1_route));
|
||||
let mut payment_params = route_params.payment_params.clone();
|
||||
payment_params.previously_failed_channels.push(chan_3_id);
|
||||
nodes[0].router.expect_find_route(RouteParameters {
|
||||
payment_params: route_params.payment_params.clone(),
|
||||
final_value_msat: amt_msat / 4, final_cltv_expiry_delta: TEST_FINAL_CLTV
|
||||
payment_params, final_value_msat: amt_msat / 4, final_cltv_expiry_delta: TEST_FINAL_CLTV
|
||||
}, Ok(retry_2_route));
|
||||
|
||||
// Send a payment that will partially fail on send, then partially fail on retry, then succeed.
|
||||
|
@ -2113,8 +2115,10 @@ fn retry_multi_path_single_failed_payment() {
|
|||
// On retry, split the payment across both channels.
|
||||
route.paths[0][0].fee_msat = 50_000_001;
|
||||
route.paths[1][0].fee_msat = 50_000_000;
|
||||
let mut pay_params = route.payment_params.clone().unwrap();
|
||||
pay_params.previously_failed_channels.push(chans[1].short_channel_id.unwrap());
|
||||
nodes[0].router.expect_find_route(RouteParameters {
|
||||
payment_params: route.payment_params.clone().unwrap(),
|
||||
payment_params: pay_params,
|
||||
// Note that the second request here requests the amount we originally failed to send,
|
||||
// not the amount remaining on the full payment, which should be changed.
|
||||
final_value_msat: 100_000_001, final_cltv_expiry_delta: TEST_FINAL_CLTV
|
||||
|
@ -2196,9 +2200,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();
|
||||
|
|
Loading…
Add table
Reference in a new issue