Skip to content

Commit af5ea24

Browse files
valentinewallacevalentinewallace
committed
tests: make PaymentSecret optional in pass_along path
and use it to make more keysend tests
1 parent d3e8ab5 commit af5ea24

File tree

4 files changed

+207
-13
lines changed

4 files changed

+207
-13
lines changed

‎lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2039,15 +2039,15 @@ fn test_path_paused_mpp() {
20392039
// Pass the first HTLC of the payment along to nodes[3].
20402040
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
20412041
assert_eq!(events.len(), 1);
2042-
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 0, payment_hash.clone(), payment_secret, events.pop().unwrap(), false, None);
2042+
pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 0, payment_hash.clone(), Some(payment_secret), events.pop().unwrap(), false, None);
20432043

20442044
// And check that, after we successfully update the monitor for chan_2 we can pass the second
20452045
// HTLC along to nodes[3] and claim the whole payment back to nodes[0].
20462046
let (outpoint, latest_update) = nodes[0].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_2_id).unwrap().clone();
20472047
nodes[0].node.channel_monitor_updated(&outpoint, latest_update);
20482048
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
20492049
assert_eq!(events.len(), 1);
2050-
pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash.clone(), payment_secret, events.pop().unwrap(), true, None);
2050+
pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash.clone(), Some(payment_secret), events.pop().unwrap(), true, None);
20512051

20522052
claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
20532053
}

‎lightning/src/ln/channelmanager.rs

Lines changed: 195 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5120,14 +5120,20 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
51205120

