From 2d44dbe0134ae09ea6937ab865fbe55f8be2a543 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 18 Jan 2023 18:58:20 -0600 Subject: [PATCH] Disallow offer_metadata in Refund The offer_metadata was optional but is redundant with invreq_metadata (i.e., payer_metadata) for refunds. It is now disallowed in the spec and was already unsupported by RefundBuilder. --- lightning/src/offers/parse.rs | 2 ++ lightning/src/offers/refund.rs | 29 +++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index a7d13e570..35c1425ac 100644 --- a/lightning/src/offers/parse.rs +++ b/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. diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index f800274c9..e022f1275 100644 --- a/lightning/src/offers/refund.rs +++ b/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(), quantity: None, 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 }) @@ -229,7 +229,6 @@ pub struct Refund { pub(super) struct RefundContents { payer: PayerContents, // offer fields - metadata: Option>, description: String, absolute_expiry: Option, issuer: Option, @@ -395,7 +394,7 @@ impl RefundContents { let offer = OfferTlvStreamRef { chains: None, - metadata: self.metadata.as_ref(), + metadata: None, currency: None, amount: None, description: Some(&self.description), @@ -497,6 +496,10 @@ impl TryFrom for RefundContents { Some(metadata) => PayerContents(metadata), }; + if metadata.is_some() { + return Err(SemanticError::UnexpectedMetadata); + } + if chains.is_some() { return Err(SemanticError::UnexpectedChain); } @@ -539,10 +542,9 @@ impl TryFrom for RefundContents { Some(payer_id) => payer_id, }; - // TODO: Should metadata be included? Ok(RefundContents { - payer, metadata, description, absolute_expiry, issuer, paths, chain, amount_msats, - features, quantity, payer_id, payer_note, + payer, description, absolute_expiry, issuer, paths, chain, amount_msats, features, + quantity, payer_id, payer_note, }) } } @@ -949,6 +951,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); -- 2.39.5