bolt12: Validate LSPS2 opening fees

Read fee limits from BOLT12 payment context metadata and reject
unsupported withholding. Record accepted fees on inbound offer payments.

Co-Authored-By: HAL 9000

Author: tnull

src/event.rs

@@ -7,7 +7,7 @@
 
 use core::future::Future;
 use core::task::{Poll, Waker};
-use std::collections::VecDeque;
+use std::collections::{BTreeMap, VecDeque};
 use std::ops::Deref;
 use std::sync::{Arc, Mutex};
 
@@ -664,13 +664,40 @@ where
 		}
 	}
 
-	fn lsps2_max_total_opening_fee_msat(payment_metadata: &[u8], amount_msat: u64) -> Option<u64> {
+	fn lsps2_max_total_opening_fee_msat(
+		payment_metadata: &[u8], amount_msat: u64, counterparty_skimmed_fee_msat: u64,
+	) -> Option<u64> {
 		let metadata = PaymentMetadata::read(&mut &payment_metadata[..]).ok()?;
+		Self::lsps2_max_total_opening_fee_msat_from_metadata(
+			metadata,
+			amount_msat,
+			counterparty_skimmed_fee_msat,
+		)
+	}
+
+	fn lsps2_bolt12_max_total_opening_fee_msat(
+		payment_metadata: &BTreeMap<u64, Vec<u8>>, amount_msat: u64,
+		counterparty_skimmed_fee_msat: u64,
+	) -> Option<u64> {
+		let metadata = PaymentMetadata::decode_from_bolt12_payment_metadata(payment_metadata)?;
+		Self::lsps2_max_total_opening_fee_msat_from_metadata(
+			metadata,
+			amount_msat,
+			counterparty_skimmed_fee_msat,
+		)
+	}
+
+	fn lsps2_max_total_opening_fee_msat_from_metadata(
+		metadata: PaymentMetadata, amount_msat: u64, counterparty_skimmed_fee_msat: u64,
+	) -> Option<u64> {
 		let lsps2_parameters = metadata.lsps2_parameters?;
 		lsps2_parameters.max_total_opening_fee_msat.or_else(|| {
 			lsps2_parameters.max_proportional_opening_fee_ppm_msat.and_then(|max_prop_fee| {
-				// If it's a variable amount payment, compute the actual fee.
-				compute_opening_fee(amount_msat, 0, max_prop_fee)
+				// `PaymentClaimable::amount_msat` excludes the fee withheld by the LSP. LSPS2
+				// proportional fees are calculated from the amount sent by the payer, so add the
+				// withheld fee back before computing the permitted fee.
+				let payment_size_msat = amount_msat.checked_add(counterparty_skimmed_fee_msat)?;
+				compute_opening_fee(payment_size_msat, 0, max_prop_fee)
 			})
 		})
 	}
@@ -864,15 +891,28 @@ where
 							.as_ref()
 							.and_then(|fields| fields.payment_metadata.as_ref())
 							.and_then(|metadata| {
-								Self::lsps2_max_total_opening_fee_msat(metadata, amount_msat)
+								Self::lsps2_max_total_opening_fee_msat(
+									metadata,
+									amount_msat,
+									counterparty_skimmed_fee_msat,
+								)
 							}),
+						PaymentPurpose::Bolt12OfferPayment { payment_context, .. } => {
+							payment_context.payment_metadata.as_ref().and_then(|metadata| {
+								Self::lsps2_bolt12_max_total_opening_fee_msat(
+									metadata,
+									amount_msat,
+									counterparty_skimmed_fee_msat,
+								)
+							})
+						},
 						_ => None,
 					};
 
 					let Some(max_total_opening_fee_msat) = max_total_opening_fee_msat else {
 						log_info!(
 							self.logger,
-							"Refusing inbound payment with hash {} as the counterparty withheld {}msat without valid BOLT11 LSPS2 payment metadata",
+							"Refusing inbound payment with hash {} as the counterparty withheld {}msat without valid LSPS2 payment metadata",
 							hex_utils::to_string(&payment_hash.0),
 							counterparty_skimmed_fee_msat,
 						);
@@ -894,7 +934,7 @@ where
 
 					if let Some(info) = payment_info.as_ref() {
 						match &info.kind {
-							PaymentKind::Bolt11 { .. } => {
+							PaymentKind::Bolt11 { .. } | PaymentKind::Bolt12Offer { .. } => {
 								let update = PaymentDetailsUpdate {
 									counterparty_skimmed_fee_msat: Some(Some(counterparty_skimmed_fee_msat)),
 									..PaymentDetailsUpdate::new(payment_id)
@@ -907,7 +947,7 @@ where
 									},
 								};
 							},
-							_ => debug_assert!(false, "We only expect the counterparty to get away with withholding fees for BOLT11 payments."),
+							_ => debug_assert!(false, "We only expect the counterparty to withhold fees for LSPS2 invoice payments."),
 						}
 					}
 				}
