Skip to content

Commit 553ad8c

Browse files
Abandon payment on behalf of the user on payment path failure
Removed retry_single_path_payment, it's replaced by automatic_retries with AutoRetry::Success
1 parent a5d85bd commit 553ad8c

File tree

8 files changed

+199
-186
lines changed

8 files changed

+199
-186
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
914914
events::Event::PaymentClaimed { .. } => {},
915915
events::Event::PaymentPathSuccessful { .. } => {},
916916
events::Event::PaymentPathFailed { .. } => {},
917+
events::Event::PaymentFailed { .. } => {},
917918
events::Event::ProbeSuccessful { .. } | events::Event::ProbeFailed { .. } => {
918919
// Even though we don't explicitly send probes, because probes are
919920
// detected based on hashing the payment hash+preimage, its rather

lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1752,12 +1752,18 @@ fn test_monitor_update_on_pending_forwards() {
17521752
commitment_signed_dance!(nodes[0], nodes[1], bs_updates.commitment_signed, false, true);
17531753

17541754
let events = nodes[0].node.get_and_clear_pending_events();
1755-
assert_eq!(events.len(), 2);
1755+
assert_eq!(events.len(), 3);
17561756
if let Event::PaymentPathFailed { payment_hash, payment_failed_permanently, .. } = events[0] {
17571757
assert_eq!(payment_hash, payment_hash_1);
17581758
assert!(payment_failed_permanently);
17591759
} else { panic!("Unexpected event!"); }
17601760
match events[1] {
1761+
Event::PaymentFailed { payment_hash, .. } => {
1762+
assert_eq!(payment_hash, payment_hash_1);
1763+
},
1764+
_ => panic!("Unexpected event"),
1765+
}
1766+
match events[2] {
17611767
Event::PendingHTLCsForwardable { .. } => { },
17621768
_ => panic!("Unexpected event"),
17631769
};

lightning/src/ln/functional_test_utils.rs

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1800,17 +1800,18 @@ macro_rules! expect_payment_failed {
18001800
}
18011801

18021802
pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
1803-
node: &'a Node<'b, 'c, 'd>, payment_failed_event: Event, expected_payment_hash: PaymentHash,
1803+
payment_failed_events: Vec<Event>, expected_payment_hash: PaymentHash,
18041804
expected_payment_failed_permanently: bool, conditions: PaymentFailedConditions<'e>
18051805
) {
1806-
let expected_payment_id = match payment_failed_event {
1806+
if conditions.expected_mpp_parts_remain { assert_eq!(payment_failed_events.len(), 1); } else { assert_eq!(payment_failed_events.len(), 2); }
1807+
let expected_payment_id = match &payment_failed_events[0] {
18071808
Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, network_update, short_channel_id,
18081809
#[cfg(test)]
18091810
error_code,
18101811
#[cfg(test)]
18111812
error_data, .. } => {
1812-
assert_eq!(payment_hash, expected_payment_hash, "unexpected payment_hash");
1813-
assert_eq!(payment_failed_permanently, expected_payment_failed_permanently, "unexpected payment_failed_permanently value");
1813+
assert_eq!(*payment_hash, expected_payment_hash, "unexpected payment_hash");
1814+
assert_eq!(*payment_failed_permanently, expected_payment_failed_permanently, "unexpected payment_failed_permanently value");
18141815
assert!(retry.is_some(), "expected retry.is_some()");
18151816
assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
18161817
assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
@@ -1839,7 +1840,7 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
18391840
},
18401841
Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if chan_closed => {
18411842
if let Some(scid) = conditions.expected_blamed_scid {
1842-
assert_eq!(short_channel_id, scid);
1843+
assert_eq!(*short_channel_id, scid);
18431844
}
18441845
assert!(is_permanent);
18451846
},
@@ -1853,10 +1854,7 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
18531854
_ => panic!("Unexpected event"),
18541855
};
18551856
if !conditions.expected_mpp_parts_remain {
1856-
node.node.abandon_payment(expected_payment_id);
1857-
let events = node.node.get_and_clear_pending_events();
1858-
assert_eq!(events.len(), 1);
1859-
match events[0] {
1857+
match &payment_failed_events[1] {
18601858
Event::PaymentFailed { ref payment_hash, ref payment_id } => {
18611859
assert_eq!(*payment_hash, expected_payment_hash, "unexpected second payment_hash");
18621860
assert_eq!(*payment_id, expected_payment_id);
@@ -1870,9 +1868,8 @@ pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>(
18701868
node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_payment_failed_permanently: bool,
18711869
conditions: PaymentFailedConditions<'e>
18721870
) {
1873-
let mut events = node.node.get_and_clear_pending_events();
1874-
assert_eq!(events.len(), 1);
1875-
expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_payment_failed_permanently, conditions);
1871+
let events = node.node.get_and_clear_pending_events();
1872+
expect_payment_failed_conditions_event(events, expected_payment_hash, expected_payment_failed_permanently, conditions);
18761873
}
18771874

18781875
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 {
@@ -2157,22 +2154,6 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
21572154
}
21582155

21592156
pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash) {
2160-
let expected_payment_id = pass_failed_payment_back_no_abandon(origin_node, expected_paths_slice, skip_last, our_payment_hash);
2161-
if !skip_last {
2162-
origin_node.node.abandon_payment(expected_payment_id.unwrap());
2163-
let events = origin_node.node.get_and_clear_pending_events();
2164-
assert_eq!(events.len(), 1);
2165-
match events[0] {
2166-
Event::PaymentFailed { ref payment_hash, ref payment_id } => {
2167-
assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
2168-
assert_eq!(*payment_id, expected_payment_id.unwrap());
2169-
}
2170-
_ => panic!("Unexpected second event"),
2171-
}
2172-
}
2173-
}
2174-
2175-
pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash) -> Option<PaymentId> {
21762157
let mut expected_paths: Vec<_> = expected_paths_slice.iter().collect();
21772158
check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());
21782159

@@ -2196,8 +2177,6 @@ pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b
21962177
per_path_msgs.sort_unstable_by(|(_, node_id_a), (_, node_id_b)| node_id_a.cmp(node_id_b));
21972178
expected_paths.sort_unstable_by(|path_a, path_b| path_a[path_a.len() - 2].node.get_our_node_id().cmp(&path_b[path_b.len() - 2].node.get_our_node_id()));
21982179

2199-
let mut expected_payment_id = None;
2200-
22012180
for (i, (expected_route, (path_msgs, next_hop))) in expected_paths.iter().zip(per_path_msgs.drain(..)).enumerate() {
22022181
let mut next_msgs = Some(path_msgs);
22032182
let mut expected_next_node = next_hop;
@@ -2245,8 +2224,9 @@ pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b
22452224
assert!(origin_node.node.get_and_clear_pending_msg_events().is_empty());
22462225
commitment_signed_dance!(origin_node, prev_node, next_msgs.as_ref().unwrap().1, false);
22472226
let events = origin_node.node.get_and_clear_pending_events();
2248-
assert_eq!(events.len(), 1);
2249-
expected_payment_id = Some(match events[0] {
2227+
if i == expected_paths.len() - 1 { assert_eq!(events.len(), 2); } else { assert_eq!(events.len(), 1); }
2228+
2229+
let expected_payment_id = match events[0] {
22502230
Event::PaymentPathFailed { payment_hash, payment_failed_permanently, all_paths_failed, ref path, ref payment_id, .. } => {
22512231
assert_eq!(payment_hash, our_payment_hash);
22522232
assert!(payment_failed_permanently);
@@ -2257,7 +2237,16 @@ pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b
22572237
payment_id.unwrap()
22582238
},
22592239
_ => panic!("Unexpected event"),
2260-
});
2240+
};
2241+
if i == expected_paths.len() - 1 {
2242+
match events[1] {
2243+
Event::PaymentFailed { ref payment_hash, ref payment_id } => {
2244+
assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
2245+
assert_eq!(*payment_id, expected_payment_id);
2246+
}
2247+
_ => panic!("Unexpected second event"),
2248+
}
2249+
}
22612250
}
22622251
}
22632252

@@ -2266,8 +2255,6 @@ pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b
22662255
assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_events().is_empty());
22672256
assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events().is_empty());
22682257
check_added_monitors!(expected_paths[0].last().unwrap(), 0);
2269-
2270-
expected_payment_id
22712258
}
22722259

22732260
pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], our_payment_hash: PaymentHash) {

0 commit comments

Comments
 (0)