Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
75 changes: 75 additions & 0 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,33 @@ impl Default for OptionalBolt11PaymentParams {
}
}

/// Optional arguments to [`ChannelManager::pay_for_bolt12_invoice`].
///
/// These fields will often not need to be set, and the provided [`Self::default`] can be used.
pub struct OptionalBolt12PaymentParams {
/// Pathfinding options which tweak how the path is constructed to the recipient.
pub route_params_config: RouteParametersConfig,
/// The number of tries or time during which we'll retry this payment if some paths to the
/// recipient fail.
///
/// Once the retry limit is reached, further path failures will not be retried and the payment
/// will ultimately fail once all pending paths have failed (generating an
/// [`Event::PaymentFailed`]).
pub retry_strategy: Retry,
}

impl Default for OptionalBolt12PaymentParams {
fn default() -> Self {
Self {
route_params_config: Default::default(),
#[cfg(feature = "std")]
retry_strategy: Retry::Timeout(core::time::Duration::from_secs(2)),
#[cfg(not(feature = "std"))]
retry_strategy: Retry::Attempts(3),
}
}
}

/// Optional arguments to [`ChannelManager::pay_for_offer`].
///
/// These fields will often not need to be set, and the provided [`Self::default`] can be used.
Expand Down Expand Up @@ -5857,6 +5884,50 @@ impl<
)
}

/// Pays a [`Bolt12Invoice`] without requiring it to have been requested through LDK.
///
/// Unlike [`ChannelManager::send_payment_for_bolt12_invoice`], this method does not verify
/// that the invoice was previously requested. The caller is responsible for invoice
/// verification and for providing a unique `payment_id`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are users supposed to call the verify method themselves? If so we should link to it and describe under what circumstances they'll want to call it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I've added links to verify_using_metadata and verify_using_payer_data, and clarified that callers should verify using the Nonce and ExpandedKey from their own InvoiceRequest.

///
/// `amount_msats` controls how much this node contributes to the payment. Set to `None` to pay

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be in the optional params.

/// the full invoice amount. For payments split across multiple senders, provide a partial
/// `amount_msats` here — the onion total is always set to the full invoice amount so that the
/// recipient can correctly validate the payment.
///
/// Returns [`Bolt12PaymentError::DuplicateInvoice`] if a payment with the given `payment_id`
/// is already pending, or [`Bolt12PaymentError::InvalidAmount`] if `amount_msats` exceeds the
/// invoice amount.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same doc inaccuracy as on the InvalidAmount variant: should also mention that zero amount_msats is rejected.

Suggested change
/// Returns [`Bolt12PaymentError::DuplicateInvoice`] if a payment with the given `payment_id`
/// is already pending, or [`Bolt12PaymentError::InvalidAmount`] if `amount_msats` exceeds the
/// invoice amount.
/// Returns [`Bolt12PaymentError::DuplicateInvoice`] if a payment with the given `payment_id`
/// is already pending, or [`Bolt12PaymentError::InvalidAmount`] if `amount_msats` is zero or
/// exceeds the invoice amount.

///
/// Either [`Event::PaymentSent`] or [`Event::PaymentFailed`] will be generated once the
/// payment completes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We previously documented how retries work in the deprecated method, may want something equivalent here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also added. The docs now explain that failures follow the configured retry_strategy, and that once a payment is abandoned, any further attempt must use a new payment_id.

pub fn pay_for_bolt12_invoice(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's at least deprecate send_payment_for_bolt12_invoice in the same PR.

&self, invoice: &Bolt12Invoice, payment_id: PaymentId, amount_msats: Option<u64>,
optional_params: OptionalBolt12PaymentParams,
) -> Result<(), Bolt12PaymentError> {
let best_block_height = self.best_block.read().unwrap().height;
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let features = self.bolt12_invoice_features();
self.pending_outbound_payments.pay_for_bolt12_invoice(
invoice,
payment_id,
amount_msats,
optional_params,
&self.router,
self.list_usable_channels(),
features,
|| self.compute_inflight_htlcs(),
&self.entropy_source,
&self.node_signer,
&self,
&self.secp_ctx,
best_block_height,
&self.pending_events,
|args| self.send_payment_along_path(args),
&WithContext::for_payment(&self.logger, None, None, None, payment_id),
)
}

