f re-add the payment_ in payment_secret, even where it wasn't

Author: TheBlueMatt

lightning/src/ln/functional_tests.rs

@@ -4104,31 +4104,34 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
 	send_payment(&nodes[1], &[&nodes[2], &nodes[3], &nodes[5]], 500000);
 	assert_eq!(get_local_commitment_txn!(nodes[3], chan_2_3.2)[0].output.len(), 2);
 
-	let ds_dust_limit = {
+	let dust_limit_msat = {
 		let per_peer_state_lock;
 		let mut peer_state_lock;
 		let chan =
 			get_channel_ref!(nodes[3], nodes[2], per_peer_state_lock, peer_state_lock, chan_2_3.2);
-		chan.context().holder_dust_limit_satoshis
+		chan.context().holder_dust_limit_satoshis * 1000
 	};
 
 	// 0th HTLC (not added - smaller than dust limit + HTLC tx fee):
 	let path_4: &[_] = &[&nodes[2], &nodes[3], &nodes[4]];
-	let (_, hash_1, ..) = route_payment(&nodes[0], path_4, ds_dust_limit*1000);
+	let (_, hash_1, ..) = route_payment(&nodes[0], path_4, dust_limit_msat);
 
 	// 1st HTLC (not added - smaller than dust limit + HTLC tx fee):
-	let (_, hash_2, ..) = route_payment(&nodes[0], path_4, ds_dust_limit*1000);
-	let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], ds_dust_limit*1000);
+	let (_, hash_2, ..) = route_payment(&nodes[0], path_4, dust_limit_msat);
+	let (route_to_5, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], dust_limit_msat);
 
 	// 2nd HTLC (not added - smaller than dust limit + HTLC tx fee):
 	let path_5: &[&[_]] = &[&[&nodes[2], &nodes[3], &nodes[5]]];
-	let secret = nodes[5].node.create_inbound_payment_for_hash(hash_1, None, 7200, None).unwrap();
-	let route_2 = route.clone();
-	send_along_route_with_secret(&nodes[1], route_2, path_5, ds_dust_limit*1000, hash_1, secret);
+	let payment_secret =
+		nodes[5].node.create_inbound_payment_for_hash(hash_1, None, 7200, None).unwrap();
+	let route = route_to_5.clone();
+	send_along_route_with_secret(&nodes[1], route, path_5, dust_limit_msat, hash_1, payment_secret);
 
 	// 3rd HTLC (not added - smaller than dust limit + HTLC tx fee):
-	let secret = nodes[5].node.create_inbound_payment_for_hash(hash_2, None, 7200, None).unwrap();
-	send_along_route_with_secret(&nodes[1], route, path_5, ds_dust_limit*1000, hash_2, secret);
+	let payment_secret =
+		nodes[5].node.create_inbound_payment_for_hash(hash_2, None, 7200, None).unwrap();
+	let route = route_to_5;
+	send_along_route_with_secret(&nodes[1], route, path_5, dust_limit_msat, hash_2, payment_secret);
 
 	// 4th HTLC:
 	let (_, hash_3, ..) = route_payment(&nodes[0], &[&nodes[2], &nodes[3], &nodes[4]], 1000000);
@@ -4138,28 +4141,32 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
 	let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
 
 	// 6th HTLC:
-	let secret = nodes[5].node.create_inbound_payment_for_hash(hash_3, None, 7200, None).unwrap();
-	send_along_route_with_secret(&nodes[1], route.clone(), path_5, 1000000, hash_3, secret);
+	let payment_secret =
+		nodes[5].node.create_inbound_payment_for_hash(hash_3, None, 7200, None).unwrap();
+	send_along_route_with_secret(&nodes[1], route.clone(), path_5, 1000000, hash_3, payment_secret);
 
 	// 7th HTLC:
-	let secret = nodes[5].node.create_inbound_payment_for_hash(hash_4, None, 7200, None).unwrap();
-	send_along_route_with_secret(&nodes[1], route, path_5, 1000000, hash_4, secret);
+	let payment_secret =
+		nodes[5].node.create_inbound_payment_for_hash(hash_4, None, 7200, None).unwrap();
+	send_along_route_with_secret(&nodes[1], route, path_5, 1000000, hash_4, payment_secret);
 
 	// 8th HTLC:
 	let (_, hash_5, ..) = route_payment(&nodes[0], path_4, 1000000);
 
 	// 9th HTLC (not added - smaller than dust limit + HTLC tx fee):
