Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions fuzz/src/invoice_request_deser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ use lightning::blinded_path::payment::{
use lightning::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA;
use lightning::ln::inbound_payment::ExpandedKey;
use lightning::offers::invoice::UnsignedBolt12Invoice;
use lightning::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields};
use lightning::offers::invoice_request::{
CurrencyConversion, InvoiceRequest, InvoiceRequestFields,
};
use lightning::offers::nonce::Nonce;
use lightning::offers::offer::OfferId;
use lightning::offers::offer::{CurrencyCode, OfferId};
use lightning::offers::parse::Bolt12SemanticError;
use lightning::sign::EntropySource;
use lightning::types::features::BlindedHopFeatures;
Expand Down Expand Up @@ -79,6 +81,14 @@ fn privkey(byte: u8) -> SecretKey {
SecretKey::from_slice(&[byte; 32]).unwrap()
}

struct FuzzCurrencyConversion;

impl CurrencyConversion for FuzzCurrencyConversion {
fn fiat_to_msats(&self, _iso4217_code: CurrencyCode) -> Result<u64, Bolt12SemanticError> {
unreachable!()
}
}

fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
invoice_request: &InvoiceRequest, secp_ctx: &Secp256k1<T>,
) -> Result<UnsignedBolt12Invoice, Bolt12SemanticError> {
Expand Down Expand Up @@ -145,7 +155,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
.unwrap();

let payment_hash = PaymentHash([42; 32]);
invoice_request.respond_with(vec![payment_path], payment_hash)?.build()
invoice_request.respond_with(&FuzzCurrencyConversion, vec![payment_path], payment_hash)?.build()
}

