Merge pull request #3028 from jkczyz/2024-04-offer-id-followups

Follow-ups to #2970

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -357,11 +357,6 @@ enum OnionPayload {
 		/// This is only here for backwards-compatibility in serialization, in the future it can be
 		/// removed, breaking clients running 0.0.106 and earlier.
 		_legacy_hop_data: Option<msgs::FinalOnionHopData>,
-		/// The context of the payment included by the recipient in a blinded path, or `None` if a
-		/// blinded path was not used.
-		///
-		/// Used in part to determine the [`events::PaymentPurpose`].
-		payment_context: Option<PaymentContext>,
 	},
 	/// Contains the payer-provided preimage.
 	Spontaneous(PaymentPreimage),
@@ -5346,7 +5341,7 @@ where
 								}
 							}) => {
 								let blinded_failure = routing.blinded_failure();
-								let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret, mut onion_fields) = match routing {
+								let (cltv_expiry, onion_payload, payment_data, payment_context, phantom_shared_secret, mut onion_fields) = match routing {
 									PendingHTLCRouting::Receive {
 										payment_data, payment_metadata, payment_context,
 										incoming_cltv_expiry, phantom_shared_secret, custom_tlvs,
@@ -5355,8 +5350,8 @@ where
 										let _legacy_hop_data = Some(payment_data.clone());
 										let onion_fields = RecipientOnionFields { payment_secret: Some(payment_data.payment_secret),
 												payment_metadata, custom_tlvs };
-										(incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data, payment_context },
-											Some(payment_data), phantom_shared_secret, onion_fields)
+										(incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data },
+											Some(payment_data), payment_context, phantom_shared_secret, onion_fields)
 									},
 									PendingHTLCRouting::ReceiveKeysend {
 										payment_data, payment_preimage, payment_metadata,
@@ -5368,7 +5363,7 @@ where
 											custom_tlvs,
 										};
 										(incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage),
