Skip to content

Commit b6d7d55

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

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
@@ -5102,14 +5102,20 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
51025102

51035103
#[cfg(test)]
51045104
mod tests {
5105-
use ln::channelmanager::PersistenceNotifier;
5106-
use std::sync::Arc;
5105+
use bitcoin::hashes::Hash;
5106+
use bitcoin::hashes::sha256::Hash as Sha256;
51075107
use core::sync::atomic::{AtomicBool, Ordering};
5108-
use std::thread;
51095108
use core::time::Duration;
5109+
use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
5110+
use ln::channelmanager::PersistenceNotifier;
5111+
use ln::features::{InitFeatures, InvoiceFeatures};
51105112
use ln::functional_test_utils::*;
5111-
use ln::features::InitFeatures;
51125113
use ln::msgs::ChannelMessageHandler;
5114+
use routing::router::get_route;
5115+
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
5116+
use util::test_utils;
5117+
use std::sync::Arc;
5118+
use std::thread;
51135119

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

52295420
#[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)