fn check_refresh_async_receive_offer_cache(&self, timer_tick_occurred: bool) {
let peers = self.get_peers_for_blinded_path();
let channels = self.list_usable_channels();
Expand Down Expand Up @@ -17059,6 +17130,10 @@ impl<
log_trace!($logger, "{}", err_msg);
InvoiceError::from_string(err_msg.to_string())
},
Err(Bolt12PaymentError::InvalidAmount) => {
log_error!($logger, "Got InvalidAmount paying internally-sourced invoice; this shouldn't happen");
return None
},
Comment thread
valentinewallace marked this conversation as resolved.
Err(Bolt12PaymentError::UnexpectedInvoice)
| Err(Bolt12PaymentError::DuplicateInvoice)
| Ok(()) => return None,
Expand Down
141 changes: 141 additions & 0 deletions lightning/src/ln/offers_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2667,3 +2667,144 @@ fn creates_and_pays_for_phantom_offer() {
assert!(nodes[0].onion_messenger.next_onion_message_for_peer(node_c_id).is_none());
}
}

/// Checks that a BOLT 12 invoice can be paid via [`ChannelManager::pay_for_bolt12_invoice`]
/// without requiring a prior LDK-managed payment request.
#[test]
fn pay_for_bolt12_invoice_with_fresh_payment_id() {
let mut manually_pay_cfg = test_default_channel_config();
manually_pay_cfg.manually_handle_bolt12_invoices = true;

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, Some(manually_pay_cfg)]);
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 alice = &nodes[0];
let alice_id = alice.node.get_our_node_id();
let bob = &nodes[1];
let bob_id = bob.node.get_our_node_id();

let offer = alice.node
.create_offer_builder().unwrap()
.amount_msats(10_000_000)
.build().unwrap();

// Use the standard offer flow to obtain an invoice, but pay it via the new API with a
// fresh payment_id rather than the one from the original request.
let orig_payment_id = PaymentId([1; 32]);
bob.node.pay_for_offer(&offer, None, orig_payment_id, Default::default()).unwrap();

let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap();
alice.onion_messenger.handle_onion_message(bob_id, &onion_message);

let (invoice_request, _) = extract_invoice_request(alice, &onion_message);
let payment_context = PaymentContext::Bolt12Offer(Bolt12OfferContext {
offer_id: offer.id(),
invoice_request: InvoiceRequestFields {
payer_signing_pubkey: invoice_request.payer_signing_pubkey(),
quantity: None,
payer_note_truncated: None,
human_readable_name: None,
},
});

let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
bob.onion_messenger.handle_onion_message(alice_id, &onion_message);

let invoice = match get_event!(bob, Event::InvoiceReceived) {
Event::InvoiceReceived { invoice, .. } => invoice,
_ => panic!("Expected InvoiceReceived"),
};

// Abandon the original payment since we're paying via a fresh payment_id below.
bob.node.abandon_payment(orig_payment_id);
get_event!(bob, Event::PaymentFailed);

let payment_id = PaymentId([2; 32]);
bob.node.pay_for_bolt12_invoice(&invoice, payment_id, None, Default::default()).unwrap();
expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id);

route_bolt12_payment(bob, &[alice], &invoice);
claim_bolt12_payment(bob, &[alice], payment_context, &invoice);
expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id);
}
Comment on lines +2672 to +2734

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Missing test coverage: Neither this test nor pay_for_bolt12_invoice_error_cases actually exercises the partial payment path (amount_msats = Some(partial_amount)) end-to-end. This is the primary feature of this PR — enabling multi-sender split payments where each node pays a portion of the invoice.

A test should verify that paying with e.g. Some(invoice.amount_msats()) (or a true partial amount in a multi-node setup) correctly sets total_mpp_amount_msat in the onion to the full invoice amount while routing only the partial amount. Without this, the total_mpp_amount_msat_override plumbing through send_payment_for_bolt12_invoice_internal is untested.


/// Checks error cases for [`ChannelManager::pay_for_bolt12_invoice`]:
/// overpaying returns [`Bolt12PaymentError::InvalidAmount`] and re-using a payment_id
/// returns [`Bolt12PaymentError::DuplicateInvoice`].
#[test]
fn pay_for_bolt12_invoice_error_cases() {
let mut manually_pay_cfg = test_default_channel_config();
manually_pay_cfg.manually_handle_bolt12_invoices = true;

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, Some(manually_pay_cfg)]);
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 alice = &nodes[0];
let alice_id = alice.node.get_our_node_id();
let bob = &nodes[1];
let bob_id = bob.node.get_our_node_id();

let offer = alice.node
.create_offer_builder().unwrap()
.amount_msats(10_000_000)
.build().unwrap();

let orig_payment_id = PaymentId([1; 32]);
bob.node.pay_for_offer(&offer, None, orig_payment_id, Default::default()).unwrap();

let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap();
alice.onion_messenger.handle_onion_message(bob_id, &onion_message);

