Drop `create_inbound_payment*_legacy` breaking downgrade to 0.0.103
authorMatt Corallo <git@bluematt.me>
Sat, 10 Jun 2023 19:52:52 +0000 (19:52 +0000)
committerMatt Corallo <git@bluematt.me>
Mon, 12 Jun 2023 16:43:01 +0000 (16:43 +0000)
0.0.103 is now downright ancient, and certainly shouldn't exist in
production anywhere today. Thus, it seems fine to remove the
ability to create legacy stateful inbound payment entries.

Users downgrading to 0.0.103 will thus not be able to claim any
payments created on modern LDK, though we still retain the ability
to claim such payments at least for one more release.

lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs
pending_changelog/no-legacy-payments.txt [new file with mode: 0644]

index 7c065f1ba44c4d956f68d791f52ad75c8135c74e..cf7b1195a4edd7cd78e0be0c648415bc4eb01b84 100644 (file)
@@ -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};
@@ -5874,37 +5874,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.
        ///
@@ -5944,23 +5913,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.
        ///
@@ -6014,20 +5966,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`].
        ///
index 0775f58bbcadb309508c3291846c36941d742eea..d24c8cf49e1c21323bd517db22461435a616032d 100644 (file)
@@ -8196,67 +8196,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
diff --git a/pending_changelog/no-legacy-payments.txt b/pending_changelog/no-legacy-payments.txt
new file mode 100644 (file)
index 0000000..f5f3f16
--- /dev/null
@@ -0,0 +1,4 @@
+ * 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.