Skip to content

Commit 5e65c49

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #1702 from TheBlueMatt/2022-09-one-hop-retryable
Mark failed one-hop HTLCs as retrably
2 parents a59b9d8 + c57bb42 commit 5e65c49

File tree

11 files changed

+86
-86
lines changed

11 files changed

+86
-86
lines changed

‎lightning-invoice/src/payment.rs‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ where
717717

718718
match event {
719719
Event::PaymentPathFailed {
720-
payment_id, payment_hash, rejected_by_dest, path, short_channel_id, retry, ..
720+
payment_id, payment_hash, payment_failed_permanently, path, short_channel_id, retry, ..
721721
} => {
722722
if let Some(short_channel_id) = short_channel_id {
723723
let path = path.iter().collect::<Vec<_>>();
@@ -726,7 +726,7 @@ where
726726

727727
if payment_id.is_none() {
728728
log_trace!(self.logger, "Payment {} has no id; not retrying", log_bytes!(payment_hash.0));
729-
} else if *rejected_by_dest {
729+
} else if *payment_failed_permanently {
730730
log_trace!(self.logger, "Payment {} rejected by destination; not retrying", log_bytes!(payment_hash.0));
731731
self.payer.abandon_payment(payment_id.unwrap());
732732
} else if retry.is_none() {
@@ -916,7 +916,7 @@ mod tests {
916916
payment_id,
917917
payment_hash,
918918
network_update: None,
919-
rejected_by_dest: false,
919+
payment_failed_permanently: false,
920920
all_paths_failed: false,
921921
path: TestRouter::path_for_value(final_value_msat),
922922
short_channel_id: None,
@@ -981,7 +981,7 @@ mod tests {
981981
payment_id,
982982
payment_hash,
983983
network_update: None,
984-
rejected_by_dest: false,
984+
payment_failed_permanently: false,
985985
all_paths_failed: false,
986986
path: TestRouter::path_for_value(final_value_msat),
987987
short_channel_id: None,
@@ -1028,7 +1028,7 @@ mod tests {
10281028
payment_id,
10291029
payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()),
10301030
network_update: None,
1031-
rejected_by_dest: false,
1031+
payment_failed_permanently: false,
10321032
all_paths_failed: true,
10331033
path: TestRouter::path_for_value(final_value_msat),
10341034
short_channel_id: None,
@@ -1042,7 +1042,7 @@ mod tests {
10421042
payment_id,
10431043
payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()),
10441044
network_update: None,
1045-
rejected_by_dest: false,
1045+
payment_failed_permanently: false,
10461046
all_paths_failed: false,
10471047
path: TestRouter::path_for_value(final_value_msat / 2),
10481048
short_channel_id: None,
@@ -1088,7 +1088,7 @@ mod tests {
10881088
payment_id,
10891089
payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()),
10901090
network_update: None,
1091-
rejected_by_dest: false,
1091+
payment_failed_permanently: false,
10921092
all_paths_failed: true,
10931093
path: TestRouter::path_for_value(final_value_msat),
10941094
short_channel_id: None,
@@ -1128,7 +1128,7 @@ mod tests {
11281128
payment_id,
11291129
payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()),
11301130
network_update: None,
1131-
rejected_by_dest: false,
1131+
payment_failed_permanently: false,
11321132
all_paths_failed: false,
11331133
path: vec![],
11341134
short_channel_id: None,
@@ -1189,7 +1189,7 @@ mod tests {
11891189
payment_id,
11901190
payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()),
11911191
network_update: None,
1192-
rejected_by_dest: false,
1192+
payment_failed_permanently: false,
11931193
all_paths_failed: false,
11941194
path: vec![],
11951195
short_channel_id: None,
@@ -1226,7 +1226,7 @@ mod tests {
12261226
payment_id,
12271227
payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()),
12281228
network_update: None,
1229-
rejected_by_dest: false,
1229+
payment_failed_permanently: false,
12301230
all_paths_failed: false,
12311231
path: TestRouter::path_for_value(final_value_msat / 2),
12321232
short_channel_id: None,
@@ -1260,7 +1260,7 @@ mod tests {
12601260
payment_id,
12611261
payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()),
12621262
network_update: None,
1263-
rejected_by_dest: true,
1263+
payment_failed_permanently: true,
12641264
all_paths_failed: false,
12651265
path: vec![],
12661266
short_channel_id: None,
@@ -1309,7 +1309,7 @@ mod tests {
13091309
payment_id,
13101310
payment_hash,
13111311
network_update: None,
1312-
rejected_by_dest: false,
1312+
payment_failed_permanently: false,
13131313
all_paths_failed: false,
13141314
path: vec![],
13151315
short_channel_id: None,
@@ -1444,7 +1444,7 @@ mod tests {
14441444
payment_id,
14451445
payment_hash,
14461446
network_update: None,
1447-
rejected_by_dest: false,
1447+
payment_failed_permanently: false,
14481448
all_paths_failed: false,
14491449
path: vec![],
14501450
short_channel_id: None,
@@ -1490,7 +1490,7 @@ mod tests {
14901490
payment_id,
14911491
payment_hash,
14921492
network_update: None,
1493-
rejected_by_dest: false,
1493+
payment_failed_permanently: false,
14941494
all_paths_failed: false,
14951495
path,
14961496
short_channel_id,
@@ -1680,7 +1680,7 @@ mod tests {
16801680
payment_id,
16811681
payment_hash,
16821682
network_update: None,
1683-
rejected_by_dest: false,
1683+
payment_failed_permanently: false,
16841684
all_paths_failed: false,
16851685
path: TestRouter::path_for_value(final_value_msat),
16861686
short_channel_id: None,
@@ -1692,7 +1692,7 @@ mod tests {
16921692
payment_id,
16931693
payment_hash,
16941694
network_update: None,
1695-
rejected_by_dest: false,
1695+
payment_failed_permanently: false,
16961696
all_paths_failed: false,
16971697
path: TestRouter::path_for_value(final_value_msat / 2),
16981698
short_channel_id: None,
@@ -2393,8 +2393,8 @@ mod tests {
23932393
// treated this as "HTLC complete" and dropped the retry counter, causing us to retry again
23942394
// if the final HTLC failed.
23952395
expected_events.borrow_mut().push_back(&|ev: &Event| {
2396-
if let Event::PaymentPathFailed { rejected_by_dest, all_paths_failed, .. } = ev {
2397-
assert!(!rejected_by_dest);
2396+
if let Event::PaymentPathFailed { payment_failed_permanently, all_paths_failed, .. } = ev {
2397+
assert!(!payment_failed_permanently);
23982398
assert!(all_paths_failed);
23992399
} else { panic!("Unexpected event"); }
24002400
});
@@ -2411,8 +2411,8 @@ mod tests {
24112411
commitment_signed_dance!(nodes[0], nodes[1], &bs_fail_update.commitment_signed, false, true);
24122412

24132413
expected_events.borrow_mut().push_back(&|ev: &Event| {
2414-
if let Event::PaymentPathFailed { rejected_by_dest, all_paths_failed, .. } = ev {
2415-
assert!(!rejected_by_dest);
2414+
if let Event::PaymentPathFailed { payment_failed_permanently, all_paths_failed, .. } = ev {
2415+
assert!(!payment_failed_permanently);
24162416
assert!(all_paths_failed);
24172417
} else { panic!("Unexpected event"); }
24182418
});

‎lightning/src/ln/chanmon_update_fail_tests.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1744,9 +1744,9 @@ fn test_monitor_update_on_pending_forwards() {
17441744

17451745
let events = nodes[0].node.get_and_clear_pending_events();
17461746
assert_eq!(events.len(), 2);
1747-
if let Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } = events[0] {
1747+
if let Event::PaymentPathFailed { payment_hash, payment_failed_permanently, .. } = events[0] {
17481748
assert_eq!(payment_hash, payment_hash_1);
1749-
assert!(rejected_by_dest);
1749+
assert!(payment_failed_permanently);
17501750
} else { panic!("Unexpected event!"); }
17511751
match events[1] {
17521752
Event::PendingHTLCsForwardable { .. } => { },

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3844,7 +3844,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
38443844
pending_events.push(events::Event::PaymentPathFailed {
38453845
payment_id: Some(payment_id),
38463846
payment_hash,
3847-
rejected_by_dest: false,
3847+
payment_failed_permanently: false,
38483848
network_update: None,
38493849
all_paths_failed: payment.get().remaining_parts() == 0,
38503850
path: path.clone(),
@@ -3959,7 +3959,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
39593959
events::Event::PaymentPathFailed {
39603960
payment_id: Some(payment_id),
39613961
payment_hash: payment_hash.clone(),
3962-
rejected_by_dest: !payment_retryable,
3962+
payment_failed_permanently: !payment_retryable,
39633963
network_update,
39643964
all_paths_failed,
39653965
path: path.clone(),
@@ -3990,7 +3990,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
39903990
events::Event::PaymentPathFailed {
39913991
payment_id: Some(payment_id),
39923992
payment_hash: payment_hash.clone(),
3993-
rejected_by_dest: path.len() == 1,
3993+
payment_failed_permanently: false,
39943994
network_update: None,
39953995
all_paths_failed,
39963996
path: path.clone(),

‎lightning/src/ln/functional_test_utils.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,38 +1580,38 @@ impl<'a> PaymentFailedConditions<'a> {
15801580

15811581
#[cfg(test)]
15821582
macro_rules! expect_payment_failed_with_update {
1583-
($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $scid: expr, $chan_closed: expr) => {
1583+
($node: expr, $expected_payment_hash: expr, $payment_failed_permanently: expr, $scid: expr, $chan_closed: expr) => {
15841584
$crate::ln::functional_test_utils::expect_payment_failed_conditions(
1585-
&$node, $expected_payment_hash, $rejected_by_dest,
1585+
&$node, $expected_payment_hash, $payment_failed_permanently,
15861586
$crate::ln::functional_test_utils::PaymentFailedConditions::new()
15871587
.blamed_scid($scid).blamed_chan_closed($chan_closed));
15881588
}
15891589
}
15901590

15911591
#[cfg(test)]
15921592
macro_rules! expect_payment_failed {
1593-
($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => {
1593+
($node: expr, $expected_payment_hash: expr, $payment_failed_permanently: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => {
15941594
#[allow(unused_mut)]
15951595
let mut conditions = $crate::ln::functional_test_utils::PaymentFailedConditions::new();
15961596
$(
15971597
conditions = conditions.expected_htlc_error_data($expected_error_code, &$expected_error_data);
15981598
)*
1599-
$crate::ln::functional_test_utils::expect_payment_failed_conditions(&$node, $expected_payment_hash, $rejected_by_dest, conditions);
1599+
$crate::ln::functional_test_utils::expect_payment_failed_conditions(&$node, $expected_payment_hash, $payment_failed_permanently, conditions);
16001600
};
16011601
}
16021602

16031603
pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
16041604
node: &'a Node<'b, 'c, 'd>, payment_failed_event: Event, expected_payment_hash: PaymentHash,
1605-
expected_rejected_by_dest: bool, conditions: PaymentFailedConditions<'e>
1605+
expected_payment_failed_permanently: bool, conditions: PaymentFailedConditions<'e>
16061606
) {
16071607
let expected_payment_id = match payment_failed_event {
1608-
Event::PaymentPathFailed { payment_hash, rejected_by_dest, path, retry, payment_id, network_update, short_channel_id,
1608+
Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, network_update, short_channel_id,
16091609
#[cfg(test)]
16101610
error_code,
16111611
#[cfg(test)]
16121612
error_data, .. } => {
16131613
assert_eq!(payment_hash, expected_payment_hash, "unexpected payment_hash");
1614-
assert_eq!(rejected_by_dest, expected_rejected_by_dest, "unexpected rejected_by_dest value");
1614+
assert_eq!(payment_failed_permanently, expected_payment_failed_permanently, "unexpected payment_failed_permanently value");
16151615
assert!(retry.is_some(), "expected retry.is_some()");
16161616
assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
16171617
assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
@@ -1668,12 +1668,12 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
16681668
}
16691669

16701670
pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>(
1671-
node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_rejected_by_dest: bool,
1671+
node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_payment_failed_permanently: bool,
16721672
conditions: PaymentFailedConditions<'e>
16731673
) {
16741674
let mut events = node.node.get_and_clear_pending_events();
16751675
assert_eq!(events.len(), 1);
1676-
expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_rejected_by_dest, conditions);
1676+
expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_payment_failed_permanently, conditions);
16771677
}
16781678

16791679
pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_paths: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) -> PaymentId {
@@ -2017,9 +2017,9 @@ pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
20172017
let events = origin_node.node.get_and_clear_pending_events();
20182018
assert_eq!(events.len(), 1);
20192019
let expected_payment_id = match events[0] {
2020-
Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, ref payment_id, .. } => {
2020+
Event::PaymentPathFailed { payment_hash, payment_failed_permanently, all_paths_failed, ref path, ref payment_id, .. } => {
20212021
assert_eq!(payment_hash, our_payment_hash);
2022-
assert!(rejected_by_dest);
2022+
assert!(payment_failed_permanently);
20232023
assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
20242024
for (idx, hop) in expected_route.iter().enumerate() {
20252025
assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);

0 commit comments

Comments
 (0)