← Watch feed
Refactor: Introduce `get_payment_info` closure for invoice creation
Public commit record
97/100 · Strong
What the developer wrote
Authored by shaavan
Refactor: Introduce `get_payment_info` closure for invoice creation
To ensure correct Bolt12 payment flow behavior, the `amount_msats`
used for generating the `payment_hash`, `payment_secret`,
and payment path must remain consistent. Previously, these steps
could inadvertently diverge due to separate sources of `amount_msats`.
This commit refactors the interface to use a `get_payment_info` closure,
which captures the required variables and provides a single source of
truth for both payment info (payment_hash, payment_secret) and path
generation. This ensures consistency and eliminates subtle bugs
that could arise from mismatched amounts across the flow.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This commit is a code cleanup (refactor) in a Bitcoin Lightning Network library. It changes how invoice creation gets its payment details so that the amount used to build the payment hash, secret, and payment path all come from one place instead of potentially different places. The commit message says this prevents subtle bugs from mismatched amounts, but the actual code change does not appear to fix any currently reachable bug by itself. It is a defensive improvement rather than a clear security fix.
Recommended action
Treat as a normal defensive refactor. Reviewers may want to confirm that all call sites now use the closure and that no alternate code path still passes a separately-derived amount. No urgent security response is indicated by the available evidence.
Security signals we found
01
Refactor to ensure amount_msats used for payment_hash/payment_secret and blinded path generation is consistent
02
Commit message describes prevention of 'subtle bugs' from mismatched amounts
03
No new input validation, bounds checks, or cryptographic hardening added
04
No CVE, advisory, researcher credit, or vendor security disclosure present in commit or references
Evidence from the diff
The patch introduces a get_payment_info closure passed into create_invoice_builder_from_refund, create_invoice_builder_from_invoice_request_with_keys, and create_invoice_builder_from_invoice_request_without_keys. Previously, callers computed amount_msats and relative_expiry separately, called create_inbound_payment to get (payment_hash, payment_secret), then passed those values plus amount_msats into the builder/path functions. Now the builder/path functions compute amount_msats and relative_expiry internally and call the closure to obtain (payment_hash, payment_secret) from the same values. This centralizes the amount used for payment info and blinded path creation. The diff shows no change to validation rules, no new error checks, and no change to cryptographic operations; it is purely a refactor to enforce single-source-of-truth.
Changed components
lightning/src/ln/channelmanager.rslightning/src/offers/flow.rsBolt12 invoice creation for offers and refundsInspect captured patch +47 / −54
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 7f6e963..8b2bc1f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -92,10 +92,7 @@ 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, UnsignedBolt12Invoice,
- DEFAULT_RELATIVE_EXPIRY,
-};
+use crate::offers::invoice::{Bolt12Invoice, UnsignedBolt12Invoice};
use crate::offers::invoice_error::InvoiceError;
use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestVerifiedFromOffer};
use crate::offers::nonce::Nonce;
@@ -12737,27 +12734,24 @@ where
) -> 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 _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 entropy = &*self.entropy_source;
+ 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),
- }
+ self.flow.enqueue_invoice(invoice.clone(), refund, self.get_peers_for_blinded_path())?;
+ Ok(invoice)
}
/// Pays for an [`Offer`] looked up using [BIP 353] Human Readable Names resolved by the DNS
@@ -14876,22 +14870,12 @@ where
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 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 get_payment_info = |amount_msats, relative_expiry| {
+ self.create_inbound_payment(
+ Some(amount_msats),
+ relative_expiry,
+ None
+ ).map_err(|_| Bolt12SemanticError::InvalidAmount)
};
let (result, context) = match invoice_request {
@@ -14900,10 +14884,8 @@ where
&self.router,
&*self.entropy_source,
&request,
- amount_msats,
- payment_hash,
- payment_secret,
self.list_usable_channels(),
+ get_payment_info,
);
match result {
@@ -14927,10 +14909,8 @@ where
&self.router,
&*self.entropy_source,
&request,
- amount_msats,
- payment_hash,
- payment_secret,
self.list_usable_channels(),
+ get_payment_info,
);
match result {
diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs
index 417a13c..74e5f02 100644
--- a/lightning/src/offers/flow.rs
+++ b/lightning/src/offers/flow.rs
@@ -884,13 +884,14 @@ where
///
/// Returns an error if the refund targets a different chain or if no valid
/// blinded path can be constructed.
- pub fn create_invoice_builder_from_refund<'a, ES: Deref, R: Deref>(
- &'a self, router: &R, entropy_source: ES, refund: &'a Refund, payment_hash: PaymentHash,
- payment_secret: PaymentSecret, usable_channels: Vec<ChannelDetails>,
+ pub fn create_invoice_builder_from_refund<'a, ES: Deref, R: Deref, F>(
+ &'a self, router: &R, entropy_source: ES, refund: &'a Refund,
+ usable_channels: Vec<ChannelDetails>, get_payment_info: F,
) -> Result<InvoiceBuilder<'a, DerivedSigningPubkey>, Bolt12SemanticError>
where
ES::Target: EntropySource,
R::Target: Router,
+ F: Fn(u64, u32) -> Result<(PaymentHash, PaymentSecret), Bolt12SemanticError>,
{
if refund.chain() != self.chain_hash {
return Err(Bolt12SemanticError::UnsupportedChain);
@@ -902,6 +903,8 @@ where
let amount_msats = refund.amount_msats();
let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32;
+ let (payment_hash, payment_secret) = get_payment_info(amount_msats, relative_expiry)?;
+
let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext {});
let payment_paths = self
.create_blinded_payment_paths(
@@ -951,20 +954,25 @@ where
/// Returns a [`Bolt12SemanticError`] if:
/// - Valid blinded payment paths could not be generated for the [`Bolt12Invoice`].
/// - The [`InvoiceBuilder`] could not be created from the [`InvoiceRequest`].
- pub fn create_invoice_builder_from_invoice_request_with_keys<'a, ES: Deref, R: Deref>(
+ pub fn create_invoice_builder_from_invoice_request_with_keys<'a, ES: Deref, R: Deref, F>(
&self, router: &R, entropy_source: ES,
- invoice_request: &'a VerifiedInvoiceRequest<DerivedSigningPubkey>, amount_msats: u64,
- payment_hash: PaymentHash, payment_secret: PaymentSecret,
- usable_channels: Vec<ChannelDetails>,
+ invoice_request: &'a VerifiedInvoiceRequest<DerivedSigningPubkey>,
+ usable_channels: Vec<ChannelDetails>, get_payment_info: F,
) -> Result<(InvoiceBuilder<'a, DerivedSigningPubkey>, MessageContext), Bolt12SemanticError>
where
ES::Target: EntropySource,
R::Target: Router,
+ F: Fn(u64, u32) -> Result<(PaymentHash, PaymentSecret), Bolt12SemanticError>,
{
let entropy = &*entropy_source;
let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32;
+ let amount_msats =
+ InvoiceBuilder::<DerivedSigningPubkey>::amount_msats(&invoice_request.inner)?;
+
+ let (payment_hash, payment_secret) = get_payment_info(amount_msats, relative_expiry)?;
+
let context = PaymentContext::Bolt12Offer(Bolt12OfferContext {
offer_id: invoice_request.offer_id,
invoice_request: invoice_request.fields(),
@@ -1011,19 +1019,24 @@ where
/// Returns a [`Bolt12SemanticError`] if:
/// - Valid blinded payment paths could not be generated for the [`Bolt12Invoice`].
/// - The [`InvoiceBuilder`] could not be created from the [`InvoiceRequest`].
- pub fn create_invoice_builder_from_invoice_request_without_keys<'a, ES: Deref, R: Deref>(
+ pub fn create_invoice_builder_from_invoice_request_without_keys<'a, ES: Deref, R: Deref, F>(
&self, router: &R, entropy_source: ES,
- invoice_request: &'a VerifiedInvoiceRequest<ExplicitSigningPubkey>, amount_msats: u64,
- payment_hash: PaymentHash, payment_secret: PaymentSecret,
- usable_channels: Vec<ChannelDetails>,
+ invoice_request: &'a VerifiedInvoiceRequest<ExplicitSigningPubkey>,
+ usable_channels: Vec<ChannelDetails>, get_payment_info: F,
) -> Result<(InvoiceBuilder<'a, ExplicitSigningPubkey>, MessageContext), Bolt12SemanticError>
where
ES::Target: EntropySource,
R::Target: Router,
+ F: Fn(u64, u32) -> Result<(PaymentHash, PaymentSecret), Bolt12SemanticError>,
{
let entropy = &*entropy_source;
let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32;
+ let amount_msats =
+ InvoiceBuilder::<DerivedSigningPubkey>::amount_msats(&invoice_request.inner)?;
+
+ let (payment_hash, payment_secret) = get_payment_info(amount_msats, relative_expiry)?;
+
let context = PaymentContext::Bolt12Offer(Bolt12OfferContext {
offer_id: invoice_request.offer_id,
invoice_request: invoice_request.fields(),
Risk score
Our methodology →Why this scored 31/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.