@@ -976,6 +1016,8 @@ where
 							hash: Some(payment_hash),
 							preimage: payment_preimage,
 							secret: Some(payment_secret),
+							counterparty_skimmed_fee_msat: (counterparty_skimmed_fee_msat > 0)
+								.then_some(counterparty_skimmed_fee_msat),
 							offer_id,
 							payer_note,
 							quantity,
@@ -2038,12 +2080,33 @@ mod tests {
 		assert_eq!(
 			EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(
 				&metadata.encode(),
-				100_000
+				100_000,
+				0,
 			),
 			Some(42_000)
 		);
 	}
 
+	#[test]
+	fn lsps2_proportional_fee_uses_gross_payment_amount() {
+		let metadata = PaymentMetadata {
+			lsps2_parameters: Some(LSPS2Parameters {
+				max_total_opening_fee_msat: None,
+				max_proportional_opening_fee_ppm_msat: Some(10_000),
+			}),
+			lsps2_lease_parameters: None,
+		};
+
+		assert_eq!(
+			EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(
+				&metadata.encode(),
+				198_000,
+				2_000,
+			),
+			Some(2_000)
+		);
+	}
+
 	#[test]
 	fn lsps2_payment_metadata_missing_or_malformed_limit_is_rejected() {
 		let empty_metadata =
@@ -2060,23 +2123,63 @@ mod tests {
 		assert_eq!(
 			EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(
 				&empty_metadata,
-				100_000
+				100_000,
+				0,
 			),
 			None
 		);
 		assert_eq!(
-			EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(&[0xff], 100_000),
+			EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(&[0xff], 100_000, 0,),
 			None
 		);
 		assert_eq!(
 			EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(
 				&metadata_without_fee_limit,
-				100_000
+				100_000,
+				0,
 			),
 			None
 		);
 	}
 
+	#[test]
+	fn bolt12_lsps2_payment_metadata_decodes_fee_limit() {
+		let metadata = PaymentMetadata {
+			lsps2_parameters: Some(LSPS2Parameters {
+				max_total_opening_fee_msat: None,
+				max_proportional_opening_fee_ppm_msat: Some(10_000),
+			}),
+			lsps2_lease_parameters: None,
+		}
+		.encode_as_bolt12_payment_metadata();
+
+		assert_eq!(
+			EventHandler::<Arc<TestLogger>>::lsps2_bolt12_max_total_opening_fee_msat(
+				&metadata, 100_000, 0,
+			),
+			Some(1_000)
+		);
+	}
+
+	#[test]
+	fn bolt12_lsps2_payment_metadata_requires_fee_limit() {
+		let empty_metadata = BTreeMap::new();
+		let metadata_without_fee_limit =
+			PaymentMetadata { lsps2_parameters: None, lsps2_lease_parameters: None }
+				.encode_as_bolt12_payment_metadata();
+		let mut malformed_metadata = metadata_without_fee_limit.clone();
+		*malformed_metadata.values_mut().next().unwrap() = vec![0xff];
+
+		for metadata in [empty_metadata, malformed_metadata, metadata_without_fee_limit] {
+			assert_eq!(
+				EventHandler::<Arc<TestLogger>>::lsps2_bolt12_max_total_opening_fee_msat(
+					&metadata, 100_000, 0,
+				),
+				None
+			);
+		}
+	}
+
 	#[tokio::test]
 	async fn event_queue_persistence() {
 		let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));

