From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 18 Jan 2023 23:29:31 +0000 (-0600)
Subject: Allow quantity in Refund
X-Git-Tag: v0.0.114-beta~37^2~2
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=6236e0d4722cdc9ccf3617f9c2ce73782213c08c;p=rust-lightning

Allow quantity in Refund

The spec always allowed this but the reason was unclear. It's useful if
the refund is for an invoice paid for offer where a quantity was given
in the request. The description in the refund would be from the offer,
which may have given a unit for each item. So allowing a quantity makes
it clear how many items the refund is for.
---

diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs
index bee6cc7f5..f800274c9 100644
--- a/lightning/src/offers/refund.rs
+++ b/lightning/src/offers/refund.rs
@@ -120,7 +120,7 @@ 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,
+			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.
@@ -224,6 +238,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 +300,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.
 	///
@@ -396,7 +416,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(),
 		};
@@ -514,11 +534,6 @@ 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,
@@ -527,7 +542,7 @@ impl TryFrom<RefundTlvStream> for RefundContents {
 		// TODO: Should metadata be included?
 		Ok(RefundContents {
 			payer, metadata, description, absolute_expiry, issuer, paths, chain, amount_msats,
-			features, payer_id, payer_note,
+			features, quantity, payer_id, payer_note,
 		})
 	}
 }
@@ -755,6 +770,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 +921,7 @@ mod tests {
 			.path(paths[1].clone())
 			.chain(Network::Testnet)
 			.features_unchecked(InvoiceRequestFeatures::unknown())
+			.quantity(10)
 			.payer_note("baz".into())
 			.build()
 			.unwrap();
@@ -900,6 +934,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),
@@ -967,16 +1002,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]