Skip to content

Commit

Permalink
Merge pull request lightningdevkit#1451 from TheBlueMatt/2022-04-moar…
Browse files Browse the repository at this point in the history 
…-mpp-fail-test

Add test coverage for failure of inconsistent MPP parts
  • Loading branch information
@TheBlueMatt
TheBlueMatt authored Apr 29, 2022
2 parents dc8479a + 7a8344a commit 9bdce47
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 46 deletions.
37 changes: 18 additions & 19 deletions lightning/src/ln/functional_test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,21 @@ macro_rules! get_payment_preimage_hash {
}
}

#[macro_export]
macro_rules! get_route {
($send_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
use $crate::chain::keysinterface::KeysInterface;
let scorer = $crate::util::test_utils::TestScorer::with_penalty(0);
let keys_manager = $crate::util::test_utils::TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet);
let random_seed_bytes = keys_manager.get_secure_random_bytes();
$crate::routing::router::get_route(
&$send_node.node.get_our_node_id(), &$payment_params, &$send_node.network_graph.read_only(),
Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
$recv_value, $cltv, $send_node.logger, &scorer, &random_seed_bytes
)
}}
}

#[cfg(test)]
#[macro_export]
macro_rules! get_route_and_payment_hash {
Expand All @@ -1176,17 +1191,9 @@ macro_rules! get_route_and_payment_hash {
$crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value, TEST_FINAL_CLTV)
}};
($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
use $crate::chain::keysinterface::KeysInterface;
let (payment_preimage, payment_hash, payment_secret) = $crate::get_payment_preimage_hash!($recv_node, Some($recv_value));
let scorer = $crate::util::test_utils::TestScorer::with_penalty(0);
let keys_manager = $crate::util::test_utils::TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet);
let random_seed_bytes = keys_manager.get_secure_random_bytes();
let route = $crate::routing::router::get_route(
&$send_node.node.get_our_node_id(), &$payment_params, &$send_node.network_graph.read_only(),
Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
$recv_value, $cltv, $send_node.logger, &scorer, &random_seed_bytes
).unwrap();
(route, payment_hash, payment_preimage, payment_secret)
let route = $crate::get_route!($send_node, $payment_params, $recv_value, $cltv);
(route.unwrap(), payment_hash, payment_preimage, payment_secret)
}}
}

Expand Down Expand Up @@ -1650,15 +1657,7 @@ pub const TEST_FINAL_CLTV: u32 = 70;
pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id())
.with_features(InvoiceFeatures::known());
let network_graph = origin_node.network_graph.read_only();
let scorer = test_utils::TestScorer::with_penalty(0);
let seed = [0u8; 32];
let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
let random_seed_bytes = keys_manager.get_secure_random_bytes();
let route = get_route(
&origin_node.node.get_our_node_id(), &payment_params, &network_graph,
Some(&origin_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer, &random_seed_bytes).unwrap();
let route = get_route!(origin_node, payment_params, recv_value, TEST_FINAL_CLTV).unwrap();
assert_eq!(route.paths.len(), 1);
assert_eq!(route.paths[0].len(), expected_route.len());
for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {
Expand Down
171 changes: 144 additions & 27 deletions lightning/src/ln/functional_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9457,12 +9457,7 @@ fn test_forwardable_regen() {
claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage_2);
}

#[test]
fn test_dup_htlc_second_fail_panic() {
// Previously, if we received two HTLCs back-to-back, where the second overran the expected
// value for the payment, we'd fail back both HTLCs after generating a `PaymentReceived` event.
// Then, if the user failed the second payment, they'd hit a "tried to fail an already failed
// HTLC" debug panic. This tests for this behavior, checking that only one HTLC is auto-failed.
fn do_test_dup_htlc_second_rejected(test_for_second_fail_panic: bool) {
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All @@ -9472,14 +9467,9 @@ fn test_dup_htlc_second_fail_panic() {

let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
.with_features(InvoiceFeatures::known());
let scorer = test_utils::TestScorer::with_penalty(0);
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
let route = get_route(
&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(),
Some(&nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),
10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
let route = get_route!(nodes[0], payment_params, 10_000, TEST_FINAL_CLTV).unwrap();

let (_, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[1]);
let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[1]);

{
nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
Expand Down Expand Up @@ -9507,26 +9497,153 @@ fn test_dup_htlc_second_fail_panic() {
// the first HTLC delivered above.
}

// Now we go fail back the first HTLC from the user end.
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();
nodes[1].node.fail_htlc_backwards(&our_payment_hash);

expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();
if test_for_second_fail_panic {
// Now we go fail back the first HTLC from the user end.
nodes[1].node.fail_htlc_backwards(&our_payment_hash);

check_added_monitors!(nodes[1], 1);
let fail_updates_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
assert_eq!(fail_updates_1.update_fail_htlcs.len(), 2);
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();

check_added_monitors!(nodes[1], 1);
let fail_updates_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
assert_eq!(fail_updates_1.update_fail_htlcs.len(), 2);

nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[1]);
commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false);

let failure_events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(failure_events.len(), 2);
if let Event::PaymentPathFailed { .. } = failure_events[0] {} else { panic!(); }
if let Event::PaymentPathFailed { .. } = failure_events[1] {} else { panic!(); }
} else {
// Let the second HTLC fail and claim the first
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();

check_added_monitors!(nodes[1], 1);
let fail_updates_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false);

expect_payment_failed_conditions!(nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain());

claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage);
}
}

