Skip to content

Commit d1445c4

Browse files
valentinewallacevalentinewallace
committed
Payment retries: copy tests from InvoicePayer
As part of migrating payment retries from InvoicePayer to ChannelManager, several tests don't need a rewrite and can be pretty much copied and pasted.
1 parent 3c7b493 commit d1445c4

File tree

1 file changed

+325
-0
lines changed

1 file changed

+325
-0
lines changed

‎lightning/src/ln/payment_tests.rs

+325
Original file line numberDiff line numberDiff line change
@@ -2063,3 +2063,328 @@ fn fails_paying_after_rejected_by_payee() {
20632063
expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], [HTLCDestination::FailedPayment { payment_hash }]);
20642064
pass_failed_payment_back(&nodes[0], &[&[&nodes[1]]], false, payment_hash);
20652065
}
2066+
2067+
#[test]
2068+
fn retry_multi_path_single_failed_payment() {
2069+
// Tests that we can/will retry after a single path of an MPP payment failed immediately
2070+
let chanmon_cfgs = create_chanmon_cfgs(2);
2071+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
2072+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]);
2073+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
2074+
2075+
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
2076+
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
2077+
let chans = nodes[0].node.list_usable_channels();
2078+
let mut route = Route {
2079+
paths: vec![
2080+
vec![RouteHop {
2081+
pubkey: nodes[1].node.get_our_node_id(),
2082+
node_features: nodes[1].node.node_features(),
2083+
short_channel_id: chans[0].short_channel_id.unwrap(),
2084+
channel_features: nodes[1].node.channel_features(),
2085+
fee_msat: 10_000,
2086+
cltv_expiry_delta: 100,
2087+
}],
2088+
vec![RouteHop {
2089+
pubkey: nodes[1].node.get_our_node_id(),
2090+
node_features: nodes[1].node.node_features(),
2091+
short_channel_id: chans[1].short_channel_id.unwrap(),
2092+
channel_features: nodes[1].node.channel_features(),
2093+
fee_msat: 100_000_001, // Our default max-HTLC-value is 10% of the channel value, which this is one more than
2094+
cltv_expiry_delta: 100,
2095+
}],
2096+
],
2097+
payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
2098+
};
2099+
nodes[0].router.expect_find_route(Ok(route.clone()));
2100+
// On retry, split the payment across both channels.
2101+
route.paths[0][0].fee_msat = 50_000_001;
2102+
route.paths[1][0].fee_msat = 50_000_000;
2103+
nodes[0].router.expect_find_route(Ok(route.clone()));
2104+
2105+
let amt_msat = 100_010_000;
2106+
let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
2107+
#[cfg(feature = "std")]
2108+
let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
2109+
#[cfg(not(feature = "std"))]
2110+
let payment_expiry_secs = 60 * 60;
2111+
let mut invoice_features = InvoiceFeatures::empty();
2112+
invoice_features.set_variable_length_onion_required();
2113+
invoice_features.set_payment_secret_required();
2114+
invoice_features.set_basic_mpp_optional();
2115+
let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
2116+
.with_expiry_time(payment_expiry_secs as u64)
2117+
.with_features(invoice_features);
2118+
let route_params = RouteParameters {
2119+
payment_params,
2120+
final_value_msat: amt_msat,
2121+
final_cltv_expiry_delta: TEST_FINAL_CLTV,
2122+
};
2123+
2124+
nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
2125+
let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
2126+
assert_eq!(htlc_msgs.len(), 2);
2127+
check_added_monitors!(nodes[0], 2);
2128+
}
2129+
2130+
#[test]
2131+
fn immediate_retry_on_failure() {
2132+
// Tests that we can/will retry immediately after a failure
2133+
let chanmon_cfgs = create_chanmon_cfgs(2);
2134+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
2135+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]);
2136+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
2137+
2138+
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
2139+
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
2140+
let chans = nodes[0].node.list_usable_channels();
2141+
let mut route = Route {
2142+
paths: vec![
2143+
vec![RouteHop {
2144+
pubkey: nodes[1].node.get_our_node_id(),
2145+
node_features: nodes[1].node.node_features(),
2146+
short_channel_id: chans[0].short_channel_id.unwrap(),
2147+
channel_features: nodes[1].node.channel_features(),
2148+
fee_msat: 100_000_001, // Our default max-HTLC-value is 10% of the channel value, which this is one more than
2149+
cltv_expiry_delta: 100,
2150+
}],
2151+
],
2152+
payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
2153+
};
2154+
nodes[0].router.expect_find_route(Ok(route.clone()));
2155+
// On retry, split the payment across both channels.
2156+
route.paths.push(route.paths[0].clone());
2157+
route.paths[0][0].short_channel_id = chans[1].short_channel_id.unwrap();
2158+
route.paths[0][0].fee_msat = 50_000_000;
2159+
route.paths[1][0].fee_msat = 50_000_001;
2160+
nodes[0].router.expect_find_route(Ok(route.clone()));
2161+
2162+
let amt_msat = 100_010_000;
2163+
let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
2164+
#[cfg(feature = "std")]
2165+
let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
2166+
#[cfg(not(feature = "std"))]
2167+
let payment_expiry_secs = 60 * 60;
2168+
let mut invoice_features = InvoiceFeatures::empty();
2169+
invoice_features.set_variable_length_onion_required();
2170+
invoice_features.set_payment_secret_required();
2171+
invoice_features.set_basic_mpp_optional();
2172+
let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
2173+
.with_expiry_time(payment_expiry_secs as u64)
2174+
.with_features(invoice_features);
2175+
let route_params = RouteParameters {
2176+
payment_params,
2177+
final_value_msat: amt_msat,
2178+
final_cltv_expiry_delta: TEST_FINAL_CLTV,
2179+
};
2180+
2181+
nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
2182+
let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
2183+
assert_eq!(htlc_msgs.len(), 2);
2184+
check_added_monitors!(nodes[0], 2);
2185+
}
2186+
2187+
#[test]
2188+
fn no_extra_retries_on_back_to_back_fail() {
2189+
// In a previous release, we had a race where we may exceed the payment retry count if we
2190+
// get two failures in a row with the second having `all_paths_failed` set.
2191+
// Generally, when we give up trying to retry a payment, we don't know for sure what the
2192+
// current state of the ChannelManager event queue is. Specifically, we cannot be sure that
2193+
// there are not multiple additional `PaymentPathFailed` or even `PaymentSent` events
2194+
// pending which we will see later. Thus, when we previously removed the retry tracking map
2195+
// entry after a `all_paths_failed` `PaymentPathFailed` event, we may have dropped the
2196+
// retry entry even though more events for the same payment were still pending. This led to
2197+
// us retrying a payment again even though we'd already given up on it.
2198+
//
2199+
// We now have a separate event - `PaymentFailed` which indicates no HTLCs remain and which
2200+
// is used to remove the payment retry counter entries instead. This tests for the specific
2201+
// excess-retry case while also testing `PaymentFailed` generation.
2202+
2203+
let chanmon_cfgs = create_chanmon_cfgs(3);
2204+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
2205+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
2206+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
2207+
2208+
let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0).0.contents.short_channel_id;
2209+
let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0).0.contents.short_channel_id;
2210+
2211+
let mut route = Route {
2212+
paths: vec![
2213+
vec![RouteHop {
2214+
pubkey: nodes[1].node.get_our_node_id(),
2215+
node_features: nodes[1].node.node_features(),
2216+
short_channel_id: chan_1_scid,
2217+
channel_features: nodes[1].node.channel_features(),
2218+
fee_msat: 0,
2219+
cltv_expiry_delta: 100,
2220+
}, RouteHop {
2221+
pubkey: nodes[2].node.get_our_node_id(),
2222+
node_features: nodes[2].node.node_features(),
2223+
short_channel_id: chan_2_scid,
2224+
channel_features: nodes[2].node.channel_features(),
2225+
fee_msat: 100_000_000,
2226+
cltv_expiry_delta: 100,
2227+
}],
2228+
vec![RouteHop {
2229+
pubkey: nodes[1].node.get_our_node_id(),
2230+
node_features: nodes[1].node.node_features(),
2231+
short_channel_id: chan_1_scid,
2232+
channel_features: nodes[1].node.channel_features(),
2233+
fee_msat: 0,
2234+
cltv_expiry_delta: 100,
2235+
}, RouteHop {
2236+
pubkey: nodes[2].node.get_our_node_id(),
2237+
node_features: nodes[2].node.node_features(),
2238+
short_channel_id: chan_2_scid,
2239+
channel_features: nodes[2].node.channel_features(),
2240+
fee_msat: 100_000_000,
2241+
cltv_expiry_delta: 100,
2242+
}]
2243+
],
2244+
payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())),
2245+
};
2246+
nodes[0].router.expect_find_route(Ok(route.clone()));
2247+
// On retry, we'll only be asked for one path
2248+
route.paths.remove(1);
2249+
nodes[0].router.expect_find_route(Ok(route.clone()));
2250+
2251+
let amt_msat = 100_010_000;
2252+
let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
2253+
#[cfg(feature = "std")]
2254+
let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
2255+
#[cfg(not(feature = "std"))]
2256+
let payment_expiry_secs = 60 * 60;
2257+
let mut invoice_features = InvoiceFeatures::empty();
2258+
invoice_features.set_variable_length_onion_required();
2259+
invoice_features.set_payment_secret_required();
2260+
invoice_features.set_basic_mpp_optional();
2261+
let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
2262+
.with_expiry_time(payment_expiry_secs as u64)
2263+
.with_features(invoice_features);
2264+
let route_params = RouteParameters {
2265+
payment_params,
2266+
final_value_msat: amt_msat,
2267+
final_cltv_expiry_delta: TEST_FINAL_CLTV,
2268+
};
2269+
2270+
nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
2271+
let htlc_updates = SendEvent::from_node(&nodes[0]);
2272+
check_added_monitors!(nodes[0], 1);
2273+
assert_eq!(htlc_updates.msgs.len(), 1);
2274+
2275+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &htlc_updates.msgs[0]);
2276+
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &htlc_updates.commitment_msg);
2277+
check_added_monitors!(nodes[1], 1);
2278+
let (bs_first_raa, bs_first_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
2279+
2280+
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa);
2281+
check_added_monitors!(nodes[0], 1);
2282+
let second_htlc_updates = SendEvent::from_node(&nodes[0]);
2283+
2284+
nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_cs);
2285+
check_added_monitors!(nodes[0], 1);
2286+
let as_first_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
2287+
2288+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &second_htlc_updates.msgs[0]);
2289+
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &second_htlc_updates.commitment_msg);
2290+
check_added_monitors!(nodes[1], 1);
2291+
let bs_second_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
2292+
2293+
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa);
2294+
check_added_monitors!(nodes[1], 1);
2295+
let bs_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
2296+
2297+
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa);
2298+
check_added_monitors!(nodes[0], 1);
2299+
2300+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_update.update_fail_htlcs[0]);
2301+
nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_fail_update.commitment_signed);
2302+
check_added_monitors!(nodes[0], 1);
2303+
let (as_second_raa, as_third_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
2304+
2305+
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa);
2306+
check_added_monitors!(nodes[1], 1);
2307+
let bs_second_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
2308+
2309+
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_third_cs);
2310+
check_added_monitors!(nodes[1], 1);
2311+
let bs_third_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
2312+
2313+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_second_fail_update.update_fail_htlcs[0]);
2314+
nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_fail_update.commitment_signed);
2315+
check_added_monitors!(nodes[0], 1);
2316+
2317+
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa);
2318+
check_added_monitors!(nodes[0], 1);
2319+
let (as_third_raa, as_fourth_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
2320+
2321+
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_third_raa);
2322+
check_added_monitors!(nodes[1], 1);
2323+
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_fourth_cs);
2324+
check_added_monitors!(nodes[1], 1);
2325+
let bs_fourth_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
2326+
2327+
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_fourth_raa);
2328+
check_added_monitors!(nodes[0], 1);
2329+
2330+
// At this point A has sent two HTLCs which both failed due to lack of fee. It now has two
2331+
// pending `PaymentPathFailed` events, one with `all_paths_failed` unset, and the second
2332+
// with it set. The first event will use up the only retry we are allowed, with the second
2333+
// `PaymentPathFailed` being passed up to the user (us, in this case). Previously, we'd
2334+
// treated this as "HTLC complete" and dropped the retry counter, causing us to retry again
2335+
// if the final HTLC failed.
2336+
let mut events = nodes[0].node.get_and_clear_pending_events();
2337+
assert_eq!(events.len(), 4);
2338+
match events[0] {
2339+
Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => {
2340+
assert_eq!(payment_hash, ev_payment_hash);
2341+
assert_eq!(payment_failed_permanently, false);
2342+
},
2343+
_ => panic!("Unexpected event"),
2344+
}
2345+
match events[1] {
2346+
Event::PendingHTLCsForwardable { .. } => {},
2347+
_ => panic!("Unexpected event"),
2348+
}
2349+
match events[2] {
2350+
Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => {
2351+
assert_eq!(payment_hash, ev_payment_hash);
2352+
assert_eq!(payment_failed_permanently, false);
2353+
},
2354+
_ => panic!("Unexpected event"),
2355+
}
2356+
match events[3] {
2357+
Event::PendingHTLCsForwardable { .. } => {},
2358+
_ => panic!("Unexpected event"),
2359+
}
2360+
2361+
nodes[0].node.process_pending_htlc_forwards();
2362+
let retry_htlc_updates = SendEvent::from_node(&nodes[0]);
2363+
check_added_monitors!(nodes[0], 1);
2364+
2365+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &retry_htlc_updates.msgs[0]);
2366+
commitment_signed_dance!(nodes[1], nodes[0], &retry_htlc_updates.commitment_msg, false, true);
2367+
let bs_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
2368+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_update.update_fail_htlcs[0]);
2369+
commitment_signed_dance!(nodes[0], nodes[1], &bs_fail_update.commitment_signed, false, true);
2370+
2371+
let mut events = nodes[0].node.get_and_clear_pending_events();
2372+
assert_eq!(events.len(), 1);
2373+
match events[0] {
2374+
Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => {
2375+
assert_eq!(payment_hash, ev_payment_hash);
2376+
assert_eq!(payment_failed_permanently, false);
2377+
},
2378+
_ => panic!("Unexpected event"),
2379+
}
2380+
nodes[0].node.abandon_payment(PaymentId(payment_hash.0));
2381+
events = nodes[0].node.get_and_clear_pending_events();
2382+
assert_eq!(events.len(), 1);
2383+
match events[0] {
2384+
Event::PaymentFailed { payment_hash: ref ev_payment_hash, payment_id: ref ev_payment_id } => {
2385+
assert_eq!(payment_hash, *ev_payment_hash);
2386+
assert_eq!(PaymentId(payment_hash.0), *ev_payment_id);
2387+
},
2388+
_ => panic!("Unexpected event"),
2389+
}
2390+
}

0 commit comments

Comments
 (0)