Skip to content

Commit f6d777f

Browse files
Remove all_paths_failed from PaymentPathFailed
This field was previous useful in manual retries for users to know when all paths of a payment have failed and it is safe to retry. Now that we support automatic retries in ChannelManager and no longer support manual retries, the field is no longer useful. We can delete the field from deserialization because it's optional.
1 parent fce7b5a commit f6d777f

File tree

7 files changed

+11
-28
lines changed

7 files changed

+11
-28
lines changed

lightning-background-processor/src/lib.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,7 +1366,6 @@ mod tests {
13661366
payment_hash: PaymentHash([42; 32]),
13671367
payment_failed_permanently: false,
13681368
network_update: None,
1369-
all_paths_failed: true,
13701369
path: path.clone(),
13711370
short_channel_id: Some(scored_scid),
13721371
retry: None,
@@ -1387,7 +1386,6 @@ mod tests {
13871386
payment_hash: PaymentHash([42; 32]),
13881387
payment_failed_permanently: true,
13891388
network_update: None,
1390-
all_paths_failed: true,
13911389
path: path.clone(),
13921390
short_channel_id: None,
13931391
retry: None,

lightning/src/ln/functional_test_utils.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2240,10 +2240,9 @@ pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
22402240
if i == expected_paths.len() - 1 { assert_eq!(events.len(), 2); } else { assert_eq!(events.len(), 1); }
22412241

22422242
let expected_payment_id = match events[0] {
2243-
Event::PaymentPathFailed { payment_hash, payment_failed_permanently, all_paths_failed, ref path, ref payment_id, .. } => {
2243+
Event::PaymentPathFailed { payment_hash, payment_failed_permanently, ref path, ref payment_id, .. } => {
22442244
assert_eq!(payment_hash, our_payment_hash);
22452245
assert!(payment_failed_permanently);
2246-
assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
22472246
for (idx, hop) in expected_route.iter().enumerate() {
22482247
assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);
22492248
}

lightning/src/ln/functional_tests.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5695,11 +5695,10 @@ fn test_fail_holding_cell_htlc_upon_free() {
56955695
let events = nodes[0].node.get_and_clear_pending_events();
56965696
assert_eq!(events.len(), 2);
56975697
match &events[0] {
5698-
&Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, .. } => {
5698+
&Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref short_channel_id, .. } => {
56995699
assert_eq!(PaymentId(our_payment_hash.0), *payment_id.as_ref().unwrap());
57005700
assert_eq!(our_payment_hash.clone(), *payment_hash);
57015701
assert_eq!(*payment_failed_permanently, false);
5702-
assert_eq!(*all_paths_failed, true);
57035702
assert_eq!(*network_update, None);
57045703
assert_eq!(*short_channel_id, Some(route.paths[0][0].short_channel_id));
57055704
},
@@ -5786,11 +5785,10 @@ fn test_free_and_fail_holding_cell_htlcs() {
57865785
let events = nodes[0].node.get_and_clear_pending_events();
57875786
assert_eq!(events.len(), 2);
57885787
match &events[0] {
5789-
&Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, .. } => {
5788+
&Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref short_channel_id, .. } => {
57905789
assert_eq!(payment_id_2, *payment_id.as_ref().unwrap());
57915790
assert_eq!(payment_hash_2.clone(), *payment_hash);
57925791
assert_eq!(*payment_failed_permanently, false);
5793-
assert_eq!(*all_paths_failed, true);
57945792
assert_eq!(*network_update, None);
57955793
assert_eq!(*short_channel_id, Some(route_2.paths[0][0].short_channel_id));
57965794
},

lightning/src/ln/onion_route_tests.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,8 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
167167

168168
let events = nodes[0].node.get_and_clear_pending_events();
169169
assert_eq!(events.len(), 2);
170-
if let &Event::PaymentPathFailed { ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, .. } = &events[0] {
170+
if let &Event::PaymentPathFailed { ref payment_failed_permanently, ref network_update, ref short_channel_id, ref error_code, .. } = &events[0] {
171171
assert_eq!(*payment_failed_permanently, !expected_retryable);
172-
assert_eq!(*all_paths_failed, true);
173172
assert_eq!(*error_code, expected_error_code);
174173
if expected_channel_update.is_some() {
175174
match network_update {

lightning/src/ln/outbound_payment.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,6 @@ impl OutboundPayments {
11061106
awaiting_retry
11071107
});
11081108

1109-
let mut all_paths_failed = false;
11101109
let mut full_failure_ev = None;
11111110
let mut pending_retry_ev = false;
11121111
let mut retry = None;
@@ -1155,7 +1154,6 @@ impl OutboundPayments {
11551154
is_retryable_now = false;
11561155
}
11571156
if payment.get().remaining_parts() == 0 {
1158-
all_paths_failed = true;
11591157
if payment.get().abandoned() {
11601158
if !payment_is_probe {
11611159
full_failure_ev = Some(events::Event::PaymentFailed {
@@ -1208,7 +1206,6 @@ impl OutboundPayments {
12081206
payment_hash: payment_hash.clone(),
12091207
payment_failed_permanently: !payment_retryable,
12101208
network_update,
1211-
all_paths_failed,
12121209
path: path.clone(),
12131210
short_channel_id,
12141211
retry,
@@ -1258,7 +1255,6 @@ fn push_payment_path_failed_evs<I: Iterator<Item = Result<(), APIError>>>(
12581255
payment_hash,
12591256
payment_failed_permanently: false,
12601257
network_update: None,
1261-
all_paths_failed: false,
12621258
path,
12631259
short_channel_id: Some(scid),
12641260
retry: None,

lightning/src/ln/payment_tests.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2135,7 +2135,7 @@ fn retry_multi_path_single_failed_payment() {
21352135
assert_eq!(events.len(), 1);
21362136
match events[0] {
21372137
Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently: false,
2138-
network_update: None, all_paths_failed: false, short_channel_id: Some(expected_scid), .. } => {
2138+
network_update: None, short_channel_id: Some(expected_scid), .. } => {
21392139
assert_eq!(payment_hash, ev_payment_hash);
21402140
assert_eq!(expected_scid, route.paths[1][0].short_channel_id);
21412141
},
@@ -2206,7 +2206,7 @@ fn immediate_retry_on_failure() {
22062206
assert_eq!(events.len(), 1);
22072207
match events[0] {
22082208
Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently: false,
2209-
network_update: None, all_paths_failed: false, short_channel_id: Some(expected_scid), .. } => {
2209+
network_update: None, short_channel_id: Some(expected_scid), .. } => {
22102210
assert_eq!(payment_hash, ev_payment_hash);
22112211
assert_eq!(expected_scid, route.paths[1][0].short_channel_id);
22122212
},
@@ -2220,7 +2220,8 @@ fn immediate_retry_on_failure() {
22202220
#[test]
22212221
fn no_extra_retries_on_back_to_back_fail() {
22222222
// In a previous release, we had a race where we may exceed the payment retry count if we
2223-
// get two failures in a row with the second having `all_paths_failed` set.
2223+
// get two failures in a row with the second indicating that all paths had failed (this field,
2224+
// `all_paths_failed`, has since been removed).
22242225
// Generally, when we give up trying to retry a payment, we don't know for sure what the
22252226
// current state of the ChannelManager event queue is. Specifically, we cannot be sure that
22262227
// there are not multiple additional `PaymentPathFailed` or even `PaymentSent` events

lightning/src/util/events.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -630,13 +630,12 @@ pub enum Event {
630630
/// handle the HTLC.
631631
///
632632
/// Note that this does *not* indicate that all paths for an MPP payment have failed, see
633-
/// [`Event::PaymentFailed`] and [`all_paths_failed`].
633+
/// [`Event::PaymentFailed`].
634634
///
635635
/// See [`ChannelManager::abandon_payment`] for giving up on this payment before its retries have
636636
/// been exhausted.
637637
///
638638
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
639-
/// [`all_paths_failed`]: Self::PaymentPathFailed::all_paths_failed
640639
PaymentPathFailed {
641640
/// The id returned by [`ChannelManager::send_payment`] and used with
642641
/// [`ChannelManager::abandon_payment`].
@@ -660,10 +659,6 @@ pub enum Event {
660659
///
661660
/// [`NetworkGraph`]: crate::routing::gossip::NetworkGraph
662661
network_update: Option<NetworkUpdate>,
663-
/// For both single-path and multi-path payments, this is set if all paths of the payment have
664-
/// failed. This will be set to false if (1) this is an MPP payment and (2) other parts of the
665-
/// larger MPP payment were still in flight when this event was generated.
666-
all_paths_failed: bool,
667662
/// The payment path that failed.
668663
path: Vec<RouteHop>,
669664
/// The channel responsible for the failed payment path.
@@ -966,7 +961,7 @@ impl Writeable for Event {
966961
},
967962
&Event::PaymentPathFailed {
968963
ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update,
969-
ref all_paths_failed, ref path, ref short_channel_id, ref retry,
964+
ref path, ref short_channel_id, ref retry,
970965
#[cfg(test)]
971966
ref error_code,
972967
#[cfg(test)]
@@ -981,7 +976,7 @@ impl Writeable for Event {
981976
(0, payment_hash, required),
982977
(1, network_update, option),
983978
(2, payment_failed_permanently, required),
984-
(3, all_paths_failed, required),
979+
(3, None::<bool>, option), // all_paths_failed in LDK versions prior to 0.0.114
985980
(5, *path, vec_type),
986981
(7, short_channel_id, option),
987982
(9, retry, option),
@@ -1198,7 +1193,6 @@ impl MaybeReadable for Event {
11981193
let mut payment_hash = PaymentHash([0; 32]);
11991194
let mut payment_failed_permanently = false;
12001195
let mut network_update = None;
1201-
let mut all_paths_failed = Some(true);
12021196
let mut path: Option<Vec<RouteHop>> = Some(vec![]);
12031197
let mut short_channel_id = None;
12041198
let mut retry = None;
@@ -1207,7 +1201,6 @@ impl MaybeReadable for Event {
12071201
(0, payment_hash, required),
12081202
(1, network_update, ignorable),
12091203
(2, payment_failed_permanently, required),
1210-
(3, all_paths_failed, option),
12111204
(5, path, vec_type),
12121205
(7, short_channel_id, option),
12131206
(9, retry, option),
@@ -1218,7 +1211,6 @@ impl MaybeReadable for Event {
12181211
payment_hash,
12191212
payment_failed_permanently,
12201213
network_update,
1221-
all_paths_failed: all_paths_failed.unwrap(),
12221214
path: path.unwrap(),
12231215
short_channel_id,
12241216
retry,

0 commit comments

Comments
 (0)