Merge pull request #1972 from jkczyz/2023-01-bolt12-spec-updates

BOLT 12 spec updates

lightning/src/offers/invoice.rs

@@ -338,8 +338,10 @@ struct InvoiceFields {
 
 impl Invoice {
 	/// Paths to the recipient originating from publicly reachable nodes, including information
-	/// needed for routing payments across them. Blinded paths provide recipient privacy by
-	/// obfuscating its node id.
+	/// needed for routing payments across them.
+	///
+	/// Blinded paths provide recipient privacy by obfuscating its node id. Note, however, that this
+	/// privacy is lost if a public node id is used for [`Invoice::signing_pubkey`].
 	pub fn payment_paths(&self) -> &[(BlindedPath, BlindedPayInfo)] {
 		&self.contents.fields().payment_paths[..]
 	}

lightning/src/offers/invoice_request.rs

@@ -333,7 +333,9 @@ impl InvoiceRequest {
 	/// for the invoice.
 	///
 	/// The `payment_paths` parameter is useful for maintaining the payment recipient's privacy. It
-	/// must contain one or more elements.
+	/// must contain one or more elements ordered from most-preferred to least-preferred, if there's
+	/// a preference. Note, however, that any privacy is lost if a public node id was used for
+	/// [`Offer::signing_pubkey`].
 	///
 	/// Errors if the request contains unknown required features.
 	///
@@ -845,11 +847,12 @@ mod tests {
 
 	#[test]
 	fn builds_invoice_request_with_quantity() {
+		let one = NonZeroU64::new(1).unwrap();
 		let ten = NonZeroU64::new(10).unwrap();
 
 		let invoice_request = OfferBuilder::new("foo".into(), recipient_pubkey())
 			.amount_msats(1000)
-			.supported_quantity(Quantity::one())
+			.supported_quantity(Quantity::One)
 			.build().unwrap()
 			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.build().unwrap()
@@ -860,7 +863,7 @@ mod tests {
 
 		match OfferBuilder::new("foo".into(), recipient_pubkey())
 			.amount_msats(1000)
-			.supported_quantity(Quantity::one())
+			.supported_quantity(Quantity::One)
 			.build().unwrap()
 			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.amount_msats(2_000).unwrap()
@@ -918,6 +921,17 @@ mod tests {
 			Ok(_) => panic!("expected error"),
 			Err(e) => assert_eq!(e, SemanticError::MissingQuantity),
 		}
+
+		match OfferBuilder::new("foo".into(), recipient_pubkey())
+			.amount_msats(1000)
+			.supported_quantity(Quantity::Bounded(one))
+			.build().unwrap()
+			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
+			.build()
+		{
+			Ok(_) => panic!("expected error"),
+			Err(e) => assert_eq!(e, SemanticError::MissingQuantity),
+		}
 	}
 
 	#[test]
@@ -1102,11 +1116,12 @@ mod tests {
 
 	#[test]
 	fn parses_invoice_request_with_quantity() {
+		let one = NonZeroU64::new(1).unwrap();
 		let ten = NonZeroU64::new(10).unwrap();
 
 		let invoice_request = OfferBuilder::new("foo".into(), recipient_pubkey())
 			.amount_msats(1000)
-			.supported_quantity(Quantity::one())
+			.supported_quantity(Quantity::One)
 			.build().unwrap()
 			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.build().unwrap()
@@ -1121,7 +1136,7 @@ mod tests {
 
 		let invoice_request = OfferBuilder::new("foo".into(), recipient_pubkey())
 			.amount_msats(1000)
-			.supported_quantity(Quantity::one())
+			.supported_quantity(Quantity::One)
 			.build().unwrap()
 			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.amount_msats(2_000).unwrap()
@@ -1206,6 +1221,22 @@ mod tests {
 			Ok(_) => panic!("expected error"),
 			Err(e) => assert_eq!(e, ParseError::InvalidSemantics(SemanticError::MissingQuantity)),
 		}
+
+		let invoice_request = OfferBuilder::new("foo".into(), recipient_pubkey())
+			.amount_msats(1000)
+			.supported_quantity(Quantity::Bounded(one))
+			.build().unwrap()
+			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
+			.build_unchecked()
+			.sign(payer_sign).unwrap();
+
+		let mut buffer = Vec::new();
+		invoice_request.write(&mut buffer).unwrap();
+
+		match InvoiceRequest::try_from(buffer) {
+			Ok(_) => panic!("expected error"),
+			Err(e) => assert_eq!(e, ParseError::InvalidSemantics(SemanticError::MissingQuantity)),
+		}
 	}
 
 	#[test]

lightning/src/offers/offer.rs

@@ -106,7 +106,7 @@ impl OfferBuilder {
 		let offer = OfferContents {
 			chains: None, metadata: None, amount: None, description,
 			features: OfferFeatures::empty(), absolute_expiry: None, issuer: None, paths: None,
-			supported_quantity: Quantity::one(), signing_pubkey,
+			supported_quantity: Quantity::One, signing_pubkey,
 		};
 		OfferBuilder { offer }
 	}
@@ -178,7 +178,7 @@ impl OfferBuilder {
 	}
 
 	/// Sets the quantity of items for [`Offer::supported_quantity`]. If not called, defaults to
-	/// [`Quantity::one`].
+	/// [`Quantity::One`].
 	///
 	/// Successive calls to this method will override the previous setting.
 	pub fn supported_quantity(mut self, quantity: Quantity) -> Self {
@@ -464,19 +464,17 @@ impl OfferContents {
 
 	fn is_valid_quantity(&self, quantity: u64) -> bool {
 		match self.supported_quantity {
-			Quantity::Bounded(n) => {
-				let n = n.get();
-				if n == 1 { false }
-				else { quantity > 0 && quantity <= n }
-			},
+			Quantity::Bounded(n) => quantity <= n.get(),
 			Quantity::Unbounded => quantity > 0,
+			Quantity::One => quantity == 1,
 		}
 	}
 
 	fn expects_quantity(&self) -> bool {
 		match self.supported_quantity {
-			Quantity::Bounded(n) => n.get() != 1,
+			Quantity::Bounded(_) => true,
 			Quantity::Unbounded => true,
+			Quantity::One => false,
 		}
 	}
 
@@ -549,25 +547,24 @@ pub type CurrencyCode = [u8; 3];
 /// Quantity of items supported by an [`Offer`].
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum Quantity {
-	/// Up to a specific number of items (inclusive).
+	/// Up to a specific number of items (inclusive). Use when more than one item can be requested
+	/// but is limited (e.g., because of per customer or inventory limits).
+	///
+	/// May be used with `NonZeroU64::new(1)` but prefer to use [`Quantity::One`] if only one item
+	/// is supported.
 	Bounded(NonZeroU64),
-	/// One or more items.
+	/// One or more items. Use when more than one item can be requested without any limit.
 	Unbounded,
+	/// Only one item. Use when only a single item can be requested.
+	One,
 }
 
 impl Quantity {
-	/// The default quantity of one.
-	pub fn one() -> Self {
-		Quantity::Bounded(NonZeroU64::new(1).unwrap())
-	}
-
 	fn to_tlv_record(&self) -> Option<u64> {
 		match self {
-			Quantity::Bounded(n) => {
-				let n = n.get();
-				if n == 1 { None } else { Some(n) }
-			},
+			Quantity::Bounded(n) => Some(n.get()),
 			Quantity::Unbounded => Some(0),
+			Quantity::One => None,
 		}
 	}
 }
@@ -639,9 +636,8 @@ impl TryFrom<OfferTlvStream> for OfferContents {
 			.map(|seconds_from_epoch| Duration::from_secs(seconds_from_epoch));
 
 		let supported_quantity = match quantity_max {
-			None => Quantity::one(),
+			None => Quantity::One,
 			Some(0) => Quantity::Unbounded,
-			Some(1) => return Err(SemanticError::InvalidQuantity),
 			Some(n) => Quantity::Bounded(NonZeroU64::new(n).unwrap()),
 		};
 
@@ -708,7 +704,7 @@ mod tests {
 		assert!(!offer.is_expired());
 		assert_eq!(offer.paths(), &[]);
 		assert_eq!(offer.issuer(), None);
-		assert_eq!(offer.supported_quantity(), Quantity::one());
+		assert_eq!(offer.supported_quantity(), Quantity::One);
 		assert_eq!(offer.signing_pubkey(), pubkey(42));
 
 		assert_eq!(
@@ -930,14 +926,15 @@ mod tests {
 
 	#[test]
 	fn builds_offer_with_supported_quantity() {
+		let one = NonZeroU64::new(1).unwrap();
 		let ten = NonZeroU64::new(10).unwrap();
 
 		let offer = OfferBuilder::new("foo".into(), pubkey(42))
-			.supported_quantity(Quantity::one())
+			.supported_quantity(Quantity::One)
 			.build()
 			.unwrap();
 		let tlv_stream = offer.as_tlv_stream();
-		assert_eq!(offer.supported_quantity(), Quantity::one());
+		assert_eq!(offer.supported_quantity(), Quantity::One);
 		assert_eq!(tlv_stream.quantity_max, None);
 
 		let offer = OfferBuilder::new("foo".into(), pubkey(42))
@@ -957,12 +954,20 @@ mod tests {
 		assert_eq!(tlv_stream.quantity_max, Some(10));
 
 		let offer = OfferBuilder::new("foo".into(), pubkey(42))
-			.supported_quantity(Quantity::Bounded(ten))
-			.supported_quantity(Quantity::one())
+			.supported_quantity(Quantity::Bounded(one))
 			.build()
 			.unwrap();
 		let tlv_stream = offer.as_tlv_stream();
-		assert_eq!(offer.supported_quantity(), Quantity::one());
+		assert_eq!(offer.supported_quantity(), Quantity::Bounded(one));
+		assert_eq!(tlv_stream.quantity_max, Some(1));
+
+		let offer = OfferBuilder::new("foo".into(), pubkey(42))
+			.supported_quantity(Quantity::Bounded(ten))
+			.supported_quantity(Quantity::One)
+			.build()
+			.unwrap();
+		let tlv_stream = offer.as_tlv_stream();
+		assert_eq!(offer.supported_quantity(), Quantity::One);
 		assert_eq!(tlv_stream.quantity_max, None);
 	}
 
@@ -1094,7 +1099,7 @@ mod tests {
 	#[test]
 	fn parses_offer_with_quantity() {
 		let offer = OfferBuilder::new("foo".into(), pubkey(42))
-			.supported_quantity(Quantity::one())
+			.supported_quantity(Quantity::One)
 			.build()
 			.unwrap();
 		if let Err(e) = offer.to_string().parse::<Offer>() {
@@ -1117,17 +1122,12 @@ mod tests {
 			panic!("error parsing offer: {:?}", e);
 		}
 
-		let mut tlv_stream = offer.as_tlv_stream();
-		tlv_stream.quantity_max = Some(1);
-
-		let mut encoded_offer = Vec::new();
-		tlv_stream.write(&mut encoded_offer).unwrap();
-
-		match Offer::try_from(encoded_offer) {
-			Ok(_) => panic!("expected error"),
-			Err(e) => {
-				assert_eq!(e, ParseError::InvalidSemantics(SemanticError::InvalidQuantity));
-			},
+		let offer = OfferBuilder::new("foo".into(), pubkey(42))
+			.supported_quantity(Quantity::Bounded(NonZeroU64::new(1).unwrap()))
+			.build()
+			.unwrap();
+		if let Err(e) = offer.to_string().parse::<Offer>() {
+			panic!("error parsing offer: {:?}", e);
 		}
 	}
 

lightning/src/offers/parse.rs

@@ -157,6 +157,8 @@ pub enum SemanticError {
 	InvalidQuantity,
 	/// A quantity or quantity bounds was provided but was not expected.
 	UnexpectedQuantity,
+	/// Metadata was provided but was not expected.
+	UnexpectedMetadata,
 	/// Payer metadata was expected but was missing.
 	MissingPayerMetadata,
 	/// A payer id was expected but was missing.

lightning/src/offers/refund.rs

@@ -118,9 +118,9 @@ impl RefundBuilder {
 		}
 
 		let refund = RefundContents {
-			payer: PayerContents(metadata), metadata: None, description, absolute_expiry: None,
-			issuer: None, paths: None, chain: None, amount_msats,
-			features: InvoiceRequestFeatures::empty(), payer_id, payer_note: None,
+			payer: PayerContents(metadata), description, absolute_expiry: None, issuer: None,
+			paths: None, chain: None, amount_msats, features: InvoiceRequestFeatures::empty(),
+			quantity: None, payer_id, payer_note: None,
 		};
 
 		Ok(RefundBuilder { refund })
@@ -162,6 +162,20 @@ impl RefundBuilder {
 		self
 	}
 
+	/// Sets [`Refund::quantity`] of items. This is purely for informational purposes. It is useful
+	/// when the refund pertains to an [`Invoice`] that paid for more than one item from an
+	/// [`Offer`] as specified by [`InvoiceRequest::quantity`].
+	///
+	/// Successive calls to this method will override the previous setting.
+	///
+	/// [`Invoice`]: crate::offers::invoice::Invoice
+	/// [`InvoiceRequest::quantity`]: crate::offers::invoice_request::InvoiceRequest::quantity
+	/// [`Offer`]: crate::offers::offer::Offer
+	pub fn quantity(mut self, quantity: u64) -> Self {
+		self.refund.quantity = Some(quantity);
+		self
+	}
+
 	/// Sets the [`Refund::payer_note`].
 	///
 	/// Successive calls to this method will override the previous setting.
@@ -215,7 +229,6 @@ pub struct Refund {
 pub(super) struct RefundContents {
 	payer: PayerContents,
 	// offer fields
-	metadata: Option<Vec<u8>>,
 	description: String,
 	absolute_expiry: Option<Duration>,
 	issuer: Option<String>,
@@ -224,6 +237,7 @@ pub(super) struct RefundContents {
 	chain: Option<ChainHash>,
 	amount_msats: u64,
 	features: InvoiceRequestFeatures,
+	quantity: Option<u64>,
 	payer_id: PublicKey,
 	payer_note: Option<String>,
 }
@@ -285,6 +299,11 @@ impl Refund {
 		&self.contents.features
 	}
 
+	/// The quantity of an item that refund is for.
+	pub fn quantity(&self) -> Option<u64> {
+		self.contents.quantity
+	}
+
 	/// A public node id to send to in the case where there are no [`paths`]. Otherwise, a possibly
 	/// transient pubkey.
 	///
@@ -312,7 +331,9 @@ impl Refund {
 	/// offer, which does have a `signing_pubkey`.
 	///
 	/// The `payment_paths` parameter is useful for maintaining the payment recipient's privacy. It
-	/// must contain one or more elements.
+	/// must contain one or more elements ordered from most-preferred to least-preferred, if there's
+	/// a preference. Note, however, that any privacy is lost if a public node id is used for
+	/// `signing_pubkey`.
 	///
 	/// Errors if the request contains unknown required features.
 	///
@@ -375,7 +396,7 @@ impl RefundContents {
 
 		let offer = OfferTlvStreamRef {
 			chains: None,
-			metadata: self.metadata.as_ref(),
+			metadata: None,
 			currency: None,
 			amount: None,
 			description: Some(&self.description),
@@ -396,7 +417,7 @@ impl RefundContents {
 			chain: self.chain.as_ref(),
 			amount: Some(self.amount_msats),
 			features,
-			quantity: None,
+			quantity: self.quantity,
 			payer_id: Some(&self.payer_id),
 			payer_note: self.payer_note.as_ref(),
 		};
@@ -477,6 +498,10 @@ impl TryFrom<RefundTlvStream> for RefundContents {
 			Some(metadata) => PayerContents(metadata),
 		};
 
+		if metadata.is_some() {
+			return Err(SemanticError::UnexpectedMetadata);
+		}
+
 		if chains.is_some() {
 			return Err(SemanticError::UnexpectedChain);
 		}
@@ -514,20 +539,14 @@ impl TryFrom<RefundTlvStream> for RefundContents {
 
 		let features = features.unwrap_or_else(InvoiceRequestFeatures::empty);
 
-		// TODO: Check why this isn't in the spec.
-		if quantity.is_some() {
-			return Err(SemanticError::UnexpectedQuantity);
-		}
-
 		let payer_id = match payer_id {
 			None => return Err(SemanticError::MissingPayerId),
 			Some(payer_id) => payer_id,
 		};
 
-		// TODO: Should metadata be included?
 		Ok(RefundContents {
-			payer, metadata, description, absolute_expiry, issuer, paths, chain, amount_msats,
-			features, payer_id, payer_note,
+			payer, description, absolute_expiry, issuer, paths, chain, amount_msats, features,
+			quantity, payer_id, payer_note,
 		})
 	}
 }
@@ -755,6 +774,24 @@ mod tests {
 		assert_eq!(tlv_stream.chain, Some(&testnet));
 	}
 
+	#[test]
+	fn builds_refund_with_quantity() {
+		let refund = RefundBuilder::new("foo".into(), vec![1; 32], payer_pubkey(), 1000).unwrap()
+			.quantity(10)
+			.build().unwrap();
+		let (_, _, tlv_stream) = refund.as_tlv_stream();
+		assert_eq!(refund.quantity(), Some(10));
+		assert_eq!(tlv_stream.quantity, Some(10));
+
+		let refund = RefundBuilder::new("foo".into(), vec![1; 32], payer_pubkey(), 1000).unwrap()
+			.quantity(10)
+			.quantity(1)
+			.build().unwrap();
+		let (_, _, tlv_stream) = refund.as_tlv_stream();
+		assert_eq!(refund.quantity(), Some(1));
+		assert_eq!(tlv_stream.quantity, Some(1));
+	}
+
 	#[test]
 	fn builds_refund_with_payer_note() {
 		let refund = RefundBuilder::new("foo".into(), vec![1; 32], payer_pubkey(), 1000).unwrap()
@@ -888,6 +925,7 @@ mod tests {
 			.path(paths[1].clone())
 			.chain(Network::Testnet)
 			.features_unchecked(InvoiceRequestFeatures::unknown())
+			.quantity(10)
 			.payer_note("baz".into())
 			.build()
 			.unwrap();
@@ -900,6 +938,7 @@ mod tests {
 				assert_eq!(refund.issuer(), Some(PrintableString("bar")));
 				assert_eq!(refund.chain(), ChainHash::using_genesis_block(Network::Testnet));
 				assert_eq!(refund.features(), &InvoiceRequestFeatures::unknown());
+				assert_eq!(refund.quantity(), Some(10));
 				assert_eq!(refund.payer_note(), Some(PrintableString("baz")));
 			},
 			Err(e) => panic!("error parsing refund: {:?}", e),
@@ -914,6 +953,17 @@ mod tests {
 			panic!("error parsing refund: {:?}", e);
 		}
 
+		let metadata = vec![42; 32];
+		let mut tlv_stream = refund.as_tlv_stream();
+		tlv_stream.1.metadata = Some(&metadata);
+
+		match Refund::try_from(tlv_stream.to_bytes()) {
+			Ok(_) => panic!("expected error"),
+			Err(e) => {
+				assert_eq!(e, ParseError::InvalidSemantics(SemanticError::UnexpectedMetadata));
+			},
+		}
+
 		let chains = vec![ChainHash::using_genesis_block(Network::Testnet)];
 		let mut tlv_stream = refund.as_tlv_stream();
 		tlv_stream.1.chains = Some(&chains);
@@ -967,16 +1017,6 @@ mod tests {
 				assert_eq!(e, ParseError::InvalidSemantics(SemanticError::UnexpectedSigningPubkey));
 			},
 		}
-
-		let mut tlv_stream = refund.as_tlv_stream();
-		tlv_stream.2.quantity = Some(10);
-
-		match Refund::try_from(tlv_stream.to_bytes()) {
-			Ok(_) => panic!("expected error"),
-			Err(e) => {
-				assert_eq!(e, ParseError::InvalidSemantics(SemanticError::UnexpectedQuantity));
-			},
-		}
 	}
 
 	#[test]