Add some simple tests of payment secret tracking

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -3716,10 +3716,6 @@ where
 					});
 					!htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
 				});
-				let mut payment_secrets = self.pending_inbound_payments.lock().unwrap();
-				payment_secrets.retain(|_, inbound_payment| {
-					inbound_payment.expiry_height > height
-				});
 			}
 		}
 

lightning/src/ln/functional_test_utils.rs

@@ -66,7 +66,7 @@ pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &T
 		connect_blocks(node, conf_height - first_connect_height);
 	}
 	let mut block = Block {
-		header: BlockHeader { version: 0x20000000, prev_blockhash: node.best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
+		header: BlockHeader { version: 0x20000000, prev_blockhash: node.best_block_hash(), merkle_root: Default::default(), time: conf_height, bits: 42, nonce: 42 },
 		txdata: Vec::new(),
 	};
 	for _ in 0..*node.network_chan_count.borrow() { // Make sure we don't end up with channels at the same short id by offsetting by chan_count
@@ -102,15 +102,16 @@ pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32) ->
 		_ => false,
 	};
 
+	let height = node.best_block_info().1 + 1;
 	let mut block = Block {
-		header: BlockHeader { version: 0x2000000, prev_blockhash: node.best_block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
+		header: BlockHeader { version: 0x2000000, prev_blockhash: node.best_block_hash(), merkle_root: Default::default(), time: height, bits: 42, nonce: 42 },
 		txdata: vec![],
 	};
 	assert!(depth >= 1);
-	for _ in 0..depth - 1 {
+	for i in 1..depth {
 		do_connect_block(node, &block, skip_intermediaries);
 		block = Block {
-			header: BlockHeader { version: 0x20000000, prev_blockhash: block.header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
+			header: BlockHeader { version: 0x20000000, prev_blockhash: block.header.block_hash(), merkle_root: Default::default(), time: height + i, bits: 42, nonce: 42 },
 			txdata: vec![],
 		};
 	}

lightning/src/ln/functional_tests.rs

@@ -19,7 +19,7 @@ use chain::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PER
 use chain::transaction::OutPoint;
 use chain::keysinterface::{KeysInterface, BaseSign};
 use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
-use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSendFailure, BREAKDOWN_TIMEOUT};
+use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentSecret, PaymentHash, PaymentSendFailure, BREAKDOWN_TIMEOUT};
 use ln::channel::{Channel, ChannelError};
 use ln::{chan_utils, onion_utils};
 use routing::router::{Route, RouteHop, get_route};
@@ -8127,6 +8127,174 @@ fn test_simple_mpp() {
 	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
 }
 
+#[test]
+fn test_preimage_storage() {
+	// Simple test of payment preimage storage allowing no client-side storage to claim payments
+	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]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
+
+	{
+		let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 7200, 42);
+
+		let logger = test_utils::TestLogger::new();
+		let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
+		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, &[], 100_000, TEST_FINAL_CLTV, &logger).unwrap();
+		nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
+		check_added_monitors!(nodes[0], 1);
+		let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+		let mut payment_event = SendEvent::from_event(events.pop().unwrap());
+		nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+		commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+	}
+	// Note that after leaving the above scope we have no knowledge of any arguments or return
+	// values from previous calls.
+	expect_pending_htlcs_forwardable!(nodes[1]);
+	let events = nodes[1].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	match events[0] {
+		Event::PaymentReceived { payment_preimage, user_payment_id, .. } => {
+			assert_eq!(user_payment_id, 42);
+			claim_payment(&nodes[0], &[&nodes[1]], payment_preimage.unwrap());
+		},
+		_ => panic!("Unexpected event"),
+	}
+}
+
+#[test]
+fn test_secret_timeout() {
+	// Simple test of payment secret storage time outs
+	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]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
+
+	let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment(Some(100_000), 2, 0);
+
+	// We should fail to register the same payment hash twice, at least until we've connected a
+	// block with time 7200 + CHAN_CONFIRM_DEPTH + 1.
+	if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 0) {
+		assert_eq!(err, "Duplicate payment hash");
+	} else { panic!(); }
+	let mut block = Block {
+		header: BlockHeader {
+			version: 0x2000000,
+			prev_blockhash: nodes[1].blocks.borrow().last().unwrap().0.block_hash(),
+			merkle_root: Default::default(),
+			time: nodes[1].blocks.borrow().len() as u32 + 7200, bits: 42, nonce: 42 },
+		txdata: vec![],
+	};
+	connect_block(&nodes[1], &block);
+	if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 0) {
+		assert_eq!(err, "Duplicate payment hash");
+	} else { panic!(); }
+
+	// If we then connect the second block, we should be able to register the same payment hash
+	// again with a different user_payment_id (this time getting a new payment secret).
+	block.header.prev_blockhash = block.header.block_hash();
+	block.header.time += 1;
+	connect_block(&nodes[1], &block);
+	let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2, 42).unwrap();
+	assert_ne!(payment_secret_1, our_payment_secret);
+
+	{
+		let logger = test_utils::TestLogger::new();
+		let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
+		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, &[], 100_000, TEST_FINAL_CLTV, &logger).unwrap();
+		nodes[0].node.send_payment(&route, payment_hash, &Some(our_payment_secret)).unwrap();
+		check_added_monitors!(nodes[0], 1);
+		let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+		let mut payment_event = SendEvent::from_event(events.pop().unwrap());
+		nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+		commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+	}
+	// Note that after leaving the above scope we have no knowledge of any arguments or return
+	// values from previous calls.
+	expect_pending_htlcs_forwardable!(nodes[1]);
+	let events = nodes[1].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1);
+	match events[0] {
+		Event::PaymentReceived { payment_preimage, payment_secret, user_payment_id, .. } => {
+			assert!(payment_preimage.is_none());
+			assert_eq!(user_payment_id, 42);
+			assert_eq!(payment_secret, our_payment_secret);
+			// We don't actually have the payment preimage with which to claim this payment!
+		},
+		_ => panic!("Unexpected event"),
+	}
+}
+
+#[test]
+fn test_bad_secret_hash() {
+	// Simple test of unregistered payment hash/invalid payment secret handling
+	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]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
+
+	let random_payment_hash = PaymentHash([42; 32]);
+	let random_payment_secret = PaymentSecret([43; 32]);
+	let (our_payment_hash, our_payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 2, 0);
+
+	let logger = test_utils::TestLogger::new();
+	let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
+	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, &[], 100_000, TEST_FINAL_CLTV, &logger).unwrap();
+
+	// All the below cases should end up being handled exactly identically, so we macro the
+	// resulting events.
+	macro_rules! handle_unknown_invalid_payment_data {
+		() => {
+			check_added_monitors!(nodes[0], 1);
+			let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+			let payment_event = SendEvent::from_event(events.pop().unwrap());
+			nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+			commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
+
+			// We have to forward pending HTLCs once to process the receipt of the HTLC and then
+			// again to process the pending backwards-failure of the HTLC
+			expect_pending_htlcs_forwardable!(nodes[1]);
+			expect_pending_htlcs_forwardable!(nodes[1]);
+			check_added_monitors!(nodes[1], 1);
+
+			// We should fail the payment back
+			let mut events = nodes[1].node.get_and_clear_pending_msg_events();
+			match events.pop().unwrap() {
+				MessageSendEvent::UpdateHTLCs { node_id: _, updates: msgs::CommitmentUpdate { update_fail_htlcs, commitment_signed, .. } } => {
+					nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &update_fail_htlcs[0]);
+					commitment_signed_dance!(nodes[0], nodes[1], commitment_signed, false);
+				},
+				_ => panic!("Unexpected event"),
+			}
+		}
+	}
+
+	let expected_error_code = 0x4000|15; // incorrect_or_unknown_payment_details
+	// Error data is the HTLC value (100,000) and current block height
+	let expected_error_data = [0, 0, 0, 0, 0, 1, 0x86, 0xa0, 0, 0, 0, CHAN_CONFIRM_DEPTH as u8];
+
+	// Send a payment with the right payment hash but the wrong payment secret
+	nodes[0].node.send_payment(&route, our_payment_hash, &Some(random_payment_secret)).unwrap();
+	handle_unknown_invalid_payment_data!();
+	expect_payment_failed!(nodes[0], our_payment_hash, true, expected_error_code, expected_error_data);
+
+	// Send a payment with a random payment hash, but the right payment secret
+	nodes[0].node.send_payment(&route, random_payment_hash, &Some(our_payment_secret)).unwrap();
+	handle_unknown_invalid_payment_data!();
+	expect_payment_failed!(nodes[0], random_payment_hash, true, expected_error_code, expected_error_data);
+
+	// Send a payment with a random payment hash and random payment secret
+	nodes[0].node.send_payment(&route, random_payment_hash, &Some(random_payment_secret)).unwrap();
+	handle_unknown_invalid_payment_data!();
+	expect_payment_failed!(nodes[0], random_payment_hash, true, expected_error_code, expected_error_data);
+}
+
 #[test]
 fn test_update_err_monitor_lockdown() {
 	// Our monitor will lock update of local commitment transaction if a broadcastion condition