From 13c3c06e652aa30f007b46501f09320d30925413 Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Thu, 18 Jul 2024 13:05:13 -0600 Subject: [PATCH] Refactor amount checks and moved currency check Modified check_amount_msats_for_quantity to trust the user-provided msats amount and moved the currency check to the InvoiceRequest parsing code. Updated fails_creating_or_paying_for_offer_without_connected_peers and fails_creating_invoice_request_without_blinded_reply_path offers tests to use fiat amounts, bypassing insufficient liquidity errors. --- lightning/src/ln/offers_tests.rs | 16 +++++++++------- lightning/src/offers/invoice_request.rs | 12 ++++++++++-- lightning/src/offers/offer.rs | 10 +++------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 88ab3794715..1f33a9a2460 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -54,6 +54,7 @@ use crate::offers::invoice::Bolt12Invoice; use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields}; use crate::offers::parse::Bolt12SemanticError; +use crate::offers::offer::{Amount}; use crate::onion_message::messenger::PeeledOnion; use crate::onion_message::offers::OffersMessage; use crate::onion_message::packet::ParsedOnionMessageContents; @@ -1267,15 +1268,15 @@ fn fails_creating_or_paying_for_offer_without_connected_peers() { reconnect_nodes(args); let offer = alice.node - .create_offer_builder(Some(absolute_expiry)).unwrap() - .amount_msats(10_000_000) - .build().unwrap(); + .create_offer_builder(None).unwrap() + .amount(Amount::Currency {iso4217_code: *b"USD", amount: 6_000}) + .build_unchecked(); let payment_id = PaymentId([1; 32]); 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::InsufficientLiquidity), + Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), } assert!(nodes[0].node.list_recent_payments().is_empty()); @@ -1431,14 +1432,15 @@ fn fails_creating_invoice_request_without_blinded_reply_path() { let offer = alice.node .create_offer_builder(None).unwrap() - .amount_msats(10_000_000) - .build().unwrap(); + .amount(Amount::Currency {iso4217_code: *b"USD", amount: 6_000}) + .build_unchecked(); + let payment_id = PaymentId([1; 32]); 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::InsufficientLiquidity), + Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), } assert!(nodes[0].node.list_recent_payments().is_empty()); diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index cdf94a2e80d..70eb9401cf4 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -72,7 +72,7 @@ use crate::ln::inbound_payment::{ExpandedKey, IV_LEN, Nonce}; use crate::ln::msgs::DecodeError; use crate::offers::invoice::BlindedPayInfo; use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, self}; -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}; @@ -1133,6 +1133,14 @@ impl TryFrom for InvoiceRequestContents { offer.check_quantity(quantity)?; offer.check_amount_msats_for_quantity(amount, quantity)?; + let amount_msats = match offer.amount() { + None => amount, + Some(Amount::Bitcoin { amount_msats }) => Some(amount_msats), + Some(Amount::Currency { iso4217_code: _ , amount: _ }) => { + return Err(Bolt12SemanticError::UnsupportedCurrency) + }, + }; + let features = features.unwrap_or_else(InvoiceRequestFeatures::empty); let payer_id = match payer_id { @@ -1146,7 +1154,7 @@ impl TryFrom for InvoiceRequestContents { Ok(InvoiceRequestContents { inner: InvoiceRequestContentsWithoutPayerId { - payer, offer, chain, amount_msats: amount, features, quantity, payer_note, + payer, offer, chain, amount_msats, features, quantity, payer_note, }, payer_id, }) diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 253de8652bb..85635966ea0 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -309,7 +309,7 @@ macro_rules! offer_builder_methods { ( /// Sets the [`Offer::amount`]. /// /// Successive calls to this method will override the previous setting. - pub(super) fn amount($($self_mut)* $self: $self_type, amount: Amount) -> $return_type { + pub(crate) fn amount($($self_mut)* $self: $self_type, amount: Amount) -> $return_type { $self.offer.amount = Some(amount); $return_value } @@ -453,7 +453,7 @@ macro_rules! offer_builder_test_methods { ( } #[cfg_attr(c_bindings, allow(dead_code))] - pub(super) fn build_unchecked($self: $self_type) -> Offer { + pub(crate) fn build_unchecked($self: $self_type) -> Offer { $self.build_without_checks() } } } @@ -854,11 +854,7 @@ impl OfferContents { pub(super) fn check_amount_msats_for_quantity( &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), - }; + let offer_amount_msats = amount_msats.unwrap_or(0); if !self.expects_quantity() || quantity.is_some() { let expected_amount_msats = offer_amount_msats.checked_mul(quantity.unwrap_or(1))