pub fn invoice_request_deser_test<Out: test_logger::Output>(data: &[u8], out: Out) {
Expand Down
177 changes: 127 additions & 50 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ use crate::ln::outbound_payment::{
use crate::ln::types::ChannelId;
use crate::offers::async_receive_offer_cache::AsyncReceiveOfferCache;
use crate::offers::flow::{HeldHtlcReplyPath, InvreqResponseInstructions, OffersMessageFlow};
use crate::offers::invoice::{
Bolt12Invoice, DerivedSigningPubkey, InvoiceBuilder, DEFAULT_RELATIVE_EXPIRY,
};
use crate::offers::invoice::{Bolt12Invoice, UnsignedBolt12Invoice};
use crate::offers::invoice_error::InvoiceError;
use crate::offers::invoice_request::InvoiceRequest;
use crate::offers::invoice_request::{
DefaultCurrencyConversion, InvoiceRequest, InvoiceRequestVerifiedFromOffer,
};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Offer, OfferFromHrn};
use crate::offers::offer::{Amount, Offer, OfferFromHrn};
use crate::offers::parse::Bolt12SemanticError;
use crate::offers::refund::Refund;
use crate::offers::signer;
Expand Down Expand Up @@ -2670,6 +2670,9 @@ pub struct ChannelManager<
fee_estimator: LowerBoundedFeeEstimator<F>,
chain_monitor: M,
tx_broadcaster: T,
#[cfg(test)]
pub(super) router: R,
#[cfg(not(test))]
router: R,

#[cfg(test)]
Expand Down Expand Up @@ -2896,6 +2899,9 @@ pub struct ChannelManager<
pub(super) entropy_source: ES,
#[cfg(not(test))]
entropy_source: ES,
#[cfg(test)]
pub(super) node_signer: NS,
#[cfg(not(test))]
node_signer: NS,
#[cfg(test)]
pub(super) signer_provider: SP,
Expand Down Expand Up @@ -3900,7 +3906,8 @@ where
let flow = OffersMessageFlow::new(
ChainHash::using_genesis_block(params.network), params.best_block,
our_network_pubkey, current_timestamp, expanded_inbound_key,
node_signer.get_receive_auth_key(), secp_ctx.clone(), message_router, logger.clone(),
node_signer.get_receive_auth_key(), secp_ctx.clone(), message_router, false,
logger.clone(),
);

ChannelManager {
Expand Down Expand Up @@ -5504,6 +5511,7 @@ where
let features = self.bolt12_invoice_features();
let outbound_pmts_res = self.pending_outbound_payments.static_invoice_received(
invoice,
&DefaultCurrencyConversion,
payment_id,
features,
best_block_height,
Expand Down Expand Up @@ -7735,7 +7743,7 @@ where
};
let payment_purpose_context =
PaymentContext::Bolt12Offer(Bolt12OfferContext {
offer_id: verified_invreq.offer_id,
offer_id: verified_invreq.offer_id(),
invoice_request: verified_invreq.fields(),
});
let from_parts_res = events::PaymentPurpose::from_parts(
Expand Down Expand Up @@ -12669,6 +12677,13 @@ where
let entropy = &*self.entropy_source;
let nonce = Nonce::from_entropy_source(entropy);

// If the offer is for a specific currency, ensure the amount is provided.
if let Some(Amount::Currency { iso4217_code: _, amount: _ }) = offer.amount() {
if amount_msats.is_none() {
return Err(Bolt12SemanticError::MissingAmount);
}
}
Comment on lines +12681 to +12685
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic error: The validation checks if amount_msats.is_none() for currency offers, but this validation occurs before the invoice request is created. At this point, amount_msats represents the user-provided amount parameter, not the final computed amount from the invoice request. Currency offers may be valid without an explicit amount if the offer itself specifies the amount. This will incorrectly reject valid currency-based offers.

Suggested change
if let Some(Amount::Currency { iso4217_code: _, amount: _ }) = offer.amount() {
if amount_msats.is_none() {
return Err(Bolt12SemanticError::MissingAmount);
}
}
if let Some(Amount::Currency { iso4217_code: _, amount }) = offer.amount() {
if amount.is_none() && amount_msats.is_none() {
return Err(Bolt12SemanticError::MissingAmount);
}
}

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


let builder = self.flow.create_invoice_request_builder(
offer, nonce, payment_id,
)?;
Expand Down Expand Up @@ -12730,33 +12745,42 @@ where
///
/// [`BlindedPaymentPath`]: crate::blinded_path::payment::BlindedPaymentPath
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
#[rustfmt::skip]
pub fn request_refund_payment(
&self, refund: &Refund
&self, refund: &Refund,
) -> Result<Bolt12Invoice, Bolt12SemanticError> {
let secp_ctx = &self.secp_ctx;

let amount_msats = refund.amount_msats();
let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32;
let entropy = &*self.entropy_source;

let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);

match self.create_inbound_payment(Some(amount_msats), relative_expiry, None) {
Ok((payment_hash, payment_secret)) => {
let entropy = &*self.entropy_source;
let builder = self.flow.create_invoice_builder_from_refund(
&self.router, entropy, refund, payment_hash,
payment_secret, self.list_usable_channels()
)?;

let invoice = builder.allow_mpp().build_and_sign(secp_ctx)?;
let builder = self.flow.create_invoice_builder_from_refund(
&self.router,
entropy,
refund,
self.list_usable_channels(),
|amount_msats, relative_expiry| {
self.create_inbound_payment(Some(amount_msats), relative_expiry, None)
.map_err(|()| Bolt12SemanticError::InvalidAmount)
},
)?;

self.flow.enqueue_invoice(invoice.clone(), refund, self.get_peers_for_blinded_path())?;
let invoice = builder.allow_mpp().build_and_sign(secp_ctx)?;

Ok(invoice)
},
Err(()) => Err(Bolt12SemanticError::InvalidAmount),
if refund.paths().is_empty() {
self.flow.enqueue_invoice_using_node_id(
invoice.clone(),
refund.payer_signing_pubkey(),
self.get_peers_for_blinded_path(),
)?;
} else {
self.flow.enqueue_invoice_using_reply_paths(
invoice.clone(),
refund.paths(),
self.get_peers_for_blinded_path(),
)?;
}

Ok(invoice)
}

/// Pays for an [`Offer`] looked up using [BIP 353] Human Readable Names resolved by the DNS
Expand Down Expand Up @@ -12978,7 +13002,7 @@ where
now
}

fn get_peers_for_blinded_path(&self) -> Vec<MessageForwardNode> {
pub(crate) fn get_peers_for_blinded_path(&self) -> Vec<MessageForwardNode> {
let per_peer_state = self.per_peer_state.read().unwrap();
per_peer_state
.iter()
Expand Down Expand Up @@ -14863,7 +14887,7 @@ where
None => return None,
};

let invoice_request = match self.flow.verify_invoice_request(invoice_request, context) {
let invoice_request = match self.flow.verify_invoice_request(invoice_request, context, responder.clone()) {
Ok(InvreqResponseInstructions::SendInvoice(invoice_request)) => invoice_request,
Ok(InvreqResponseInstructions::SendStaticInvoice { recipient_id, invoice_slot, invoice_request }) => {
self.pending_events.lock().unwrap().push_back((Event::StaticInvoiceRequested {
Expand All @@ -14872,37 +14896,89 @@ where

return None
},
Ok(InvreqResponseInstructions::AsynchronouslyHandleResponse) => return None,
Err(_) => return None,
};

let amount_msats = match InvoiceBuilder::<DerivedSigningPubkey>::amount_msats(
&invoice_request.inner
) {
Ok(amount_msats) => amount_msats,
Err(error) => return Some((OffersMessage::InvoiceError(error.into()), responder.respond())),
let get_payment_info = |amount_msats, relative_expiry| {
self.create_inbound_payment(
Some(amount_msats),
relative_expiry,
None
).map_err(|_| Bolt12SemanticError::InvalidAmount)
};

let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32;
let (payment_hash, payment_secret) = match self.create_inbound_payment(
Some(amount_msats), relative_expiry, None
) {
Ok((payment_hash, payment_secret)) => (payment_hash, payment_secret),
Err(()) => {
let error = Bolt12SemanticError::InvalidAmount;
return Some((OffersMessage::InvoiceError(error.into()), responder.respond()));
let (result, context) = match invoice_request {
InvoiceRequestVerifiedFromOffer::DerivedKeys(request) => {
let result = self.flow.create_invoice_builder_from_invoice_request_with_keys(
&self.router,
&*self.entropy_source,
&DefaultCurrencyConversion,
&request,
self.list_usable_channels(),
get_payment_info,
);

match result {
Ok((builder, context)) => {
let res = builder
.build_and_sign(&self.secp_ctx)
.map_err(InvoiceError::from);

(res, context)
},
Err(error) => {
return Some((
OffersMessage::InvoiceError(InvoiceError::from(error)),
responder.respond(),
));
},
}
},
};
InvoiceRequestVerifiedFromOffer::ExplicitKeys(request) => {
let result = self.flow.create_invoice_builder_from_invoice_request_without_keys(
&self.router,
&*self.entropy_source,
&DefaultCurrencyConversion,
&request,
self.list_usable_channels(),
get_payment_info,
);

let entropy = &*self.entropy_source;
let (response, context) = self.flow.create_response_for_invoice_request(
&self.node_signer, &self.router, entropy, invoice_request, amount_msats,
payment_hash, payment_secret, self.list_usable_channels()
);
match result {
Ok((builder, context)) => {
let res = builder
.build()
.map_err(InvoiceError::from)
.and_then(|invoice| {
#[cfg(c_bindings)]
let mut invoice = invoice;
invoice
.sign(|invoice: &UnsignedBolt12Invoice| self.node_signer.sign_bolt12_invoice(invoice))
.map_err(InvoiceError::from)
});
(res, context)
},
Err(error) => {
return Some((
OffersMessage::InvoiceError(InvoiceError::from(error)),
responder.respond(),
));
},
}
}
};

match context {
Some(context) => Some((response, responder.respond_with_reply_path(context))),
None => Some((response, responder.respond()))
}
Some(match result {
Ok(invoice) => (
OffersMessage::Invoice(invoice),
responder.respond_with_reply_path(context),
),
Err(error) => (
OffersMessage::InvoiceError(error),
responder.respond(),
),
})
},
OffersMessage::Invoice(invoice) => {
let payment_id = match self.flow.verify_bolt12_invoice(&invoice, context.as_ref()) {
Expand Down Expand Up @@ -17589,6 +17665,7 @@ where
args.node_signer.get_receive_auth_key(),
secp_ctx.clone(),
args.message_router,
false,
args.logger.clone(),
)
.with_async_payments_offers_cache(async_receive_offer_cache);
Expand Down
Loading