#[test]
fn test_dup_htlc_second_fail_panic() {
// Previously, if we received two HTLCs back-to-back, where the second overran the expected
// value for the payment, we'd fail back both HTLCs after generating a `PaymentReceived` event.
// Then, if the user failed the second payment, they'd hit a "tried to fail an already failed
// HTLC" debug panic. This tests for this behavior, checking that only one HTLC is auto-failed.
do_test_dup_htlc_second_rejected(true);
}

#[test]
fn test_dup_htlc_second_rejected() {
// Test that if we receive a second HTLC for an MPP payment that overruns the payment amount we
// simply reject the second HTLC but are still able to claim the first HTLC.
do_test_dup_htlc_second_rejected(false);
}

#[test]
fn test_inconsistent_mpp_params() {
// Test that if we recieve two HTLCs with different payment parameters we fail back the first
// such HTLC and allow the second to stay.
let chanmon_cfgs = create_chanmon_cfgs(4);
let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
let nodes = create_network(4, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 100_000, 0, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 100_000, 0, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 100_000, 0, InitFeatures::known(), InitFeatures::known());

let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id())
.with_features(InvoiceFeatures::known());
let mut route = get_route!(nodes[0], payment_params, 15_000_000, TEST_FINAL_CLTV).unwrap();
assert_eq!(route.paths.len(), 2);
route.paths.sort_by(|path_a, _| {
// Sort the path so that the path through nodes[1] comes first
if path_a[0].pubkey == nodes[1].node.get_our_node_id() {
core::cmp::Ordering::Less } else { core::cmp::Ordering::Greater }
});
let payment_params_opt = Some(payment_params);

let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[3]);

let cur_height = nodes[0].best_block_info().1;
let payment_id = PaymentId([42; 32]);
{
nodes[0].node.send_payment_along_path(&route.paths[0], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None).unwrap();
check_added_monitors!(nodes[0], 1);

let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), false, None);
}
assert!(nodes[3].node.get_and_clear_pending_events().is_empty());

{
nodes[0].node.send_payment_along_path(&route.paths[1], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 14_000_000, cur_height, payment_id, &None).unwrap();
check_added_monitors!(nodes[0], 1);

let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
let payment_event = SendEvent::from_event(events.pop().unwrap());

nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
commitment_signed_dance!(nodes[2], nodes[0], payment_event.commitment_msg, false);

nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[1]);
commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);

let mut events = nodes[2].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
let payment_event = SendEvent::from_event(events.pop().unwrap());

nodes[3].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &payment_event.msgs[0]);
check_added_monitors!(nodes[3], 0);
commitment_signed_dance!(nodes[3], nodes[2], payment_event.commitment_msg, true, true);

// At this point, nodes[3] should notice the two HTLCs don't contain the same total payment
// amount. It will assume the second is a privacy attack (no longer particularly relevant
// post-payment_secrets) and fail back the new HTLC.
}
expect_pending_htlcs_forwardable_ignore!(nodes[3]);
nodes[3].node.process_pending_htlc_forwards();
expect_pending_htlcs_forwardable_ignore!(nodes[3]);
nodes[3].node.process_pending_htlc_forwards();

check_added_monitors!(nodes[3], 1);

let fail_updates_1 = get_htlc_update_msgs!(nodes[3], nodes[2].node.get_our_node_id());
nodes[2].node.handle_update_fail_htlc(&nodes[3].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]);
commitment_signed_dance!(nodes[2], nodes[3], fail_updates_1.commitment_signed, false);

expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);

let fail_updates_2 = get_htlc_update_msgs!(nodes[2], nodes[0].node.get_our_node_id());
nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &fail_updates_2.update_fail_htlcs[0]);
commitment_signed_dance!(nodes[0], nodes[2], fail_updates_2.commitment_signed, false);

expect_payment_failed_conditions!(nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain());

nodes[0].node.send_payment_along_path(&route.paths[1], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None).unwrap();
check_added_monitors!(nodes[0], 1);

let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), true, None);

let failure_events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(failure_events.len(), 2);
if let Event::PaymentPathFailed { .. } = failure_events[0] {} else { panic!(); }
if let Event::PaymentPathFailed { .. } = failure_events[1] {} else { panic!(); }
claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
}

#[test]
Expand Down

0 comments on commit 9bdce47

Please sign in to comment.