src/payment/bolt12/mod.rs

@@ -163,6 +163,7 @@ impl Bolt12Payment {
 					hash: None,
 					preimage: None,
 					secret: None,
+					counterparty_skimmed_fee_msat: None,
 					offer_id: offer.id(),
 					payer_note: payer_note.map(UntrustedString),
 					quantity,
@@ -188,6 +189,7 @@ impl Bolt12Payment {
 							hash: None,
 							preimage: None,
 							secret: None,
+							counterparty_skimmed_fee_msat: None,
 							offer_id: offer.id(),
 							payer_note: payer_note.map(UntrustedString),
 							quantity,
@@ -344,6 +346,7 @@ impl Bolt12Payment {
 					hash: None,
 					preimage: None,
 					secret: None,
+					counterparty_skimmed_fee_msat: None,
 					offer_id: offer.id(),
 					payer_note: payer_note.map(UntrustedString),
 					quantity,
@@ -369,6 +372,7 @@ impl Bolt12Payment {
 							hash: None,
 							preimage: None,
 							secret: None,
+							counterparty_skimmed_fee_msat: None,
 							offer_id: offer.id(),
 							payer_note: payer_note.map(UntrustedString),
 							quantity,

src/payment/metadata.rs

@@ -77,4 +77,19 @@ mod tests {
 
 		assert_eq!(metadata, decoded);
 	}
+
+	#[test]
+	fn bolt12_metadata_roundtrips() {
+		let metadata = PaymentMetadata {
+			lsps2_parameters: Some(LSPS2Parameters {
+				max_total_opening_fee_msat: Some(42_000),
+				max_proportional_opening_fee_ppm_msat: None,
+			}),
+			lsps2_lease_parameters: None,
+		};
+
+		let encoded = metadata.encode_as_bolt12_payment_metadata();
+
+		assert_eq!(PaymentMetadata::decode_from_bolt12_payment_metadata(&encoded), Some(metadata));
+	}
 }

src/payment/store.rs

@@ -253,12 +253,13 @@ impl StorableObject for PaymentDetails {
 
 		if let Some(skimmed_fee_msat) = update.counterparty_skimmed_fee_msat {
 			match self.kind {
-				PaymentKind::Bolt11 { ref mut counterparty_skimmed_fee_msat, .. } => {
+				PaymentKind::Bolt11 { ref mut counterparty_skimmed_fee_msat, .. }
+				| PaymentKind::Bolt12Offer { ref mut counterparty_skimmed_fee_msat, .. } => {
 					update_if_necessary!(*counterparty_skimmed_fee_msat, skimmed_fee_msat);
 				},
 				_ => debug_assert!(
 					false,
-					"We should only ever override counterparty_skimmed_fee_msat for BOLT11 payments"
+					"We should only override counterparty_skimmed_fee_msat for invoice payments"
 				),
 			}
 		}
@@ -545,6 +546,17 @@ pub enum PaymentKind {
 		preimage: Option<PaymentPreimage>,
 		/// The secret used by the payment.
 		secret: Option<PaymentSecret>,
+		/// The value, in thousands of a satoshi, that was deducted from this payment as an extra
+		/// fee taken by our channel counterparty.
+		///
+		/// Will only ever be `Some` for inbound payments received via an [bLIP-52 / LSPS 2]
+		/// just-in-time channel, and only after the payment is observed; `None` otherwise.
+		///
+		/// This will always be `None` for payments serialized by versions that did not record
+		/// BOLT12 LSPS2 fees.
+		///
+		/// [bLIP-52 / LSPS 2]: https://github.com/lightning/blips/blob/master/blip-0052.md
+		counterparty_skimmed_fee_msat: Option<u64>,
 		/// The ID of the offer this payment is for.
 		offer_id: OfferId,
 		/// The payer note for the payment.
@@ -617,6 +629,7 @@ impl_writeable_tlv_based_enum!(PaymentKind,
 		(2, preimage, option),
 		(3, quantity, option),
 		(4, secret, option),
+		(5, counterparty_skimmed_fee_msat, option),
 		(6, offer_id, required),
 	},
 	(8, Spontaneous) => {
@@ -732,7 +745,8 @@ impl From<&PaymentDetails> for PaymentDetailsUpdate {
 		};
 
 		let counterparty_skimmed_fee_msat = match value.kind {
-			PaymentKind::Bolt11 { counterparty_skimmed_fee_msat, .. } => {
+			PaymentKind::Bolt11 { counterparty_skimmed_fee_msat, .. }
+			| PaymentKind::Bolt12Offer { counterparty_skimmed_fee_msat, .. } => {
 				Some(counterparty_skimmed_fee_msat)
 			},
 			_ => None,
@@ -1022,6 +1036,38 @@ mod tests {
 		}
 	}
 
+	#[test]
+	fn bolt12_offer_records_counterparty_skimmed_fee() {
+		let payment_id = PaymentId([41; 32]);
+		let mut payment = PaymentDetails::new(
+			payment_id,
+			PaymentKind::Bolt12Offer {
+				hash: Some(PaymentHash([42; 32])),
+				preimage: None,
+				secret: None,
+				counterparty_skimmed_fee_msat: None,
+				offer_id: OfferId([43; 32]),
+				payer_note: None,
+				quantity: None,
+			},
+			Some(100_000),
+			None,
+			PaymentDirection::Inbound,
+			PaymentStatus::Pending,
+		);
+
+		assert!(payment.update(PaymentDetailsUpdate {
+			counterparty_skimmed_fee_msat: Some(Some(1_000)),
+			..PaymentDetailsUpdate::new(payment_id)
+		}));
+		assert!(matches!(
+			payment.kind,
+			PaymentKind::Bolt12Offer { counterparty_skimmed_fee_msat: Some(1_000), .. }
+		));
+		let encoded = payment.encode();
+		assert_eq!(payment, PaymentDetails::read(&mut &*encoded).unwrap());
+	}
+
 	#[derive(Clone, Debug, PartialEq, Eq)]
 	struct LegacyBolt11JitKind {
 		hash: PaymentHash,

tests/integration_tests_rust.rs

@@ -2332,12 +2332,14 @@ async fn simple_bolt12_send_receive() {
 			hash,
 			preimage,
 			secret: _,
+			counterparty_skimmed_fee_msat,
 			offer_id,
 			quantity: ref qty,
 			payer_note: ref note,
 		} => {
 			assert!(hash.is_some());
 			assert!(preimage.is_some());
+			assert_eq!(counterparty_skimmed_fee_msat, None);
 			assert_eq!(offer_id, offer.id());
 			assert_eq!(&expected_quantity, qty);
 			assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0);
@@ -2399,12 +2401,14 @@ async fn simple_bolt12_send_receive() {
 			hash,
 			preimage,
 			secret: _,
+			counterparty_skimmed_fee_msat,
 			offer_id,
 			quantity: ref qty,
 			payer_note: ref note,
 		} => {
 			assert!(hash.is_some());
 			assert!(preimage.is_some());
+			assert_eq!(counterparty_skimmed_fee_msat, None);
 			assert_eq!(offer_id, offer.id());
 			assert_eq!(&expected_quantity, qty);
 			assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0);