Allow quantity in Refund
authorJeffrey Czyz <jkczyz@gmail.com>
Wed, 18 Jan 2023 23:29:31 +0000 (17:29 -0600)
committerJeffrey Czyz <jkczyz@gmail.com>
Mon, 30 Jan 2023 21:44:39 +0000 (15:44 -0600)
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.

lightning/src/offers/refund.rs

index bee6cc7f5f61ddd39a89744bf33f9d6bc8ea66f5..f800274c973341d0ed33fd9c89dffc8fcb475b19 100644 (file)
@@ -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]