Default to padding blinded paths

lightning-dns-resolver/src/lib.rs

@@ -236,6 +236,7 @@ mod test {
 				recipient,
 				local_node_receive_key,
 				context,
+				false,
 				&keys,
 				secp_ctx,
 			)])
@@ -345,6 +346,7 @@ mod test {
 			payer_id,
 			receive_key,
 			query_context,
+			false,
 			&*payer_keys,
 			&secp_ctx,
 		);

lightning/src/blinded_path/message.rs

@@ -54,21 +54,38 @@ impl Readable for BlindedMessagePath {
 
 impl BlindedMessagePath {
 	/// Create a one-hop blinded path for a message.
+	///
+	/// `compact_padding` selects between space-inefficient padding which better hides contents and
+	/// a space-constrained padding which does very little to hide the contents, especially for the
+	/// last hop. It should only be set when the blinded path needs to be as compact as possible.
 	pub fn one_hop<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
 		recipient_node_id: PublicKey, local_node_receive_key: ReceiveAuthKey,
-		context: MessageContext, entropy_source: ES, secp_ctx: &Secp256k1<T>,
+		context: MessageContext, compact_padding: bool, entropy_source: ES,
+		secp_ctx: &Secp256k1<T>,
 	) -> Self
 	where
 		ES::Target: EntropySource,
 	{
-		Self::new(&[], recipient_node_id, local_node_receive_key, context, entropy_source, secp_ctx)
+		Self::new(
+			&[],
+			recipient_node_id,
+			local_node_receive_key,
+			context,
+			compact_padding,
+			entropy_source,
+			secp_ctx,
+		)
 	}
 
 	/// Create a path for an onion message, to be forwarded along `node_pks`.
+	///
+	/// `compact_padding` selects between space-inefficient padding which better hides contents and
+	/// a space-constrained padding which does very little to hide the contents, especially for the
+	/// last hop. It should only be set when the blinded path needs to be as compact as possible.
 	pub fn new<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
 		intermediate_nodes: &[MessageForwardNode], recipient_node_id: PublicKey,
-		local_node_receive_key: ReceiveAuthKey, context: MessageContext, entropy_source: ES,
-		secp_ctx: &Secp256k1<T>,
+		local_node_receive_key: ReceiveAuthKey, context: MessageContext, compact_padding: bool,
+		entropy_source: ES, secp_ctx: &Secp256k1<T>,
 	) -> Self
 	where
 		ES::Target: EntropySource,
@@ -79,19 +96,23 @@ impl BlindedMessagePath {
 			0,
 			local_node_receive_key,
 			context,
+			compact_padding,
 			entropy_source,
 			secp_ctx,
 		)
 	}
 
 	/// Same as [`BlindedMessagePath::new`], but allows specifying a number of dummy hops.
 	///
-	/// Note:
-	/// At most [`MAX_DUMMY_HOPS_COUNT`] dummy hops can be added to the blinded path.
+	/// `compact_padding` selects between space-inefficient padding which better hides contents and
+	/// a space-constrained padding which does very little to hide the contents, especially for the
+	/// last hop. It should only be set when the blinded path needs to be as compact as possible.
+	///
+	/// Note: At most [`MAX_DUMMY_HOPS_COUNT`] dummy hops can be added to the blinded path.
 	pub fn new_with_dummy_hops<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
 		intermediate_nodes: &[MessageForwardNode], recipient_node_id: PublicKey,
 		dummy_hop_count: usize, local_node_receive_key: ReceiveAuthKey, context: MessageContext,
-		entropy_source: ES, secp_ctx: &Secp256k1<T>,
+		compact_padding: bool, entropy_source: ES, secp_ctx: &Secp256k1<T>,
 	) -> Self
 	where
 		ES::Target: EntropySource,
@@ -114,6 +135,7 @@ impl BlindedMessagePath {
 				context,
 				&blinding_secret,
 				local_node_receive_key,
+				compact_padding,
 			),
 		})
 	}
@@ -416,28 +438,45 @@ pub enum OffersContext {
 		/// Useful to timeout async recipients that are no longer supported as clients.
 		path_absolute_expiry: Duration,
 	},
-	/// Context used by a [`BlindedMessagePath`] within a [`Refund`] or as a reply path for an
-	/// [`InvoiceRequest`].
+	/// Context used by a [`BlindedMessagePath`] within a [`Refund`].
 	///
 	/// This variant is intended to be received when handling a [`Bolt12Invoice`] or an
 	/// [`InvoiceError`].
 	///
 	/// [`Refund`]: crate::offers::refund::Refund