let (invoice_request, _) = extract_invoice_request(alice, &onion_message);
let payment_context = PaymentContext::Bolt12Offer(Bolt12OfferContext {
offer_id: offer.id(),
invoice_request: InvoiceRequestFields {
payer_signing_pubkey: invoice_request.payer_signing_pubkey(),
quantity: None,
payer_note_truncated: None,
human_readable_name: None,
},
});

let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
bob.onion_messenger.handle_onion_message(alice_id, &onion_message);

let invoice = match get_event!(bob, Event::InvoiceReceived) {
Event::InvoiceReceived { invoice, .. } => invoice,
_ => panic!("Expected InvoiceReceived"),
};

bob.node.abandon_payment(orig_payment_id);
get_event!(bob, Event::PaymentFailed);

let payment_id = PaymentId([2; 32]);

// Overpaying is rejected before any state is inserted.
assert_eq!(
bob.node.pay_for_bolt12_invoice(
&invoice, payment_id, Some(invoice.amount_msats() + 1), Default::default()
),
Err(Bolt12PaymentError::InvalidAmount),
);

// First call succeeds and starts the payment.
bob.node.pay_for_bolt12_invoice(&invoice, payment_id, None, Default::default()).unwrap();

// Re-using the same payment_id is rejected.
assert_eq!(
bob.node.pay_for_bolt12_invoice(&invoice, payment_id, None, Default::default()),
Err(Bolt12PaymentError::DuplicateInvoice),
);

route_bolt12_payment(bob, &[alice], &invoice);
claim_bolt12_payment(bob, &[alice], payment_context, &invoice);
expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id);
}
81 changes: 75 additions & 6 deletions lightning/src/ln/outbound_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ use crate::blinded_path::{IntroductionNode, NodeIdLookUp};
use crate::events::{self, PaidBolt12Invoice, PaymentFailureReason};
use crate::ln::channel_state::ChannelDetails;
use crate::ln::channelmanager::{
EventCompletionAction, HTLCSource, OptionalBolt11PaymentParams, PaymentCompleteUpdate,
PaymentId,
EventCompletionAction, HTLCSource, OptionalBolt11PaymentParams, OptionalBolt12PaymentParams,
PaymentCompleteUpdate, PaymentId,
};
use crate::ln::msgs::DecodeError;
use crate::ln::onion_utils;
Expand Down Expand Up @@ -657,6 +657,12 @@ pub enum Bolt12PaymentError {
DuplicateInvoice,
/// The invoice was valid for the corresponding [`PaymentId`], but required unknown features.
UnknownRequiredFeatures,
/// Incorrect amount was provided to [`ChannelManager::pay_for_bolt12_invoice`].
///
/// This occurs when `amount_msats` exceeds the invoice amount.
///
/// [`ChannelManager::pay_for_bolt12_invoice`]: crate::ln::channelmanager::ChannelManager::pay_for_bolt12_invoice
InvalidAmount,
Comment on lines +660 to +665

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: The doc says "This occurs when amount_msats exceeds the invoice amount" but the actual validation (send_amount == 0 || send_amount > invoice_amount) also rejects zero amounts. The same incomplete description appears in the pay_for_bolt12_invoice method doc at channelmanager.rs:5899.

Suggested change
/// Incorrect amount was provided to [`ChannelManager::pay_for_bolt12_invoice`].
///
/// This occurs when `amount_msats` exceeds the invoice amount.
///
/// [`ChannelManager::pay_for_bolt12_invoice`]: crate::ln::channelmanager::ChannelManager::pay_for_bolt12_invoice
InvalidAmount,
/// Incorrect amount was provided to [`ChannelManager::pay_for_bolt12_invoice`].
///
/// This occurs when `amount_msats` is zero or exceeds the invoice amount.
///
/// [`ChannelManager::pay_for_bolt12_invoice`]: crate::ln::channelmanager::ChannelManager::pay_for_bolt12_invoice
InvalidAmount,

/// The invoice was valid for the corresponding [`PaymentId`], but sending the payment failed.
SendingFailed(RetryableSendFailure),
/// Failed to create a blinded path back to ourselves.
Expand Down Expand Up @@ -1124,20 +1130,82 @@ impl OutboundPayments {
}
let invoice = PaidBolt12Invoice::Bolt12Invoice(invoice.clone());
self.send_payment_for_bolt12_invoice_internal(
payment_id, payment_hash, None, None, invoice, route_params, retry_strategy, false, router,
first_hops, inflight_htlcs, entropy_source, node_signer, node_id_lookup, secp_ctx,
payment_id, payment_hash, None, None, invoice, route_params, retry_strategy, false, None,
router, first_hops, inflight_htlcs, entropy_source, node_signer, node_id_lookup, secp_ctx,
best_block_height, pending_events, send_payment_along_path, logger,
)
}