-											payment_data, None, onion_fields)
+											payment_data, None, None, onion_fields)
 									},
 									_ => {
 										panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive");
@@ -5530,7 +5525,7 @@ where
 								match payment_secrets.entry(payment_hash) {
 									hash_map::Entry::Vacant(_) => {
 										match claimable_htlc.onion_payload {
-											OnionPayload::Invoice { ref payment_context, .. } => {
+											OnionPayload::Invoice { .. } => {
 												let payment_data = payment_data.unwrap();
 												let (payment_preimage, min_final_cltv_expiry_delta) = match inbound_payment::verify(payment_hash, &payment_data, self.highest_seen_timestamp.load(Ordering::Acquire) as u64, &self.inbound_payment_key, &self.logger) {
 													Ok(result) => result,
@@ -5548,9 +5543,9 @@ where
 													}
 												}
 												let purpose = events::PaymentPurpose::from_parts(
-													payment_preimage.clone(),
+													payment_preimage,
 													payment_data.payment_secret,
-													payment_context.clone(),
+													payment_context,
 												);
 												check_total_value!(purpose);
 											},
@@ -5561,13 +5556,10 @@ where
 										}
 									},
 									hash_map::Entry::Occupied(inbound_payment) => {
-										let payment_context = match claimable_htlc.onion_payload {
-											OnionPayload::Spontaneous(_) => {
-												log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} because we already have an inbound payment with the same payment hash", &payment_hash);
-												fail_htlc!(claimable_htlc, payment_hash);
-											},
-											OnionPayload::Invoice { ref payment_context, .. } => payment_context,
-										};
+										if let OnionPayload::Spontaneous(_) = claimable_htlc.onion_payload {
+											log_trace!(self.logger, "Failing new keysend HTLC with payment_hash {} because we already have an inbound payment with the same payment hash", &payment_hash);
+											fail_htlc!(claimable_htlc, payment_hash);
+										}
 										let payment_data = payment_data.unwrap();
 										if inbound_payment.get().payment_secret != payment_data.payment_secret {
 											log_trace!(self.logger, "Failing new HTLC with payment_hash {} as it didn't match our expected payment secret.", &payment_hash);
@@ -5580,7 +5572,7 @@ where
 											let purpose = events::PaymentPurpose::from_parts(
 												inbound_payment.get().payment_preimage,
 												payment_data.payment_secret,
-												payment_context.clone(),
+												payment_context,
 											);
 											let payment_claimable_generated = check_total_value!(purpose);
 											if payment_claimable_generated {
@@ -10815,11 +10807,11 @@ impl_writeable_tlv_based!(HTLCPreviousHopData, {
 
 impl Writeable for ClaimableHTLC {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
-		let (payment_data, keysend_preimage, payment_context) = match &self.onion_payload {
-			OnionPayload::Invoice { _legacy_hop_data, payment_context } => {
-				(_legacy_hop_data.as_ref(), None, payment_context.as_ref())
+		let (payment_data, keysend_preimage) = match &self.onion_payload {
+			OnionPayload::Invoice { _legacy_hop_data } => {
+				(_legacy_hop_data.as_ref(), None)
 			},
-			OnionPayload::Spontaneous(preimage) => (None, Some(preimage), None),
+			OnionPayload::Spontaneous(preimage) => (None, Some(preimage)),
 		};
 		write_tlv_fields!(writer, {
 			(0, self.prev_hop, required),
@@ -10831,7 +10823,6 @@ impl Writeable for ClaimableHTLC {
 			(6, self.cltv_expiry, required),
 			(8, keysend_preimage, option),
 			(10, self.counterparty_skimmed_fee_msat, option),
-			(11, payment_context, option),
 		});
 		Ok(())
 	}
@@ -10849,7 +10840,6 @@ impl Readable for ClaimableHTLC {
 			(6, cltv_expiry, required),
 			(8, keysend_preimage, option),
 			(10, counterparty_skimmed_fee_msat, option),
-			(11, payment_context, option),
 		});
 		let payment_data: Option<msgs::FinalOnionHopData> = payment_data_opt;
 		let value = value_ser.0.unwrap();
@@ -10870,7 +10860,7 @@ impl Readable for ClaimableHTLC {
 					}
 					total_msat = Some(payment_data.as_ref().unwrap().total_msat);
 				}
-				OnionPayload::Invoice { _legacy_hop_data: payment_data, payment_context }
+				OnionPayload::Invoice { _legacy_hop_data: payment_data }
 			},
 		};
 		Ok(Self {
@@ -12108,7 +12098,7 @@ where
 					return Err(DecodeError::InvalidValue);
 				}
 				let purpose = match &htlcs[0].onion_payload {
-					OnionPayload::Invoice { _legacy_hop_data, payment_context: _ } => {
+					OnionPayload::Invoice { _legacy_hop_data } => {
 						if let Some(hop_data) = _legacy_hop_data {
 							events::PaymentPurpose::Bolt11InvoicePayment {
 								payment_preimage: match pending_inbound_payments.get(&payment_hash) {

lightning/src/ln/offers_tests.rs

@@ -46,7 +46,6 @@ use crate::blinded_path::{BlindedPath, IntroductionNode};
 use crate::blinded_path::payment::{Bolt12OfferContext, Bolt12RefundContext, PaymentContext};
 use crate::events::{Event, MessageSendEventsProvider, PaymentPurpose};
 use crate::ln::channelmanager::{PaymentId, RecentPaymentDetails, Retry, self};
-use crate::ln::features::InvoiceRequestFeatures;
 use crate::ln::functional_test_utils::*;
 use crate::ln::msgs::{ChannelMessageHandler, Init, NodeAnnouncement, OnionMessage, OnionMessageHandler, RoutingMessageHandler, SocketAddress, UnsignedGossipMessage, UnsignedNodeAnnouncement};
 use crate::offers::invoice::Bolt12Invoice;
@@ -412,8 +411,6 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() {
 		offer_id: offer.id(),
 		invoice_request: InvoiceRequestFields {
 			payer_id: invoice_request.payer_id(),
-			amount_msats: None,
-			features: InvoiceRequestFeatures::empty(),
 			quantity: None,
 			payer_note_truncated: None,
 		},
@@ -565,8 +562,6 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() {
 		offer_id: offer.id(),
 		invoice_request: InvoiceRequestFields {
 			payer_id: invoice_request.payer_id(),
-			amount_msats: None,
-			features: InvoiceRequestFeatures::empty(),
 			quantity: None,
 			payer_note_truncated: None,
 		},
@@ -687,8 +682,6 @@ fn pays_for_offer_without_blinded_paths() {
 		offer_id: offer.id(),
 		invoice_request: InvoiceRequestFields {
 			payer_id: invoice_request.payer_id(),
-			amount_msats: None,
-			features: InvoiceRequestFeatures::empty(),
 			quantity: None,
 			payer_note_truncated: None,
 		},

lightning/src/offers/invoice_request.rs

@@ -877,14 +877,12 @@ impl VerifiedInvoiceRequest {
 		let InvoiceRequestContents {
 			payer_id,
 			inner: InvoiceRequestContentsWithoutPayerId {
-				payer: _, offer: _, chain: _, amount_msats, features, quantity, payer_note
+				payer: _, offer: _, chain: _, amount_msats: _, features: _, quantity, payer_note
 			},
 		} = &self.inner.contents;
 
 		InvoiceRequestFields {
 			payer_id: *payer_id,
-			amount_msats: *amount_msats,
-			features: features.clone(),
 			quantity: *quantity,
 			payer_note_truncated: payer_note.clone()
 				.map(|mut s| { s.truncate(PAYER_NOTE_LIMIT); UntrustedString(s) }),
@@ -1126,15 +1124,6 @@ pub struct InvoiceRequestFields {
 	/// A possibly transient pubkey used to sign the invoice request.
 	pub payer_id: PublicKey,
 
-	/// The amount to pay in msats (i.e., the minimum lightning-payable unit for [`chain`]), which
-	/// must be greater than or equal to [`Offer::amount`], converted if necessary.
-	///
-	/// [`chain`]: InvoiceRequest::chain
-	pub amount_msats: Option<u64>,
-
-	/// Features pertaining to requesting an invoice.
-	pub features: InvoiceRequestFeatures,
-
 	/// The quantity of the offer's item conforming to [`Offer::is_valid_quantity`].
 	pub quantity: Option<u64>,
 
@@ -1150,10 +1139,8 @@ impl Writeable for InvoiceRequestFields {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
 		write_tlv_fields!(writer, {
 			(0, self.payer_id, required),
-			(2, self.amount_msats.map(|v| HighZeroBytesDroppedBigSize(v)), option),
-			(4, WithoutLength(&self.features), required),
-			(6, self.quantity.map(|v| HighZeroBytesDroppedBigSize(v)), option),
-			(8, self.payer_note_truncated.as_ref().map(|s| WithoutLength(&s.0)), option),
+			(2, self.quantity.map(|v| HighZeroBytesDroppedBigSize(v)), option),
+			(4, self.payer_note_truncated.as_ref().map(|s| WithoutLength(&s.0)), option),
 		});
 		Ok(())
 	}
@@ -1163,15 +1150,13 @@ impl Readable for InvoiceRequestFields {
 	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
 		_init_and_read_len_prefixed_tlv_fields!(reader, {
 			(0, payer_id, required),
-			(2, amount_msats, (option, encoding: (u64, HighZeroBytesDroppedBigSize))),
-			(4, features, (option, encoding: (InvoiceRequestFeatures, WithoutLength))),
-			(6, quantity, (option, encoding: (u64, HighZeroBytesDroppedBigSize))),
-			(8, payer_note_truncated, (option, encoding: (String, WithoutLength))),
+			(2, quantity, (option, encoding: (u64, HighZeroBytesDroppedBigSize))),
+			(4, payer_note_truncated, (option, encoding: (String, WithoutLength))),
 		});
-		let features = features.unwrap_or(InvoiceRequestFeatures::empty());
 
 		Ok(InvoiceRequestFields {
-			payer_id: payer_id.0.unwrap(), amount_msats, features, quantity,
+			payer_id: payer_id.0.unwrap(),
+			quantity,
 			payer_note_truncated: payer_note_truncated.map(|s| UntrustedString(s)),
 		})
 	}
@@ -2264,7 +2249,6 @@ mod tests {
 
 		let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.chain(Network::Testnet).unwrap()
-			.amount_msats(1001).unwrap()
 			.quantity(1).unwrap()
 			.payer_note("0".repeat(PAYER_NOTE_LIMIT * 2))
 			.build().unwrap()
@@ -2277,8 +2261,6 @@ mod tests {
 					fields,
 					InvoiceRequestFields {
 						payer_id: payer_pubkey(),
-						amount_msats: Some(1001),
-						features: InvoiceRequestFeatures::empty(),
 						quantity: Some(1),
 						payer_note_truncated: Some(UntrustedString("0".repeat(PAYER_NOTE_LIMIT))),
 					}