-	/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 	/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 	/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
-	OutboundPayment {
-		/// Payment ID used when creating a [`Refund`] or [`InvoiceRequest`].
+	OutboundPaymentInRefund {
+		/// Payment ID used when creating a [`Refund`].
 		///
 		/// [`Refund`]: crate::offers::refund::Refund
-		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 		payment_id: PaymentId,
 
-		/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid [`Refund`] or
-		/// [`InvoiceRequest`] and for deriving their signing keys.
+		/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid [`Refund`] and
+		/// for deriving its signing keys.
 		///
 		/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 		/// [`Refund`]: crate::offers::refund::Refund
+		nonce: Nonce,
+	},
+	/// Context used by a [`BlindedMessagePath`] as a reply path for an [`InvoiceRequest`].
+	///
+	/// This variant is intended to be received when handling a [`Bolt12Invoice`] or an
+	/// [`InvoiceError`].
+	///
+	/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
+	/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
+	/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
+	OutboundPaymentInInvReq {
+		/// Payment ID used when creating an [`InvoiceRequest`].
+		///
+		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
+		payment_id: PaymentId,
+
+		/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid
+		/// [`InvoiceRequest`] and for deriving its signing keys.
+		///
+		/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 		nonce: Nonce,
 	},
@@ -619,7 +658,7 @@ impl_writeable_tlv_based_enum!(OffersContext,
 	(0, InvoiceRequest) => {
 		(0, nonce, required),
 	},
-	(1, OutboundPayment) => {
+	(1, OutboundPaymentInRefund) => {
 		(0, payment_id, required),
 		(1, nonce, required),
 	},
@@ -631,6 +670,10 @@ impl_writeable_tlv_based_enum!(OffersContext,
 		(2, invoice_slot, required),
 		(4, path_absolute_expiry, required),
 	},
+	(4, OutboundPaymentInInvReq) => {
+		(0, payment_id, required),
+		(1, nonce, required),
+	},
 );
 
 impl_writeable_tlv_based_enum!(AsyncPaymentsContext,
@@ -693,7 +736,7 @@ pub const MAX_DUMMY_HOPS_COUNT: usize = 10;
 pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
 	secp_ctx: &Secp256k1<T>, intermediate_nodes: &[MessageForwardNode],
 	recipient_node_id: PublicKey, dummy_hop_count: usize, context: MessageContext,
-	session_priv: &SecretKey, local_node_receive_key: ReceiveAuthKey,
+	session_priv: &SecretKey, local_node_receive_key: ReceiveAuthKey, compact_padding: bool,
 ) -> Vec<BlindedHop> {
 	let dummy_count = cmp::min(dummy_hop_count, MAX_DUMMY_HOPS_COUNT);
 	let pks = intermediate_nodes
@@ -703,9 +746,8 @@ pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
 			core::iter::repeat((recipient_node_id, Some(local_node_receive_key))).take(dummy_count),
 		)
 		.chain(core::iter::once((recipient_node_id, Some(local_node_receive_key))));
-	let is_compact = intermediate_nodes.iter().any(|node| node.short_channel_id.is_some());
 
-	let tlvs = pks
+	let intermediate_tlvs = pks
 		.clone()
 		.skip(1) // The first node's TLVs contains the next node's pubkey
 		.zip(intermediate_nodes.iter().map(|node| node.short_channel_id))
@@ -716,18 +758,43 @@ pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
 		.map(|next_hop| {
 			ControlTlvs::Forward(ForwardTlvs { next_hop, next_blinding_override: None })
 		})
-		.chain((0..dummy_count).map(|_| ControlTlvs::Dummy))
-		.chain(core::iter::once(ControlTlvs::Receive(ReceiveTlvs { context: Some(context) })));
-
-	if is_compact {
-		let path = pks.zip(tlvs);
-		utils::construct_blinded_hops(secp_ctx, path, session_priv)
+		.chain((0..dummy_count).map(|_| ControlTlvs::Dummy));
+
+	let max_intermediate_len =
+		intermediate_tlvs.clone().map(|tlvs| tlvs.serialized_length()).max().unwrap_or(0);
+	let have_intermediate_one_byte_smaller =
+		intermediate_tlvs.clone().any(|tlvs| tlvs.serialized_length() == max_intermediate_len - 1);
+
+	let round_off = if compact_padding {
+		// We can only pad by a minimum of two bytes (we can only go from no-TLV to a type + length
+		// byte). Thus, if there are any intermediate hops that need to be padded by exactly one
+		// byte, we have to instead pad everything by two.
+		if have_intermediate_one_byte_smaller {
+			max_intermediate_len + 2
+		} else {
+			max_intermediate_len
+		}
 	} else {
-		let path =
-			pks.zip(tlvs.map(|tlv| BlindedPathWithPadding {
-				tlvs: tlv,
-				round_off: MESSAGE_PADDING_ROUND_OFF,
-			}));
-		utils::construct_blinded_hops(secp_ctx, path, session_priv)
-	}
+		MESSAGE_PADDING_ROUND_OFF
+	};
+
+	let tlvs = intermediate_tlvs
+		.map(|tlvs| {
+			let res = BlindedPathWithPadding { tlvs, round_off };
+			if compact_padding {
+				debug_assert_eq!(res.serialized_length(), max_intermediate_len);
+			} else {
+				// We don't currently ever push extra fields to intermediate hops, so they should
+				// never go over `MESSAGE_PADDING_ROUND_OFF`.
+				debug_assert_eq!(res.serialized_length(), MESSAGE_PADDING_ROUND_OFF);
+			}
+			res
+		})
+		.chain(core::iter::once(BlindedPathWithPadding {
+			tlvs: ControlTlvs::Receive(ReceiveTlvs { context: Some(context) }),
+			round_off: if compact_padding { 0 } else { MESSAGE_PADDING_ROUND_OFF },
+		}));
+
+	let path = pks.zip(tlvs);
+	utils::construct_blinded_hops(secp_ctx, path, session_priv)
 }

lightning/src/blinded_path/utils.rs

@@ -256,9 +256,12 @@ impl<T: Writeable> Writeable for BlindedPathWithPadding<T> {
 		let tlv_length = self.tlvs.serialized_length();
 		let total_length = tlv_length + TLV_OVERHEAD;
 
-		let padding_length = total_length.div_ceil(self.round_off) * self.round_off - total_length;
-
-		let padding = Some(BlindedPathPadding::new(padding_length));
+		let padding = if self.round_off == 0 || tlv_length % self.round_off == 0 {
+			None
+		} else {
+			let length = total_length.div_ceil(self.round_off) * self.round_off - total_length;
+			Some(BlindedPathPadding::new(length))
+		};
 
 		encode_tlv_stream!(writer, {
 			(1, padding, option),

lightning/src/ln/channelmanager.rs

@@ -5593,29 +5593,12 @@ where
 	pub fn send_payment_for_bolt12_invoice(
 		&self, invoice: &Bolt12Invoice, context: Option<&OffersContext>,
 	) -> Result<(), Bolt12PaymentError> {
-		match self.verify_bolt12_invoice(invoice, context) {
+		match self.flow.verify_bolt12_invoice(invoice, context) {
 			Ok(payment_id) => self.send_payment_for_verified_bolt12_invoice(invoice, payment_id),
 			Err(()) => Err(Bolt12PaymentError::UnexpectedInvoice),
 		}
 	}
 
-	fn verify_bolt12_invoice(
-		&self, invoice: &Bolt12Invoice, context: Option<&OffersContext>,
-	) -> Result<PaymentId, ()> {
-		let secp_ctx = &self.secp_ctx;
-		let expanded_key = &self.inbound_payment_key;
-
-		match context {
-			None if invoice.is_for_refund_without_paths() => {
-				invoice.verify_using_metadata(expanded_key, secp_ctx)
-			},
-			Some(&OffersContext::OutboundPayment { payment_id, nonce, .. }) => {
-				invoice.verify_using_payer_data(payment_id, nonce, expanded_key, secp_ctx)
-			},
-			_ => Err(()),
-		}
-	}
-
 	fn send_payment_for_verified_bolt12_invoice(
 		&self, invoice: &Bolt12Invoice, payment_id: PaymentId,
 	) -> Result<(), Bolt12PaymentError> {
@@ -15366,7 +15349,7 @@ where
 			},
 			OffersMessage::StaticInvoice(invoice) => {
 				let payment_id = match context {
-					Some(OffersContext::OutboundPayment { payment_id, .. }) => payment_id,
+					Some(OffersContext::OutboundPaymentInInvReq { payment_id, .. }) => payment_id,
 					_ => return None
 				};
 				let res = self.initiate_async_payment(&invoice, payment_id);
@@ -15382,7 +15365,8 @@ where
 				log_trace!(logger, "Received invoice_error: {}", invoice_error);
 
 				match context {
-					Some(OffersContext::OutboundPayment { payment_id, .. }) => {
+					Some(OffersContext::OutboundPaymentInInvReq { payment_id, .. })
+					|Some(OffersContext::OutboundPaymentInRefund { payment_id, .. }) => {
 						self.abandon_payment_with_reason(
 							payment_id, PaymentFailureReason::InvoiceRequestRejected,
 						);