Skip to content

Commit 0ec10bd

Browse files
valentinewallacevalentinewallace
committed
tests: make PaymentSecret optional in pass_along path
and use it to make more keysend tests
1 parent 16de087 commit 0ec10bd

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

50945094
#[cfg(test)]
50955095
mod tests {
5096-
use ln::channelmanager::PersistenceNotifier;
5097-
use std::sync::Arc;
5096+
use bitcoin::hashes::Hash;
5097+
use bitcoin::hashes::sha256::Hash as Sha256;
50985098
use core::sync::atomic::{AtomicBool, Ordering};
5099-
use std::thread;
51005099
use core::time::Duration;
5100+
use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
5101+
use ln::channelmanager::PersistenceNotifier;
5102+
use ln::features::{InitFeatures, InvoiceFeatures};
51015103
use ln::functional_test_utils::*;
5102-
use ln::features::InitFeatures;
51035104
use ln::msgs::ChannelMessageHandler;
5105+
use routing::router::get_route;
5106+
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
5107+
use util::test_utils;
5108+
use std::sync::Arc;
5109+
use std::thread;
51045110

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

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