#[rustfmt::skip]
Comment thread
Alkamal01 marked this conversation as resolved.
Outdated
pub(super) fn pay_for_bolt12_invoice<
R: Router, ES: EntropySource, NS: NodeSigner, NL: NodeIdLookUp, IH, SP, L: Logger,
>(
&self, invoice: &Bolt12Invoice, payment_id: PaymentId, amount_msats: Option<u64>,
optional_params: OptionalBolt12PaymentParams,
router: &R, first_hops: Vec<ChannelDetails>, features: Bolt12InvoiceFeatures, inflight_htlcs: IH,
entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL,
secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32,
pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
send_payment_along_path: SP, logger: &WithContext<L>,
) -> Result<(), Bolt12PaymentError>
where
IH: Fn() -> InFlightHtlcs,
SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
{
let OptionalBolt12PaymentParams { retry_strategy, route_params_config } = optional_params;

let invoice_amount = invoice.amount_msats();
let send_amount = amount_msats.unwrap_or(invoice_amount);

if send_amount > invoice_amount {
return Err(Bolt12PaymentError::InvalidAmount);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bug: amount_msats = Some(0) passes this check (0 <= invoice_amount), which inserts an InvoiceReceived entry and then proceeds to route finding for 0 msat. Routing will fail, triggering abandon_payment and a PaymentFailed event, but a zero-value payment is never meaningful and should be rejected upfront.

Consider:

Suggested change
if send_amount > invoice_amount {
return Err(Bolt12PaymentError::InvalidAmount);
}
if send_amount == 0 || send_amount > invoice_amount {
return Err(Bolt12PaymentError::InvalidAmount);
}


if invoice.invoice_features().requires_unknown_bits_from(&features) {
return Err(Bolt12PaymentError::UnknownRequiredFeatures);
}

let payment_hash = invoice.payment_hash();

match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
hash_map::Entry::Occupied(_) => return Err(Bolt12PaymentError::DuplicateInvoice),
hash_map::Entry::Vacant(entry) => {
entry.insert(PendingOutboundPayment::InvoiceReceived {
payment_hash,
retry_strategy,
route_params_config,
});
},
}

let mut route_params = RouteParameters::from_payment_params_and_value(
PaymentParameters::from_bolt12_invoice(invoice)
.with_user_config_ignoring_fee_limit(route_params_config),
send_amount,
);
if let Some(max_fee_msat) = route_params_config.max_total_routing_fee_msat {
route_params.max_total_routing_fee_msat = Some(max_fee_msat);
}
// The onion total must always reflect the full invoice amount so that the recipient can
// correctly validate MPP payments, including when this node pays only a partial amount.
let invoice = PaidBolt12Invoice::Bolt12Invoice(invoice.clone());
self.send_payment_for_bolt12_invoice_internal(
payment_id, payment_hash, None, None, invoice, route_params, retry_strategy,
false, Some(invoice_amount), router, first_hops, inflight_htlcs,
entropy_source, node_signer, node_id_lookup, secp_ctx, best_block_height,
pending_events, send_payment_along_path, logger,
)
}

#[rustfmt::skip]
fn send_payment_for_bolt12_invoice_internal<
R: Router, ES: EntropySource, NS: NodeSigner, NL: NodeIdLookUp, IH, SP, L: Logger,
>(
&self, payment_id: PaymentId, payment_hash: PaymentHash,
keysend_preimage: Option<PaymentPreimage>, invoice_request: Option<&InvoiceRequest>,
bolt12_invoice: PaidBolt12Invoice,
mut route_params: RouteParameters, retry_strategy: Retry, hold_htlcs_at_next_hop: bool, router: &R,
mut route_params: RouteParameters, retry_strategy: Retry, hold_htlcs_at_next_hop: bool,
total_mpp_amount_msat_override: Option<u64>, router: &R,
first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS,
node_id_lookup: &NL, secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32,
pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
Expand Down Expand Up @@ -1169,7 +1237,7 @@ impl OutboundPayments {
payment_secret: None,
payment_metadata: None,
custom_tlvs: vec![],
total_mpp_amount_msat: route_params.final_value_msat,
total_mpp_amount_msat: total_mpp_amount_msat_override.unwrap_or(route_params.final_value_msat),
};
let route = match self.find_initial_route(
payment_id, payment_hash, &recipient_onion, keysend_preimage, invoice_request,
Expand Down Expand Up @@ -1405,6 +1473,7 @@ impl OutboundPayments {
route_params,
retry_strategy,
hold_htlcs_at_next_hop,
None,
router,
first_hops,
inflight_htlcs,
Expand Down
Loading