Merge pull request #3017 from jkczyz/2024-04-optional-pubkey

Sending to `Offer` without `signing_pubkey`

lightning/src/ln/channelmanager.rs

@@ -8757,14 +8757,7 @@ where
 			.map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?;
 
 		let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap();
-		if offer.paths().is_empty() {
-			let message = new_pending_onion_message(
-				OffersMessage::InvoiceRequest(invoice_request),
-				Destination::Node(offer.signing_pubkey()),
-				Some(reply_path),
-			);
-			pending_offers_messages.push(message);
-		} else {
+		if !offer.paths().is_empty() {
 			// Send as many invoice requests as there are paths in the offer (with an upper bound).
 			// Using only one path could result in a failure if the path no longer exists. But only
 			// one invoice for a given payment id will be paid, even if more than one is received.
@@ -8777,6 +8770,16 @@ where
 				);
 				pending_offers_messages.push(message);
 			}
+		} else if let Some(signing_pubkey) = offer.signing_pubkey() {
+			let message = new_pending_onion_message(
+				OffersMessage::InvoiceRequest(invoice_request),
+				Destination::Node(signing_pubkey),
+				Some(reply_path),
+			);
+			pending_offers_messages.push(message);
+		} else {
+			debug_assert!(false);
+			return Err(Bolt12SemanticError::MissingSigningPubkey);
 		}
 
 		Ok(())

lightning/src/ln/offers_tests.rs

