From b2c3ddb1527976ba5639bdc344043aad058a256f Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Thu, 18 Jul 2024 13:05:13 -0600 Subject: [PATCH 01/14] Refactor amount validation and relocate currency check Update `InvoiceRequest` parsing to handle currency amounts in offers. The currency check has been moved from `OfferContents::check_amount_msats_for_quantity` to the `InvoiceRequest` parsing code. This makes sure that an error is returned early if an offer specifies an unsupported currency, preventing issues with parsing a `Bolt12Invoice`. This change also simplifies `OfferContents::check_amount_msats_for_quantity` by removing redundant currency validation, allowing it to trust the sender-provided amount. --- lightning/src/offers/invoice_request.rs | 13 +++++++++---- lightning/src/offers/offer.rs | 3 +-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index c6c9da82a4e..a8507ba458b 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -71,7 +71,7 @@ use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::ln::msgs::DecodeError; use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, self}; use crate::offers::nonce::Nonce; -use crate::offers::offer::{Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef}; +use crate::offers::offer::{Amount, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef}; use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError}; use crate::offers::payer::{PayerContents, PayerTlvStream, PayerTlvStreamRef}; use crate::offers::signer::{Metadata, MetadataMaterial}; @@ -1163,6 +1163,14 @@ impl TryFrom for InvoiceRequestContents { }; let offer = OfferContents::try_from(offer_tlv_stream)?; + match offer.amount() { + Some(Amount::Currency {..}) => return Err(Bolt12SemanticError::UnsupportedCurrency), + _ => { + offer.check_amount_msats_for_quantity(amount, quantity)?; + offer.check_quantity(quantity)?; + }, + }; + if !offer.supports_chain(chain.unwrap_or_else(|| offer.implied_chain())) { return Err(Bolt12SemanticError::UnsupportedChain); } @@ -1171,9 +1179,6 @@ impl TryFrom for InvoiceRequestContents { return Err(Bolt12SemanticError::MissingAmount); } - offer.check_quantity(quantity)?; - offer.check_amount_msats_for_quantity(amount, quantity)?; - let features = features.unwrap_or_else(InvoiceRequestFeatures::empty); let payer_signing_pubkey = match payer_id { diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 8501b8d4651..282b054d082 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -873,9 +873,8 @@ impl OfferContents { &self, amount_msats: Option, quantity: Option ) -> Result<(), Bolt12SemanticError> { let offer_amount_msats = match self.amount { - None => 0, Some(Amount::Bitcoin { amount_msats }) => amount_msats, - Some(Amount::Currency { .. }) => return Err(Bolt12SemanticError::UnsupportedCurrency), + _ => 0, }; if !self.expects_quantity() || quantity.is_some() { From e7edc86658fb5f445ed94c4a063763715478e719 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Mon, 26 Aug 2024 14:05:58 -0600 Subject: [PATCH 02/14] f Remove currency validation from parsing code This fixup commit removes the currency validation from the `InvoiceRequest` parsing code. Additionally, updates a test that previously expected a failure due to an unsupported currency. Since the logic now parses any amount the test no longer triggers the `UnsupportedCurrency` error during parsing. The next commit adds the currency validation during handling in `ChannelManager` instead. This way, when currency support is added, we avoid the need for exchange rate lookups at parsing time. --- lightning/src/offers/invoice_request.rs | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index a8507ba458b..a8a23e2868b 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -71,7 +71,7 @@ use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::ln::msgs::DecodeError; use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, self}; use crate::offers::nonce::Nonce; -use crate::offers::offer::{Amount, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef}; +use crate::offers::offer::{Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef}; use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError}; use crate::offers::payer::{PayerContents, PayerTlvStream, PayerTlvStreamRef}; use crate::offers::signer::{Metadata, MetadataMaterial}; @@ -1163,14 +1163,6 @@ impl TryFrom for InvoiceRequestContents { }; let offer = OfferContents::try_from(offer_tlv_stream)?; - match offer.amount() { - Some(Amount::Currency {..}) => return Err(Bolt12SemanticError::UnsupportedCurrency), - _ => { - offer.check_amount_msats_for_quantity(amount, quantity)?; - offer.check_quantity(quantity)?; - }, - }; - if !offer.supports_chain(chain.unwrap_or_else(|| offer.implied_chain())) { return Err(Bolt12SemanticError::UnsupportedChain); } @@ -1179,6 +1171,9 @@ impl TryFrom for InvoiceRequestContents { return Err(Bolt12SemanticError::MissingAmount); } + offer.check_quantity(quantity)?; + offer.check_amount_msats_for_quantity(amount, quantity)?; + let features = features.unwrap_or_else(InvoiceRequestFeatures::empty); let payer_signing_pubkey = match payer_id { @@ -2048,11 +2043,8 @@ mod tests { 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, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::UnsupportedCurrency)); - }, + if let Err(e) = InvoiceRequest::try_from(buffer) { + panic!("error parsing invoice_request: {:?}", e); } let invoice_request = OfferBuilder::new(recipient_pubkey()) From 2f029a97b3165fd3926eb70d0f23f0500cb333a7 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Mon, 26 Aug 2024 17:01:22 -0600 Subject: [PATCH 03/14] Move currency check to `OffersMessageHandler` Add and move the check for unsupported currencies from the parsing code to `handle_message` in `ChannelManager`. This change ensures that we don't fail when parsing an `InvoiceRequest` for an Offer with a currency. --- lightning/src/ln/channelmanager.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index bdf67b5da7c..89a93f27fc5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -67,7 +67,7 @@ use crate::offers::invoice::{Bolt12Invoice, DEFAULT_RELATIVE_EXPIRY, DerivedSign use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{DerivedPayerSigningPubkey, InvoiceRequest, InvoiceRequestBuilder}; use crate::offers::nonce::Nonce; -use crate::offers::offer::{Offer, OfferBuilder}; +use crate::offers::offer::{Amount, Offer, OfferBuilder}; use crate::offers::parse::Bolt12SemanticError; use crate::offers::refund::{Refund, RefundBuilder}; use crate::offers::signer; @@ -11135,6 +11135,13 @@ where None => return None, }; + if let Some(amount) = invoice_request.amount() { + if let Amount::Currency { .. } = amount { + let error = Bolt12SemanticError::UnsupportedCurrency; + return Some((OffersMessage::InvoiceError(error.into()), responder.respond())); + } + } + let nonce = match context { None if invoice_request.metadata().is_some() => None, Some(OffersContext::InvoiceRequest { nonce }) => Some(nonce), From 61f7348912ef7aeba877a760915d156811b67023 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Fri, 19 Jul 2024 13:00:35 -0600 Subject: [PATCH 04/14] Add `Bolt12CreationError` error type Introduce the `Bolt12CreationError` error type for handling BOLT12-related errors in the `ChannelManager`. This error type consolidates various variants, which were previously part of `Bolt12SemanticError`. Additionally, updated the relevant code to replace `Bolt12SemanticError` with `Bolt12CreationError` throughout the affected files. Note: The following commit will revert the changes in the `create_refund_builder` and `pay_for_offer` methods, switching back to `Bolt12SemanticError` temporarily. A future commit will introduce a `Bolt12RequestError` to handle these scenarios more accurately. --- lightning/src/ln/channelmanager.rs | 59 ++++++++++++++++++++---------- lightning/src/ln/offers_tests.rs | 22 +++++------ lightning/src/offers/parse.rs | 4 +- 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 89a93f27fc5..dffa80ac6c7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1844,10 +1844,9 @@ where /// /// ``` /// # use lightning::events::{Event, EventsProvider, PaymentPurpose}; -/// # use lightning::ln::channelmanager::AChannelManager; -/// # use lightning::offers::parse::Bolt12SemanticError; +/// # use lightning::ln::channelmanager::{AChannelManager, Bolt12CreationError}; /// # -/// # fn example(channel_manager: T) -> Result<(), Bolt12SemanticError> { +/// # fn example(channel_manager: T) -> Result<(), Bolt12CreationError> { /// # let channel_manager = channel_manager.get_cm(); /// # let absolute_expiry = None; /// let offer = channel_manager @@ -1947,13 +1946,12 @@ where /// ``` /// # use core::time::Duration; /// # use lightning::events::{Event, EventsProvider}; -/// # use lightning::ln::channelmanager::{AChannelManager, PaymentId, RecentPaymentDetails, Retry}; -/// # use lightning::offers::parse::Bolt12SemanticError; +/// # use lightning::ln::channelmanager::{AChannelManager, Bolt12CreationError, PaymentId, RecentPaymentDetails, Retry}; /// # /// # fn example( /// # channel_manager: T, amount_msats: u64, absolute_expiry: Duration, retry: Retry, /// # max_total_routing_fee_msat: Option -/// # ) -> Result<(), Bolt12SemanticError> { +/// # ) -> Result<(), Bolt12CreationError> { /// # let channel_manager = channel_manager.get_cm(); /// let payment_id = PaymentId([42; 32]); /// let refund = channel_manager @@ -2685,6 +2683,26 @@ pub enum RecentPaymentDetails { }, } +/// Error during creation and handling of BOLT 12 related payments. +#[derive(Debug, Clone, PartialEq)] +pub enum Bolt12CreationError { + /// Error from BOLT 12 semantic checks. + InvalidSemantics(Bolt12SemanticError), + /// The payment id for a refund or request is already in use. + DuplicatePaymentId, + /// There is insufficient liquidity to complete the payment. + InsufficientLiquidity, + /// Failed to create a blinded path. + BlindedPathCreationFailed, +} + +impl From for Bolt12CreationError { + fn from(err: Bolt12SemanticError) -> Self { + Bolt12CreationError::InvalidSemantics(err) + } +} + + /// Route hints used in constructing invoices for [phantom node payents]. /// /// [phantom node payments]: crate::sign::PhantomKeysManager @@ -9114,7 +9132,7 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => { /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest pub fn create_offer_builder( &$self, absolute_expiry: Option - ) -> Result<$builder, Bolt12SemanticError> { + ) -> Result<$builder, Bolt12CreationError> { let node_id = $self.get_our_node_id(); let expanded_key = &$self.inbound_payment_key; let entropy = &*$self.entropy_source; @@ -9124,7 +9142,7 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => { let context = OffersContext::InvoiceRequest { nonce }; let path = $self.create_blinded_paths_using_absolute_expiry(context, absolute_expiry) .and_then(|paths| paths.into_iter().next().ok_or(())) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; + .map_err(|()| Bolt12CreationError::BlindedPathCreationFailed)?; let builder = OfferBuilder::deriving_signing_pubkey(node_id, expanded_key, nonce, secp_ctx) .chain_hash($self.chain_hash) .path(path); @@ -9187,7 +9205,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { pub fn create_refund_builder( &$self, amount_msats: u64, absolute_expiry: Duration, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option - ) -> Result<$builder, Bolt12SemanticError> { + ) -> Result<$builder, Bolt12CreationError> { let node_id = $self.get_our_node_id(); let expanded_key = &$self.inbound_payment_key; let entropy = &*$self.entropy_source; @@ -9197,7 +9215,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { let context = OffersContext::OutboundPayment { payment_id, nonce, hmac: None }; let path = $self.create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry)) .and_then(|paths| paths.into_iter().next().ok_or(())) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; + .map_err(|()| Bolt12CreationError::BlindedPathCreationFailed)?; let builder = RefundBuilder::deriving_signing_pubkey( node_id, expanded_key, nonce, secp_ctx, amount_msats, payment_id @@ -9213,7 +9231,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { .add_new_awaiting_invoice( payment_id, expiration, retry_strategy, max_total_routing_fee_msat, None, ) - .map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?; + .map_err(|()| Bolt12CreationError::DuplicatePaymentId)?; Ok(builder.into()) } @@ -9305,7 +9323,7 @@ where &self, offer: &Offer, quantity: Option, amount_msats: Option, payer_note: Option, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option - ) -> Result<(), Bolt12SemanticError> { + ) -> Result<(), Bolt12CreationError> { let expanded_key = &self.inbound_payment_key; let entropy = &*self.entropy_source; let secp_ctx = &self.secp_ctx; @@ -9335,7 +9353,7 @@ where OffersContext::OutboundPayment { payment_id, nonce, hmac: Some(hmac) } ); let reply_paths = self.create_blinded_paths(context) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; + .map_err(|()| Bolt12CreationError::BlindedPathCreationFailed)?; let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); @@ -9349,9 +9367,10 @@ where payment_id, expiration, retry_strategy, max_total_routing_fee_msat, Some(retryable_invoice_request) ) - .map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?; + .map_err(|()| Bolt12CreationError::DuplicatePaymentId)?; - self.enqueue_invoice_request(invoice_request, reply_paths) + self.enqueue_invoice_request(invoice_request, reply_paths)?; + Ok(()) } fn enqueue_invoice_request( @@ -9414,7 +9433,7 @@ where /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice pub fn request_refund_payment( &self, refund: &Refund - ) -> Result { + ) -> Result { let expanded_key = &self.inbound_payment_key; let entropy = &*self.entropy_source; let secp_ctx = &self.secp_ctx; @@ -9423,7 +9442,7 @@ where let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; if refund.chain() != self.chain_hash { - return Err(Bolt12SemanticError::UnsupportedChain); + return Err(Bolt12CreationError::InvalidSemantics(Bolt12SemanticError::UnsupportedChain)); } let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); @@ -9434,7 +9453,7 @@ where let payment_paths = self.create_blinded_payment_paths( amount_msats, payment_secret, payment_context ) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; + .map_err(|()| Bolt12CreationError::BlindedPathCreationFailed)?; #[cfg(feature = "std")] let builder = refund.respond_using_derived_keys( @@ -9457,7 +9476,7 @@ where payment_hash: invoice.payment_hash(), nonce, hmac }); let reply_paths = self.create_blinded_paths(context) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; + .map_err(|()| Bolt12CreationError::BlindedPathCreationFailed)?; let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap(); if refund.paths().is_empty() { @@ -9486,7 +9505,7 @@ where Ok(invoice) }, - Err(()) => Err(Bolt12SemanticError::InvalidAmount), + Err(()) => Err(Bolt12CreationError::InvalidSemantics(Bolt12SemanticError::InvalidAmount)), } } diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 12b10754e41..eb74101a570 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -48,7 +48,7 @@ use crate::blinded_path::message::BlindedMessagePath; use crate::blinded_path::payment::{Bolt12OfferContext, Bolt12RefundContext, PaymentContext}; use crate::blinded_path::message::{MessageContext, OffersContext}; use crate::events::{ClosureReason, Event, MessageSendEventsProvider, PaymentFailureReason, PaymentPurpose}; -use crate::ln::channelmanager::{Bolt12PaymentError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, Retry, self}; +use crate::ln::channelmanager::{Bolt12CreationError, Bolt12PaymentError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, Retry, self}; use crate::ln::features::Bolt12InvoiceFeatures; use crate::ln::functional_test_utils::*; use crate::ln::inbound_payment::ExpandedKey; @@ -1681,7 +1681,7 @@ fn fails_creating_or_paying_for_offer_without_connected_peers() { let absolute_expiry = alice.node.duration_since_epoch() + MAX_SHORT_LIVED_RELATIVE_EXPIRY; match alice.node.create_offer_builder(Some(absolute_expiry)) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), } let mut args = ReconnectArgs::new(alice, bob); @@ -1697,7 +1697,7 @@ fn fails_creating_or_paying_for_offer_without_connected_peers() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), } assert!(nodes[0].node.list_recent_payments().is_empty()); @@ -1755,7 +1755,7 @@ fn fails_creating_refund_or_sending_invoice_without_connected_peers() { 10_000_000, absolute_expiry, payment_id, Retry::Attempts(0), None ) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), } let mut args = ReconnectArgs::new(charlie, david); @@ -1769,7 +1769,7 @@ fn fails_creating_refund_or_sending_invoice_without_connected_peers() { match alice.node.request_refund_payment(&refund) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), } let mut args = ReconnectArgs::new(alice, bob); @@ -1801,7 +1801,7 @@ fn fails_creating_invoice_request_for_unsupported_chain() { let payment_id = PaymentId([1; 32]); match bob.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::UnsupportedChain), + Err(e) => assert_eq!(e, Bolt12CreationError::InvalidSemantics(Bolt12SemanticError::UnsupportedChain)), } } @@ -1828,7 +1828,7 @@ fn fails_sending_invoice_with_unsupported_chain_for_refund() { match alice.node.request_refund_payment(&refund) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::UnsupportedChain), + Err(e) => assert_eq!(e, Bolt12CreationError::InvalidSemantics(Bolt12SemanticError::UnsupportedChain)), } } @@ -1860,7 +1860,7 @@ fn fails_creating_invoice_request_without_blinded_reply_path() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), } assert!(nodes[0].node.list_recent_payments().is_empty()); @@ -1900,7 +1900,7 @@ fn fails_creating_invoice_request_with_duplicate_payment_id() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::DuplicatePaymentId), + Err(e) => assert_eq!(e, Bolt12CreationError::DuplicatePaymentId), } expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); @@ -1928,7 +1928,7 @@ fn fails_creating_refund_with_duplicate_payment_id() { 10_000, absolute_expiry, payment_id, Retry::Attempts(0), None ) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::DuplicatePaymentId), + Err(e) => assert_eq!(e, Bolt12CreationError::DuplicatePaymentId), } expect_recent_payment!(nodes[0], RecentPaymentDetails::AwaitingInvoice, payment_id); @@ -2051,7 +2051,7 @@ fn fails_sending_invoice_without_blinded_payment_paths_for_refund() { match alice.node.request_refund_payment(&refund) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), } } diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index a46e90de6be..6aaa24a6f78 100644 --- a/lightning/src/offers/parse.rs +++ b/lightning/src/offers/parse.rs @@ -178,8 +178,8 @@ pub enum Bolt12SemanticError { MissingPayerMetadata, /// A payer signing pubkey was expected but was missing. MissingPayerSigningPubkey, - /// The payment id for a refund or request is already in use. - DuplicatePaymentId, + /// A payer id was expected but was missing. + MissingPayerId, /// Blinded paths were expected but were missing. MissingPaths, /// Blinded paths were provided but were not expected. From 5c8be849cdea9a72b6c10aa31b6b2a44d673f441 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Fri, 23 Aug 2024 12:48:10 -0600 Subject: [PATCH 05/14] f Revert `Bolt12CreationError` to `Bolt12SemanticError` Revert the return type of `create_refund_builder` and `pay_for_offer` from `Bolt12CreationError` back to `Bolt12SemanticError`. This change is temporary, as a future commit will introduce `Bolt12RequestError` for these functions. --- lightning/src/ln/channelmanager.rs | 21 +++++++++------------ lightning/src/ln/offers_tests.rs | 12 ++++++------ lightning/src/offers/parse.rs | 2 ++ 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index dffa80ac6c7..bfaaa7596f9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1946,12 +1946,13 @@ where /// ``` /// # use core::time::Duration; /// # use lightning::events::{Event, EventsProvider}; -/// # use lightning::ln::channelmanager::{AChannelManager, Bolt12CreationError, PaymentId, RecentPaymentDetails, Retry}; +/// # use lightning::ln::channelmanager::{AChannelManager, PaymentId, RecentPaymentDetails, Retry}; +/// # use lightning::offers::parse::Bolt12SemanticError; /// # /// # fn example( /// # channel_manager: T, amount_msats: u64, absolute_expiry: Duration, retry: Retry, /// # max_total_routing_fee_msat: Option -/// # ) -> Result<(), Bolt12CreationError> { +/// # ) -> Result<(), Bolt12SemanticError> { /// # let channel_manager = channel_manager.get_cm(); /// let payment_id = PaymentId([42; 32]); /// let refund = channel_manager @@ -2688,10 +2689,6 @@ pub enum RecentPaymentDetails { pub enum Bolt12CreationError { /// Error from BOLT 12 semantic checks. InvalidSemantics(Bolt12SemanticError), - /// The payment id for a refund or request is already in use. - DuplicatePaymentId, - /// There is insufficient liquidity to complete the payment. - InsufficientLiquidity, /// Failed to create a blinded path. BlindedPathCreationFailed, } @@ -9205,7 +9202,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { pub fn create_refund_builder( &$self, amount_msats: u64, absolute_expiry: Duration, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option - ) -> Result<$builder, Bolt12CreationError> { + ) -> Result<$builder, Bolt12SemanticError> { let node_id = $self.get_our_node_id(); let expanded_key = &$self.inbound_payment_key; let entropy = &*$self.entropy_source; @@ -9215,7 +9212,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { let context = OffersContext::OutboundPayment { payment_id, nonce, hmac: None }; let path = $self.create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry)) .and_then(|paths| paths.into_iter().next().ok_or(())) - .map_err(|()| Bolt12CreationError::BlindedPathCreationFailed)?; + .map_err(|_| Bolt12SemanticError::MissingPaths)?; let builder = RefundBuilder::deriving_signing_pubkey( node_id, expanded_key, nonce, secp_ctx, amount_msats, payment_id @@ -9231,7 +9228,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { .add_new_awaiting_invoice( payment_id, expiration, retry_strategy, max_total_routing_fee_msat, None, ) - .map_err(|()| Bolt12CreationError::DuplicatePaymentId)?; + .map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?; Ok(builder.into()) } @@ -9323,7 +9320,7 @@ where &self, offer: &Offer, quantity: Option, amount_msats: Option, payer_note: Option, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option - ) -> Result<(), Bolt12CreationError> { + ) -> Result<(), Bolt12SemanticError> { let expanded_key = &self.inbound_payment_key; let entropy = &*self.entropy_source; let secp_ctx = &self.secp_ctx; @@ -9353,7 +9350,7 @@ where OffersContext::OutboundPayment { payment_id, nonce, hmac: Some(hmac) } ); let reply_paths = self.create_blinded_paths(context) - .map_err(|()| Bolt12CreationError::BlindedPathCreationFailed)?; + .map_err(|_| Bolt12SemanticError::MissingPaths)?; let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); @@ -9367,7 +9364,7 @@ where payment_id, expiration, retry_strategy, max_total_routing_fee_msat, Some(retryable_invoice_request) ) - .map_err(|()| Bolt12CreationError::DuplicatePaymentId)?; + .map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?; self.enqueue_invoice_request(invoice_request, reply_paths)?; Ok(()) diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index eb74101a570..eb9be5ba027 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -1697,7 +1697,7 @@ fn fails_creating_or_paying_for_offer_without_connected_peers() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), + Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), } assert!(nodes[0].node.list_recent_payments().is_empty()); @@ -1755,7 +1755,7 @@ fn fails_creating_refund_or_sending_invoice_without_connected_peers() { 10_000_000, absolute_expiry, payment_id, Retry::Attempts(0), None ) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), + Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), } let mut args = ReconnectArgs::new(charlie, david); @@ -1801,7 +1801,7 @@ fn fails_creating_invoice_request_for_unsupported_chain() { let payment_id = PaymentId([1; 32]); match bob.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12CreationError::InvalidSemantics(Bolt12SemanticError::UnsupportedChain)), + Err(e) => assert_eq!(e, Bolt12SemanticError::UnsupportedChain), } } @@ -1860,7 +1860,7 @@ fn fails_creating_invoice_request_without_blinded_reply_path() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12CreationError::BlindedPathCreationFailed), + Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), } assert!(nodes[0].node.list_recent_payments().is_empty()); @@ -1900,7 +1900,7 @@ fn fails_creating_invoice_request_with_duplicate_payment_id() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12CreationError::DuplicatePaymentId), + Err(e) => assert_eq!(e, Bolt12SemanticError::DuplicatePaymentId), } expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); @@ -1928,7 +1928,7 @@ fn fails_creating_refund_with_duplicate_payment_id() { 10_000, absolute_expiry, payment_id, Retry::Attempts(0), None ) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12CreationError::DuplicatePaymentId), + Err(e) => assert_eq!(e, Bolt12SemanticError::DuplicatePaymentId), } expect_recent_payment!(nodes[0], RecentPaymentDetails::AwaitingInvoice, payment_id); diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index 6aaa24a6f78..b25ab58155b 100644 --- a/lightning/src/offers/parse.rs +++ b/lightning/src/offers/parse.rs @@ -180,6 +180,8 @@ pub enum Bolt12SemanticError { MissingPayerSigningPubkey, /// A payer id was expected but was missing. MissingPayerId, + /// The payment id for a refund or request is already in use. + DuplicatePaymentId, /// Blinded paths were expected but were missing. MissingPaths, /// Blinded paths were provided but were not expected. From 6f9ac1100d5d875235d4cf2f89fae92663174dd6 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Wed, 7 Aug 2024 14:20:23 -0600 Subject: [PATCH 06/14] Add a `Bolt12RequestError` error type Introduced the `Bolt12RequestError` type to handle errors associated with BOLT12 payment and refund requests. This type includes variants for handling `InvalidSemantics`, `DuplicatePaymentId`, `InsufficientLiquidity`, and `BlindedPathCreationFailed`. --- lightning/src/ln/channelmanager.rs | 39 +++++++++++++++++++++--------- lightning/src/ln/offers_tests.rs | 17 ++++++------- lightning/src/offers/parse.rs | 2 -- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index bfaaa7596f9..c350a8303fa 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1946,13 +1946,12 @@ where /// ``` /// # use core::time::Duration; /// # use lightning::events::{Event, EventsProvider}; -/// # use lightning::ln::channelmanager::{AChannelManager, PaymentId, RecentPaymentDetails, Retry}; -/// # use lightning::offers::parse::Bolt12SemanticError; +/// # use lightning::ln::channelmanager::{AChannelManager, Bolt12RequestError, PaymentId, RecentPaymentDetails, Retry}; /// # /// # fn example( /// # channel_manager: T, amount_msats: u64, absolute_expiry: Duration, retry: Retry, /// # max_total_routing_fee_msat: Option -/// # ) -> Result<(), Bolt12SemanticError> { +/// # ) -> Result<(), Bolt12RequestError> { /// # let channel_manager = channel_manager.get_cm(); /// let payment_id = PaymentId([42; 32]); /// let refund = channel_manager @@ -2699,6 +2698,24 @@ impl From for Bolt12CreationError { } } +/// Error during requesting a BOLT 12 related payment. +#[derive(Debug, Clone, PartialEq)] +pub enum Bolt12RequestError { + /// Error from BOLT 12 semantic checks. + InvalidSemantics(Bolt12SemanticError), + /// The payment id for a refund or request is already in use. + DuplicatePaymentId, + /// There is insufficient liquidity to complete the payment. + InsufficientLiquidity, + /// Failed to create a blinded path. + BlindedPathCreationFailed, +} + +impl From for Bolt12RequestError { + fn from(err: Bolt12SemanticError) -> Self { + Bolt12RequestError::InvalidSemantics(err) + } +} /// Route hints used in constructing invoices for [phantom node payents]. /// @@ -9202,7 +9219,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { pub fn create_refund_builder( &$self, amount_msats: u64, absolute_expiry: Duration, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option - ) -> Result<$builder, Bolt12SemanticError> { + ) -> Result<$builder, Bolt12RequestError> { let node_id = $self.get_our_node_id(); let expanded_key = &$self.inbound_payment_key; let entropy = &*$self.entropy_source; @@ -9212,7 +9229,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { let context = OffersContext::OutboundPayment { payment_id, nonce, hmac: None }; let path = $self.create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry)) .and_then(|paths| paths.into_iter().next().ok_or(())) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; + .map_err(|()| Bolt12RequestError::BlindedPathCreationFailed)?; let builder = RefundBuilder::deriving_signing_pubkey( node_id, expanded_key, nonce, secp_ctx, amount_msats, payment_id @@ -9228,7 +9245,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { .add_new_awaiting_invoice( payment_id, expiration, retry_strategy, max_total_routing_fee_msat, None, ) - .map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?; + .map_err(|()| Bolt12RequestError::DuplicatePaymentId)?; Ok(builder.into()) } @@ -9320,7 +9337,7 @@ where &self, offer: &Offer, quantity: Option, amount_msats: Option, payer_note: Option, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option - ) -> Result<(), Bolt12SemanticError> { + ) -> Result<(), Bolt12RequestError> { let expanded_key = &self.inbound_payment_key; let entropy = &*self.entropy_source; let secp_ctx = &self.secp_ctx; @@ -9350,7 +9367,7 @@ where OffersContext::OutboundPayment { payment_id, nonce, hmac: Some(hmac) } ); let reply_paths = self.create_blinded_paths(context) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; + .map_err(|()| Bolt12RequestError::BlindedPathCreationFailed)?; let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); @@ -9364,7 +9381,7 @@ where payment_id, expiration, retry_strategy, max_total_routing_fee_msat, Some(retryable_invoice_request) ) - .map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?; + .map_err(|()| Bolt12RequestError::DuplicatePaymentId)?; self.enqueue_invoice_request(invoice_request, reply_paths)?; Ok(()) @@ -9374,7 +9391,7 @@ where &self, invoice_request: InvoiceRequest, reply_paths: Vec, - ) -> Result<(), Bolt12SemanticError> { + ) -> Result<(), Bolt12RequestError> { let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap(); if !invoice_request.paths().is_empty() { reply_paths @@ -9400,7 +9417,7 @@ where } } else { debug_assert!(false); - return Err(Bolt12SemanticError::MissingIssuerSigningPubkey); + return Err(Bolt12RequestError::InvalidSemantics(Bolt12SemanticError::MissingIssuerSigningPubkey)); } Ok(()) diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index eb9be5ba027..c3a832627a2 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -48,7 +48,7 @@ use crate::blinded_path::message::BlindedMessagePath; use crate::blinded_path::payment::{Bolt12OfferContext, Bolt12RefundContext, PaymentContext}; use crate::blinded_path::message::{MessageContext, OffersContext}; use crate::events::{ClosureReason, Event, MessageSendEventsProvider, PaymentFailureReason, PaymentPurpose}; -use crate::ln::channelmanager::{Bolt12CreationError, Bolt12PaymentError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, Retry, self}; +use crate::ln::channelmanager::{Bolt12CreationError, Bolt12PaymentError, Bolt12RequestError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, Retry, self}; use crate::ln::features::Bolt12InvoiceFeatures; use crate::ln::functional_test_utils::*; use crate::ln::inbound_payment::ExpandedKey; @@ -1697,7 +1697,7 @@ fn fails_creating_or_paying_for_offer_without_connected_peers() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12RequestError::BlindedPathCreationFailed), } assert!(nodes[0].node.list_recent_payments().is_empty()); @@ -1755,7 +1755,7 @@ fn fails_creating_refund_or_sending_invoice_without_connected_peers() { 10_000_000, absolute_expiry, payment_id, Retry::Attempts(0), None ) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12RequestError::BlindedPathCreationFailed), } let mut args = ReconnectArgs::new(charlie, david); @@ -1801,7 +1801,7 @@ fn fails_creating_invoice_request_for_unsupported_chain() { let payment_id = PaymentId([1; 32]); match bob.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::UnsupportedChain), + Err(e) => assert_eq!(e, Bolt12RequestError::InvalidSemantics(Bolt12SemanticError::UnsupportedChain)), } } @@ -1860,7 +1860,7 @@ fn fails_creating_invoice_request_without_blinded_reply_path() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), + Err(e) => assert_eq!(e, Bolt12RequestError::BlindedPathCreationFailed), } assert!(nodes[0].node.list_recent_payments().is_empty()); @@ -1900,7 +1900,7 @@ fn fails_creating_invoice_request_with_duplicate_payment_id() { match david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::DuplicatePaymentId), + Err(e) => assert_eq!(e, Bolt12RequestError::DuplicatePaymentId), } expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); @@ -1928,7 +1928,7 @@ fn fails_creating_refund_with_duplicate_payment_id() { 10_000, absolute_expiry, payment_id, Retry::Attempts(0), None ) { Ok(_) => panic!("Expected error"), - Err(e) => assert_eq!(e, Bolt12SemanticError::DuplicatePaymentId), + Err(e) => assert_eq!(e, Bolt12RequestError::DuplicatePaymentId), } expect_recent_payment!(nodes[0], RecentPaymentDetails::AwaitingInvoice, payment_id); @@ -2336,5 +2336,4 @@ fn no_double_pay_with_stale_channelmanager() { // Alice and Bob is closed. Since no 2nd attempt should be made, check that no events are // generated in response to the duplicate invoice. assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); -} - +} \ No newline at end of file diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index b25ab58155b..6aaa24a6f78 100644 --- a/lightning/src/offers/parse.rs +++ b/lightning/src/offers/parse.rs @@ -180,8 +180,6 @@ pub enum Bolt12SemanticError { MissingPayerSigningPubkey, /// A payer id was expected but was missing. MissingPayerId, - /// The payment id for a refund or request is already in use. - DuplicatePaymentId, /// Blinded paths were expected but were missing. MissingPaths, /// Blinded paths were provided but were not expected. From d38acd31ca6b6738c434efaca1c75694fd6806eb Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Fri, 12 Jul 2024 16:25:40 -0600 Subject: [PATCH 07/14] Abort payment if insufficient liquidity in `pay_for_offer` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check for sufficient channel liquidity in `pay_for_offer` and abort the payment if there is insufficient liquidity to fulfill the payment. This makes sure that the payment attempt is only made when there is enough outbound liquidity available, preventing failed payment attempts due to liquidity issues. --- lightning/src/ln/channelmanager.rs | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c350a8303fa..92eee22105f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9369,6 +9369,37 @@ where let reply_paths = self.create_blinded_paths(context) .map_err(|()| Bolt12RequestError::BlindedPathCreationFailed)?; + let total_liquidity: u64 = { + let per_peer_state = &self.per_peer_state.read().unwrap(); + let mut sum: u64 = 0; + + per_peer_state.iter().for_each(|(_, peer_state_mutex)| { + let peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &*peer_state_lock; + + sum += peer_state.channel_by_id + .iter() + .map(|(_, phase)| phase.context().get_available_balances(&self.fee_estimator).outbound_capacity_msat) + .sum::(); + }); + sum + }; + + let requested_amount_msats = match invoice_request.amount_msats() { + Some(amount_msats) => Some(amount_msats), + None => match offer.amount() { + Some(Amount::Bitcoin { amount_msats }) => Some(amount_msats), + _ => None, + }, + }; + + if let Some(amount) = requested_amount_msats { + if amount > total_liquidity { + log_error!(self.logger, "Insufficient liquidity for payment with payment id: {}", payment_id); + return Err(Bolt12RequestError::InsufficientLiquidity); + } + } + let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); let expiration = StaleExpiration::TimerTicks(1); From 7dfcbc5318585d17e21a332267d8457b48484fe2 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Tue, 23 Jul 2024 13:28:59 -0600 Subject: [PATCH 08/14] Add test for insufficient liquidity in `pay_for_offer` Adds the `fails_paying_offer_with_insufficient_liquidity` test to ensure that the `pay_for_offer` method correctly handles cases where the liquidity is insufficient. The test verifies that the method returns the `Bolt12RequestError::InsufficientLiquidity` error when the requested payment amount exceeds the available liquidity. --- lightning/src/ln/offers_tests.rs | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index c3a832627a2..533488d73d9 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -2336,4 +2336,37 @@ fn no_double_pay_with_stale_channelmanager() { // Alice and Bob is closed. Since no 2nd attempt should be made, check that no events are // generated in response to the duplicate invoice. assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); +} + +#[test] +fn fails_paying_offer_with_insufficient_liquidity() { + let channel_mon_config = create_chanmon_cfgs(2); + let node_config = create_node_cfgs(2, &channel_mon_config); + let node_chanmgrs = create_node_chanmgrs(2, &node_config, &[None, None]); + let nodes = create_network(2, &node_config, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + + let (alice, bob) = (&nodes[0], &nodes[1]); + let alice_id = alice.node.get_our_node_id(); + + let offer = alice.node + .create_offer_builder(None).unwrap() + .clear_paths() + .amount_msats(950_000_000) + .build().unwrap(); + assert_eq!(offer.issuer_signing_pubkey(), Some(alice_id)); + assert!(offer.paths().is_empty()); + + let payment_id = PaymentId([1; 32]); + + let result = bob.node + .pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None); + + match result { + Ok(_) => panic!("Expected error with insufficient liquidity."), + Err(e) => { + assert_eq!(e, Bolt12RequestError::InsufficientLiquidity); + } + } } \ No newline at end of file From f6f752f5e744e88a6770a8b6c8b5e685fce90cc0 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Thu, 25 Jul 2024 10:24:57 -0600 Subject: [PATCH 09/14] Add tests for `Offers` with fiat amount Introduces tests to ensure the successful creation of `InvoiceRequest` when specifying a currency amount with no amount_msats. And verifies that when both a currency amount and amount_msats are specified, the values are handled correctly. This ensures that `InvoiceRequest` behaves as expected with various amount configs. --- lightning/src/offers/invoice_request.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index a8a23e2868b..33d6dae23ec 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -1737,6 +1737,29 @@ mod tests { Ok(_) => panic!("expected error"), Err(e) => assert_eq!(e, Bolt12SemanticError::InvalidAmount), } + + let invoice_request = OfferBuilder::new(recipient_pubkey()) + .amount(Amount::Currency { iso4217_code: *b"USD", amount: 1000 }) + .build_unchecked() + .request_invoice(vec![1; 32], payer_pubkey()).unwrap() + .build().unwrap() + .sign(payer_sign).unwrap(); + let (_, _, tlv_stream, _) = invoice_request.as_tlv_stream(); + assert_eq!(invoice_request.amount(), Some(Amount::Currency { iso4217_code: *b"USD", amount: 1000 })); + assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(tlv_stream.amount, None); + + let invoice_request = OfferBuilder::new(recipient_pubkey()) + .amount(Amount::Currency { iso4217_code: *b"USD", amount: 100 }) + .build_unchecked() + .request_invoice(vec![1; 32], payer_pubkey()).unwrap() + .amount_msats(150_000_000) + .unwrap() + .build().unwrap() + .sign(payer_sign).unwrap(); + let (_, _, tlv_stream, _) = invoice_request.as_tlv_stream(); + assert_eq!(invoice_request.amount_msats(), Some(150_000_000)); + assert_eq!(tlv_stream.amount, Some(150_000_000)); } #[test] From 15653e8272551b86bf639335ae8189a19456f1d9 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Thu, 8 Aug 2024 08:08:23 -0600 Subject: [PATCH 10/14] Update docs for `InvoiceRequestBuilder::amount_msats` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves documentation for the amount_msats method in  `InvoiceRequestBuilder`. The update describes the behavior when an `Offer` specifies a currency. --- lightning/src/offers/invoice_request.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 33d6dae23ec..f5f41d38959 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -270,6 +270,11 @@ macro_rules! invoice_request_builder_methods { ( /// Sets the [`InvoiceRequest::amount_msats`] for paying an invoice. Errors if `amount_msats` is /// not at least the expected invoice amount (i.e., [`Offer::amount`] times [`quantity`]). /// + /// When the [`Offer`] specifies a currency, this method allows setting the `amount_msats` in the + /// `InvoiceRequest`. The issuer of the Offer will validate this amount and must set a matching + /// amount in the final invoice. If the issuer disagrees with the specfified amount, the invoice + /// request will be rejected. This is also useful if the Offer doesn't specify an amount. + /// /// Successive calls to this method will override the previous setting. /// /// [`quantity`]: Self::quantity From b22a843b868e7822282278143717e31d4c6bf49f Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Wed, 7 Aug 2024 12:48:58 -0600 Subject: [PATCH 11/14] Abort `RefundBuilder` creation on insufficient liquidity Add a liquidity check in `create_refund_builder` to ensure that refund creation is aborted if there is insufficient outbound liquidity. This change prevents attempts to create a refund when the available liquidity is below the required amount, ensuring that refunds are only created when there is adequate liquidity. --- lightning/src/ln/channelmanager.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 92eee22105f..cfd8590a68d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9231,6 +9231,27 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { .and_then(|paths| paths.into_iter().next().ok_or(())) .map_err(|()| Bolt12RequestError::BlindedPathCreationFailed)?; + let total_liquidity: u64 = { + let per_peer_state = &$self.per_peer_state.read().unwrap(); + let mut sum: u64 = 0; + + per_peer_state.iter().for_each(|(_, peer_state_mutex)| { + let peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &*peer_state_lock; + + sum += peer_state.channel_by_id + .iter() + .map(|(_, phase)| phase.context().get_available_balances(&$self.fee_estimator).outbound_capacity_msat) + .sum::(); + }); + sum + }; + + if amount_msats > total_liquidity { + log_error!($self.logger, "Insufficient liquidity for payment with payment id: {}", payment_id); + return Err(Bolt12RequestError::InsufficientLiquidity); + } + let builder = RefundBuilder::deriving_signing_pubkey( node_id, expanded_key, nonce, secp_ctx, amount_msats, payment_id )? From 7b465f919632a1138b93ff7520d9a0b75e121d3d Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Sat, 24 Aug 2024 13:36:55 -0600 Subject: [PATCH 12/14] Add test for refund creation with insufficient liquidity Adds the `fails_creating_refund_with_insufficient_liquidity` test to verify that the `create_refund_builder` method correctly handles cases where there is insufficient channel liquidity. The test verifies that the method returns the `Bolt12RequestError::InsufficientLiquidity` error when the refund amount exceeds the available liquidity. --- lightning/src/ln/offers_tests.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 533488d73d9..d68392ec70a 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -2369,4 +2369,26 @@ fn fails_paying_offer_with_insufficient_liquidity() { assert_eq!(e, Bolt12RequestError::InsufficientLiquidity); } } +} + +#[test] +fn fails_creating_refund_with_insufficient_liquidity() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + + let bob = &nodes[1]; + + let absolute_expiry = Duration::from_secs(u64::MAX); + let payment_id = PaymentId([1; 32]); + let refund = bob.node + .create_refund_builder(950_000_000, absolute_expiry, payment_id, Retry::Attempts(0), None); + + match refund { + Ok(_) => panic!("Expected error with insufficient liquidity."), + Err(e) => assert_eq!(e, Bolt12RequestError::InsufficientLiquidity), + } } \ No newline at end of file From f15db05d4d29f08e6f7a7cf403b8dc30a0dc98cc Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Thu, 8 Aug 2024 07:50:03 -0600 Subject: [PATCH 13/14] Validate amount_msats against invoice and refund amounts Add a check to ensure that the amount_msats in an invoice matches the amount_msats specified in the invoice_request or refund. Reject the invoice as invalid if there is a mismatch between these amounts. This validation ensures consistency in invoice handling. --- lightning/src/offers/invoice.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 648c0fba651..966b31e2684 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -1381,11 +1381,23 @@ impl TryFrom for InvoiceContents { let refund = RefundContents::try_from( (payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream) )?; + + if amount_msats != refund.amount_msats() { + return Err(Bolt12SemanticError::InvalidAmount); + } + Ok(InvoiceContents::ForRefund { refund, fields }) } else { let invoice_request = InvoiceRequestContents::try_from( (payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream) )?; + + if let Some(requested_amount_msats) = invoice_request.amount_msats() { + if amount_msats != requested_amount_msats { + return Err(Bolt12SemanticError::InvalidAmount); + } + } + Ok(InvoiceContents::ForOffer { invoice_request, fields }) } } From 35e3daae429beeb631efd23ed68727e8cfc27a75 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Fri, 23 Aug 2024 18:38:00 -0600 Subject: [PATCH 14/14] Update docs for `Bolt12SemanticError::InvalidAmount` Added doc update to clarify that `InvalidAmount` now also covers cases where the amount in an invoice does not match the expected amount specified in the associated `InvoiceRequest` or `Refund`. --- lightning/src/offers/parse.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index 6aaa24a6f78..3be98108cbc 100644 --- a/lightning/src/offers/parse.rs +++ b/lightning/src/offers/parse.rs @@ -147,6 +147,9 @@ pub enum Bolt12SemanticError { /// An amount was expected but was missing. MissingAmount, /// The amount exceeded the total bitcoin supply. + /// + /// This error can also occur if the amount in an invoice does not match the expected + /// amount specified in the associated `InvoiceRequest` or `Refund`. InvalidAmount, /// An amount was provided but was not sufficient in value. InsufficientAmount,