51215121
#[cfg(test)]
51225122
mod tests {
5123-
use ln::channelmanager::PersistenceNotifier;
5124-
use std::sync::Arc;
5123+
use bitcoin::hashes::Hash;
5124+
use bitcoin::hashes::sha256::Hash as Sha256;
51255125
use core::sync::atomic::{AtomicBool, Ordering};
5126-
use std::thread;
51275126
use core::time::Duration;
5127+
use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
5128+
use ln::channelmanager::PersistenceNotifier;
5129+
use ln::features::{InitFeatures, InvoiceFeatures};
51285130
use ln::functional_test_utils::*;
5129-
use ln::features::InitFeatures;
51305131
use ln::msgs::ChannelMessageHandler;
5132+
use routing::router::get_route;
5133+
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
5134+
use util::test_utils;
5135+
use std::sync::Arc;
5136+
use std::thread;
51315137

51325138
#[test]
51335139
fn test_wait_timeout() {
@@ -5242,6 +5248,191 @@ mod tests {
52425248
assert_ne!(nodes[0].node.list_channels()[0], node_a_chan_info);
52435249
assert_ne!(nodes[1].node.list_channels()[0], node_b_chan_info);
52445250
}
5251+
5252+
#[test]
5253+
fn test_keysend_dup_hash_partial_mpp() {
5254+
// Test that a keysend payment with a duplicate hash to an existing partial MPP payment fails as
5255+
// expected.
5256+
let chanmon_cfgs = create_chanmon_cfgs(2);
5257+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
5258+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
5259+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
5260+
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
5261+
let logger = test_utils::TestLogger::new();
5262+
5263+
// First, send a partial MPP payment.
5264+
let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
5265+
let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
5266+
let (payment_preimage, our_payment_hash, payment_secret) = get_payment_preimage_hash!(&nodes[1]);
5267+
// Use the utility function send_payment_along_path to send the payment with MPP data which
5268+
// indicates there are more HTLCs coming.
5269+
let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match.
5270+
nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, &None).unwrap();
5271+
check_added_monitors!(nodes[0], 1);
5272+
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
5273+
assert_eq!(events.len(), 1);
5274+
pass_along_path(&nodes[0], &[&nodes[1]], 200_000, our_payment_hash, Some(payment_secret), events.drain(..).next().unwrap(), false, None);
5275+
5276+
// Next, send a keysend payment with the same payment_hash and make sure it fails.
5277+
nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
5278+
check_added_monitors!(nodes[0], 1);
5279+
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
5280+
assert_eq!(events.len(), 1);
5281+
let ev = events.drain(..).next().unwrap();
5282+
let payment_event = SendEvent::from_event(ev);
5283+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
5284+
check_added_monitors!(nodes[1], 0);
5285+
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
5286+
expect_pending_htlcs_forwardable!(nodes[1]);
5287+
expect_pending_htlcs_forwardable!(nodes[1]);
5288+
check_added_monitors!(nodes[1], 1);
5289+
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
5290+
assert!(updates.update_add_htlcs.is_empty());
5291+
assert!(updates.update_fulfill_htlcs.is_empty());
5292+
assert_eq!(updates.update_fail_htlcs.len(), 1);
5293+
assert!(updates.update_fail_malformed_htlcs.is_empty());
5294+
assert!(updates.update_fee.is_none());
5295+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
5296+
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, true, true);
5297+
expect_payment_failed!(nodes[0], our_payment_hash, true);
5298+
5299+
// Send the second half of the original MPP payment.
5300+
nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, &None).unwrap();
5301+
check_added_monitors!(nodes[0], 1);
5302+
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
5303+
assert_eq!(events.len(), 1);
5304+
pass_along_path(&nodes[0], &[&nodes[1]], 200_000, our_payment_hash, Some(payment_secret), events.drain(..).next().unwrap(), true, None);
5305+
5306+
// Claim the full MPP payment. Note that we can't use a test utility like
5307+
// claim_funds_along_route because the ordering of the messages causes the second half of the
5308+
// payment to be put in the holding cell, which confuses the test utilities. So we exchange the
5309+
// lightning messages manually.
5310+
assert!(nodes[1].node.claim_funds(payment_preimage));
5311+
check_added_monitors!(nodes[1], 2);
5312+
let bs_first_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
5313+
nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_first_updates.update_fulfill_htlcs[0]);
5314+
nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_updates.commitment_signed);
5315+
check_added_monitors!(nodes[0], 1);
5316+
let (as_first_raa, as_first_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
5317+
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa);
5318+
check_added_monitors!(nodes[1], 1);
5319+
let bs_second_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
5320+
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_first_cs);
5321+
check_added_monitors!(nodes[1], 1);
5322+
let bs_first_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
5323+
nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_second_updates.update_fulfill_htlcs[0]);
5324+
nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_updates.commitment_signed);
5325+
check_added_monitors!(nodes[0], 1);
5326+
let as_second_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
5327+
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa);
5328+
let as_second_updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
5329+
check_added_monitors!(nodes[0], 1);
5330+
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa);
5331+
check_added_monitors!(nodes[1], 1);
5332+
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_second_updates.commitment_signed);
5333+
check_added_monitors!(nodes[1], 1);
5334+
let bs_third_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
5335+
nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa);
5336+
check_added_monitors!(nodes[0], 1);
5337+
5338+
// There's an existing bug that generates a PaymentSent event for each MPP path, so handle that here.
5339+
let events = nodes[0].node.get_and_clear_pending_events();
5340+
match events[0] {
5341+
Event::PaymentSent { payment_preimage: ref preimage } => {
5342+
assert_eq!(payment_preimage, *preimage);
5343+
},
5344+
_ => panic!("Unexpected event"),
5345+
}
5346+
match events[1] {
5347+
Event::PaymentSent { payment_preimage: ref preimage } => {
5348+
assert_eq!(payment_preimage, *preimage);
5349+
},
5350+
_ => panic!("Unexpected event"),
5351+
}
5352+
}
5353+
5354+
#[test]
5355+
fn test_keysend_dup_payment_hash() {
5356+
// (1): Test that a keysend payment with a duplicate payment hash to an existing pending
5357+
// outbound regular payment fails as expected.
5358+
// (2): Test that a regular payment with a duplicate payment hash to an existing keysend payment
5359+
// fails as expected.
5360+
let chanmon_cfgs = create_chanmon_cfgs(2);
5361+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
5362+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
5363+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
5364+
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
5365+
let logger = test_utils::TestLogger::new();
5366+
5367+
// To start (1), send a regular payment but don't claim it.
5368+
let expected_route = [&nodes[1]];
5369+
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &expected_route, 100_000);
5370+
5371+
// Next, attempt a keysend payment and make sure it fails.
5372+
let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph.read().unwrap(), &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
5373+
nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
5374+
check_added_monitors!(nodes[0], 1);
5375+
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
5376+
assert_eq!(events.len(), 1);
5377+
let ev = events.drain(..).next().unwrap();
5378+
let payment_event = SendEvent::from_event(ev);
5379+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
5380+
check_added_monitors!(nodes[1], 0);
5381+
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
5382+
expect_pending_htlcs_forwardable!(nodes[1]);
5383+
expect_pending_htlcs_forwardable!(nodes[1]);
5384+
check_added_monitors!(nodes[1], 1);
5385+
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
5386+
assert!(updates.update_add_htlcs.is_empty());
5387+
assert!(updates.update_fulfill_htlcs.is_empty());
5388+
assert_eq!(updates.update_fail_htlcs.len(), 1);
5389+
assert!(updates.update_fail_malformed_htlcs.is_empty());
5390+
assert!(updates.update_fee.is_none());
5391+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
5392+
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, true, true);
5393+
expect_payment_failed!(nodes[0], payment_hash, true);
5394+
5395+
// Finally, claim the original payment.
5396+
claim_payment(&nodes[0], &expected_route, payment_preimage);
5397+
5398+
// To start (2), send a keysend payment but don't claim it.
5399+
let payment_preimage = PaymentPreimage([42; 32]);
5400+
let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph.read().unwrap(), &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
5401+
let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
5402+
check_added_monitors!(nodes[0], 1);
5403+
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
5404+
assert_eq!(events.len(), 1);
5405+
let event = events.pop().unwrap();
5406+
let path = vec![&nodes[1]];
5407+
pass_along_path(&nodes[0], &path, 100_000, payment_hash, None, event, true, Some(payment_preimage));
5408+
5409+
// Next, attempt a regular payment and make sure it fails.
5410+
let payment_secret = PaymentSecret([43; 32]);
5411+
nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
5412+
check_added_monitors!(nodes[0], 1);
5413+
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
5414+
assert_eq!(events.len(), 1);
5415+
let ev = events.drain(..).next().unwrap();
5416+
let payment_event = SendEvent::from_event(ev);
5417+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
5418+
check_added_monitors!(nodes[1], 0);
5419+
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
5420+
expect_pending_htlcs_forwardable!(nodes[1]);
5421+
expect_pending_htlcs_forwardable!(nodes[1]);
5422+
check_added_monitors!(nodes[1], 1);
5423+
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
5424+
assert!(updates.update_add_htlcs.is_empty());
5425+
assert!(updates.update_fulfill_htlcs.is_empty());
5426+
assert_eq!(updates.update_fail_htlcs.len(), 1);
5427+
assert!(updates.update_fail_malformed_htlcs.is_empty());
5428+
assert!(updates.update_fee.is_none());
5429+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
5430+
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, true, true);
5431+
expect_payment_failed!(nodes[0], payment_hash, true);
5432+
5433+
// Finally, succeed the keysend payment.
5434+
claim_payment(&nodes[0], &expected_route, payment_preimage);
5435+
}
52455436
}
52465437

