Merge pull request #2351 from TheBlueMatt/2023-04-remove-legacy-recv

Drop `create_inbound_payment*_legacy` breaking downgrade to 0.0.103

Author: TheBlueMatt

lightning/src/events/mod.rs

@@ -855,7 +855,7 @@ impl Writeable for Event {
 					(3, via_channel_id, option),
 					(4, amount_msat, required),
 					(5, via_user_channel_id, option),
-					(6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier
+					// Type 6 was `user_payment_id` on 0.0.103 and earlier
 					(7, claim_deadline, option),
 					(8, payment_preimage, option),
 					(9, onion_fields, option),
@@ -1059,7 +1059,7 @@ impl MaybeReadable for Event {
 					let mut payment_secret = None;
 					let mut amount_msat = 0;
 					let mut receiver_node_id = None;
-					let mut _user_payment_id = None::<u64>; // For compatibility with 0.0.103 and earlier
+					let mut _user_payment_id = None::<u64>; // Used in 0.0.103 and earlier, no longer written in 0.0.116+.
 					let mut via_channel_id = None;
 					let mut claim_deadline = None;
 					let mut via_user_channel_id = None;

lightning/src/ln/channelmanager.rs

@@ -50,7 +50,7 @@ use crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParame
 use crate::ln::msgs;
 use crate::ln::onion_utils;
 use crate::ln::onion_utils::HTLCFailReason;
-use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
+use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
 #[cfg(test)]
 use crate::ln::outbound_payment;
 use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment};
@@ -6005,37 +6005,6 @@ where
 		}
 	}
 
-	fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option<PaymentPreimage>, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
-		assert!(invoice_expiry_delta_secs <= 60*60*24*365); // Sadly bitcoin timestamps are u32s, so panic before 2106
-
-		if min_value_msat.is_some() && min_value_msat.unwrap() > MAX_VALUE_MSAT {
-			return Err(APIError::APIMisuseError { err: format!("min_value_msat of {} greater than total 21 million bitcoin supply", min_value_msat.unwrap()) });
-		}
-
-		let payment_secret = PaymentSecret(self.entropy_source.get_secure_random_bytes());
-
-		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
-		let mut payment_secrets = self.pending_inbound_payments.lock().unwrap();
-		match payment_secrets.entry(payment_hash) {
-			hash_map::Entry::Vacant(e) => {
-				e.insert(PendingInboundPayment {
-					payment_secret, min_value_msat, payment_preimage,
-					user_payment_id: 0, // For compatibility with version 0.0.103 and earlier
-					// We assume that highest_seen_timestamp is pretty close to the current time -
-					// it's updated when we receive a new block with the maximum time we've seen in
-					// a header. It should never be more than two hours in the future.
-					// Thus, we add two hours here as a buffer to ensure we absolutely
-					// never fail a payment too early.
-					// Note that we assume that received blocks have reasonably up-to-date
-					// timestamps.
-					expiry_time: self.highest_seen_timestamp.load(Ordering::Acquire) as u64 + invoice_expiry_delta_secs as u64 + 7200,
-				});
-			},
-			hash_map::Entry::Occupied(_) => return Err(APIError::APIMisuseError { err: "Duplicate payment hash".to_owned() }),
-		}
-		Ok(payment_secret)
-	}
-
 	/// Gets a payment secret and payment hash for use in an invoice given to a third party wishing
 	/// to pay us.
 	///
@@ -6075,23 +6044,6 @@ where
 			min_final_cltv_expiry_delta)
 	}
 
-	/// Legacy version of [`create_inbound_payment`]. Use this method if you wish to share
-	/// serialized state with LDK node(s) running 0.0.103 and earlier.
-	///
-	/// May panic if `invoice_expiry_delta_secs` is greater than one year.
-	///
-	/// # Note
-	/// This method is deprecated and will be removed soon.
-	///
-	/// [`create_inbound_payment`]: Self::create_inbound_payment
-	#[deprecated]
-	pub fn create_inbound_payment_legacy(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<(PaymentHash, PaymentSecret), APIError> {
-		let payment_preimage = PaymentPreimage(self.entropy_source.get_secure_random_bytes());
-		let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
-		let payment_secret = self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs)?;
-		Ok((payment_hash, payment_secret))
-	}
-
 	/// Gets a [`PaymentSecret`] for a given [`PaymentHash`], for which the payment preimage is
 	/// stored external to LDK.
 	///
@@ -6145,20 +6097,6 @@ where
 			min_final_cltv_expiry)
 	}
 
-	/// Legacy version of [`create_inbound_payment_for_hash`]. Use this method if you wish to share
-	/// serialized state with LDK node(s) running 0.0.103 and earlier.
-	///
-	/// May panic if `invoice_expiry_delta_secs` is greater than one year.
-	///
-	/// # Note
-	/// This method is deprecated and will be removed soon.
-	///
-	/// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
-	#[deprecated]
-	pub fn create_inbound_payment_for_hash_legacy(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
-		self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs)
-	}
-
 	/// Gets an LDK-generated payment preimage from a payment hash and payment secret that were
 	/// previously returned from [`create_inbound_payment`].
 	///

lightning/src/ln/functional_tests.rs

@@ -8202,67 +8202,6 @@ fn test_preimage_storage() {
 	}
 }
 
-#[test]
-#[allow(deprecated)]
-fn test_secret_timeout() {
-	// Simple test of payment secret storage time outs. After
-	// `create_inbound_payment(_for_hash)_legacy` is removed, this test will be removed as well.
-	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).0.contents.short_channel_id;
-
-	let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment_legacy(Some(100_000), 2).unwrap();
-
-	// 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_legacy(payment_hash, Some(100_000), 2) {
-		assert_eq!(err, "Duplicate payment hash");
-	} else { panic!(); }
-	let mut block = {
-		let node_1_blocks = nodes[1].blocks.lock().unwrap();
-		create_dummy_block(node_1_blocks.last().unwrap().0.block_hash(), node_1_blocks.len() as u32 + 7200, Vec::new())
-	};
-	connect_block(&nodes[1], &block);
-	if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash_legacy(payment_hash, Some(100_000), 2) {
-		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 (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_legacy(payment_hash, Some(100_000), 2).unwrap();
-	assert_ne!(payment_secret_1, our_payment_secret);
-
-	{
-		let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
-		nodes[0].node.send_payment_with_route(&route, payment_hash,
-			RecipientOnionFields::secret_only(our_payment_secret), PaymentId(payment_hash.0)).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::PaymentClaimable { purpose: PaymentPurpose::InvoicePayment { payment_preimage, payment_secret }, .. } => {
-			assert!(payment_preimage.is_none());
-			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

pending_changelog/no-legacy-payments.txt

@@ -0,0 +1,6 @@
+ * Legacy inbound payment creation has been removed, thus there is no way to
+   create a pending inbound payment which will still be claimable on LDK
+   0.0.103 or earlier. Support for claiming such payments is still retained,
+   however is likely to be removed in the next release (#2351).
+ * Some fields required in 0.0.103 and earlier are no longer written, thus
+   deserializing objects written in 0.0.116 with 0.0.103 may now fail (#2351).