-	let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], ds_dust_limit*1000);
-	let secret = nodes[5].node.create_inbound_payment_for_hash(hash_5, None, 7200, None).unwrap();
-	send_along_route_with_secret(&nodes[1], route, path_5, ds_dust_limit*1000, hash_5, secret);
+	let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], dust_limit_msat);
+	let payment_secret =
+		nodes[5].node.create_inbound_payment_for_hash(hash_5, None, 7200, None).unwrap();
+	send_along_route_with_secret(&nodes[1], route, path_5, dust_limit_msat, hash_5, payment_secret);
 
 	// 10th HTLC (not added - smaller than dust limit + HTLC tx fee):
-	let (_, hash_6, ..) = route_payment(&nodes[0], path_4, ds_dust_limit*1000);
+	let (_, hash_6, ..) = route_payment(&nodes[0], path_4, dust_limit_msat);
 
 	// 11th HTLC:
 	let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
-	let secret = nodes[5].node.create_inbound_payment_for_hash(hash_6, None, 7200, None).unwrap();
-	send_along_route_with_secret(&nodes[1], route, path_5, 1000000, hash_6, secret);
+	let payment_secret =
+		nodes[5].node.create_inbound_payment_for_hash(hash_6, None, 7200, None).unwrap();
+	send_along_route_with_secret(&nodes[1], route, path_5, 1000000, hash_6, payment_secret);
 
 	// Double-check that six of the new HTLC were added
 	// We now have six HTLCs pending over the dust limit and six HTLCs under the dust limit (ie,
@@ -6683,7 +6690,8 @@ pub fn test_onion_value_mpp_set_calculation() {
 
 	let total_msat = 100_000;
 	let expected_paths: &[&[&Node]] = &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]];
-	let (mut route, hash, preimage, secret) = get_route_and_payment_hash!(&nodes[0], nodes[3], total_msat);
+	let (mut route, hash, preimage, payment_secret) =
+		get_route_and_payment_hash!(&nodes[0], nodes[3], total_msat);
 	let sample_path = route.paths.pop().unwrap();
 
 	let mut path_1 = sample_path.clone();
@@ -6704,7 +6712,7 @@ pub fn test_onion_value_mpp_set_calculation() {
 
 	// Send payment
 	let id = PaymentId(nodes[0].keys_manager.backing.get_secure_random_bytes());
-	let onion = RecipientOnionFields::secret_only(secret);
+	let onion = RecipientOnionFields::secret_only(payment_secret);
 	let onion_session_privs = nodes[0]
 		.node
 		.test_add_new_pending_payment(hash, onion.clone(), id, &route)
@@ -6732,7 +6740,7 @@ pub fn test_onion_value_mpp_set_calculation() {
 			let height = nodes[0].best_block_info().1;
 			let session_priv = SecretKey::from_slice(&session_priv).unwrap();
 			let mut onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv);
-			let recipient_onion_fields = RecipientOnionFields::secret_only(secret);
+			let recipient_onion_fields = RecipientOnionFields::secret_only(payment_secret);
 			let (mut onion_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], 100_000,
 				&recipient_onion_fields, height + 1, &None, None, None).unwrap();
 			// Edit amt_to_forward to simulate the sender having set