52475438
#[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))]

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,7 +1056,7 @@ pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
10561056
pass_along_route(origin_node, expected_paths, recv_value, our_payment_hash, our_payment_secret);
10571057
}
10581058

1059-
pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret, ev: MessageSendEvent, payment_received_expected: bool, expected_preimage: Option<PaymentPreimage>) {
1059+
pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option<PaymentSecret>, ev: MessageSendEvent, payment_received_expected: bool, expected_preimage: Option<PaymentPreimage>) {
10601060
let mut payment_event = SendEvent::from_event(ev);
10611061
let mut prev_node = origin_node;
10621062

@@ -1079,9 +1079,12 @@ pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path
10791079
match &purpose {
10801080
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
10811081
assert_eq!(expected_preimage, *payment_preimage);
1082-
assert_eq!(our_payment_secret, *payment_secret);
1082+
assert_eq!(our_payment_secret.unwrap(), *payment_secret);
1083+
},
1084+
PaymentPurpose::SpontaneousPayment(payment_preimage) => {
1085+
assert_eq!(expected_preimage.unwrap(), *payment_preimage);
1086+
assert!(our_payment_secret.is_none());
10831087
},
1084-
_ => {},
10851088
}
10861089
assert_eq!(amt, recv_value);
10871090
},
@@ -1109,7 +1112,7 @@ pub fn pass_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
11091112
// Once we've gotten through all the HTLCs, the last one should result in a
11101113
// PaymentReceived (but each previous one should not!), .
11111114
let expect_payment = path_idx == expected_route.len() - 1;
1112-
pass_along_path(origin_node, expected_path, recv_value, our_payment_hash.clone(), our_payment_secret, ev, expect_payment, None);
1115+
pass_along_path(origin_node, expected_path, recv_value, our_payment_hash.clone(), Some(our_payment_secret), ev, expect_payment, None);
11131116
}
11141117
}
11151118

‎lightning/src/ln/functional_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4173,7 +4173,7 @@ fn do_test_htlc_timeout(send_partial_mpp: bool) {
41734173
assert_eq!(events.len(), 1);
41744174
// Now do the relevant commitment_signed/RAA dances along the path, noting that the final
41754175
// hop should *not* yet generate any PaymentReceived event(s).
4176-
pass_along_path(&nodes[0], &[&nodes[1]], 100000, our_payment_hash, payment_secret, events.drain(..).next().unwrap(), false, None);
4176+
pass_along_path(&nodes[0], &[&nodes[1]], 100000, our_payment_hash, Some(payment_secret), events.drain(..).next().unwrap(), false, None);
41774177
our_payment_hash
41784178
} else {
41794179
route_payment(&nodes[0], &[&nodes[1]], 100000).1
@@ -9599,7 +9599,7 @@ fn test_keysend_payments_to_public_node() {
95999599
assert_eq!(events.len(), 1);
96009600
let event = events.pop().unwrap();
96019601
let path = vec![&nodes[1]];
9602-
pass_along_path(&nodes[0], &path, 10000, payment_hash, PaymentSecret([0; 32]), event, true, Some(test_preimage));
9602+
pass_along_path(&nodes[0], &path, 10000, payment_hash, None, event, true, Some(test_preimage));
96039603
claim_payment(&nodes[0], &path, test_preimage);
96049604
}
96059605

@@ -9629,6 +9629,6 @@ fn test_keysend_payments_to_private_node() {
96299629
assert_eq!(events.len(), 1);
96309630
let event = events.pop().unwrap();
96319631
let path = vec![&nodes[1]];
9632-
pass_along_path(&nodes[0], &path, 10000, payment_hash, PaymentSecret([0; 32]), event, true, Some(test_preimage));
9632+
pass_along_path(&nodes[0], &path, 10000, payment_hash, None, event, true, Some(test_preimage));
96339633
claim_payment(&nodes[0], &path, test_preimage);
96349634
}

0 commit comments

Comments
 (0)