@@ -270,7 +270,7 @@ fn prefers_non_tor_nodes_in_blinded_paths() {
 		.create_offer_builder("coffee".to_string()).unwrap()
 		.amount_msats(10_000_000)
 		.build().unwrap();
-	assert_ne!(offer.signing_pubkey(), bob_id);
+	assert_ne!(offer.signing_pubkey(), Some(bob_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
 		assert_ne!(path.introduction_node, IntroductionNode::NodeId(bob_id));
@@ -285,7 +285,7 @@ fn prefers_non_tor_nodes_in_blinded_paths() {
 		.create_offer_builder("coffee".to_string()).unwrap()
 		.amount_msats(10_000_000)
 		.build().unwrap();
-	assert_ne!(offer.signing_pubkey(), bob_id);
+	assert_ne!(offer.signing_pubkey(), Some(bob_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
 		assert_eq!(path.introduction_node, IntroductionNode::NodeId(bob_id));
@@ -335,7 +335,7 @@ fn prefers_more_connected_nodes_in_blinded_paths() {
 		.create_offer_builder("coffee".to_string()).unwrap()
 		.amount_msats(10_000_000)
 		.build().unwrap();
-	assert_ne!(offer.signing_pubkey(), bob_id);
+	assert_ne!(offer.signing_pubkey(), Some(bob_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
 		assert_eq!(path.introduction_node, IntroductionNode::NodeId(nodes[4].node.get_our_node_id()));
@@ -385,7 +385,7 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() {
 		.unwrap()
 		.amount_msats(10_000_000)
 		.build().unwrap();
-	assert_ne!(offer.signing_pubkey(), alice_id);
+	assert_ne!(offer.signing_pubkey(), Some(alice_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
 		assert_eq!(path.introduction_node, IntroductionNode::NodeId(bob_id));
@@ -544,7 +544,7 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() {
 		.create_offer_builder("coffee".to_string()).unwrap()
 		.amount_msats(10_000_000)
 		.build().unwrap();
-	assert_ne!(offer.signing_pubkey(), alice_id);
+	assert_ne!(offer.signing_pubkey(), Some(alice_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
 		assert_eq!(path.introduction_node, IntroductionNode::NodeId(alice_id));
@@ -667,7 +667,7 @@ fn pays_for_offer_without_blinded_paths() {
 		.clear_paths()
 		.amount_msats(10_000_000)
 		.build().unwrap();
-	assert_eq!(offer.signing_pubkey(), alice_id);
+	assert_eq!(offer.signing_pubkey(), Some(alice_id));
 	assert!(offer.paths().is_empty());
 
 	let payment_id = PaymentId([1; 32]);

lightning/src/offers/invoice.rs

@@ -210,10 +210,9 @@ macro_rules! invoice_explicit_signing_pubkey_builder_methods { ($self: ident, $s
 	#[cfg_attr(c_bindings, allow(dead_code))]
 	pub(super) fn for_offer(
 		invoice_request: &'a InvoiceRequest, payment_paths: Vec<(BlindedPayInfo, BlindedPath)>,
-		created_at: Duration, payment_hash: PaymentHash
+		created_at: Duration, payment_hash: PaymentHash, signing_pubkey: PublicKey
 	) -> Result<Self, Bolt12SemanticError> {
 		let amount_msats = Self::amount_msats(invoice_request)?;
-		let signing_pubkey = invoice_request.contents.inner.offer.signing_pubkey();
 		let contents = InvoiceContents::ForOffer {
 			invoice_request: invoice_request.contents.clone(),
 			fields: Self::fields(
@@ -272,7 +271,7 @@ macro_rules! invoice_derived_signing_pubkey_builder_methods { ($self: ident, $se
 		created_at: Duration, payment_hash: PaymentHash, keys: KeyPair
 	) -> Result<Self, Bolt12SemanticError> {
 		let amount_msats = Self::amount_msats(invoice_request)?;
-		let signing_pubkey = invoice_request.contents.inner.offer.signing_pubkey();
+		let signing_pubkey = keys.public_key();
 		let contents = InvoiceContents::ForOffer {
 			invoice_request: invoice_request.contents.clone(),
 			fields: Self::fields(
@@ -1435,8 +1434,8 @@ impl TryFrom<PartialInvoiceTlvStream> for InvoiceContents {
 			features, signing_pubkey,
 		};
 
-		match offer_tlv_stream.node_id {
-			Some(expected_signing_pubkey) => {
+		match (offer_tlv_stream.node_id, &offer_tlv_stream.paths) {
+			(Some(expected_signing_pubkey), _) => {
 				if fields.signing_pubkey != expected_signing_pubkey {
 					return Err(Bolt12SemanticError::InvalidSigningPubkey);
 				}
@@ -1446,7 +1445,21 @@ impl TryFrom<PartialInvoiceTlvStream> for InvoiceContents {
 				)?;
 				Ok(InvoiceContents::ForOffer { invoice_request, fields })
 			},
-			None => {
+			(None, Some(paths)) => {
+				if !paths
+					.iter()
+					.filter_map(|path| path.blinded_hops.last())
+					.any(|last_hop| fields.signing_pubkey == last_hop.blinded_node_id)
+				{
+					return Err(Bolt12SemanticError::InvalidSigningPubkey);
+				}
+
+				let invoice_request = InvoiceRequestContents::try_from(
+					(payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream)
+				)?;
+				Ok(InvoiceContents::ForOffer { invoice_request, fields })
+			},
+			(None, None) => {
 				let refund = RefundContents::try_from(
 					(payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream)
 				)?;
@@ -1464,7 +1477,7 @@ mod tests {
 	use bitcoin::blockdata::script::ScriptBuf;
 	use bitcoin::hashes::Hash;
 	use bitcoin::network::constants::Network;
-	use bitcoin::secp256k1::{Message, Secp256k1, XOnlyPublicKey, self};
+	use bitcoin::secp256k1::{KeyPair, Message, Secp256k1, SecretKey, XOnlyPublicKey, self};
 	use bitcoin::address::{Address, Payload, WitnessProgram, WitnessVersion};
 	use bitcoin::key::TweakedPublicKey;
 
@@ -1633,6 +1646,7 @@ mod tests {
 					quantity: None,
 					payer_id: Some(&payer_pubkey()),
 					payer_note: None,
+					paths: None,
 				},
 				InvoiceTlvStreamRef {
 					paths: Some(Iterable(payment_paths.iter().map(|(_, path)| path))),
@@ -1725,6 +1739,7 @@ mod tests {
 					quantity: None,
 					payer_id: Some(&payer_pubkey()),
 					payer_note: None,
+					paths: None,
 				},
 				InvoiceTlvStreamRef {
 					paths: Some(Iterable(payment_paths.iter().map(|(_, path)| path))),
@@ -2365,6 +2380,81 @@ mod tests {
 		}
 	}
 
+	#[test]
+	fn parses_invoice_with_node_id_from_blinded_path() {
+		let paths = vec![
+			BlindedPath {
+				introduction_node: IntroductionNode::NodeId(pubkey(40)),
+				blinding_point: pubkey(41),
+				blinded_hops: vec![
+					BlindedHop { blinded_node_id: pubkey(43), encrypted_payload: vec![0; 43] },
+					BlindedHop { blinded_node_id: pubkey(44), encrypted_payload: vec![0; 44] },
+				],
+			},
+			BlindedPath {
+				introduction_node: IntroductionNode::NodeId(pubkey(40)),
+				blinding_point: pubkey(41),
+				blinded_hops: vec![
+					BlindedHop { blinded_node_id: pubkey(45), encrypted_payload: vec![0; 45] },
+					BlindedHop { blinded_node_id: pubkey(46), encrypted_payload: vec![0; 46] },
+				],
+			},
+		];
+
+		let blinded_node_id_sign = |message: &UnsignedBolt12Invoice| {
+			let secp_ctx = Secp256k1::new();
+			let keys = KeyPair::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap());
+			Ok(secp_ctx.sign_schnorr_no_aux_rand(message.as_ref().as_digest(), &keys))
+		};
+
+		let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
+			.clear_signing_pubkey()
+			.amount_msats(1000)
+			.path(paths[0].clone())
+			.path(paths[1].clone())
+			.build().unwrap()
+			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
+			.build().unwrap()
+			.sign(payer_sign).unwrap()
+			.respond_with_no_std_using_signing_pubkey(
+				payment_paths(), payment_hash(), now(), pubkey(46)
+			).unwrap()
+			.build().unwrap()
+			.sign(blinded_node_id_sign).unwrap();
+
+		let mut buffer = Vec::new();
+		invoice.write(&mut buffer).unwrap();
+
+		if let Err(e) = Bolt12Invoice::try_from(buffer) {
+			panic!("error parsing invoice: {:?}", e);
+		}
+
+		let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
+			.clear_signing_pubkey()
+			.amount_msats(1000)
+			.path(paths[0].clone())
+			.path(paths[1].clone())
+			.build().unwrap()
+			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
+			.build().unwrap()
+			.sign(payer_sign).unwrap()
+			.respond_with_no_std_using_signing_pubkey(
+				payment_paths(), payment_hash(), now(), recipient_pubkey()
+			).unwrap()
+			.build().unwrap()
+			.sign(recipient_sign).unwrap();
+
+		let mut buffer = Vec::new();
+		invoice.write(&mut buffer).unwrap();
+
+		match Bolt12Invoice::try_from(buffer) {
+			Ok(_) => panic!("expected error"),
+			Err(e) => {
+				assert_eq!(e, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::InvalidSigningPubkey));
+			},
+		}
+	}
+
 	#[test]
 	fn fails_parsing_invoice_without_signature() {
 		let mut buffer = Vec::new();

lightning/src/offers/invoice_request.rs

@@ -747,7 +747,27 @@ macro_rules! invoice_request_respond_with_explicit_signing_pubkey_methods { (
 			return Err(Bolt12SemanticError::UnknownRequiredFeatures);
 		}
 
-		<$builder>::for_offer(&$contents, payment_paths, created_at, payment_hash)
+		let signing_pubkey = match $contents.contents.inner.offer.signing_pubkey() {
+			Some(signing_pubkey) => signing_pubkey,
+			None => return Err(Bolt12SemanticError::MissingSigningPubkey),
+		};
+
+		<$builder>::for_offer(&$contents, payment_paths, created_at, payment_hash, signing_pubkey)
+	}
+
+	#[cfg(test)]
+	#[allow(dead_code)]
+	pub(super) fn respond_with_no_std_using_signing_pubkey(
+		&$self, payment_paths: Vec<(BlindedPayInfo, BlindedPath)>, payment_hash: PaymentHash,
+		created_at: core::time::Duration, signing_pubkey: PublicKey
+	) -> Result<$builder, Bolt12SemanticError> {
+		debug_assert!($contents.contents.inner.offer.signing_pubkey().is_none());
+
+		if $contents.invoice_request_features().requires_unknown_bits() {
+			return Err(Bolt12SemanticError::UnknownRequiredFeatures);
+		}
+
+		<$builder>::for_offer(&$contents, payment_paths, created_at, payment_hash, signing_pubkey)
 	}
 } }
 
@@ -855,6 +875,11 @@ macro_rules! invoice_request_respond_with_derived_signing_pubkey_methods { (
 			Some(keys) => keys,
 		};
 
+		match $contents.contents.inner.offer.signing_pubkey() {
+			Some(signing_pubkey) => debug_assert_eq!(signing_pubkey, keys.public_key()),
+			None => return Err(Bolt12SemanticError::MissingSigningPubkey),
+		}
+
 		<$builder>::for_offer_using_keys(
 			&$self.inner, payment_paths, created_at, payment_hash, keys
 		)
@@ -959,6 +984,7 @@ impl InvoiceRequestContentsWithoutPayerId {
 			quantity: self.quantity,
 			payer_id: None,
 			payer_note: self.payer_note.as_ref(),
+			paths: None,
 		};
 
 		(payer, offer, invoice_request)
@@ -991,13 +1017,17 @@ pub(super) const INVOICE_REQUEST_TYPES: core::ops::Range<u64> = 80..160;
 /// [`Refund::payer_id`]: crate::offers::refund::Refund::payer_id
 pub(super) const INVOICE_REQUEST_PAYER_ID_TYPE: u64 = 88;
 
+// This TLV stream is used for both InvoiceRequest and Refund, but not all TLV records are valid for
+// InvoiceRequest as noted below.
 tlv_stream!(InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef, INVOICE_REQUEST_TYPES, {
 	(80, chain: ChainHash),
 	(82, amount: (u64, HighZeroBytesDroppedBigSize)),
 	(84, features: (InvoiceRequestFeatures, WithoutLength)),
 	(86, quantity: (u64, HighZeroBytesDroppedBigSize)),
 	(INVOICE_REQUEST_PAYER_ID_TYPE, payer_id: PublicKey),
 	(89, payer_note: (String, WithoutLength)),
+	// Only used for Refund since the onion message of an InvoiceRequest has a reply path.
+	(90, paths: (Vec<BlindedPath>, WithoutLength)),
 });
 
 type FullInvoiceRequestTlvStream =
@@ -1080,7 +1110,9 @@ impl TryFrom<PartialInvoiceRequestTlvStream> for InvoiceRequestContents {
 		let (
 			PayerTlvStream { metadata },
 			offer_tlv_stream,
-			InvoiceRequestTlvStream { chain, amount, features, quantity, payer_id, payer_note },
+			InvoiceRequestTlvStream {
+				chain, amount, features, quantity, payer_id, payer_note, paths,
+			},
 		) = tlv_stream;
 
 		let payer = match metadata {
@@ -1107,6 +1139,10 @@ impl TryFrom<PartialInvoiceRequestTlvStream> for InvoiceRequestContents {
 			Some(payer_id) => payer_id,
 		};
 
+		if paths.is_some() {
+			return Err(Bolt12SemanticError::UnexpectedPaths);
+		}
+
 		Ok(InvoiceRequestContents {
 			inner: InvoiceRequestContentsWithoutPayerId {
 				payer, offer, chain, amount_msats: amount, features, quantity, payer_note,
@@ -1218,7 +1254,7 @@ mod tests {
 		assert_eq!(unsigned_invoice_request.paths(), &[]);
 		assert_eq!(unsigned_invoice_request.issuer(), None);
 		assert_eq!(unsigned_invoice_request.supported_quantity(), Quantity::One);
-		assert_eq!(unsigned_invoice_request.signing_pubkey(), recipient_pubkey());
+		assert_eq!(unsigned_invoice_request.signing_pubkey(), Some(recipient_pubkey()));
 		assert_eq!(unsigned_invoice_request.chain(), ChainHash::using_genesis_block(Network::Bitcoin));
 		assert_eq!(unsigned_invoice_request.amount_msats(), None);
 		assert_eq!(unsigned_invoice_request.invoice_request_features(), &InvoiceRequestFeatures::empty());
@@ -1250,7 +1286,7 @@ mod tests {
 		assert_eq!(invoice_request.paths(), &[]);
 		assert_eq!(invoice_request.issuer(), None);
 		assert_eq!(invoice_request.supported_quantity(), Quantity::One);
-		assert_eq!(invoice_request.signing_pubkey(), recipient_pubkey());
+		assert_eq!(invoice_request.signing_pubkey(), Some(recipient_pubkey()));
 		assert_eq!(invoice_request.chain(), ChainHash::using_genesis_block(Network::Bitcoin));
 		assert_eq!(invoice_request.amount_msats(), None);
 		assert_eq!(invoice_request.invoice_request_features(), &InvoiceRequestFeatures::empty());
@@ -1285,6 +1321,7 @@ mod tests {
 					quantity: None,
 					payer_id: Some(&payer_pubkey()),
 					payer_note: None,
+					paths: None,
 				},
 				SignatureTlvStreamRef { signature: Some(&invoice_request.signature()) },
 			),
@@ -2245,7 +2282,7 @@ mod tests {
 			.amount_msats(1000)
 			.supported_quantity(Quantity::Unbounded)
 			.build().unwrap();
-		assert_eq!(offer.signing_pubkey(), node_id);
+		assert_eq!(offer.signing_pubkey(), Some(node_id));
 
 		let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.chain(Network::Testnet).unwrap()