@@ -6769,7 +6777,8 @@ pub fn test_onion_value_mpp_set_calculation() {
 
 	// Second path
 	let ev = remove_first_msg_event_to_node(&expected_paths[1][0].node.get_our_node_id(), &mut events);
-	pass_along_path(&nodes[0], expected_paths[1], 101_000, hash, Some(secret), ev, true, None);
+	let payment_secret = Some(payment_secret);
+	pass_along_path(&nodes[0], expected_paths[1], 101_000, hash, payment_secret, ev, true, None);
 
 	claim_payment_along_route(
 		ClaimAlongRouteArgs::new(&nodes[0], expected_paths, preimage)
@@ -6808,7 +6817,7 @@ fn do_test_overshoot_mpp(msat_amounts: &[u64], total_msat: u64){
 
 	// Create a route for each amount
 	let example_amount = 100000;
-	let (mut route, hash, preimage, secret) = get_route_and_payment_hash!(&nodes[src_idx], nodes[dst_idx], example_amount);
+	let (mut route, hash, preimage, payment_secret) = get_route_and_payment_hash!(&nodes[src_idx], nodes[dst_idx], example_amount);
 	let sample_path = route.paths.pop().unwrap();
 	for i in 0..routing_node_count {
 		let routing_node = 2 + i;
@@ -6823,12 +6832,12 @@ fn do_test_overshoot_mpp(msat_amounts: &[u64], total_msat: u64){
 
 	// Send payment with manually set total_msat
 	let id = PaymentId(nodes[src_idx].keys_manager.backing.get_secure_random_bytes());
-	let onion = RecipientOnionFields::secret_only(secret);
+	let onion = RecipientOnionFields::secret_only(payment_secret);
 	let onion_session_privs = nodes[src_idx]
 		.node
 		.test_add_new_pending_payment(hash, onion, id, &route)
 		.unwrap();
-	let onion = RecipientOnionFields::secret_only(secret);
+	let onion = RecipientOnionFields::secret_only(payment_secret);
 	let amt = Some(total_msat);
 	nodes[src_idx]
 		.node
@@ -6845,7 +6854,7 @@ fn do_test_overshoot_mpp(msat_amounts: &[u64], total_msat: u64){
 		let current_path_amount = msat_amounts[path_idx];
 		amount_received += current_path_amount;
 		let became_claimable_now = amount_received >= total_msat && amount_received - current_path_amount < total_msat;
-		pass_along_path(&nodes[src_idx], expected_path, amount_received, hash.clone(), Some(secret), ev, became_claimable_now, None);
+		pass_along_path(&nodes[src_idx], expected_path, amount_received, hash.clone(), Some(payment_secret), ev, became_claimable_now, None);
 	}
 
 	claim_payment_along_route(
@@ -8355,7 +8364,7 @@ pub fn test_inconsistent_mpp_params() {
 			core::cmp::Ordering::Less } else { core::cmp::Ordering::Greater }
 	});
 
-	let (preimage, hash, secret) = get_payment_preimage_hash!(&nodes[3]);
+	let (preimage, hash, payment_secret) = get_payment_preimage_hash!(&nodes[3]);
 
 	let cur_height = nodes[0].best_block_info().1;
 	let id = PaymentId([42; 32]);
@@ -8365,10 +8374,10 @@ pub fn test_inconsistent_mpp_params() {
 		// ultimately have, just not right away.
 		let mut dup_route = route.clone();
 		dup_route.paths.push(route.paths[1].clone());
-		let onion = RecipientOnionFields::secret_only(secret);
+		let onion = RecipientOnionFields::secret_only(payment_secret);
 		nodes[0].node.test_add_new_pending_payment(hash, onion, id, &dup_route).unwrap()
 	};
-	let onion = RecipientOnionFields::secret_only(secret);
+	let onion = RecipientOnionFields::secret_only(payment_secret);
 	let path_a = &route.paths[0];
 	let real_amt = 15_000_000;
 	let priv_a = session_privs[0];
@@ -8382,11 +8391,11 @@ pub fn test_inconsistent_mpp_params() {
 	assert_eq!(events.len(), 1);
 	let path_a = &[&nodes[1], &nodes[3]];
 	let event = events.pop().unwrap();
-	pass_along_path(&nodes[0], path_a, real_amt, hash, Some(secret), event, false, None);
+	pass_along_path(&nodes[0], path_a, real_amt, hash, Some(payment_secret), event, false, None);
 	assert!(nodes[3].node.get_and_clear_pending_events().is_empty());
 
 	let path_b = &route.paths[1];
-	let onion = RecipientOnionFields::secret_only(secret);
+	let onion = RecipientOnionFields::secret_only(payment_secret);
 	let amt_b = 14_000_000;
 	let priv_b = session_privs[1];
 	nodes[0]
@@ -8440,7 +8449,7 @@ pub fn test_inconsistent_mpp_params() {
 	let conditions = PaymentFailedConditions::new().mpp_parts_remain();
 	expect_payment_failed_conditions(&nodes[0], hash, true, conditions);
 
-	let onion = RecipientOnionFields::secret_only(secret);
+	let onion = RecipientOnionFields::secret_only(payment_secret);
 	let path_b = &route.paths[1];
 	let priv_c = session_privs[2];
 	nodes[0]
@@ -8453,7 +8462,7 @@ pub fn test_inconsistent_mpp_params() {
 	assert_eq!(events.len(), 1);
 	let event = events.pop().unwrap();
 	let path_b = &[&nodes[2], &nodes[3]];
-	pass_along_path(&nodes[0], path_b, real_amt, hash, Some(secret), event, true, None);
+	pass_along_path(&nodes[0], path_b, real_amt, hash, Some(payment_secret), event, true, None);
 
 	do_claim_payment_along_route(
 		ClaimAlongRouteArgs::new(&nodes[0], &[path_a, path_b], preimage)
@@ -8479,7 +8488,7 @@ pub fn test_double_partial_claim() {
 	create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 100_000, 0);
 	create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 100_000, 0);
 
-	let (mut route, payment_hash, payment_preimage, secret) = get_route_and_payment_hash!(nodes[0], nodes[3], 15_000_000);
+	let (mut route, hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[3], 15_000_000);
 	assert_eq!(route.paths.len(), 2);
 	route.paths.sort_by(|path_a, _| {
 		// Sort the path so that the path through nodes[1] comes first
@@ -8488,7 +8497,7 @@ pub fn test_double_partial_claim() {
 	});
 
 	let paths: &[&[_]] = &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]];
-	send_along_route_with_secret(&nodes[0], route.clone(), paths, 15_000_000, payment_hash, secret);
+	send_along_route_with_secret(&nodes[0], route.clone(), paths, 15_000_000, hash, payment_secret);
 	// nodes[3] has now received a PaymentClaimable event...which it will take some (exorbitant)
 	// amount of time to respond to.
 
@@ -8497,25 +8506,25 @@ pub fn test_double_partial_claim() {
 	connect_blocks(&nodes[0], TEST_FINAL_CLTV); // To get the same height for sending later
 
 	let failed_destinations = vec![
-		HTLCHandlingFailureType::Receive { payment_hash },
-		HTLCHandlingFailureType::Receive { payment_hash },
+		HTLCHandlingFailureType::Receive { payment_hash: hash },
+		HTLCHandlingFailureType::Receive { payment_hash: hash },
 	];
 	expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[3], failed_destinations);
 
 	let reason = PaymentFailureReason::RecipientRejected;
-	pass_failed_payment_back(&nodes[0], paths, false, payment_hash, reason);
+	pass_failed_payment_back(&nodes[0], paths, false, hash, reason);
 
 	// nodes[1] now retries one of the two paths...
-	let onion = RecipientOnionFields::secret_only(secret);
-	let id = PaymentId(payment_hash.0);
-	nodes[0].node.send_payment_with_route(route, payment_hash, onion, id).unwrap();
+	let onion = RecipientOnionFields::secret_only(payment_secret);
+	let id = PaymentId(hash.0);
+	nodes[0].node.send_payment_with_route(route, hash, onion, id).unwrap();
 	check_added_monitors!(nodes[0], 2);
 
 	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
 	assert_eq!(events.len(), 2);
 	let msgs = remove_first_msg_event_to_node(&node_b_id, &mut events);
 	let path = &[&nodes[1], &nodes[3]];
-	pass_along_path(&nodes[0], path, 15_000_000, payment_hash, Some(secret), msgs, false, None);
+	pass_along_path(&nodes[0], path, 15_000_000, hash, Some(payment_secret), msgs, false, None);
 
 	// At this point nodes[3] has received one half of the payment, and the user goes to handle
 	// that PaymentClaimable event they got hours ago and never handled...we should refuse to claim.
@@ -9263,20 +9272,20 @@ fn do_payment_with_custom_min_final_cltv_expiry(valid_delta: bool, use_user_hash
 
 	let payment_parameters =
 		PaymentParameters::from_node_id(node_b_id, final_cltv_expiry_delta as u32);
-	let (hash, payment_preimage, secret) = if use_user_hash {
-		let (payment_preimage, hash, secret) =
+	let (hash, payment_preimage, payment_secret) = if use_user_hash {
+		let (payment_preimage, hash, payment_secret) =
 			get_payment_preimage_hash!(nodes[1], Some(recv_value), Some(min_cltv_expiry_delta));
-		(hash, payment_preimage, secret)
+		(hash, payment_preimage, payment_secret)
 	} else {
-		let (hash, secret) =
+		let (hash, payment_secret) =
 			nodes[1]
 				.node
 				.create_inbound_payment(Some(recv_value), 7200, Some(min_cltv_expiry_delta))
 				.unwrap();
-		(hash, nodes[1].node.get_payment_preimage(hash, secret).unwrap(), secret)
+		(hash, nodes[1].node.get_payment_preimage(hash, payment_secret).unwrap(), payment_secret)
 	};
 	let route = get_route!(nodes[0], payment_parameters, recv_value).unwrap();
-	let onion = RecipientOnionFields::secret_only(secret);
+	let onion = RecipientOnionFields::secret_only(payment_secret);
 	nodes[0].node.send_payment_with_route(route, hash, onion, PaymentId(hash.0)).unwrap();
 	check_added_monitors!(nodes[0], 1);
 	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -9288,7 +9297,7 @@ fn do_payment_with_custom_min_final_cltv_expiry(valid_delta: bool, use_user_hash
 
 	if valid_delta {
 		let preimage = if use_user_hash { None } else { Some(payment_preimage) };
-		expect_payment_claimable!(nodes[1], hash, secret, recv_value, preimage, node_b_id);
+		expect_payment_claimable!(nodes[1], hash, payment_secret, recv_value, preimage, node_b_id);
 
 		claim_payment(&nodes[0], &[&nodes[1]], payment_preimage);
 	} else {