From 4431b84db8e28f37b9401531631c96f8c8300326 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 4 Feb 2026 10:55:56 +0100 Subject: [PATCH 01/13] Make `payment_id` a required field in `Event`s The switch to tracking payments by ID happened with LDK Node v0.3.0, which is >1.5 years old by now. We can be pretty certain that nobody is upgrading from an older version to the upcoming v0.8. Here we hence make the `payment_id` fields in `Event` required which is a nice API simplification that will also be utilized in the next commit. Co-Authored-By: HAL 9000 --- CHANGELOG.md | 5 ++++ benches/payments.rs | 8 ++---- src/event.rs | 25 +++++++----------- tests/common/mod.rs | 6 ++--- tests/integration_tests_hrn.rs | 2 +- tests/integration_tests_migration.rs | 4 +-- tests/integration_tests_rust.rs | 39 ++++++++++++++-------------- tests/upgrade_downgrade_tests.rs | 5 ++-- 8 files changed, 43 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 115b0ed058..2a1b582a3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ a persisted `ChannelClosed` event. - Users of the VSS storage backend must upgrade their VSS server to at least version `v0.1.0-alpha.0` before upgrading LDK Node. +- The `payment_id` field on the `PaymentSuccessful`, `PaymentFailed`, and + `PaymentReceived` events is now a required (non-optional) `PaymentId`. Events + persisted by LDK Node v0.2.1 or earlier (which stored `payment_id` as + optional) will fail to deserialize on read; users upgrading from those + versions need to drain pending events before the upgrade. ## Feature and API updates - The Bitcoin Core RPC and REST chain-source builder methods now accept an optional diff --git a/benches/payments.rs b/benches/payments.rs index 52769d7949..e4de689763 100644 --- a/benches/payments.rs +++ b/benches/payments.rs @@ -75,12 +75,8 @@ async fn send_payments(node_a: Arc, node_b: Arc) -> std::time::Durat while success_count < total_payments { match node_a.next_event_async().await { Event::PaymentSuccessful { payment_id, payment_hash, .. } => { - if let Some(id) = payment_id { - success_count += 1; - println!("{}: Payment with id {:?} completed", payment_hash.0.as_hex(), id); - } else { - println!("Payment completed (no payment_id)"); - } + success_count += 1; + println!("{}: Payment with id {:?} completed", payment_hash.0.as_hex(), payment_id); }, Event::PaymentFailed { payment_id, payment_hash, .. } => { println!("{}: Payment {:?} failed", payment_hash.unwrap().0.as_hex(), payment_id); diff --git a/src/event.rs b/src/event.rs index 91ab7b27de..7c3b11f311 100644 --- a/src/event.rs +++ b/src/event.rs @@ -105,9 +105,7 @@ pub enum Event { /// A sent payment was successful. PaymentSuccessful { /// A local identifier used to track the payment. - /// - /// Will only be `None` for events serialized with LDK Node v0.2.1 or prior. - payment_id: Option, + payment_id: PaymentId, /// The hash of the payment. payment_hash: PaymentHash, /// The preimage to the `payment_hash`. @@ -133,9 +131,7 @@ pub enum Event { /// A sent payment has failed. PaymentFailed { /// A local identifier used to track the payment. - /// - /// Will only be `None` for events serialized with LDK Node v0.2.1 or prior. - payment_id: Option, + payment_id: PaymentId, /// The hash of the payment. /// /// This will be `None` if the payment failed before receiving an invoice when paying a @@ -151,9 +147,7 @@ pub enum Event { /// A payment has been received. PaymentReceived { /// A local identifier used to track the payment. - /// - /// Will only be `None` for events serialized with LDK Node v0.2.1 or prior. - payment_id: Option, + payment_id: PaymentId, /// The hash of the payment. payment_hash: PaymentHash, /// The value, in thousandths of a satoshi, that has been received. @@ -298,18 +292,18 @@ impl_writeable_tlv_based_enum!(Event, (0, PaymentSuccessful) => { (0, payment_hash, required), (1, fee_paid_msat, option), - (3, payment_id, option), + (3, payment_id, required), (5, payment_preimage, option), (7, bolt12_invoice, option), }, (1, PaymentFailed) => { (0, payment_hash, option), (1, reason, upgradable_option), - (3, payment_id, option), + (3, payment_id, required), }, (2, PaymentReceived) => { (0, payment_hash, required), - (1, payment_id, option), + (1, payment_id, required), (2, amount_msat, required), (3, custom_records, optional_vec), }, @@ -1097,7 +1091,7 @@ where } let event = Event::PaymentReceived { - payment_id: Some(payment_id), + payment_id, payment_hash, amount_msat, custom_records: onion_fields @@ -1162,7 +1156,7 @@ where ); }); let event = Event::PaymentSuccessful { - payment_id: Some(payment_id), + payment_id, payment_hash, payment_preimage: Some(payment_preimage), fee_paid_msat, @@ -1198,8 +1192,7 @@ where }, }; - let event = - Event::PaymentFailed { payment_id: Some(payment_id), payment_hash, reason }; + let event = Event::PaymentFailed { payment_id, payment_hash, reason }; match self.event_queue.add_event(event).await { Ok(_) => return Ok(()), Err(e) => { diff --git a/tests/common/mod.rs b/tests/common/mod.rs index aeacef464b..0bf1500323 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -224,7 +224,7 @@ macro_rules! expect_payment_received_event { ref e @ Event::PaymentReceived { payment_id, amount_msat, .. } => { println!("{} got event {:?}", $node.node_id(), e); assert_eq!(amount_msat, $amount_msat); - let payment = $node.payment(&payment_id.unwrap()).unwrap(); + let payment = $node.payment(&payment_id).unwrap(); if !matches!(payment.kind, ldk_node::payment::PaymentKind::Onchain { .. }) { assert_eq!(payment.fee_paid_msat, None); } @@ -292,7 +292,7 @@ macro_rules! expect_payment_successful_event { if let Some(fee_msat) = $fee_paid_msat { assert_eq!(fee_paid_msat, fee_msat); } - let payment = $node.payment(&$payment_id.unwrap()).unwrap(); + let payment = $node.payment(&$payment_id).unwrap(); assert_eq!(payment.fee_paid_msat, fee_paid_msat); assert_eq!(payment_id, $payment_id); $node.event_handled().unwrap(); @@ -1412,7 +1412,7 @@ pub(crate) async fn do_channel_full_cycle( .claim_for_hash(manual_payment_hash, claimable_amount_msat, manual_preimage) .unwrap(); expect_payment_received_event!(node_b, claimable_amount_msat); - expect_payment_successful_event!(node_a, Some(manual_payment_id), None); + expect_payment_successful_event!(node_a, manual_payment_id, None); assert_eq!(node_a.payment(&manual_payment_id).unwrap().status, PaymentStatus::Succeeded); assert_eq!(node_a.payment(&manual_payment_id).unwrap().direction, PaymentDirection::Outbound); assert_eq!( diff --git a/tests/integration_tests_hrn.rs b/tests/integration_tests_hrn.rs index 9102400398..8f910b8606 100644 --- a/tests/integration_tests_hrn.rs +++ b/tests/integration_tests_hrn.rs @@ -79,5 +79,5 @@ async fn unified_send_to_hrn() { }, }; - expect_payment_successful_event!(node_a, Some(offer_payment_id), None); + expect_payment_successful_event!(node_a, offer_payment_id, None); } diff --git a/tests/integration_tests_migration.rs b/tests/integration_tests_migration.rs index c4e63451a8..5962147617 100644 --- a/tests/integration_tests_migration.rs +++ b/tests/integration_tests_migration.rs @@ -203,7 +203,7 @@ async fn migrate_node_across_all_backends() { Bolt11InvoiceDescription::Direct(Description::new("ln send".to_string()).unwrap()); let invoice = node_b.bolt11_payment().receive(10_000, &description.into(), 3600).unwrap(); let ln_send_id = node.bolt11_payment().send(&invoice, None).unwrap(); - expect_payment_successful_event!(node, Some(ln_send_id), None); + expect_payment_successful_event!(node, ln_send_id, None); expect_payment_received_event!(node_b, 10_000); // Lightning receive: node B -> node. @@ -211,7 +211,7 @@ async fn migrate_node_across_all_backends() { Bolt11InvoiceDescription::Direct(Description::new("ln receive".to_string()).unwrap()); let invoice = node.bolt11_payment().receive(5_000, &description.into(), 3600).unwrap(); let ln_receive_id = node_b.bolt11_payment().send(&invoice, None).unwrap(); - expect_payment_successful_event!(node_b, Some(ln_receive_id), None); + expect_payment_successful_event!(node_b, ln_receive_id, None); expect_payment_received_event!(node, 5_000); // On-chain send: node -> a foreign address. diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index b07a90629f..fe1f50f75f 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -460,13 +460,12 @@ async fn split_underpaid_bolt11_payment() { .unwrap(); let receiver_payment_id = expect_payment_received_event!(node_c, amount_msat); - assert_eq!(receiver_payment_id, Some(PaymentId(invoice.payment_hash().0))); - expect_payment_successful_event!(node_a, Some(payment_id_a), None); - expect_payment_successful_event!(node_b, Some(payment_id_b), None); + assert_eq!(receiver_payment_id, PaymentId(invoice.payment_hash().0)); + expect_payment_successful_event!(node_a, payment_id_a, None); + expect_payment_successful_event!(node_b, payment_id_b, None); // The receiver records the full invoice amount; each payer records only its own half. - let receiver_payments = - node_c.list_payments_with_filter(|p| p.id == receiver_payment_id.unwrap()); + let receiver_payments = node_c.list_payments_with_filter(|p| p.id == receiver_payment_id); assert_eq!(receiver_payments.len(), 1); assert_eq!(receiver_payments.first().unwrap().amount_msat, Some(amount_msat)); @@ -1648,7 +1647,7 @@ async fn splice_channel() { let payment_id = node_b.spontaneous_payment().send(amount_msat, node_a.node_id(), None).unwrap(); - expect_payment_successful_event!(node_b, Some(payment_id), None); + expect_payment_successful_event!(node_b, payment_id, None); expect_payment_received_event!(node_a, amount_msat); // Mine a block to give time for the HTLC to resolve @@ -2170,7 +2169,7 @@ async fn simple_bolt12_send_receive() { match event { ref e @ Event::PaymentSuccessful { payment_id: ref evt_id, ref bolt12_invoice, .. } => { println!("{} got event {:?}", node_a.node_id(), e); - assert_eq!(*evt_id, Some(payment_id)); + assert_eq!(*evt_id, payment_id); assert!( bolt12_invoice.is_some(), "bolt12_invoice should be present for BOLT12 payments" @@ -2244,7 +2243,7 @@ async fn simple_bolt12_send_receive() { ) .unwrap(); - expect_payment_successful_event!(node_a, Some(payment_id), None); + expect_payment_successful_event!(node_a, payment_id, None); let node_a_payments = node_a.list_payments_with_filter(|p| { matches!(p.kind, PaymentKind::Bolt12Offer { .. }) && p.id == payment_id }); @@ -2317,7 +2316,7 @@ async fn simple_bolt12_send_receive() { .first() .unwrap() .id; - expect_payment_successful_event!(node_b, Some(node_b_payment_id), None); + expect_payment_successful_event!(node_b, node_b_payment_id, None); let node_b_payments = node_b.list_payments_with_filter(|p| { matches!(p.kind, PaymentKind::Bolt12Refund { .. }) && p.id == node_b_payment_id @@ -2491,7 +2490,7 @@ async fn async_payment() { node_receiver.start().unwrap(); - expect_payment_successful_event!(node_sender, Some(payment_id), None); + expect_payment_successful_event!(node_sender, payment_id, None); } #[tokio::test(flavor = "multi_thread", worker_threads = 1)] @@ -2717,7 +2716,7 @@ async fn unified_send_receive_bip21_uri() { }, }; - expect_payment_successful_event!(node_a, Some(offer_payment_id), None); + expect_payment_successful_event!(node_a, offer_payment_id, None); // Cut off the BOLT12 part to fallback to BOLT11. let uri_str_without_offer = uri_str.split("&lno=").next().unwrap(); @@ -2737,7 +2736,7 @@ async fn unified_send_receive_bip21_uri() { panic!("Expected Bolt11 payment but got error: {:?}", e); }, }; - expect_payment_successful_event!(node_a, Some(invoice_payment_id), None); + expect_payment_successful_event!(node_a, invoice_payment_id, None); let expect_onchain_amount_sats = 800_000; let onchain_uni_payment = @@ -2872,9 +2871,9 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { let service_fee_msat = (jit_amount_msat * channel_opening_fee_ppm as u64) / 1_000_000; let expected_received_amount_msat = jit_amount_msat - service_fee_msat; - expect_payment_successful_event!(payer_node, Some(payment_id), None); + expect_payment_successful_event!(payer_node, payment_id, None); let client_payment_id = - expect_payment_received_event!(client_node, expected_received_amount_msat).unwrap(); + expect_payment_received_event!(client_node, expected_received_amount_msat); let client_payment = client_node.payment(&client_payment_id).unwrap(); match client_payment.kind { PaymentKind::Bolt11 { counterparty_skimmed_fee_msat, .. } => { @@ -2901,7 +2900,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { // are working as expected. println!("Paying regular invoice!"); let payment_id = payer_node.bolt11_payment().send(&invoice, None).unwrap(); - expect_payment_successful_event!(payer_node, Some(payment_id), None); + expect_payment_successful_event!(payer_node, payment_id, None); expect_event!(service_node, PaymentForwarded); expect_payment_received_event!(client_node, amount_msat); @@ -2947,9 +2946,9 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { .unwrap(); expect_event!(service_node, PaymentForwarded); - expect_payment_successful_event!(payer_node, Some(payment_id), None); + expect_payment_successful_event!(payer_node, payment_id, None); let client_payment_id = - expect_payment_received_event!(client_node, expected_received_amount_msat).unwrap(); + expect_payment_received_event!(client_node, expected_received_amount_msat); let client_payment = client_node.payment(&client_payment_id).unwrap(); match client_payment.kind { PaymentKind::Bolt11 { counterparty_skimmed_fee_msat, .. } => { @@ -3054,7 +3053,7 @@ async fn spontaneous_send_with_custom_preimage() { .unwrap(); // check payment status and verify stored preimage - expect_payment_successful_event!(node_a, Some(payment_id), None); + expect_payment_successful_event!(node_a, payment_id, None); let details: PaymentDetails = node_a.list_payments_with_filter(|p| p.id == payment_id).first().unwrap().clone(); assert_eq!(details.status, PaymentStatus::Succeeded); @@ -3240,9 +3239,9 @@ async fn lsps2_client_trusts_lsp() { .claim_for_hash(manual_payment_hash, jit_amount_msat, manual_preimage) .unwrap(); - expect_payment_successful_event!(payer_node, Some(payment_id), None); + expect_payment_successful_event!(payer_node, payment_id, None); - let _ = expect_payment_received_event!(client_node, expected_received_amount_msat).unwrap(); + let _ = expect_payment_received_event!(client_node, expected_received_amount_msat); // Check the nodes pick up on the confirmed funding tx now. wait_for_tx(&electrsd.client, funding_txo.txid).await; diff --git a/tests/upgrade_downgrade_tests.rs b/tests/upgrade_downgrade_tests.rs index de5bef96e8..1b9a92a1a3 100644 --- a/tests/upgrade_downgrade_tests.rs +++ b/tests/upgrade_downgrade_tests.rs @@ -304,7 +304,7 @@ // ) { // match next_current_event(node).await { // ldk_node::Event::PaymentSuccessful { payment_id, .. } => { -// assert_eq!(payment_id.as_ref(), Some(expected_payment_id)); +// assert_eq!(&payment_id, expected_payment_id); // node.event_handled().unwrap(); // }, // event => panic!("{} got unexpected event: {:?}", node.node_id(), event), @@ -313,9 +313,8 @@ // // async fn expect_current_payment_received(node: &CurrentNode, expected_amount_msat: u64) { // match next_current_event(node).await { -// ldk_node::Event::PaymentReceived { amount_msat, payment_id, .. } => { +// ldk_node::Event::PaymentReceived { amount_msat, .. } => { // assert_eq!(amount_msat, expected_amount_msat); -// assert!(payment_id.is_some()); // node.event_handled().unwrap(); // }, // event => panic!("{} got unexpected event: {:?}", node.node_id(), event), From 8302b2c190205fe666059b3144812a2df6c15e28 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 30 Jun 2026 14:11:15 +0200 Subject: [PATCH 02/13] Add pending payment expiry metadata Pending manual invoice records need persisted expiry times so stale reservations can be pruned before checking for duplicate payment hashes. Pending BOLT11 reservations also need indexed lookup by payment hash so duplicate checks can avoid scanning the full pending store. Co-Authored-By: HAL 9000 --- src/payment/mod.rs | 4 +- src/payment/pending_payment_store.rs | 179 ++++++++++++++++++++++++++- src/types.rs | 4 +- 3 files changed, 181 insertions(+), 6 deletions(-) diff --git a/src/payment/mod.rs b/src/payment/mod.rs index fd75322ceb..9b1753d82a 100644 --- a/src/payment/mod.rs +++ b/src/payment/mod.rs @@ -20,7 +20,9 @@ pub use bolt11::Bolt11Payment; pub(crate) use bolt11::PaymentMetadata; pub use bolt12::Bolt12Payment; pub use onchain::OnchainPayment; -pub(crate) use pending_payment_store::{FundingTxCandidate, PendingPaymentDetails}; +pub(crate) use pending_payment_store::{ + FundingTxCandidate, PendingPaymentDetails, PendingPaymentStore, +}; pub use spontaneous::SpontaneousPayment; pub use store::{ Channel, ConfirmationStatus, LSPS2Parameters, PaymentDetails, PaymentDirection, PaymentKind, diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index f5f2fa40a2..5864808bd3 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -5,13 +5,21 @@ // http://opensource.org/licenses/MIT>, at your option. You may not use this file except in // accordance with one or both of these licenses. +use std::collections::HashMap; +use std::ops::Deref; +use std::sync::{Arc, Mutex}; + use bitcoin::Txid; use lightning::impl_writeable_tlv_based; use lightning::ln::channelmanager::PaymentId; +use lightning_types::payment::PaymentHash; -use crate::data_store::{StorableObject, StorableObjectUpdate}; +use crate::data_store::{DataStore, StorableObject, StorableObjectUpdate}; +use crate::logger::LdkLogger; use crate::payment::store::PaymentDetailsUpdate; -use crate::payment::{PaymentDetails, PaymentKind}; +use crate::payment::{PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus}; +use crate::types::DynStore; +use crate::Error; /// One candidate transaction in an interactive-funding (splice) RBF history, holding this node's /// share of the funding amount and fee for that candidate. Both are `None` for a candidate this @@ -47,25 +55,38 @@ pub struct PendingPaymentDetails { /// RBF history, keyed by each candidate's txid. Empty for non-funding payments and for /// records written before per-candidate tracking existed. pub(crate) candidates: Vec, + /// The timestamp after which this pending payment can be pruned. + pub expires_at: Option, } impl PendingPaymentDetails { pub(crate) fn new( details: PaymentDetails, conflicting_txids: Vec, candidates: Vec, ) -> Self { - Self { details, conflicting_txids, candidates } + Self { details, conflicting_txids, candidates, expires_at: None } + } + + pub(crate) fn new_with_expiry( + details: PaymentDetails, conflicting_txids: Vec, expires_at: Option, + ) -> Self { + Self { details, conflicting_txids, candidates: Vec::new(), expires_at } } /// Returns this node's recorded funding figures for the candidate with the given txid, if any. pub(crate) fn candidate(&self, txid: Txid) -> Option<&FundingTxCandidate> { self.candidates.iter().find(|candidate| candidate.txid == txid) } + + pub(crate) fn has_expired(&self, now: u64) -> bool { + self.expires_at.map_or(false, |expires_at| expires_at <= now) + } } impl_writeable_tlv_based!(PendingPaymentDetails, { (0, details, required), (2, conflicting_txids, optional_vec), (4, candidates, optional_vec), + (6, expires_at, option), }); #[derive(Clone, Debug, PartialEq, Eq)] @@ -74,6 +95,7 @@ pub(crate) struct PendingPaymentDetailsUpdate { pub payment_update: Option, pub conflicting_txids: Option>, pub candidates: Vec, + pub expires_at: Option>, } impl StorableObject for PendingPaymentDetails { @@ -112,6 +134,13 @@ impl StorableObject for PendingPaymentDetails { updated = true; } + if let Some(new_expires_at) = update.expires_at { + if self.expires_at != new_expires_at { + self.expires_at = new_expires_at; + updated = true; + } + } + updated } @@ -138,7 +167,151 @@ impl From<&PendingPaymentDetails> for PendingPaymentDetailsUpdate { payment_update: Some(value.details.to_update()), conflicting_txids, candidates: value.candidates.clone(), + expires_at: Some(value.expires_at), + } + } +} + +pub(crate) struct PendingPaymentStore +where + L::Target: LdkLogger, +{ + inner: DataStore, + mutation_lock: tokio::sync::Mutex<()>, + manual_bolt11_payment_hash_index: Mutex>>, +} + +impl PendingPaymentStore +where + L::Target: LdkLogger, +{ + pub(crate) fn new( + pending_payments: Vec, primary_namespace: String, + secondary_namespace: String, kv_store: Arc, logger: L, + ) -> Self { + // TODO: Revisit this initialization once pending payments are no longer all kept in + // memory. + let manual_bolt11_payment_hash_index = + Mutex::new(Self::build_manual_bolt11_payment_hash_index(&pending_payments)); + let inner = DataStore::new( + pending_payments, + primary_namespace, + secondary_namespace, + kv_store, + logger, + ); + Self { inner, mutation_lock: tokio::sync::Mutex::new(()), manual_bolt11_payment_hash_index } + } + + pub(crate) async fn insert_or_update( + &self, pending_payment: PendingPaymentDetails, + ) -> Result { + let _guard = self.mutation_lock.lock().await; + let id = pending_payment.id(); + let before = self.inner.get(&id); + let updated = self.inner.insert_or_update(pending_payment).await?; + if updated { + let after = self.inner.get(&id); + self.replace_in_index(before.as_ref(), after.as_ref()); + } + Ok(updated) + } + + pub(crate) async fn remove(&self, id: &PaymentId) -> Result<(), Error> { + let _guard = self.mutation_lock.lock().await; + let before = self.inner.get(id); + self.inner.remove(id).await?; + if let Some(payment) = before.as_ref() { + self.remove_from_index(payment); } + Ok(()) + } + + pub(crate) fn get(&self, id: &PaymentId) -> Option { + self.inner.get(id) + } + + pub(crate) fn contains_key(&self, id: &PaymentId) -> bool { + self.inner.contains_key(id) + } + + pub(crate) fn list_filter bool>( + &self, f: F, + ) -> Vec { + self.inner.list_filter(f) + } + + pub(crate) fn get_pending_manual_bolt11_by_payment_hash( + &self, payment_hash: &PaymentHash, + ) -> Option { + let ids = self + .manual_bolt11_payment_hash_index + .lock() + .expect("lock") + .get(payment_hash) + .cloned() + .unwrap_or_default(); + ids.into_iter().find_map(|id| self.inner.get(&id)) + } + + fn build_manual_bolt11_payment_hash_index( + pending_payments: &[PendingPaymentDetails], + ) -> HashMap> { + let mut index = HashMap::new(); + for payment in pending_payments { + Self::insert_into_manual_bolt11_hash_index(&mut index, payment); + } + index + } + + fn replace_in_index( + &self, before: Option<&PendingPaymentDetails>, after: Option<&PendingPaymentDetails>, + ) { + let mut index = self.manual_bolt11_payment_hash_index.lock().expect("lock"); + if let Some(payment) = before { + Self::remove_from_manual_bolt11_hash_index(&mut index, payment); + } + if let Some(payment) = after { + Self::insert_into_manual_bolt11_hash_index(&mut index, payment); + } + } + + fn remove_from_index(&self, payment: &PendingPaymentDetails) { + let mut index = self.manual_bolt11_payment_hash_index.lock().expect("lock"); + Self::remove_from_manual_bolt11_hash_index(&mut index, payment); + } + + fn insert_into_manual_bolt11_hash_index( + index: &mut HashMap>, payment: &PendingPaymentDetails, + ) { + if let Some(payment_hash) = manual_bolt11_payment_hash(&payment.details) { + index.entry(payment_hash).or_default().push(payment.details.id); + } + } + + fn remove_from_manual_bolt11_hash_index( + index: &mut HashMap>, payment: &PendingPaymentDetails, + ) { + if let Some(payment_hash) = manual_bolt11_payment_hash(&payment.details) { + if let Some(ids) = index.get_mut(&payment_hash) { + ids.retain(|id| *id != payment.details.id); + if ids.is_empty() { + index.remove(&payment_hash); + } + } + } + } +} + +fn manual_bolt11_payment_hash(payment: &PaymentDetails) -> Option { + match payment.kind { + PaymentKind::Bolt11 { hash, preimage: None, .. } + if payment.direction == PaymentDirection::Inbound + && payment.status == PaymentStatus::Pending => + { + Some(hash) + }, + _ => None, } } diff --git a/src/types.rs b/src/types.rs index e24db4d253..2aa09cb1b4 100644 --- a/src/types.rs +++ b/src/types.rs @@ -48,7 +48,7 @@ use crate::fee_estimator::OnchainFeeEstimator; use crate::ffi::maybe_wrap; use crate::logger::Logger; use crate::message_handler::NodeCustomMessageHandler; -use crate::payment::{PaymentDetails, PendingPaymentDetails}; +use crate::payment::{PaymentDetails, PendingPaymentStore as PendingPaymentStoreImpl}; use crate::runtime::RuntimeSpawner; #[cfg(not(feature = "uniffi"))] @@ -755,4 +755,4 @@ impl From<&(u64, Vec)> for CustomTlvRecord { } } -pub(crate) type PendingPaymentStore = DataStore>; +pub(crate) type PendingPaymentStore = PendingPaymentStoreImpl>; From 64802f3b59516ba507f72ccb2f48da81c8134e16 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 13:28:10 +0200 Subject: [PATCH 03/13] f Avoid cloning pending payment ids Avoid copying the full id list during manual BOLT11 lookups. Holding the index lock while reading the in-memory store does not change persistence ordering. Co-Authored-By: HAL 9000 --- src/payment/pending_payment_store.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index 5864808bd3..7834dcf7b1 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -244,14 +244,9 @@ where pub(crate) fn get_pending_manual_bolt11_by_payment_hash( &self, payment_hash: &PaymentHash, ) -> Option { - let ids = self - .manual_bolt11_payment_hash_index - .lock() - .expect("lock") - .get(payment_hash) - .cloned() - .unwrap_or_default(); - ids.into_iter().find_map(|id| self.inner.get(&id)) + let index = self.manual_bolt11_payment_hash_index.lock().expect("lock"); + let ids = index.get(payment_hash)?; + ids.iter().find_map(|id| self.inner.get(id)) } fn build_manual_bolt11_payment_hash_index( From 59e97075282a0b25a7b14daec88f41074357540b Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 13:29:13 +0200 Subject: [PATCH 04/13] f Assert unique manual payment hashes Document the pending-store invariant that a manual BOLT11 hash maps to at most one pending payment id. Co-Authored-By: HAL 9000 --- src/payment/pending_payment_store.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index 7834dcf7b1..43cae53d22 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -246,6 +246,10 @@ where ) -> Option { let index = self.manual_bolt11_payment_hash_index.lock().expect("lock"); let ids = index.get(payment_hash)?; + debug_assert!( + ids.len() <= 1, + "manual BOLT11 payment hash maps to multiple pending payment IDs" + ); ids.iter().find_map(|id| self.inner.get(id)) } From d4e67d1396539b6aeebcef37a6b3e363dce70da4 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 14:27:06 +0200 Subject: [PATCH 05/13] f Use enum pending expiries Represent pending-payment pruning conditions as either wall-clock time or block height so later manual BOLT11 handling can keep the claimable phase tied to the HTLC claim deadline. Co-Authored-By: HAL 9000 --- src/payment/mod.rs | 2 +- src/payment/pending_payment_store.rs | 50 ++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/payment/mod.rs b/src/payment/mod.rs index 9b1753d82a..60750db4a8 100644 --- a/src/payment/mod.rs +++ b/src/payment/mod.rs @@ -21,7 +21,7 @@ pub(crate) use bolt11::PaymentMetadata; pub use bolt12::Bolt12Payment; pub use onchain::OnchainPayment; pub(crate) use pending_payment_store::{ - FundingTxCandidate, PendingPaymentDetails, PendingPaymentStore, + FundingTxCandidate, PendingPaymentDetails, PendingPaymentExpiry, PendingPaymentStore, }; pub use spontaneous::SpontaneousPayment; pub use store::{ diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index 43cae53d22..d996bd41af 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -10,8 +10,8 @@ use std::ops::Deref; use std::sync::{Arc, Mutex}; use bitcoin::Txid; -use lightning::impl_writeable_tlv_based; use lightning::ln::channelmanager::PaymentId; +use lightning::{impl_writeable_tlv_based, impl_writeable_tlv_based_enum}; use lightning_types::payment::PaymentHash; use crate::data_store::{DataStore, StorableObject, StorableObjectUpdate}; @@ -44,6 +44,24 @@ impl_writeable_tlv_based!(FundingTxCandidate, { (4, fee_paid_msat, option), }); +/// The condition after which this pending payment can be pruned. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum PendingPaymentExpiry { + /// The Unix timestamp after which this pending payment can be pruned. + Time { timestamp: u64 }, + /// The block height after which this pending payment can be pruned. + Height { height: u32 }, +} + +impl_writeable_tlv_based_enum!(PendingPaymentExpiry, + (0, Time) => { + (0, timestamp, required), + }, + (2, Height) => { + (0, height, required), + }, +); + /// Represents a pending payment #[derive(Clone, Debug, PartialEq, Eq)] pub struct PendingPaymentDetails { @@ -55,21 +73,21 @@ pub struct PendingPaymentDetails { /// RBF history, keyed by each candidate's txid. Empty for non-funding payments and for /// records written before per-candidate tracking existed. pub(crate) candidates: Vec, - /// The timestamp after which this pending payment can be pruned. - pub expires_at: Option, + /// The condition after which this pending payment can be pruned. + pub(crate) expiry: Option, } impl PendingPaymentDetails { pub(crate) fn new( details: PaymentDetails, conflicting_txids: Vec, candidates: Vec, ) -> Self { - Self { details, conflicting_txids, candidates, expires_at: None } + Self { details, conflicting_txids, candidates, expiry: None } } pub(crate) fn new_with_expiry( - details: PaymentDetails, conflicting_txids: Vec, expires_at: Option, + details: PaymentDetails, conflicting_txids: Vec, expiry: Option, ) -> Self { - Self { details, conflicting_txids, candidates: Vec::new(), expires_at } + Self { details, conflicting_txids, candidates: Vec::new(), expiry } } /// Returns this node's recorded funding figures for the candidate with the given txid, if any. @@ -77,8 +95,12 @@ impl PendingPaymentDetails { self.candidates.iter().find(|candidate| candidate.txid == txid) } - pub(crate) fn has_expired(&self, now: u64) -> bool { - self.expires_at.map_or(false, |expires_at| expires_at <= now) + pub(crate) fn has_expired(&self, now: u64, current_height: u32) -> bool { + match self.expiry { + Some(PendingPaymentExpiry::Time { timestamp }) => timestamp <= now, + Some(PendingPaymentExpiry::Height { height }) => height <= current_height, + None => false, + } } } @@ -86,7 +108,7 @@ impl_writeable_tlv_based!(PendingPaymentDetails, { (0, details, required), (2, conflicting_txids, optional_vec), (4, candidates, optional_vec), - (6, expires_at, option), + (6, expiry, option), }); #[derive(Clone, Debug, PartialEq, Eq)] @@ -95,7 +117,7 @@ pub(crate) struct PendingPaymentDetailsUpdate { pub payment_update: Option, pub conflicting_txids: Option>, pub candidates: Vec, - pub expires_at: Option>, + pub expiry: Option>, } impl StorableObject for PendingPaymentDetails { @@ -134,9 +156,9 @@ impl StorableObject for PendingPaymentDetails { updated = true; } - if let Some(new_expires_at) = update.expires_at { - if self.expires_at != new_expires_at { - self.expires_at = new_expires_at; + if let Some(new_expiry) = update.expiry { + if self.expiry != new_expiry { + self.expiry = new_expiry; updated = true; } } @@ -167,7 +189,7 @@ impl From<&PendingPaymentDetails> for PendingPaymentDetailsUpdate { payment_update: Some(value.details.to_update()), conflicting_txids, candidates: value.candidates.clone(), - expires_at: Some(value.expires_at), + expiry: Some(value.expiry), } } } From 4ef9c0382761a633f8c1f21d37d9cf76133aab1a Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 13:58:21 +0200 Subject: [PATCH 06/13] Add pending payment batch removal Expose a pending-store batch removal path so callers can prune expired pending payments without repeatedly taking the pending-store mutation lock. Co-Authored-By: HAL 9000 --- src/data_store.rs | 60 ++++++++++++++++++++++++++-- src/payment/pending_payment_store.rs | 11 +++-- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/data_store.rs b/src/data_store.rs index b1ed816df9..f1af820a4b 100644 --- a/src/data_store.rs +++ b/src/data_store.rs @@ -112,9 +112,19 @@ where } pub(crate) async fn remove(&self, id: &SO::Id) -> Result<(), Error> { + self.remove_batch(std::slice::from_ref(id)).await?; + Ok(()) + } + + pub(crate) async fn remove_batch(&self, ids: &[SO::Id]) -> Result, Error> { let _guard = self.mutation_lock.lock().await; - let should_remove = { self.objects.lock().expect("lock").contains_key(id) }; - if should_remove { + let mut removed_objects = Vec::new(); + for id in ids { + let should_remove = { self.objects.lock().expect("lock").contains_key(id) }; + if !should_remove { + continue; + } + let store_key = id.encode_to_hex_str(); KVStore::remove( &*self.kv_store, @@ -135,9 +145,12 @@ where ); Error::PersistenceFailed })?; - self.objects.lock().expect("lock").remove(id); + + if let Some(object) = self.objects.lock().expect("lock").remove(id) { + removed_objects.push(object); + } } - Ok(()) + Ok(removed_objects) } /// Returns the current in-memory object for `id`. @@ -422,6 +435,45 @@ mod tests { assert!(data_store.get(&new_id).is_none()); } + #[tokio::test] + async fn batch_remove_removes_persisted_objects() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let logger = Arc::new(TestLogger::new()); + let primary_namespace = "datastore_batch_remove_test_primary".to_string(); + let secondary_namespace = "datastore_batch_remove_test_secondary".to_string(); + let data_store: DataStore> = DataStore::new( + Vec::new(), + primary_namespace.clone(), + secondary_namespace.clone(), + Arc::clone(&store), + logger, + ); + + let first = TestObject { id: TestObjectId { id: [1u8; 4] }, data: [23u8; 3] }; + let second = TestObject { id: TestObjectId { id: [2u8; 4] }, data: [42u8; 3] }; + let missing_id = TestObjectId { id: [3u8; 4] }; + assert_eq!(Ok(false), data_store.insert(first).await); + assert_eq!(Ok(false), data_store.insert(second).await); + + assert_eq!( + Ok(vec![first, second]), + data_store.remove_batch(&[first.id, missing_id, second.id]).await + ); + assert_eq!(None, data_store.get(&first.id)); + assert_eq!(None, data_store.get(&second.id)); + + for id in [first.id, second.id] { + assert!(KVStore::read( + &*store, + &primary_namespace, + &secondary_namespace, + &id.encode_to_hex_str() + ) + .await + .is_err()); + } + } + #[tokio::test] async fn insert_does_not_mutate_memory_if_persist_fails() { let id = TestObjectId { id: [42u8; 4] }; diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index d996bd41af..2ee98dc377 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -240,11 +240,14 @@ where } pub(crate) async fn remove(&self, id: &PaymentId) -> Result<(), Error> { + self.remove_batch(std::slice::from_ref(id)).await + } + + pub(crate) async fn remove_batch(&self, ids: &[PaymentId]) -> Result<(), Error> { let _guard = self.mutation_lock.lock().await; - let before = self.inner.get(id); - self.inner.remove(id).await?; - if let Some(payment) = before.as_ref() { - self.remove_from_index(payment); + let removed_payments = self.inner.remove_batch(ids).await?; + for payment in removed_payments { + self.remove_from_index(&payment); } Ok(()) } From 4a62c033f315e6ac801b052179c0793ccbe62589 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 2 Jul 2026 14:04:02 +0200 Subject: [PATCH 07/13] Track manual BOLT11 invoices Manual BOLT11 invoices need a pending entry before HTLC arrival. That lets duplicate registrations be rejected without scanning finalized payments. Keep the legacy hash-derived payment ID in this step; the next commit switches BOLT11 payments to randomized IDs. Co-Authored-By: HAL 9000 --- src/builder.rs | 1 + src/event.rs | 139 +++++++++++++++++++++++++-- src/lib.rs | 8 +- src/payment/bolt11.rs | 218 +++++++++++++++++++++++++++++++----------- tests/common/mod.rs | 9 ++ 5 files changed, 309 insertions(+), 66 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index a70b04b2ab..228b69ef4f 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -2333,6 +2333,7 @@ fn build_with_store_internal( scorer, peer_store, payment_store, + pending_payment_store, lnurl_auth, is_running, node_metrics, diff --git a/src/event.rs b/src/event.rs index 7c3b11f311..50debc66c7 100644 --- a/src/event.rs +++ b/src/event.rs @@ -10,6 +10,7 @@ use core::task::{Poll, Waker}; use std::collections::VecDeque; use std::ops::Deref; use std::sync::{Arc, Mutex}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use bitcoin::blockdata::locktime::absolute::LockTime; use bitcoin::secp256k1::PublicKey; @@ -51,11 +52,12 @@ use crate::payment::asynchronous::static_invoice_store::StaticInvoiceStore; use crate::payment::store::{ PaymentDetails, PaymentDetailsUpdate, PaymentDirection, PaymentKind, PaymentStatus, }; -use crate::payment::PaymentMetadata; +use crate::payment::{PaymentMetadata, PendingPaymentDetails}; use crate::probing::Prober; use crate::runtime::Runtime; use crate::types::{ - CustomTlvRecord, DynStore, KeysManager, OnionMessenger, PaymentStore, Sweeper, Wallet, + CustomTlvRecord, DynStore, KeysManager, OnionMessenger, PaymentStore, PendingPaymentStore, + Sweeper, Wallet, }; use crate::{ hex_utils, BumpTransactionEventHandler, ChannelManager, Error, Graph, PeerInfo, PeerStore, @@ -529,6 +531,7 @@ where network_graph: Arc, liquidity_source: Arc>>, payment_store: Arc, + pending_payment_store: Arc, peer_store: Arc>, keys_manager: Arc, static_invoice_store: Option, @@ -550,10 +553,10 @@ where channel_manager: Arc, connection_manager: Arc>, output_sweeper: Arc, network_graph: Arc, liquidity_source: Arc>>, payment_store: Arc, - peer_store: Arc>, keys_manager: Arc, - static_invoice_store: Option, onion_messenger: Arc, - om_mailbox: Option>, prober: Option>, - runtime: Arc, logger: L, config: Arc, + pending_payment_store: Arc, peer_store: Arc>, + keys_manager: Arc, static_invoice_store: Option, + onion_messenger: Arc, om_mailbox: Option>, + prober: Option>, runtime: Arc, logger: L, config: Arc, ) -> Self { Self { event_queue, @@ -565,6 +568,7 @@ where network_graph, liquidity_source, payment_store, + pending_payment_store, peer_store, keys_manager, static_invoice_store, @@ -607,6 +611,41 @@ where }) } + fn current_time_secs() -> u64 { + SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or(Duration::from_secs(0)).as_secs() + } + + async fn prune_expired_pending_payments(&self) -> Result<(), ReplayEvent> { + let now = Self::current_time_secs(); + let expired_payment_ids = self + .pending_payment_store + .list_filter(|payment| payment.has_expired(now)) + .into_iter() + .map(|payment| payment.details.id) + .collect::>(); + + for payment_id in expired_payment_ids { + if let Err(e) = self.pending_payment_store.remove(&payment_id).await { + log_error!( + self.logger, + "Failed to remove expired pending payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + } + } + + Ok(()) + } + + async fn find_pending_inbound_payment( + &self, payment_hash: &PaymentHash, + ) -> Result, ReplayEvent> { + self.prune_expired_pending_payments().await?; + Ok(self.pending_payment_store.get_pending_manual_bolt11_by_payment_hash(payment_hash)) + } + pub async fn handle_event(&self, event: LdkEvent) -> Result<(), ReplayEvent> { match event { LdkEvent::FundingGenerationReady { @@ -720,6 +759,11 @@ where } => { let payment_id = PaymentId(payment_hash.0); let payment_info = self.payment_store.get(&payment_id); + let pending_payment = if payment_info.is_none() { + self.find_pending_inbound_payment(&payment_hash).await? + } else { + None + }; if let Some(info) = payment_info.as_ref() { if info.direction == PaymentDirection::Outbound { log_info!( @@ -805,6 +849,17 @@ where counterparty_skimmed_fee_msat, ); self.fail_claimable_payment(payment_id, &payment_hash).await?; + if pending_payment.is_some() { + if let Err(e) = self.pending_payment_store.remove(&payment_id).await { + log_error!( + self.logger, + "Failed to remove pending payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + } + } return Ok(()); }; @@ -817,6 +872,17 @@ where max_total_opening_fee_msat, ); self.fail_claimable_payment(payment_id, &payment_hash).await?; + if pending_payment.is_some() { + if let Err(e) = self.pending_payment_store.remove(&payment_id).await { + log_error!( + self.logger, + "Failed to remove pending payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + } + } return Ok(()); } @@ -840,6 +906,57 @@ where } } + let payment_info = if let Some(pending_payment) = pending_payment.as_ref() { + let mut payment = pending_payment.details.clone(); + if let PaymentKind::Bolt11 { + counterparty_skimmed_fee_msat: stored_fee, .. + } = &mut payment.kind + { + if counterparty_skimmed_fee_msat > 0 { + *stored_fee = Some(counterparty_skimmed_fee_msat); + } + } + + match self.payment_store.insert(payment.clone()).await { + Ok(false) => (), + Ok(true) => { + log_error!( + self.logger, + "Bolt11InvoicePayment with ID {} was previously known", + payment_id, + ); + debug_assert!(false); + }, + Err(e) => { + log_error!( + self.logger, + "Failed to insert payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + }, + } + + let mut pending_payment = pending_payment.clone(); + pending_payment.expires_at = None; + if let Err(e) = + self.pending_payment_store.insert_or_update(pending_payment).await + { + log_error!( + self.logger, + "Failed to update pending payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + } + + Some(payment) + } else { + payment_info + }; + if let Some(info) = payment_info { // If this is known by the store but ChannelManager doesn't know the preimage, // the payment has been registered via `_for_hash` variants and needs to be manually claimed via @@ -1090,6 +1207,16 @@ where }, } + if let Err(e) = self.pending_payment_store.remove(&payment_id).await { + log_error!( + self.logger, + "Failed to remove pending payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + } + let event = Event::PaymentReceived { payment_id, payment_hash, diff --git a/src/lib.rs b/src/lib.rs index acfcbc0d48..a06b10af93 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -181,8 +181,8 @@ use runtime::Runtime; pub use tokio; use types::{ Broadcaster, BumpTransactionEventHandler, ChainMonitor, ChannelManager, DynStore, Graph, - HRNResolver, KeysManager, OnionMessenger, PaymentStore, PeerManager, Router, Scorer, Sweeper, - Wallet, + HRNResolver, KeysManager, OnionMessenger, PaymentStore, PeerManager, PendingPaymentStore, + Router, Scorer, Sweeper, Wallet, }; pub use types::{ ChannelCounterparty, ChannelDetails, CustomTlvRecord, PeerDetails, ReserveType, UserChannelId, @@ -249,6 +249,7 @@ pub struct Node { scorer: Arc>, peer_store: Arc>>, payment_store: Arc, + pending_payment_store: Arc, lnurl_auth: Arc, is_running: Arc>, node_metrics: Arc, @@ -611,6 +612,7 @@ impl Node { Arc::clone(&self.network_graph), Arc::clone(&self.liquidity_source), Arc::clone(&self.payment_store), + Arc::clone(&self.pending_payment_store), Arc::clone(&self.peer_store), Arc::clone(&self.keys_manager), static_invoice_store, @@ -962,6 +964,7 @@ impl Node { Arc::clone(&self.connection_manager), Arc::clone(&self.liquidity_source), Arc::clone(&self.payment_store), + Arc::clone(&self.pending_payment_store), Arc::clone(&self.peer_store), Arc::clone(&self.config), Arc::clone(&self.is_running), @@ -980,6 +983,7 @@ impl Node { Arc::clone(&self.connection_manager), Arc::clone(&self.liquidity_source), Arc::clone(&self.payment_store), + Arc::clone(&self.pending_payment_store), Arc::clone(&self.peer_store), Arc::clone(&self.config), Arc::clone(&self.is_running), diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index 4503dfa061..904d95abcb 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -10,6 +10,7 @@ //! [BOLT 11]: https://github.com/lightning/bolts/blob/master/11-payment-encoding.md use std::sync::{Arc, RwLock}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; @@ -22,7 +23,7 @@ use lightning::routing::router::{PaymentParameters, RouteParameters, RouteParame use lightning_invoice::{ Bolt11Invoice as LdkBolt11Invoice, Bolt11InvoiceDescription as LdkBolt11InvoiceDescription, }; -use lightning_types::payment::{PaymentHash, PaymentPreimage}; +use lightning_types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::config::{Config, LDK_PAYMENT_RETRY_TIMEOUT}; use crate::connection::ConnectionManager; @@ -35,9 +36,10 @@ use crate::payment::store::{ LSPS2Parameters, PaymentDetails, PaymentDetailsUpdate, PaymentDirection, PaymentKind, PaymentStatus, }; +use crate::payment::PendingPaymentDetails; use crate::peer_store::{PeerInfo, PeerStore}; use crate::runtime::Runtime; -use crate::types::{ChannelManager, PaymentStore}; +use crate::types::{ChannelManager, PaymentStore, PendingPaymentStore}; #[cfg(not(feature = "uniffi"))] type Bolt11Invoice = LdkBolt11Invoice; @@ -72,6 +74,7 @@ pub struct Bolt11Payment { connection_manager: Arc>>, liquidity_source: Arc>>, payment_store: Arc, + pending_payment_store: Arc, peer_store: Arc>>, config: Arc, is_running: Arc>, @@ -83,8 +86,8 @@ impl Bolt11Payment { runtime: Arc, channel_manager: Arc, connection_manager: Arc>>, liquidity_source: Arc>>, payment_store: Arc, - peer_store: Arc>>, config: Arc, - is_running: Arc>, logger: Arc, + pending_payment_store: Arc, peer_store: Arc>>, + config: Arc, is_running: Arc>, logger: Arc, ) -> Self { Self { runtime, @@ -92,6 +95,7 @@ impl Bolt11Payment { connection_manager, liquidity_source, payment_store, + pending_payment_store, peer_store, config, is_running, @@ -99,10 +103,68 @@ impl Bolt11Payment { } } + fn current_time_secs() -> u64 { + SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or(Duration::from_secs(0)).as_secs() + } + + fn prune_expired_pending_payments(&self) -> Result<(), Error> { + let now = Self::current_time_secs(); + let expired_payment_ids = self + .pending_payment_store + .list_filter(|payment| payment.has_expired(now)) + .into_iter() + .map(|payment| payment.details.id) + .collect::>(); + + for payment_id in expired_payment_ids { + self.runtime.block_on(self.pending_payment_store.remove(&payment_id))?; + } + + Ok(()) + } + + fn has_pending_inbound_payment(&self, payment_hash: &PaymentHash) -> bool { + self.pending_payment_store.get_pending_manual_bolt11_by_payment_hash(payment_hash).is_some() + } + + fn register_manual_claim_invoice( + &self, payment_hash: PaymentHash, amount_msat: Option, payment_secret: PaymentSecret, + expiry_secs: u32, + ) -> Result<(), Error> { + let payment_id = PaymentId(payment_hash.0); + let kind = PaymentKind::Bolt11 { + hash: payment_hash, + preimage: None, + secret: Some(payment_secret), + counterparty_skimmed_fee_msat: None, + }; + let payment = PaymentDetails::new( + payment_id, + kind, + amount_msat, + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); + let expires_at = Some(Self::current_time_secs().saturating_add(expiry_secs as u64)); + let pending_payment = + PendingPaymentDetails::new_with_expiry(payment, Vec::new(), expires_at); + self.runtime.block_on(self.pending_payment_store.insert_or_update(pending_payment))?; + Ok(()) + } + pub(crate) fn receive_inner( &self, amount_msat: Option, invoice_description: &LdkBolt11InvoiceDescription, expiry_secs: u32, manual_claim_payment_hash: Option, ) -> Result { + if let Some(payment_hash) = manual_claim_payment_hash { + self.prune_expired_pending_payments()?; + if self.has_pending_inbound_payment(&payment_hash) { + log_error!(self.logger, "Payment error: an invoice must not be paid twice."); + return Err(Error::DuplicatePayment); + } + } + let invoice = { let invoice_params = Bolt11InvoiceParameters { amount_msats: amount_msat, @@ -124,14 +186,19 @@ impl Bolt11Payment { } }; - let payment_hash = invoice.payment_hash(); - let payment_secret = invoice.payment_secret(); - let id = PaymentId(payment_hash.0); - let preimage = if manual_claim_payment_hash.is_none() { - // If the user hasn't registered a custom payment hash, we're positive ChannelManager - // will know the preimage at this point. + if let Some(payment_hash) = manual_claim_payment_hash { + self.register_manual_claim_invoice( + payment_hash, + amount_msat, + *invoice.payment_secret(), + expiry_secs, + )?; + } else { + let payment_hash = invoice.payment_hash(); + let payment_secret = invoice.payment_secret(); + let id = PaymentId(payment_hash.0); let mut payment_metadata = invoice.payment_metadata().cloned(); - let res = self + let preimage = self .channel_manager .get_payment_preimage_decrypt_metadata( payment_hash, @@ -139,26 +206,23 @@ impl Bolt11Payment { payment_metadata.as_deref_mut(), ) .ok(); - debug_assert!(res.is_some(), "We just let ChannelManager create an inbound payment, it can't have forgotten the preimage by now."); - res - } else { - None - }; - let kind = PaymentKind::Bolt11 { - hash: payment_hash, - preimage, - secret: Some(payment_secret.clone()), - counterparty_skimmed_fee_msat: None, - }; - let payment = PaymentDetails::new( - id, - kind, - amount_msat, - None, - PaymentDirection::Inbound, - PaymentStatus::Pending, - ); - self.runtime.block_on(self.payment_store.insert(payment))?; + debug_assert!(preimage.is_some(), "We just let ChannelManager create an inbound payment, it can't have forgotten the preimage by now."); + let kind = PaymentKind::Bolt11 { + hash: payment_hash, + preimage, + secret: Some(payment_secret.clone()), + counterparty_skimmed_fee_msat: None, + }; + let payment = PaymentDetails::new( + id, + kind, + amount_msat, + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); + self.runtime.block_on(self.payment_store.insert(payment))?; + } Ok(invoice) } @@ -168,6 +232,14 @@ impl Bolt11Payment { expiry_secs: u32, max_total_lsp_fee_limit_msat: Option, max_proportional_lsp_fee_limit_ppm_msat: Option, payment_hash: Option, ) -> Result { + if let Some(payment_hash) = payment_hash { + self.prune_expired_pending_payments()?; + if self.has_pending_inbound_payment(&payment_hash) { + log_error!(self.logger, "Payment error: an invoice must not be paid twice."); + return Err(Error::DuplicatePayment); + } + } + let connection_manager = Arc::clone(&self.connection_manager); let (invoice, chosen_lsp) = self.runtime.block_on(async move { if let Some(amount_msat) = amount_msat { @@ -196,34 +268,43 @@ impl Bolt11Payment { } })?; - // Register payment in payment store. - let payment_hash = invoice.payment_hash(); - let payment_secret = invoice.payment_secret(); - let id = PaymentId(payment_hash.0); - let mut payment_metadata = invoice.payment_metadata().cloned(); - let preimage = self - .channel_manager - .get_payment_preimage_decrypt_metadata( + if let Some(payment_hash) = payment_hash { + self.register_manual_claim_invoice( payment_hash, - payment_secret.clone(), - payment_metadata.as_deref_mut(), - ) - .ok(); - let kind = PaymentKind::Bolt11 { - hash: payment_hash, - preimage, - secret: Some(payment_secret.clone()), - counterparty_skimmed_fee_msat: None, - }; - let payment = PaymentDetails::new( - id, - kind, - amount_msat, - None, - PaymentDirection::Inbound, - PaymentStatus::Pending, - ); - self.runtime.block_on(self.payment_store.insert(payment))?; + amount_msat, + *invoice.payment_secret(), + expiry_secs, + )?; + } else { + // Register payment in payment store. + let payment_hash = invoice.payment_hash(); + let payment_secret = invoice.payment_secret(); + let id = PaymentId(payment_hash.0); + let mut payment_metadata = invoice.payment_metadata().cloned(); + let preimage = self + .channel_manager + .get_payment_preimage_decrypt_metadata( + payment_hash, + payment_secret.clone(), + payment_metadata.as_deref_mut(), + ) + .ok(); + let kind = PaymentKind::Bolt11 { + hash: payment_hash, + preimage, + secret: Some(payment_secret.clone()), + counterparty_skimmed_fee_msat: None, + }; + let payment = PaymentDetails::new( + id, + kind, + amount_msat, + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); + self.runtime.block_on(self.payment_store.insert(payment))?; + } // Persist the chosen LSP peer to make sure we reconnect on restart. let peer_info = PeerInfo { node_id: chosen_lsp.node_id, address: chosen_lsp.address }; @@ -582,6 +663,7 @@ impl Bolt11Payment { } self.channel_manager.fail_htlc_backwards(&payment_hash); + self.runtime.block_on(self.pending_payment_store.remove(&payment_id))?; Ok(()) } @@ -603,6 +685,11 @@ impl Bolt11Payment { /// We will register the given payment hash and emit a [`PaymentClaimable`] event once /// the inbound payment arrives. /// + /// **Warning:** it is the user's responsibility to never reuse the same payment hash. + /// Reusing a payment hash is unsafe and can lead to loss of funds. We only reject duplicates + /// while a matching manual-claim invoice is still pending; we do not prevent reuse after the + /// pending registration has been claimed, failed, expired, or pruned. + /// /// **Note:** users *MUST* handle this event and claim the payment manually via /// [`claim_for_hash`] as soon as they have obtained access to the preimage of the given /// payment hash. If they're unable to obtain the preimage, they *MUST* immediately fail the payment via @@ -639,6 +726,11 @@ impl Bolt11Payment { /// We will register the given payment hash and emit a [`PaymentClaimable`] event once /// the inbound payment arrives. /// + /// **Warning:** it is the user's responsibility to never reuse the same payment hash. + /// Reusing a payment hash is unsafe and can lead to loss of funds. We only reject duplicates + /// while a matching manual-claim invoice is still pending; we do not prevent reuse after the + /// pending registration has been claimed, failed, expired, or pruned. + /// /// **Note:** users *MUST* handle this event and claim the payment manually via /// [`claim_for_hash`] as soon as they have obtained access to the preimage of the given /// payment hash. If they're unable to obtain the preimage, they *MUST* immediately fail the payment via @@ -694,6 +786,11 @@ impl Bolt11Payment { /// the inbound payment arrives. The check that [`counterparty_skimmed_fee_msat`] is within the limits /// is performed *before* emitting the event. /// + /// **Warning:** it is the user's responsibility to never reuse the same payment hash. + /// Reusing a payment hash is unsafe and can lead to loss of funds. We only reject duplicates + /// while a matching manual-claim invoice is still pending; we do not prevent reuse after the + /// pending registration has been claimed, failed, expired, or pruned. + /// /// **Note:** users *MUST* handle this event and claim the payment manually via /// [`claim_for_hash`] as soon as they have obtained access to the preimage of the given /// payment hash. If they're unable to obtain the preimage, they *MUST* immediately fail the payment via @@ -761,6 +858,11 @@ impl Bolt11Payment { /// the inbound payment arrives. The check that [`counterparty_skimmed_fee_msat`] is within the limits /// is performed *before* emitting the event. /// + /// **Warning:** it is the user's responsibility to never reuse the same payment hash. + /// Reusing a payment hash is unsafe and can lead to loss of funds. We only reject duplicates + /// while a matching manual-claim invoice is still pending; we do not prevent reuse after the + /// pending registration has been claimed, failed, expired, or pruned. + /// /// **Note:** users *MUST* handle this event and claim the payment manually via /// [`claim_for_hash`] as soon as they have obtained access to the preimage of the given /// payment hash. If they're unable to obtain the preimage, they *MUST* immediately fail the payment via diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 0bf1500323..8b8cce13e9 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1399,6 +1399,15 @@ pub(crate) async fn do_channel_full_cycle( manual_payment_hash, ) .unwrap(); + assert!(matches!( + node_b.bolt11_payment().receive_for_hash( + invoice_amount_3_msat, + &invoice_description.clone().into(), + 9217, + manual_payment_hash, + ), + Err(NodeError::DuplicatePayment) + )); let manual_payment_id = node_a.bolt11_payment().send(&manual_invoice, None).unwrap(); let claimable_amount_msat = expect_payment_claimable_event!( From da39b529ee5a30c5c21a43e5cb9b2084f84d5a5d Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 13:36:04 +0200 Subject: [PATCH 08/13] f Reserve manual invoice hashes first Reject duplicate manual BOLT11 hashes from the pending store before creating the invoice. This keeps duplicate detection and registration serialized, while preserving the persist-before-index ordering for crash recovery. Co-Authored-By: HAL 9000 --- src/payment/bolt11.rs | 72 +++++++++++++++++------- src/payment/pending_payment_store.rs | 82 ++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 19 deletions(-) diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index 904d95abcb..e4db3821b5 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -123,19 +123,15 @@ impl Bolt11Payment { Ok(()) } - fn has_pending_inbound_payment(&self, payment_hash: &PaymentHash) -> bool { - self.pending_payment_store.get_pending_manual_bolt11_by_payment_hash(payment_hash).is_some() - } - - fn register_manual_claim_invoice( - &self, payment_hash: PaymentHash, amount_msat: Option, payment_secret: PaymentSecret, + fn pending_manual_claim_invoice( + payment_hash: PaymentHash, amount_msat: Option, payment_secret: Option, expiry_secs: u32, - ) -> Result<(), Error> { + ) -> PendingPaymentDetails { let payment_id = PaymentId(payment_hash.0); let kind = PaymentKind::Bolt11 { hash: payment_hash, preimage: None, - secret: Some(payment_secret), + secret: payment_secret, counterparty_skimmed_fee_msat: None, }; let payment = PaymentDetails::new( @@ -147,22 +143,51 @@ impl Bolt11Payment { PaymentStatus::Pending, ); let expires_at = Some(Self::current_time_secs().saturating_add(expiry_secs as u64)); + PendingPaymentDetails::new_with_expiry(payment, Vec::new(), expires_at) + } + + fn reserve_manual_claim_invoice( + &self, payment_hash: PaymentHash, amount_msat: Option, expiry_secs: u32, + ) -> Result<(), Error> { let pending_payment = - PendingPaymentDetails::new_with_expiry(payment, Vec::new(), expires_at); + Self::pending_manual_claim_invoice(payment_hash, amount_msat, None, expiry_secs); + if let Err(e) = + self.runtime.block_on(self.pending_payment_store.insert_manual_bolt11(pending_payment)) + { + if e == Error::DuplicatePayment { + log_error!(self.logger, "Payment error: an invoice must not be paid twice."); + } + return Err(e); + } + Ok(()) + } + + fn register_manual_claim_invoice( + &self, payment_hash: PaymentHash, amount_msat: Option, payment_secret: PaymentSecret, + expiry_secs: u32, + ) -> Result<(), Error> { + let pending_payment = Self::pending_manual_claim_invoice( + payment_hash, + amount_msat, + Some(payment_secret), + expiry_secs, + ); self.runtime.block_on(self.pending_payment_store.insert_or_update(pending_payment))?; Ok(()) } + fn remove_manual_claim_invoice(&self, payment_hash: PaymentHash) -> Result<(), Error> { + let payment_id = PaymentId(payment_hash.0); + self.runtime.block_on(self.pending_payment_store.remove(&payment_id)) + } + pub(crate) fn receive_inner( &self, amount_msat: Option, invoice_description: &LdkBolt11InvoiceDescription, expiry_secs: u32, manual_claim_payment_hash: Option, ) -> Result { if let Some(payment_hash) = manual_claim_payment_hash { self.prune_expired_pending_payments()?; - if self.has_pending_inbound_payment(&payment_hash) { - log_error!(self.logger, "Payment error: an invoice must not be paid twice."); - return Err(Error::DuplicatePayment); - } + self.reserve_manual_claim_invoice(payment_hash, amount_msat, expiry_secs)?; } let invoice = { @@ -181,6 +206,9 @@ impl Bolt11Payment { }, Err(e) => { log_error!(self.logger, "Failed to create invoice: {}", e); + if let Some(payment_hash) = manual_claim_payment_hash { + self.remove_manual_claim_invoice(payment_hash)?; + } return Err(Error::InvoiceCreationFailed); }, } @@ -234,14 +262,11 @@ impl Bolt11Payment { ) -> Result { if let Some(payment_hash) = payment_hash { self.prune_expired_pending_payments()?; - if self.has_pending_inbound_payment(&payment_hash) { - log_error!(self.logger, "Payment error: an invoice must not be paid twice."); - return Err(Error::DuplicatePayment); - } + self.reserve_manual_claim_invoice(payment_hash, amount_msat, expiry_secs)?; } let connection_manager = Arc::clone(&self.connection_manager); - let (invoice, chosen_lsp) = self.runtime.block_on(async move { + let res = self.runtime.block_on(async move { if let Some(amount_msat) = amount_msat { self.liquidity_source .lsps2_client() @@ -266,7 +291,16 @@ impl Bolt11Payment { ) .await } - })?; + }); + let (invoice, chosen_lsp) = match res { + Ok(res) => res, + Err(e) => { + if let Some(payment_hash) = payment_hash { + self.remove_manual_claim_invoice(payment_hash)?; + } + return Err(e); + }, + }; if let Some(payment_hash) = payment_hash { self.register_manual_claim_invoice( diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index 2ee98dc377..9be2094dbe 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -229,6 +229,33 @@ where &self, pending_payment: PendingPaymentDetails, ) -> Result { let _guard = self.mutation_lock.lock().await; + self.insert_or_update_locked(pending_payment).await + } + + pub(crate) async fn insert_manual_bolt11( + &self, pending_payment: PendingPaymentDetails, + ) -> Result<(), Error> { + let _guard = self.mutation_lock.lock().await; + let Some(payment_hash) = manual_bolt11_payment_hash(&pending_payment.details) else { + debug_assert!(false, "manual BOLT11 insert requires a pending inbound BOLT11 payment"); + self.insert_or_update_locked(pending_payment).await?; + return Ok(()); + }; + let duplicate = { + let index = self.manual_bolt11_payment_hash_index.lock().expect("lock"); + index.get(&payment_hash).map_or(false, |ids| !ids.is_empty()) + }; + if duplicate { + return Err(Error::DuplicatePayment); + } + + self.insert_or_update_locked(pending_payment).await?; + Ok(()) + } + + async fn insert_or_update_locked( + &self, pending_payment: PendingPaymentDetails, + ) -> Result { let id = pending_payment.id(); let before = self.inner.get(&id); let updated = self.inner.insert_or_update(pending_payment).await?; @@ -342,10 +369,14 @@ fn manual_bolt11_payment_hash(payment: &PaymentDetails) -> Option { #[cfg(test)] mod tests { use bitcoin::hashes::Hash; + use lightning::util::test_utils::TestLogger; + use lightning_types::payment::PaymentSecret; use super::*; + use crate::io::test_utils::InMemoryStore; use crate::payment::store::ConfirmationStatus; use crate::payment::{PaymentDirection, PaymentKind, PaymentStatus}; + use crate::types::{DynStore, DynStoreWrapper}; #[test] fn pending_payment_candidate_lookup() { @@ -414,6 +445,57 @@ mod tests { ) } + fn pending_store() -> PendingPaymentStore> { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let logger = Arc::new(TestLogger::new()); + PendingPaymentStore::new( + Vec::new(), + "pending_payment_store_test_primary".to_string(), + "pending_payment_store_test_secondary".to_string(), + store, + logger, + ) + } + + fn pending_manual_bolt11_payment( + payment_hash: PaymentHash, payment_secret: Option, + ) -> PendingPaymentDetails { + let payment_id = PaymentId(payment_hash.0); + let details = PaymentDetails::new( + payment_id, + PaymentKind::Bolt11 { + hash: payment_hash, + preimage: None, + secret: payment_secret, + counterparty_skimmed_fee_msat: None, + }, + Some(1_000), + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); + PendingPaymentDetails::new_with_expiry(details, Vec::new(), Some(1_000_000)) + } + + #[tokio::test] + async fn manual_bolt11_insert_rejects_duplicate_hash() { + let store = pending_store(); + let payment_hash = PaymentHash([42; 32]); + let pending_payment = pending_manual_bolt11_payment(payment_hash, None); + assert_eq!(store.insert_manual_bolt11(pending_payment.clone()).await, Ok(())); + + let duplicate = pending_manual_bolt11_payment(payment_hash, None); + assert_eq!(store.insert_manual_bolt11(duplicate).await, Err(Error::DuplicatePayment)); + assert_eq!( + store.get_pending_manual_bolt11_by_payment_hash(&payment_hash), + Some(pending_payment) + ); + + let updated = pending_manual_bolt11_payment(payment_hash, Some(PaymentSecret([43; 32]))); + assert_eq!(store.insert_or_update(updated.clone()).await, Ok(true)); + assert_eq!(store.get_pending_manual_bolt11_by_payment_hash(&payment_hash), Some(updated)); + } + #[test] fn pending_onchain_conflicts_exclude_current_txid_after_txid_rotation() { let original_txid = test_txid(1); From ccd89715ab80d2524c8765eecd5960fe3ce3c1eb Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 13:59:14 +0200 Subject: [PATCH 09/13] f Batch-prune expired pending payments Use the batch removal path for both manual BOLT11 pruning callsites introduced by the manual-invoice tracking change. Co-Authored-By: HAL 9000 --- src/event.rs | 13 +++---------- src/payment/bolt11.rs | 6 +----- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/event.rs b/src/event.rs index 50debc66c7..6b5317f60f 100644 --- a/src/event.rs +++ b/src/event.rs @@ -624,16 +624,9 @@ where .map(|payment| payment.details.id) .collect::>(); - for payment_id in expired_payment_ids { - if let Err(e) = self.pending_payment_store.remove(&payment_id).await { - log_error!( - self.logger, - "Failed to remove expired pending payment with ID {}: {}", - payment_id, - e - ); - return Err(ReplayEvent()); - } + if let Err(e) = self.pending_payment_store.remove_batch(&expired_payment_ids).await { + log_error!(self.logger, "Failed to remove expired pending payments: {}", e); + return Err(ReplayEvent()); } Ok(()) diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index e4db3821b5..419001f80f 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -116,11 +116,7 @@ impl Bolt11Payment { .map(|payment| payment.details.id) .collect::>(); - for payment_id in expired_payment_ids { - self.runtime.block_on(self.pending_payment_store.remove(&payment_id))?; - } - - Ok(()) + self.runtime.block_on(self.pending_payment_store.remove_batch(&expired_payment_ids)) } fn pending_manual_claim_invoice( From 1d29fcc6a1c9b0fdffd5bf6547485c51da16f98f Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 14:29:10 +0200 Subject: [PATCH 10/13] f Use claim-height pending expiry Keep manual BOLT11 invoice reservations time-based until the claimable event arrives, then prune by the HTLC claim deadline height so cleanup cannot run before the claim window closes. Co-Authored-By: HAL 9000 --- src/event.rs | 8 +++++--- src/payment/bolt11.rs | 10 ++++++---- src/payment/pending_payment_store.rs | 6 +++++- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/event.rs b/src/event.rs index 6b5317f60f..c4d5c03f3a 100644 --- a/src/event.rs +++ b/src/event.rs @@ -52,7 +52,7 @@ use crate::payment::asynchronous::static_invoice_store::StaticInvoiceStore; use crate::payment::store::{ PaymentDetails, PaymentDetailsUpdate, PaymentDirection, PaymentKind, PaymentStatus, }; -use crate::payment::{PaymentMetadata, PendingPaymentDetails}; +use crate::payment::{PaymentMetadata, PendingPaymentDetails, PendingPaymentExpiry}; use crate::probing::Prober; use crate::runtime::Runtime; use crate::types::{ @@ -617,9 +617,10 @@ where async fn prune_expired_pending_payments(&self) -> Result<(), ReplayEvent> { let now = Self::current_time_secs(); + let current_height = self.channel_manager.current_best_block().height; let expired_payment_ids = self .pending_payment_store - .list_filter(|payment| payment.has_expired(now)) + .list_filter(|payment| payment.has_expired(now, current_height)) .into_iter() .map(|payment| payment.details.id) .collect::>(); @@ -932,7 +933,8 @@ where } let mut pending_payment = pending_payment.clone(); - pending_payment.expires_at = None; + pending_payment.expiry = + claim_deadline.map(|height| PendingPaymentExpiry::Height { height }); if let Err(e) = self.pending_payment_store.insert_or_update(pending_payment).await { diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index 419001f80f..7e4a558a98 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -36,7 +36,7 @@ use crate::payment::store::{ LSPS2Parameters, PaymentDetails, PaymentDetailsUpdate, PaymentDirection, PaymentKind, PaymentStatus, }; -use crate::payment::PendingPaymentDetails; +use crate::payment::{PendingPaymentDetails, PendingPaymentExpiry}; use crate::peer_store::{PeerInfo, PeerStore}; use crate::runtime::Runtime; use crate::types::{ChannelManager, PaymentStore, PendingPaymentStore}; @@ -109,9 +109,10 @@ impl Bolt11Payment { fn prune_expired_pending_payments(&self) -> Result<(), Error> { let now = Self::current_time_secs(); + let current_height = self.channel_manager.current_best_block().height; let expired_payment_ids = self .pending_payment_store - .list_filter(|payment| payment.has_expired(now)) + .list_filter(|payment| payment.has_expired(now, current_height)) .into_iter() .map(|payment| payment.details.id) .collect::>(); @@ -138,8 +139,9 @@ impl Bolt11Payment { PaymentDirection::Inbound, PaymentStatus::Pending, ); - let expires_at = Some(Self::current_time_secs().saturating_add(expiry_secs as u64)); - PendingPaymentDetails::new_with_expiry(payment, Vec::new(), expires_at) + let timestamp = Self::current_time_secs().saturating_add(expiry_secs as u64); + let expiry = Some(PendingPaymentExpiry::Time { timestamp }); + PendingPaymentDetails::new_with_expiry(payment, Vec::new(), expiry) } fn reserve_manual_claim_invoice( diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index 9be2094dbe..f80ddc447f 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -474,7 +474,11 @@ mod tests { PaymentDirection::Inbound, PaymentStatus::Pending, ); - PendingPaymentDetails::new_with_expiry(details, Vec::new(), Some(1_000_000)) + PendingPaymentDetails::new_with_expiry( + details, + Vec::new(), + Some(PendingPaymentExpiry::Time { timestamp: 1_000_000 }), + ) } #[tokio::test] From 16f33f1b4cf48181dd77d3b0bf8f9f84d5bc99a0 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 15:11:03 +0200 Subject: [PATCH 11/13] f Keep pending index consistent on prune fail A failed batch prune can already have durably removed earlier entries. Keep the pending-payment secondary index in step with those partial removals so a retry can finish pruning and later reuse expired manual hashes. Co-Authored-By: HAL 9000 --- src/data_store.rs | 17 +++- src/payment/pending_payment_store.rs | 128 ++++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 5 deletions(-) diff --git a/src/data_store.rs b/src/data_store.rs index f1af820a4b..4a1bb9b7cb 100644 --- a/src/data_store.rs +++ b/src/data_store.rs @@ -117,6 +117,14 @@ where } pub(crate) async fn remove_batch(&self, ids: &[SO::Id]) -> Result, Error> { + let (removed_objects, result) = self.remove_batch_with_partial_result(ids).await; + result?; + Ok(removed_objects) + } + + pub(crate) async fn remove_batch_with_partial_result( + &self, ids: &[SO::Id], + ) -> (Vec, Result<(), Error>) { let _guard = self.mutation_lock.lock().await; let mut removed_objects = Vec::new(); for id in ids { @@ -126,7 +134,7 @@ where } let store_key = id.encode_to_hex_str(); - KVStore::remove( + let remove_result = KVStore::remove( &*self.kv_store, &self.primary_namespace, &self.secondary_namespace, @@ -144,13 +152,16 @@ where e ); Error::PersistenceFailed - })?; + }); + if let Err(e) = remove_result { + return (removed_objects, Err(e)); + } if let Some(object) = self.objects.lock().expect("lock").remove(id) { removed_objects.push(object); } } - Ok(removed_objects) + (removed_objects, Ok(())) } /// Returns the current in-memory object for `id`. diff --git a/src/payment/pending_payment_store.rs b/src/payment/pending_payment_store.rs index f80ddc447f..8b5f9e2487 100644 --- a/src/payment/pending_payment_store.rs +++ b/src/payment/pending_payment_store.rs @@ -272,11 +272,11 @@ where pub(crate) async fn remove_batch(&self, ids: &[PaymentId]) -> Result<(), Error> { let _guard = self.mutation_lock.lock().await; - let removed_payments = self.inner.remove_batch(ids).await?; + let (removed_payments, result) = self.inner.remove_batch_with_partial_result(ids).await; for payment in removed_payments { self.remove_from_index(&payment); } - Ok(()) + result } pub(crate) fn get(&self, id: &PaymentId) -> Option { @@ -368,7 +368,13 @@ fn manual_bolt11_payment_hash(payment: &PaymentDetails) -> Option { #[cfg(test)] mod tests { + use std::future::Future; + use std::pin::Pin; + use std::sync::atomic::{AtomicUsize, Ordering}; + use bitcoin::hashes::Hash; + use lightning::io; + use lightning::util::persist::{KVStore, PageToken, PaginatedKVStore, PaginatedListResponse}; use lightning::util::test_utils::TestLogger; use lightning_types::payment::PaymentSecret; @@ -457,6 +463,83 @@ mod tests { ) } + struct FailSecondRemoveStore { + inner: InMemoryStore, + remove_calls: AtomicUsize, + } + + impl FailSecondRemoveStore { + fn new() -> Self { + Self { inner: InMemoryStore::new(), remove_calls: AtomicUsize::new(0) } + } + } + + impl KVStore for FailSecondRemoveStore { + fn read( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, + ) -> impl std::future::Future, io::Error>> + 'static + Send { + KVStore::read(&self.inner, primary_namespace, secondary_namespace, key) + } + + fn write( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec, + ) -> impl std::future::Future> + 'static + Send { + KVStore::write(&self.inner, primary_namespace, secondary_namespace, key, buf) + } + + fn remove( + &self, primary_namespace: &str, secondary_namespace: &str, key: &str, _lazy: bool, + ) -> impl std::future::Future> + 'static + Send { + let remove_call = self.remove_calls.fetch_add(1, Ordering::Relaxed) + 1; + let fut: Pin> + Send>> = + if remove_call == 2 { + Box::pin(async { Err(io::Error::new(io::ErrorKind::Other, "remove failed")) }) + } else { + Box::pin(KVStore::remove( + &self.inner, + primary_namespace, + secondary_namespace, + key, + false, + )) + }; + fut + } + + fn list( + &self, primary_namespace: &str, secondary_namespace: &str, + ) -> impl std::future::Future, io::Error>> + 'static + Send { + KVStore::list(&self.inner, primary_namespace, secondary_namespace) + } + } + + impl PaginatedKVStore for FailSecondRemoveStore { + fn list_paginated( + &self, primary_namespace: &str, secondary_namespace: &str, + page_token: Option, + ) -> impl std::future::Future> + 'static + Send + { + PaginatedKVStore::list_paginated( + &self.inner, + primary_namespace, + secondary_namespace, + page_token, + ) + } + } + + fn pending_store_with_fail_second_remove() -> PendingPaymentStore> { + let store: Arc = Arc::new(DynStoreWrapper(FailSecondRemoveStore::new())); + let logger = Arc::new(TestLogger::new()); + PendingPaymentStore::new( + Vec::new(), + "pending_payment_store_test_primary".to_string(), + "pending_payment_store_test_secondary".to_string(), + store, + logger, + ) + } + fn pending_manual_bolt11_payment( payment_hash: PaymentHash, payment_secret: Option, ) -> PendingPaymentDetails { @@ -481,6 +564,17 @@ mod tests { ) } + async fn prune_expired( + store: &PendingPaymentStore>, now: u64, + ) -> Result<(), Error> { + let expired_payment_ids = store + .list_filter(|payment| payment.has_expired(now, 0)) + .into_iter() + .map(|payment| payment.details.id) + .collect::>(); + store.remove_batch(&expired_payment_ids).await + } + #[tokio::test] async fn manual_bolt11_insert_rejects_duplicate_hash() { let store = pending_store(); @@ -500,6 +594,36 @@ mod tests { assert_eq!(store.get_pending_manual_bolt11_by_payment_hash(&payment_hash), Some(updated)); } + #[tokio::test] + async fn expired_manual_bolt11_entries_can_be_retried_after_partial_prune_failure() { + let store = pending_store_with_fail_second_remove(); + let first_hash = PaymentHash([41; 32]); + let second_hash = PaymentHash([42; 32]); + assert_eq!( + store.insert_manual_bolt11(pending_manual_bolt11_payment(first_hash, None)).await, + Ok(()) + ); + assert_eq!( + store.insert_manual_bolt11(pending_manual_bolt11_payment(second_hash, None)).await, + Ok(()) + ); + + assert_eq!(prune_expired(&store, 1_000_001).await, Err(Error::PersistenceFailed)); + assert_eq!(store.list_filter(|_| true).len(), 1); + + assert_eq!(prune_expired(&store, 1_000_001).await, Ok(())); + assert!(store.list_filter(|_| true).is_empty()); + + assert_eq!( + store.insert_manual_bolt11(pending_manual_bolt11_payment(first_hash, None)).await, + Ok(()) + ); + assert_eq!( + store.insert_manual_bolt11(pending_manual_bolt11_payment(second_hash, None)).await, + Ok(()) + ); + } + #[test] fn pending_onchain_conflicts_exclude_current_txid_after_txid_rotation() { let original_txid = test_txid(1); From bf7aadbc4cc8e6f801da9ce925c57537197f67ba Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 2 Jul 2026 14:05:16 +0200 Subject: [PATCH 12/13] Use payment IDs for BOLT11 payments Create inbound BOLT11 records from claimable and claimed events. Stop pre-creating automatic BOLT11 records when invoices are generated. Generate outbound BOLT11 IDs from KeysManager entropy instead of deriving them from the payment hash. Co-Authored-By: HAL 9000 --- bindings/python/src/ldk_node/test_ldk_node.py | 2 +- src/event.rs | 412 +++++++++++++----- src/lib.rs | 2 + src/payment/bolt11.rs | 273 ++++++------ tests/common/mod.rs | 259 +++++------ tests/integration_tests_rust.rs | 65 ++- 6 files changed, 586 insertions(+), 427 deletions(-) diff --git a/bindings/python/src/ldk_node/test_ldk_node.py b/bindings/python/src/ldk_node/test_ldk_node.py index 304caf9c04..063c95f700 100644 --- a/bindings/python/src/ldk_node/test_ldk_node.py +++ b/bindings/python/src/ldk_node/test_ldk_node.py @@ -235,7 +235,7 @@ def test_spontaneous_payment(self): self.assertEqual(received_event.custom_records, custom_tlvs) sender_payment = node_1.payment(keysend_payment_id) - receiver_payment = node_2.payment(keysend_payment_id) + receiver_payment = node_2.payment(received_event.payment_id) self.assertIsNotNone(sender_payment) self.assertIsNotNone(receiver_payment) diff --git a/src/event.rs b/src/event.rs index c4d5c03f3a..cc5586aeb6 100644 --- a/src/event.rs +++ b/src/event.rs @@ -198,15 +198,15 @@ pub enum Event { }, /// A payment for a previously-registered payment hash has been received. /// - /// This needs to be manually claimed by supplying the correct preimage to [`claim_for_hash`]. + /// This needs to be manually claimed by supplying the correct preimage to [`claim_for_id`]. /// /// If the provided parameters don't match the expectations or the preimage can't be - /// retrieved in time, should be failed-back via [`fail_for_hash`]. + /// retrieved in time, should be failed-back via [`fail_for_id`]. /// /// Note claiming will necessarily fail after the `claim_deadline` has been reached. /// - /// [`claim_for_hash`]: crate::payment::Bolt11Payment::claim_for_hash - /// [`fail_for_hash`]: crate::payment::Bolt11Payment::fail_for_hash + /// [`claim_for_id`]: crate::payment::Bolt11Payment::claim_for_id + /// [`fail_for_id`]: crate::payment::Bolt11Payment::fail_for_id PaymentClaimable { /// A local identifier used to track the payment. payment_id: PaymentId, @@ -640,6 +640,23 @@ where Ok(self.pending_payment_store.get_pending_manual_bolt11_by_payment_hash(payment_hash)) } + fn resolve_inbound_payment_id( + &self, event_payment_id: PaymentId, payment_hash: &PaymentHash, + ) -> (PaymentId, Option) { + if let Some(info) = self.payment_store.get(&event_payment_id) { + return (event_payment_id, Some(info)); + } + + let legacy_id = PaymentId(payment_hash.0); + if legacy_id != event_payment_id { + if let Some(info) = self.payment_store.get(&legacy_id) { + return (legacy_id, Some(info)); + } + } + + (event_payment_id, None) + } + pub async fn handle_event(&self, event: LdkEvent) -> Result<(), ReplayEvent> { match event { LdkEvent::FundingGenerationReady { @@ -743,6 +760,7 @@ where .lsps2_funding_tx_broadcast_safe(user_channel_id, counterparty_node_id); }, LdkEvent::PaymentClaimable { + payment_id, payment_hash, purpose, amount_msat, @@ -751,13 +769,17 @@ where counterparty_skimmed_fee_msat, .. } => { - let payment_id = PaymentId(payment_hash.0); - let payment_info = self.payment_store.get(&payment_id); + let event_payment_id = payment_id.unwrap_or(PaymentId(payment_hash.0)); + let (mut payment_id, payment_info) = + self.resolve_inbound_payment_id(event_payment_id, &payment_hash); let pending_payment = if payment_info.is_none() { self.find_pending_inbound_payment(&payment_hash).await? } else { None }; + if let Some(pending_payment) = pending_payment.as_ref() { + payment_id = pending_payment.details.id; + } if let Some(info) = payment_info.as_ref() { if info.direction == PaymentDirection::Outbound { log_info!( @@ -952,11 +974,11 @@ where payment_info }; - if let Some(info) = payment_info { + if let Some(info) = payment_info.as_ref() { // If this is known by the store but ChannelManager doesn't know the preimage, // the payment has been registered via `_for_hash` variants and needs to be manually claimed via // user interaction. - match info.kind { + match &info.kind { PaymentKind::Bolt11 { preimage, .. } => { if purpose.preimage().is_none() { debug_assert!( @@ -1000,7 +1022,55 @@ where amount_msat, ); let payment_preimage = match purpose { - PaymentPurpose::Bolt11InvoicePayment { payment_preimage, .. } => { + PaymentPurpose::Bolt11InvoicePayment { + payment_preimage, + payment_secret, + .. + } => { + if payment_info.is_none() { + let kind = PaymentKind::Bolt11 { + hash: payment_hash, + preimage: payment_preimage, + secret: Some(payment_secret), + counterparty_skimmed_fee_msat: if counterparty_skimmed_fee_msat > 0 + { + Some(counterparty_skimmed_fee_msat) + } else { + None + }, + }; + + let payment = PaymentDetails::new( + payment_id, + kind, + Some(amount_msat), + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); + + match self.payment_store.insert(payment).await { + Ok(false) => (), + Ok(true) => { + log_error!( + self.logger, + "Bolt11InvoicePayment with ID {} was previously known", + payment_id, + ); + debug_assert!(false); + }, + Err(e) => { + log_error!( + self.logger, + "Failed to insert payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + }, + } + } + payment_preimage }, PaymentPurpose::Bolt12OfferPayment { @@ -1012,84 +1082,130 @@ where let payer_note = payment_context.invoice_request.payer_note_truncated; let offer_id = payment_context.offer_id; let quantity = payment_context.invoice_request.quantity; - let kind = PaymentKind::Bolt12Offer { - hash: Some(payment_hash), - preimage: payment_preimage, - secret: Some(payment_secret), - offer_id, - payer_note, - quantity, - }; - - let payment = PaymentDetails::new( - payment_id, - kind, - Some(amount_msat), - None, - PaymentDirection::Inbound, - PaymentStatus::Pending, - ); + if payment_info.is_none() { + let kind = PaymentKind::Bolt12Offer { + hash: Some(payment_hash), + preimage: payment_preimage, + secret: Some(payment_secret), + offer_id, + payer_note, + quantity, + }; + + let payment = PaymentDetails::new( + payment_id, + kind, + Some(amount_msat), + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); - match self.payment_store.insert(payment).await { - Ok(false) => (), - Ok(true) => { - log_error!( - self.logger, - "Bolt12OfferPayment with ID {} was previously known", - payment_id, - ); - debug_assert!(false); - }, - Err(e) => { - log_error!( - self.logger, - "Failed to insert payment with ID {}: {}", - payment_id, - e - ); - debug_assert!(false); - }, + match self.payment_store.insert(payment).await { + Ok(false) => (), + Ok(true) => { + log_error!( + self.logger, + "Bolt12OfferPayment with ID {} was previously known", + payment_id, + ); + debug_assert!(false); + }, + Err(e) => { + log_error!( + self.logger, + "Failed to insert payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + }, + } } payment_preimage }, - PaymentPurpose::Bolt12RefundPayment { payment_preimage, .. } => { + PaymentPurpose::Bolt12RefundPayment { + payment_preimage, + payment_secret, + .. + } => { + if payment_info.is_none() { + let kind = PaymentKind::Bolt12Refund { + hash: Some(payment_hash), + preimage: payment_preimage, + secret: Some(payment_secret), + payer_note: None, + quantity: None, + }; + + let payment = PaymentDetails::new( + payment_id, + kind, + Some(amount_msat), + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); + + match self.payment_store.insert(payment).await { + Ok(false) => (), + Ok(true) => { + log_error!( + self.logger, + "Bolt12RefundPayment with ID {} was previously known", + payment_id, + ); + debug_assert!(false); + }, + Err(e) => { + log_error!( + self.logger, + "Failed to insert payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + }, + } + } payment_preimage }, PaymentPurpose::SpontaneousPayment(preimage) => { - // Since it's spontaneous, we insert it now into our store. - let kind = PaymentKind::Spontaneous { - hash: payment_hash, - preimage: Some(preimage), - }; + if payment_info.is_none() { + let kind = PaymentKind::Spontaneous { + hash: payment_hash, + preimage: Some(preimage), + }; - let payment = PaymentDetails::new( - payment_id, - kind, - Some(amount_msat), - None, - PaymentDirection::Inbound, - PaymentStatus::Pending, - ); + let payment = PaymentDetails::new( + payment_id, + kind, + Some(amount_msat), + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); - match self.payment_store.insert(payment).await { - Ok(false) => (), - Ok(true) => { - log_error!( - self.logger, - "Spontaneous payment with ID {} was previously known", - payment_id, - ); - debug_assert!(false); - }, - Err(e) => { - log_error!( - self.logger, - "Failed to insert payment with ID {}: {}", - payment_id, - e - ); - debug_assert!(false); - }, + match self.payment_store.insert(payment).await { + Ok(false) => (), + Ok(true) => { + log_error!( + self.logger, + "Spontaneous payment with ID {} was previously known", + payment_id, + ); + debug_assert!(false); + }, + Err(e) => { + log_error!( + self.logger, + "Failed to insert payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + }, + } } Some(preimage) @@ -1121,6 +1237,7 @@ where } }, LdkEvent::PaymentClaimed { + payment_id, payment_hash, purpose, amount_msat, @@ -1128,9 +1245,18 @@ where htlcs: _, sender_intended_total_msat: _, onion_fields, - payment_id: _, } => { - let payment_id = PaymentId(payment_hash.0); + let event_payment_id = payment_id.unwrap_or(PaymentId(payment_hash.0)); + let (mut payment_id, payment_info) = + self.resolve_inbound_payment_id(event_payment_id, &payment_hash); + let pending_payment = if payment_info.is_none() { + self.find_pending_inbound_payment(&payment_hash).await? + } else { + None + }; + if let Some(pending_payment) = pending_payment.as_ref() { + payment_id = pending_payment.details.id; + } log_info!( self.logger, "Claimed payment with ID {} from payment hash {} of {}msat.", @@ -1139,43 +1265,83 @@ where amount_msat, ); - let update = match purpose { + let (update, kind_for_insert) = match purpose { PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. - } => PaymentDetailsUpdate { - preimage: Some(payment_preimage), - secret: Some(Some(payment_secret)), - amount_msat: Some(Some(amount_msat)), - status: Some(PaymentStatus::Succeeded), - ..PaymentDetailsUpdate::new(payment_id) + } => { + let kind = PaymentKind::Bolt11 { + hash: payment_hash, + preimage: payment_preimage, + secret: Some(payment_secret), + counterparty_skimmed_fee_msat: None, + }; + let update = PaymentDetailsUpdate { + preimage: Some(payment_preimage), + secret: Some(Some(payment_secret)), + amount_msat: Some(Some(amount_msat)), + status: Some(PaymentStatus::Succeeded), + ..PaymentDetailsUpdate::new(payment_id) + }; + (update, kind) }, PaymentPurpose::Bolt12OfferPayment { - payment_preimage, payment_secret, .. - } => PaymentDetailsUpdate { - preimage: Some(payment_preimage), - secret: Some(Some(payment_secret)), - amount_msat: Some(Some(amount_msat)), - status: Some(PaymentStatus::Succeeded), - ..PaymentDetailsUpdate::new(payment_id) + payment_preimage, + payment_secret, + payment_context, + .. + } => { + let kind = PaymentKind::Bolt12Offer { + hash: Some(payment_hash), + preimage: payment_preimage, + secret: Some(payment_secret), + offer_id: payment_context.offer_id, + payer_note: payment_context.invoice_request.payer_note_truncated, + quantity: payment_context.invoice_request.quantity, + }; + let update = PaymentDetailsUpdate { + preimage: Some(payment_preimage), + secret: Some(Some(payment_secret)), + amount_msat: Some(Some(amount_msat)), + status: Some(PaymentStatus::Succeeded), + ..PaymentDetailsUpdate::new(payment_id) + }; + (update, kind) }, PaymentPurpose::Bolt12RefundPayment { payment_preimage, payment_secret, .. - } => PaymentDetailsUpdate { - preimage: Some(payment_preimage), - secret: Some(Some(payment_secret)), - amount_msat: Some(Some(amount_msat)), - status: Some(PaymentStatus::Succeeded), - ..PaymentDetailsUpdate::new(payment_id) + } => { + let kind = PaymentKind::Bolt12Refund { + hash: Some(payment_hash), + preimage: payment_preimage, + secret: Some(payment_secret), + payer_note: None, + quantity: None, + }; + let update = PaymentDetailsUpdate { + preimage: Some(payment_preimage), + secret: Some(Some(payment_secret)), + amount_msat: Some(Some(amount_msat)), + status: Some(PaymentStatus::Succeeded), + ..PaymentDetailsUpdate::new(payment_id) + }; + (update, kind) }, - PaymentPurpose::SpontaneousPayment(preimage) => PaymentDetailsUpdate { - preimage: Some(Some(preimage)), - amount_msat: Some(Some(amount_msat)), - status: Some(PaymentStatus::Succeeded), - ..PaymentDetailsUpdate::new(payment_id) + PaymentPurpose::SpontaneousPayment(preimage) => { + let kind = PaymentKind::Spontaneous { + hash: payment_hash, + preimage: Some(preimage), + }; + let update = PaymentDetailsUpdate { + preimage: Some(Some(preimage)), + amount_msat: Some(Some(amount_msat)), + status: Some(PaymentStatus::Succeeded), + ..PaymentDetailsUpdate::new(payment_id) + }; + (update, kind) }, }; @@ -1185,11 +1351,23 @@ where // be the result of a replayed event. ), Ok(DataStoreUpdateResult::NotFound) => { - log_error!( - self.logger, - "Claimed payment with ID {} couldn't be found in store", + let payment = PaymentDetails::new( payment_id, + kind_for_insert, + Some(amount_msat), + None, + PaymentDirection::Inbound, + PaymentStatus::Succeeded, ); + if let Err(e) = self.payment_store.insert(payment).await { + log_error!( + self.logger, + "Failed to insert payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + } }, Err(e) => { log_error!( @@ -1202,14 +1380,16 @@ where }, } - if let Err(e) = self.pending_payment_store.remove(&payment_id).await { - log_error!( - self.logger, - "Failed to remove pending payment with ID {}: {}", - payment_id, - e - ); - return Err(ReplayEvent()); + if pending_payment.is_some() { + if let Err(e) = self.pending_payment_store.remove(&payment_id).await { + log_error!( + self.logger, + "Failed to remove pending payment with ID {}: {}", + payment_id, + e + ); + return Err(ReplayEvent()); + } } let event = Event::PaymentReceived { diff --git a/src/lib.rs b/src/lib.rs index a06b10af93..db3d4bb04a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -961,6 +961,7 @@ impl Node { Bolt11Payment::new( Arc::clone(&self.runtime), Arc::clone(&self.channel_manager), + Arc::clone(&self.keys_manager), Arc::clone(&self.connection_manager), Arc::clone(&self.liquidity_source), Arc::clone(&self.payment_store), @@ -980,6 +981,7 @@ impl Node { Arc::new(Bolt11Payment::new( Arc::clone(&self.runtime), Arc::clone(&self.channel_manager), + Arc::clone(&self.keys_manager), Arc::clone(&self.connection_manager), Arc::clone(&self.liquidity_source), Arc::clone(&self.payment_store), diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index 7e4a558a98..8c9271f67c 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -20,6 +20,7 @@ use lightning::ln::channelmanager::{ }; use lightning::ln::outbound_payment::{Bolt11PaymentError, Retry, RetryableSendFailure}; use lightning::routing::router::{PaymentParameters, RouteParameters, RouteParametersConfig}; +use lightning::sign::EntropySource; use lightning_invoice::{ Bolt11Invoice as LdkBolt11Invoice, Bolt11InvoiceDescription as LdkBolt11InvoiceDescription, }; @@ -39,7 +40,7 @@ use crate::payment::store::{ use crate::payment::{PendingPaymentDetails, PendingPaymentExpiry}; use crate::peer_store::{PeerInfo, PeerStore}; use crate::runtime::Runtime; -use crate::types::{ChannelManager, PaymentStore, PendingPaymentStore}; +use crate::types::{ChannelManager, KeysManager, PaymentStore, PendingPaymentStore}; #[cfg(not(feature = "uniffi"))] type Bolt11Invoice = LdkBolt11Invoice; @@ -71,6 +72,7 @@ impl_writeable_tlv_based!(PaymentMetadata, { pub struct Bolt11Payment { runtime: Arc, channel_manager: Arc, + keys_manager: Arc, connection_manager: Arc>>, liquidity_source: Arc>>, payment_store: Arc, @@ -84,7 +86,7 @@ pub struct Bolt11Payment { impl Bolt11Payment { pub(crate) fn new( runtime: Arc, channel_manager: Arc, - connection_manager: Arc>>, + keys_manager: Arc, connection_manager: Arc>>, liquidity_source: Arc>>, payment_store: Arc, pending_payment_store: Arc, peer_store: Arc>>, config: Arc, is_running: Arc>, logger: Arc, @@ -92,6 +94,7 @@ impl Bolt11Payment { Self { runtime, channel_manager, + keys_manager, connection_manager, liquidity_source, payment_store, @@ -121,10 +124,9 @@ impl Bolt11Payment { } fn pending_manual_claim_invoice( - payment_hash: PaymentHash, amount_msat: Option, payment_secret: Option, - expiry_secs: u32, + payment_id: PaymentId, payment_hash: PaymentHash, amount_msat: Option, + payment_secret: Option, expiry_secs: u32, ) -> PendingPaymentDetails { - let payment_id = PaymentId(payment_hash.0); let kind = PaymentKind::Bolt11 { hash: payment_hash, preimage: None, @@ -146,9 +148,15 @@ impl Bolt11Payment { fn reserve_manual_claim_invoice( &self, payment_hash: PaymentHash, amount_msat: Option, expiry_secs: u32, - ) -> Result<(), Error> { - let pending_payment = - Self::pending_manual_claim_invoice(payment_hash, amount_msat, None, expiry_secs); + ) -> Result { + let payment_id = PaymentId(self.keys_manager.get_secure_random_bytes()); + let pending_payment = Self::pending_manual_claim_invoice( + payment_id, + payment_hash, + amount_msat, + None, + expiry_secs, + ); if let Err(e) = self.runtime.block_on(self.pending_payment_store.insert_manual_bolt11(pending_payment)) { @@ -157,14 +165,15 @@ impl Bolt11Payment { } return Err(e); } - Ok(()) + Ok(payment_id) } fn register_manual_claim_invoice( - &self, payment_hash: PaymentHash, amount_msat: Option, payment_secret: PaymentSecret, - expiry_secs: u32, + &self, payment_id: PaymentId, payment_hash: PaymentHash, amount_msat: Option, + payment_secret: PaymentSecret, expiry_secs: u32, ) -> Result<(), Error> { let pending_payment = Self::pending_manual_claim_invoice( + payment_id, payment_hash, amount_msat, Some(payment_secret), @@ -174,8 +183,7 @@ impl Bolt11Payment { Ok(()) } - fn remove_manual_claim_invoice(&self, payment_hash: PaymentHash) -> Result<(), Error> { - let payment_id = PaymentId(payment_hash.0); + fn remove_manual_claim_invoice(&self, payment_id: PaymentId) -> Result<(), Error> { self.runtime.block_on(self.pending_payment_store.remove(&payment_id)) } @@ -183,10 +191,12 @@ impl Bolt11Payment { &self, amount_msat: Option, invoice_description: &LdkBolt11InvoiceDescription, expiry_secs: u32, manual_claim_payment_hash: Option, ) -> Result { - if let Some(payment_hash) = manual_claim_payment_hash { + let manual_claim_payment_id = if let Some(payment_hash) = manual_claim_payment_hash { self.prune_expired_pending_payments()?; - self.reserve_manual_claim_invoice(payment_hash, amount_msat, expiry_secs)?; - } + Some(self.reserve_manual_claim_invoice(payment_hash, amount_msat, expiry_secs)?) + } else { + None + }; let invoice = { let invoice_params = Bolt11InvoiceParameters { @@ -204,50 +214,24 @@ impl Bolt11Payment { }, Err(e) => { log_error!(self.logger, "Failed to create invoice: {}", e); - if let Some(payment_hash) = manual_claim_payment_hash { - self.remove_manual_claim_invoice(payment_hash)?; + if let Some(payment_id) = manual_claim_payment_id { + self.remove_manual_claim_invoice(payment_id)?; } return Err(Error::InvoiceCreationFailed); }, } }; - if let Some(payment_hash) = manual_claim_payment_hash { + if let (Some(payment_hash), Some(payment_id)) = + (manual_claim_payment_hash, manual_claim_payment_id) + { self.register_manual_claim_invoice( + payment_id, payment_hash, amount_msat, *invoice.payment_secret(), expiry_secs, )?; - } else { - let payment_hash = invoice.payment_hash(); - let payment_secret = invoice.payment_secret(); - let id = PaymentId(payment_hash.0); - let mut payment_metadata = invoice.payment_metadata().cloned(); - let preimage = self - .channel_manager - .get_payment_preimage_decrypt_metadata( - payment_hash, - payment_secret.clone(), - payment_metadata.as_deref_mut(), - ) - .ok(); - debug_assert!(preimage.is_some(), "We just let ChannelManager create an inbound payment, it can't have forgotten the preimage by now."); - let kind = PaymentKind::Bolt11 { - hash: payment_hash, - preimage, - secret: Some(payment_secret.clone()), - counterparty_skimmed_fee_msat: None, - }; - let payment = PaymentDetails::new( - id, - kind, - amount_msat, - None, - PaymentDirection::Inbound, - PaymentStatus::Pending, - ); - self.runtime.block_on(self.payment_store.insert(payment))?; } Ok(invoice) @@ -258,10 +242,12 @@ impl Bolt11Payment { expiry_secs: u32, max_total_lsp_fee_limit_msat: Option, max_proportional_lsp_fee_limit_ppm_msat: Option, payment_hash: Option, ) -> Result { - if let Some(payment_hash) = payment_hash { + let manual_claim_payment_id = if let Some(payment_hash) = payment_hash { self.prune_expired_pending_payments()?; - self.reserve_manual_claim_invoice(payment_hash, amount_msat, expiry_secs)?; - } + Some(self.reserve_manual_claim_invoice(payment_hash, amount_msat, expiry_secs)?) + } else { + None + }; let connection_manager = Arc::clone(&self.connection_manager); let res = self.runtime.block_on(async move { @@ -293,55 +279,27 @@ impl Bolt11Payment { let (invoice, chosen_lsp) = match res { Ok(res) => res, Err(e) => { - if let Some(payment_hash) = payment_hash { - self.remove_manual_claim_invoice(payment_hash)?; + if let Some(payment_id) = manual_claim_payment_id { + self.remove_manual_claim_invoice(payment_id)?; } return Err(e); }, }; - if let Some(payment_hash) = payment_hash { + // Persist the chosen LSP peer to make sure we reconnect on restart. + let peer_info = PeerInfo { node_id: chosen_lsp.node_id, address: chosen_lsp.address }; + self.runtime.block_on(self.peer_store.add_peer(peer_info))?; + + if let (Some(payment_hash), Some(payment_id)) = (payment_hash, manual_claim_payment_id) { self.register_manual_claim_invoice( + payment_id, payment_hash, amount_msat, *invoice.payment_secret(), expiry_secs, )?; - } else { - // Register payment in payment store. - let payment_hash = invoice.payment_hash(); - let payment_secret = invoice.payment_secret(); - let id = PaymentId(payment_hash.0); - let mut payment_metadata = invoice.payment_metadata().cloned(); - let preimage = self - .channel_manager - .get_payment_preimage_decrypt_metadata( - payment_hash, - payment_secret.clone(), - payment_metadata.as_deref_mut(), - ) - .ok(); - let kind = PaymentKind::Bolt11 { - hash: payment_hash, - preimage, - secret: Some(payment_secret.clone()), - counterparty_skimmed_fee_msat: None, - }; - let payment = PaymentDetails::new( - id, - kind, - amount_msat, - None, - PaymentDirection::Inbound, - PaymentStatus::Pending, - ); - self.runtime.block_on(self.payment_store.insert(payment))?; } - // Persist the chosen LSP peer to make sure we reconnect on restart. - let peer_info = PeerInfo { node_id: chosen_lsp.node_id, address: chosen_lsp.address }; - self.runtime.block_on(self.peer_store.add_peer(peer_info))?; - Ok(invoice) } } @@ -388,15 +346,7 @@ impl Bolt11Payment { } let payment_hash = invoice.payment_hash(); - let payment_id = PaymentId(invoice.payment_hash().0); - if let Some(payment) = self.payment_store.get(&payment_id) { - if payment.status == PaymentStatus::Pending - || payment.status == PaymentStatus::Succeeded - { - log_error!(self.logger, "Payment error: an invoice must not be paid twice."); - return Err(Error::DuplicatePayment); - } - } + let payment_id = PaymentId(self.keys_manager.get_secure_random_bytes()); let route_params_config = route_parameters.or(self.config.route_parameters).unwrap_or_default(); @@ -603,13 +553,31 @@ impl Bolt11Payment { /// [`receive_variable_amount_for_hash`]: Self::receive_variable_amount_for_hash /// [`PaymentClaimable`]: crate::Event::PaymentClaimable /// [`PaymentReceived`]: crate::Event::PaymentReceived - pub fn claim_for_hash( - &self, payment_hash: PaymentHash, claimable_amount_msat: u64, preimage: PaymentPreimage, + pub fn claim_for_id( + &self, payment_id: PaymentId, claimable_amount_msat: u64, preimage: PaymentPreimage, ) -> Result<(), Error> { - let payment_id = PaymentId(payment_hash.0); + let details = self.payment_store.get(&payment_id).ok_or_else(|| { + log_error!( + self.logger, + "Failed to manually claim unknown payment with ID: {}", + payment_id + ); + Error::InvalidPaymentId + })?; - let expected_payment_hash = PaymentHash(Sha256::hash(&preimage.0).to_byte_array()); + let payment_hash = match details.kind { + PaymentKind::Bolt11 { hash, .. } => hash, + _ => { + log_error!( + self.logger, + "Failed to manually claim payment with ID {} of unsupported kind", + payment_id + ); + return Err(Error::InvalidPaymentId); + }, + }; + let expected_payment_hash = PaymentHash(Sha256::hash(&preimage.0).to_byte_array()); if expected_payment_hash != payment_hash { log_error!( self.logger, @@ -619,40 +587,30 @@ impl Bolt11Payment { return Err(Error::InvalidPaymentPreimage); } - if let Some(details) = self.payment_store.get(&payment_id) { - // For payments requested via `receive*_via_jit_channel_for_hash()` - // `skimmed_fee_msat` held by LSP must be taken into account. - let skimmed_fee_msat = match details.kind { - PaymentKind::Bolt11 { - counterparty_skimmed_fee_msat: Some(skimmed_fee_msat), - .. - } => skimmed_fee_msat, - _ => 0, - }; - if let Some(invoice_amount_msat) = details.amount_msat { - if claimable_amount_msat < invoice_amount_msat.saturating_sub(skimmed_fee_msat) { - log_error!( - self.logger, - "Failed to manually claim payment {} as the claimable amount is less than expected", - payment_id - ); - return Err(Error::InvalidAmount); - } + // For payments requested via `receive*_via_jit_channel_for_hash()` + // `skimmed_fee_msat` held by LSP must be taken into account. + let skimmed_fee_msat = match details.kind { + PaymentKind::Bolt11 { + counterparty_skimmed_fee_msat: Some(skimmed_fee_msat), .. + } => skimmed_fee_msat, + _ => 0, + }; + if let Some(invoice_amount_msat) = details.amount_msat { + if claimable_amount_msat < invoice_amount_msat.saturating_sub(skimmed_fee_msat) { + log_error!( + self.logger, + "Failed to manually claim payment {} as the claimable amount is less than expected", + payment_id + ); + return Err(Error::InvalidAmount); } - } else { - log_error!( - self.logger, - "Failed to manually claim unknown payment with hash: {}", - payment_hash - ); - return Err(Error::InvalidPaymentHash); } self.channel_manager.claim_funds(preimage); Ok(()) } - /// Allows to manually fail payments with the given hash that have previously + /// Allows to manually fail payments with the given id that have previously /// been registered via [`receive_for_hash`] or [`receive_variable_amount_for_hash`]. /// /// This should be called in reponse to a [`PaymentClaimable`] event if the payment needs to be @@ -665,8 +623,27 @@ impl Bolt11Payment { /// [`receive_for_hash`]: Self::receive_for_hash /// [`receive_variable_amount_for_hash`]: Self::receive_variable_amount_for_hash /// [`PaymentClaimable`]: crate::Event::PaymentClaimable - pub fn fail_for_hash(&self, payment_hash: PaymentHash) -> Result<(), Error> { - let payment_id = PaymentId(payment_hash.0); + pub fn fail_for_id(&self, payment_id: PaymentId) -> Result<(), Error> { + let details = self.payment_store.get(&payment_id).ok_or_else(|| { + log_error!( + self.logger, + "Failed to manually fail unknown payment with ID {}", + payment_id, + ); + Error::InvalidPaymentId + })?; + + let payment_hash = match details.kind { + PaymentKind::Bolt11 { hash, .. } => hash, + _ => { + log_error!( + self.logger, + "Failed to manually fail payment with ID {} of unsupported kind", + payment_id + ); + return Err(Error::InvalidPaymentId); + }, + }; let update = PaymentDetailsUpdate { status: Some(PaymentStatus::Failed), @@ -678,10 +655,10 @@ impl Bolt11Payment { Ok(DataStoreUpdateResult::NotFound) => { log_error!( self.logger, - "Failed to manually fail unknown payment with hash {}", - payment_hash, + "Failed to manually fail unknown payment with ID {}", + payment_id, ); - return Err(Error::InvalidPaymentHash); + return Err(Error::InvalidPaymentId); }, Err(e) => { log_error!( @@ -723,13 +700,13 @@ impl Bolt11Payment { /// pending registration has been claimed, failed, expired, or pruned. /// /// **Note:** users *MUST* handle this event and claim the payment manually via - /// [`claim_for_hash`] as soon as they have obtained access to the preimage of the given + /// [`claim_for_id`] as soon as they have obtained access to the preimage of the given /// payment hash. If they're unable to obtain the preimage, they *MUST* immediately fail the payment via - /// [`fail_for_hash`]. + /// [`fail_for_id`]. /// /// [`PaymentClaimable`]: crate::Event::PaymentClaimable - /// [`claim_for_hash`]: Self::claim_for_hash - /// [`fail_for_hash`]: Self::fail_for_hash + /// [`claim_for_id`]: Self::claim_for_id + /// [`fail_for_id`]: Self::fail_for_id pub fn receive_for_hash( &self, amount_msat: u64, description: &Bolt11InvoiceDescription, expiry_secs: u32, payment_hash: PaymentHash, @@ -764,13 +741,13 @@ impl Bolt11Payment { /// pending registration has been claimed, failed, expired, or pruned. /// /// **Note:** users *MUST* handle this event and claim the payment manually via - /// [`claim_for_hash`] as soon as they have obtained access to the preimage of the given + /// [`claim_for_id`] as soon as they have obtained access to the preimage of the given /// payment hash. If they're unable to obtain the preimage, they *MUST* immediately fail the payment via - /// [`fail_for_hash`]. + /// [`fail_for_id`]. /// /// [`PaymentClaimable`]: crate::Event::PaymentClaimable - /// [`claim_for_hash`]: Self::claim_for_hash - /// [`fail_for_hash`]: Self::fail_for_hash + /// [`claim_for_id`]: Self::claim_for_id + /// [`fail_for_id`]: Self::fail_for_id pub fn receive_variable_amount_for_hash( &self, description: &Bolt11InvoiceDescription, expiry_secs: u32, payment_hash: PaymentHash, ) -> Result { @@ -824,14 +801,14 @@ impl Bolt11Payment { /// pending registration has been claimed, failed, expired, or pruned. /// /// **Note:** users *MUST* handle this event and claim the payment manually via - /// [`claim_for_hash`] as soon as they have obtained access to the preimage of the given + /// [`claim_for_id`] as soon as they have obtained access to the preimage of the given /// payment hash. If they're unable to obtain the preimage, they *MUST* immediately fail the payment via - /// [`fail_for_hash`]. + /// [`fail_for_id`]. /// /// [LSPS2]: https://github.com/BitcoinAndLightningLayerSpecs/lsp/blob/main/LSPS2/README.md /// [`PaymentClaimable`]: crate::Event::PaymentClaimable - /// [`claim_for_hash`]: Self::claim_for_hash - /// [`fail_for_hash`]: Self::fail_for_hash + /// [`claim_for_id`]: Self::claim_for_id + /// [`fail_for_id`]: Self::fail_for_id /// [`counterparty_skimmed_fee_msat`]: crate::payment::PaymentKind::Bolt11::counterparty_skimmed_fee_msat pub fn receive_via_jit_channel_for_hash( &self, amount_msat: u64, description: &Bolt11InvoiceDescription, expiry_secs: u32, @@ -896,14 +873,14 @@ impl Bolt11Payment { /// pending registration has been claimed, failed, expired, or pruned. /// /// **Note:** users *MUST* handle this event and claim the payment manually via - /// [`claim_for_hash`] as soon as they have obtained access to the preimage of the given + /// [`claim_for_id`] as soon as they have obtained access to the preimage of the given /// payment hash. If they're unable to obtain the preimage, they *MUST* immediately fail the payment via - /// [`fail_for_hash`]. + /// [`fail_for_id`]. /// /// [LSPS2]: https://github.com/BitcoinAndLightningLayerSpecs/lsp/blob/main/LSPS2/README.md /// [`PaymentClaimable`]: crate::Event::PaymentClaimable - /// [`claim_for_hash`]: Self::claim_for_hash - /// [`fail_for_hash`]: Self::fail_for_hash + /// [`claim_for_id`]: Self::claim_for_id + /// [`fail_for_id`]: Self::fail_for_id /// [`counterparty_skimmed_fee_msat`]: crate::payment::PaymentKind::Bolt11::counterparty_skimmed_fee_msat pub fn receive_variable_amount_via_jit_channel_for_hash( &self, description: &Bolt11InvoiceDescription, expiry_secs: u32, diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 8b8cce13e9..3864507cad 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -241,6 +241,36 @@ macro_rules! expect_payment_received_event { pub(crate) use expect_payment_received_event; macro_rules! expect_payment_claimable_event { + ($node:expr, $payment_hash:expr, $claimable_amount_msat:expr) => {{ + let event = tokio::time::timeout( + std::time::Duration::from_secs(crate::common::INTEROP_TIMEOUT_SECS), + $node.next_event_async(), + ) + .await + .unwrap_or_else(|_| { + panic!( + "{} timed out waiting for PaymentClaimable event after 60s", + std::stringify!($node) + ) + }); + match event { + ref e @ Event::PaymentClaimable { + payment_id, + payment_hash, + claimable_amount_msat, + .. + } => { + println!("{} got event {:?}", std::stringify!($node), e); + assert_eq!(payment_hash, $payment_hash); + assert_eq!(claimable_amount_msat, $claimable_amount_msat); + $node.event_handled().unwrap(); + (payment_id, claimable_amount_msat) + }, + ref e => { + panic!("{} got unexpected event!: {:?}", std::stringify!($node), e); + }, + } + }}; ($node:expr, $payment_id:expr, $payment_hash:expr, $claimable_amount_msat:expr) => {{ let event = tokio::time::timeout( std::time::Duration::from_secs(crate::common::INTEROP_TIMEOUT_SECS), @@ -1249,10 +1279,9 @@ pub(crate) async fn do_channel_full_cycle( .unwrap(); println!("\nA send"); - let payment_id = node_a.bolt11_payment().send(&invoice, None).unwrap(); - assert_eq!(node_a.bolt11_payment().send(&invoice, None), Err(NodeError::DuplicatePayment)); + let outbound_payment_id = node_a.bolt11_payment().send(&invoice, None).unwrap(); - assert!(!node_a.list_payments_with_filter(|p| p.id == payment_id).is_empty()); + assert!(!node_a.list_payments_with_filter(|p| p.id == outbound_payment_id).is_empty()); let outbound_payments_a = node_a.list_payments_with_filter(|p| { p.direction == PaymentDirection::Outbound && matches!(p.kind, PaymentKind::Bolt11 { .. }) @@ -1272,7 +1301,7 @@ pub(crate) async fn do_channel_full_cycle( let inbound_payments_b = node_b.list_payments_with_filter(|p| { p.direction == PaymentDirection::Inbound && matches!(p.kind, PaymentKind::Bolt11 { .. }) }); - assert_eq!(inbound_payments_b.len(), 1); + assert_eq!(inbound_payments_b.len(), 0); // Verify bolt12_invoice is None for BOLT11 payments match node_a.next_event_async().await { @@ -1285,24 +1314,17 @@ pub(crate) async fn do_channel_full_cycle( panic!("{} got unexpected event!: {:?}", std::stringify!(node_a), e); }, } - expect_event!(node_b, PaymentReceived); - assert_eq!(node_a.payment(&payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_a.payment(&payment_id).unwrap().direction, PaymentDirection::Outbound); - assert_eq!(node_a.payment(&payment_id).unwrap().amount_msat, Some(invoice_amount_1_msat)); - assert!(matches!(node_a.payment(&payment_id).unwrap().kind, PaymentKind::Bolt11 { .. })); - assert_eq!(node_b.payment(&payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_b.payment(&payment_id).unwrap().direction, PaymentDirection::Inbound); - assert_eq!(node_b.payment(&payment_id).unwrap().amount_msat, Some(invoice_amount_1_msat)); - assert!(matches!(node_b.payment(&payment_id).unwrap().kind, PaymentKind::Bolt11 { .. })); - - // Assert we fail duplicate outbound payments and check the status hasn't changed. - assert_eq!(Err(NodeError::DuplicatePayment), node_a.bolt11_payment().send(&invoice, None)); - assert_eq!(node_a.payment(&payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_a.payment(&payment_id).unwrap().direction, PaymentDirection::Outbound); - assert_eq!(node_a.payment(&payment_id).unwrap().amount_msat, Some(invoice_amount_1_msat)); - assert_eq!(node_b.payment(&payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_b.payment(&payment_id).unwrap().direction, PaymentDirection::Inbound); - assert_eq!(node_b.payment(&payment_id).unwrap().amount_msat, Some(invoice_amount_1_msat)); + let inbound_payment_id = expect_payment_received_event!(node_b, invoice_amount_1_msat); + let outbound_payment = node_a.payment(&outbound_payment_id).unwrap(); + assert_eq!(outbound_payment.status, PaymentStatus::Succeeded); + assert_eq!(outbound_payment.direction, PaymentDirection::Outbound); + assert_eq!(outbound_payment.amount_msat, Some(invoice_amount_1_msat)); + assert!(matches!(&outbound_payment.kind, PaymentKind::Bolt11 { .. })); + let inbound_payment = node_b.payment(&inbound_payment_id).unwrap(); + assert_eq!(inbound_payment.status, PaymentStatus::Succeeded); + assert_eq!(inbound_payment.direction, PaymentDirection::Inbound); + assert_eq!(inbound_payment.amount_msat, Some(invoice_amount_1_msat)); + assert!(matches!(&inbound_payment.kind, PaymentKind::Bolt11 { .. })); // Test under-/overpayment let invoice_amount_2_msat = 2500_000; @@ -1325,28 +1347,30 @@ pub(crate) async fn do_channel_full_cycle( let overpaid_amount_msat = invoice_amount_2_msat + 100; println!("\nA overpaid send"); - let payment_id = + let outbound_payment_id = node_a.bolt11_payment().send_using_amount(&invoice, overpaid_amount_msat, None).unwrap(); expect_event!(node_a, PaymentSuccessful); - let received_amount = match node_b.next_event_async().await { - ref e @ Event::PaymentReceived { amount_msat, .. } => { + let (inbound_payment_id, received_amount) = match node_b.next_event_async().await { + ref e @ Event::PaymentReceived { payment_id, amount_msat, .. } => { println!("{} got event {:?}", std::stringify!(node_b), e); node_b.event_handled().unwrap(); - amount_msat + (payment_id, amount_msat) }, ref e => { panic!("{} got unexpected event!: {:?}", std::stringify!(node_b), e); }, }; assert_eq!(received_amount, overpaid_amount_msat); - assert_eq!(node_a.payment(&payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_a.payment(&payment_id).unwrap().direction, PaymentDirection::Outbound); - assert_eq!(node_a.payment(&payment_id).unwrap().amount_msat, Some(overpaid_amount_msat)); - assert!(matches!(node_a.payment(&payment_id).unwrap().kind, PaymentKind::Bolt11 { .. })); - assert_eq!(node_b.payment(&payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_b.payment(&payment_id).unwrap().direction, PaymentDirection::Inbound); - assert_eq!(node_b.payment(&payment_id).unwrap().amount_msat, Some(overpaid_amount_msat)); - assert!(matches!(node_b.payment(&payment_id).unwrap().kind, PaymentKind::Bolt11 { .. })); + let outbound_payment = node_a.payment(&outbound_payment_id).unwrap(); + assert_eq!(outbound_payment.status, PaymentStatus::Succeeded); + assert_eq!(outbound_payment.direction, PaymentDirection::Outbound); + assert_eq!(outbound_payment.amount_msat, Some(overpaid_amount_msat)); + assert!(matches!(&outbound_payment.kind, PaymentKind::Bolt11 { .. })); + let inbound_payment = node_b.payment(&inbound_payment_id).unwrap(); + assert_eq!(inbound_payment.status, PaymentStatus::Succeeded); + assert_eq!(inbound_payment.direction, PaymentDirection::Inbound); + assert_eq!(inbound_payment.amount_msat, Some(overpaid_amount_msat)); + assert!(matches!(&inbound_payment.kind, PaymentKind::Bolt11 { .. })); // Test "zero-amount" invoice payment println!("\nB receive_variable_amount_payment"); @@ -1360,31 +1384,33 @@ pub(crate) async fn do_channel_full_cycle( node_a.bolt11_payment().send(&variable_amount_invoice, None) ); println!("\nA send_using_amount"); - let payment_id = node_a + let outbound_payment_id = node_a .bolt11_payment() .send_using_amount(&variable_amount_invoice, determined_amount_msat, None) .unwrap(); expect_event!(node_a, PaymentSuccessful); - let received_amount = match node_b.next_event_async().await { - ref e @ Event::PaymentReceived { amount_msat, .. } => { + let (inbound_payment_id, received_amount) = match node_b.next_event_async().await { + ref e @ Event::PaymentReceived { payment_id, amount_msat, .. } => { println!("{} got event {:?}", std::stringify!(node_b), e); node_b.event_handled().unwrap(); - amount_msat + (payment_id, amount_msat) }, ref e => { panic!("{} got unexpected event!: {:?}", std::stringify!(node_b), e); }, }; assert_eq!(received_amount, determined_amount_msat); - assert_eq!(node_a.payment(&payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_a.payment(&payment_id).unwrap().direction, PaymentDirection::Outbound); - assert_eq!(node_a.payment(&payment_id).unwrap().amount_msat, Some(determined_amount_msat)); - assert!(matches!(node_a.payment(&payment_id).unwrap().kind, PaymentKind::Bolt11 { .. })); - assert_eq!(node_b.payment(&payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_b.payment(&payment_id).unwrap().direction, PaymentDirection::Inbound); - assert_eq!(node_b.payment(&payment_id).unwrap().amount_msat, Some(determined_amount_msat)); - assert!(matches!(node_b.payment(&payment_id).unwrap().kind, PaymentKind::Bolt11 { .. })); + let outbound_payment = node_a.payment(&outbound_payment_id).unwrap(); + assert_eq!(outbound_payment.status, PaymentStatus::Succeeded); + assert_eq!(outbound_payment.direction, PaymentDirection::Outbound); + assert_eq!(outbound_payment.amount_msat, Some(determined_amount_msat)); + assert!(matches!(&outbound_payment.kind, PaymentKind::Bolt11 { .. })); + let inbound_payment = node_b.payment(&inbound_payment_id).unwrap(); + assert_eq!(inbound_payment.status, PaymentStatus::Succeeded); + assert_eq!(inbound_payment.direction, PaymentDirection::Inbound); + assert_eq!(inbound_payment.amount_msat, Some(determined_amount_msat)); + assert!(matches!(&inbound_payment.kind, PaymentKind::Bolt11 { .. })); // Test claiming manually registered payments. let invoice_amount_3_msat = 5_532_000; @@ -1408,34 +1434,28 @@ pub(crate) async fn do_channel_full_cycle( ), Err(NodeError::DuplicatePayment) )); - let manual_payment_id = node_a.bolt11_payment().send(&manual_invoice, None).unwrap(); + let outbound_manual_payment_id = node_a.bolt11_payment().send(&manual_invoice, None).unwrap(); - let claimable_amount_msat = expect_payment_claimable_event!( - node_b, - manual_payment_id, - manual_payment_hash, - invoice_amount_3_msat - ); + let (manual_payment_id, claimable_amount_msat) = + expect_payment_claimable_event!(node_b, manual_payment_hash, invoice_amount_3_msat); + assert_ne!(manual_payment_id.0, manual_payment_hash.0); node_b .bolt11_payment() - .claim_for_hash(manual_payment_hash, claimable_amount_msat, manual_preimage) + .claim_for_id(manual_payment_id, claimable_amount_msat, manual_preimage) .unwrap(); - expect_payment_received_event!(node_b, claimable_amount_msat); - expect_payment_successful_event!(node_a, manual_payment_id, None); - assert_eq!(node_a.payment(&manual_payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_a.payment(&manual_payment_id).unwrap().direction, PaymentDirection::Outbound); - assert_eq!( - node_a.payment(&manual_payment_id).unwrap().amount_msat, - Some(invoice_amount_3_msat) - ); - assert!(matches!(node_a.payment(&manual_payment_id).unwrap().kind, PaymentKind::Bolt11 { .. })); - assert_eq!(node_b.payment(&manual_payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_b.payment(&manual_payment_id).unwrap().direction, PaymentDirection::Inbound); - assert_eq!( - node_b.payment(&manual_payment_id).unwrap().amount_msat, - Some(invoice_amount_3_msat) - ); - assert!(matches!(node_b.payment(&manual_payment_id).unwrap().kind, PaymentKind::Bolt11 { .. })); + let received_payment_id = expect_payment_received_event!(node_b, claimable_amount_msat); + assert_eq!(received_payment_id, manual_payment_id); + expect_payment_successful_event!(node_a, outbound_manual_payment_id, None); + let outbound_manual_payment = node_a.payment(&outbound_manual_payment_id).unwrap(); + assert_eq!(outbound_manual_payment.status, PaymentStatus::Succeeded); + assert_eq!(outbound_manual_payment.direction, PaymentDirection::Outbound); + assert_eq!(outbound_manual_payment.amount_msat, Some(invoice_amount_3_msat)); + assert!(matches!(&outbound_manual_payment.kind, PaymentKind::Bolt11 { .. })); + let manual_payment = node_b.payment(&manual_payment_id).unwrap(); + assert_eq!(manual_payment.status, PaymentStatus::Succeeded); + assert_eq!(manual_payment.direction, PaymentDirection::Inbound); + assert_eq!(manual_payment.amount_msat, Some(invoice_amount_3_msat)); + assert!(matches!(&manual_payment.kind, PaymentKind::Bolt11 { .. })); // Test failing manually registered payments. let invoice_amount_4_msat = 5_532_000; @@ -1451,42 +1471,24 @@ pub(crate) async fn do_channel_full_cycle( manual_fail_payment_hash, ) .unwrap(); - let manual_fail_payment_id = node_a.bolt11_payment().send(&manual_fail_invoice, None).unwrap(); + let outbound_manual_fail_payment_id = + node_a.bolt11_payment().send(&manual_fail_invoice, None).unwrap(); - expect_payment_claimable_event!( - node_b, - manual_fail_payment_id, - manual_fail_payment_hash, - invoice_amount_4_msat - ); - node_b.bolt11_payment().fail_for_hash(manual_fail_payment_hash).unwrap(); + let (manual_fail_payment_id, _) = + expect_payment_claimable_event!(node_b, manual_fail_payment_hash, invoice_amount_4_msat); + assert_ne!(manual_fail_payment_id.0, manual_fail_payment_hash.0); + node_b.bolt11_payment().fail_for_id(manual_fail_payment_id).unwrap(); expect_event!(node_a, PaymentFailed); - assert_eq!(node_a.payment(&manual_fail_payment_id).unwrap().status, PaymentStatus::Failed); - assert_eq!( - node_a.payment(&manual_fail_payment_id).unwrap().direction, - PaymentDirection::Outbound - ); - assert_eq!( - node_a.payment(&manual_fail_payment_id).unwrap().amount_msat, - Some(invoice_amount_4_msat) - ); - assert!(matches!( - node_a.payment(&manual_fail_payment_id).unwrap().kind, - PaymentKind::Bolt11 { .. } - )); - assert_eq!(node_b.payment(&manual_fail_payment_id).unwrap().status, PaymentStatus::Failed); - assert_eq!( - node_b.payment(&manual_fail_payment_id).unwrap().direction, - PaymentDirection::Inbound - ); - assert_eq!( - node_b.payment(&manual_fail_payment_id).unwrap().amount_msat, - Some(invoice_amount_4_msat) - ); - assert!(matches!( - node_b.payment(&manual_fail_payment_id).unwrap().kind, - PaymentKind::Bolt11 { .. } - )); + let outbound_manual_fail_payment = node_a.payment(&outbound_manual_fail_payment_id).unwrap(); + assert_eq!(outbound_manual_fail_payment.status, PaymentStatus::Failed); + assert_eq!(outbound_manual_fail_payment.direction, PaymentDirection::Outbound); + assert_eq!(outbound_manual_fail_payment.amount_msat, Some(invoice_amount_4_msat)); + assert!(matches!(&outbound_manual_fail_payment.kind, PaymentKind::Bolt11 { .. })); + let manual_fail_payment = node_b.payment(&manual_fail_payment_id).unwrap(); + assert_eq!(manual_fail_payment.status, PaymentStatus::Failed); + assert_eq!(manual_fail_payment.direction, PaymentDirection::Inbound); + assert_eq!(manual_fail_payment.amount_msat, Some(invoice_amount_4_msat)); + assert!(matches!(&manual_fail_payment.kind, PaymentKind::Bolt11 { .. })); // Test spontaneous/keysend payments println!("\nA send_spontaneous_payment"); @@ -1498,39 +1500,38 @@ pub(crate) async fn do_channel_full_cycle( .unwrap(); expect_event!(node_a, PaymentSuccessful); let next_event = node_b.next_event_async().await; - let (received_keysend_amount, received_custom_records) = match next_event { - ref e @ Event::PaymentReceived { amount_msat, ref custom_records, .. } => { - println!("{} got event {:?}", std::stringify!(node_b), e); - node_b.event_handled().unwrap(); - (amount_msat, custom_records) - }, - ref e => { - panic!("{} got unexpected event!: {:?}", std::stringify!(node_b), e); - }, - }; + let (received_keysend_payment_id, received_keysend_amount, received_custom_records) = + match next_event { + ref e @ Event::PaymentReceived { + payment_id, amount_msat, ref custom_records, .. + } => { + println!("{} got event {:?}", std::stringify!(node_b), e); + node_b.event_handled().unwrap(); + (payment_id, amount_msat, custom_records) + }, + ref e => { + panic!("{} got unexpected event!: {:?}", std::stringify!(node_b), e); + }, + }; assert_eq!(received_keysend_amount, keysend_amount_msat); - assert_eq!(node_a.payment(&keysend_payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_a.payment(&keysend_payment_id).unwrap().direction, PaymentDirection::Outbound); - assert_eq!(node_a.payment(&keysend_payment_id).unwrap().amount_msat, Some(keysend_amount_msat)); - assert!(matches!( - node_a.payment(&keysend_payment_id).unwrap().kind, - PaymentKind::Spontaneous { .. } - )); + let keysend_payment = node_a.payment(&keysend_payment_id).unwrap(); + assert_eq!(keysend_payment.status, PaymentStatus::Succeeded); + assert_eq!(keysend_payment.direction, PaymentDirection::Outbound); + assert_eq!(keysend_payment.amount_msat, Some(keysend_amount_msat)); + assert!(matches!(&keysend_payment.kind, PaymentKind::Spontaneous { .. })); assert_eq!(received_custom_records, &custom_tlvs); - assert_eq!(node_b.payment(&keysend_payment_id).unwrap().status, PaymentStatus::Succeeded); - assert_eq!(node_b.payment(&keysend_payment_id).unwrap().direction, PaymentDirection::Inbound); - assert_eq!(node_b.payment(&keysend_payment_id).unwrap().amount_msat, Some(keysend_amount_msat)); - assert!(matches!( - node_b.payment(&keysend_payment_id).unwrap().kind, - PaymentKind::Spontaneous { .. } - )); + let received_keysend_payment = node_b.payment(&received_keysend_payment_id).unwrap(); + assert_eq!(received_keysend_payment.status, PaymentStatus::Succeeded); + assert_eq!(received_keysend_payment.direction, PaymentDirection::Inbound); + assert_eq!(received_keysend_payment.amount_msat, Some(keysend_amount_msat)); + assert!(matches!(&received_keysend_payment.kind, PaymentKind::Spontaneous { .. })); assert_eq!( node_a.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Bolt11 { .. })).len(), 5 ); assert_eq!( node_b.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Bolt11 { .. })).len(), - 6 + 5 ); assert_eq!( node_a diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index fe1f50f75f..1926892f17 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -370,7 +370,7 @@ async fn multi_hop_sending() { .bolt11_payment() .receive(2_500_000, &invoice_description.clone().into(), 9217) .unwrap(); - nodes[0].bolt11_payment().send(&invoice, Some(route_params)).unwrap(); + let outbound_payment_id = nodes[0].bolt11_payment().send(&invoice, Some(route_params)).unwrap(); expect_event!(nodes[1], PaymentForwarded); @@ -379,9 +379,9 @@ async fn multi_hop_sending() { let node_3_fwd_event = matches!(nodes[3].next_event(), Some(Event::PaymentForwarded { .. })); assert!(node_2_fwd_event || node_3_fwd_event); - let payment_id = expect_payment_received_event!(&nodes[4], 2_500_000); + expect_payment_received_event!(&nodes[4], 2_500_000); let fee_paid_msat = Some(2000); - expect_payment_successful_event!(nodes[0], payment_id, Some(fee_paid_msat)); + expect_payment_successful_event!(nodes[0], outbound_payment_id, Some(fee_paid_msat)); } #[tokio::test(flavor = "multi_thread", worker_threads = 1)] @@ -460,7 +460,6 @@ async fn split_underpaid_bolt11_payment() { .unwrap(); let receiver_payment_id = expect_payment_received_event!(node_c, amount_msat); - assert_eq!(receiver_payment_id, PaymentId(invoice.payment_hash().0)); expect_payment_successful_event!(node_a, payment_id_a, None); expect_payment_successful_event!(node_b, payment_id_b, None); @@ -2248,7 +2247,7 @@ async fn simple_bolt12_send_receive() { matches!(p.kind, PaymentKind::Bolt12Offer { .. }) && p.id == payment_id }); assert_eq!(node_a_payments.len(), 1); - let payment_hash = match node_a_payments.first().unwrap().kind { + match node_a_payments.first().unwrap().kind { PaymentKind::Bolt12Offer { hash, preimage, @@ -2264,7 +2263,6 @@ async fn simple_bolt12_send_receive() { assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0); // TODO: We should eventually set and assert the secret sender-side, too, but the BOLT12 // API currently doesn't allow to do that. - hash.unwrap() }, _ => { panic!("Unexpected payment kind"); @@ -2272,8 +2270,7 @@ async fn simple_bolt12_send_receive() { }; assert_eq!(node_a_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); - expect_payment_received_event!(node_b, expected_amount_msat); - let node_b_payment_id = PaymentId(payment_hash.0); + let node_b_payment_id = expect_payment_received_event!(node_b, expected_amount_msat); let node_b_payments = node_b.list_payments_with_filter(|p| { matches!(p.kind, PaymentKind::Bolt12Offer { .. }) && p.id == node_b_payment_id }); @@ -2305,8 +2302,8 @@ async fn simple_bolt12_send_receive() { None, ) .unwrap(); - let invoice = node_a.bolt12_payment().request_refund_payment(&refund).unwrap(); - expect_payment_received_event!(node_a, overpaid_amount); + let _invoice = node_a.bolt12_payment().request_refund_payment(&refund).unwrap(); + let node_a_payment_id = expect_payment_received_event!(node_a, overpaid_amount); let node_b_payment_id = node_b .list_payments_with_filter(|p| { @@ -2343,7 +2340,6 @@ async fn simple_bolt12_send_receive() { } assert_eq!(node_b_payments.first().unwrap().amount_msat, Some(overpaid_amount)); - let node_a_payment_id = PaymentId(invoice.payment_hash().0); let node_a_payments = node_a.list_payments_with_filter(|p| { matches!(p.kind, PaymentKind::Bolt12Refund { .. }) && p.id == node_a_payment_id }); @@ -2862,7 +2858,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { // Have the payer_node pay the invoice, therby triggering channel open service_node -> client_node. println!("Paying JIT invoice!"); - let payment_id = payer_node.bolt11_payment().send(&jit_invoice, None).unwrap(); + let payer_payment_id = payer_node.bolt11_payment().send(&jit_invoice, None).unwrap(); expect_channel_pending_event!(service_node, client_node.node_id()); expect_channel_ready_event!(service_node, client_node.node_id()); expect_event!(service_node, PaymentForwarded); @@ -2871,7 +2867,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { let service_fee_msat = (jit_amount_msat * channel_opening_fee_ppm as u64) / 1_000_000; let expected_received_amount_msat = jit_amount_msat - service_fee_msat; - expect_payment_successful_event!(payer_node, payment_id, None); + expect_payment_successful_event!(payer_node, payer_payment_id, None); let client_payment_id = expect_payment_received_event!(client_node, expected_received_amount_msat); let client_payment = client_node.payment(&client_payment_id).unwrap(); @@ -2905,7 +2901,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { expect_payment_received_event!(client_node, amount_msat); //////////////////////////////////////////////////////////////////////////// - // receive_via_jit_channel_for_hash and claim_for_hash + // receive_via_jit_channel_for_hash and claim_for_id //////////////////////////////////////////////////////////////////////////// println!("Generating JIT invoice!"); // Increase the amount to make sure it does not fit into the existing channels. @@ -2925,7 +2921,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { // Have the payer_node pay the invoice, therby triggering channel open service_node -> client_node. println!("Paying JIT invoice!"); - let payment_id = payer_node.bolt11_payment().send(&jit_invoice, None).unwrap(); + let payer_payment_id = payer_node.bolt11_payment().send(&jit_invoice, None).unwrap(); expect_channel_pending_event!(service_node, client_node.node_id()); expect_channel_ready_event!(service_node, client_node.node_id()); expect_channel_pending_event!(client_node, service_node.node_id()); @@ -2933,22 +2929,24 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { let service_fee_msat = (jit_amount_msat * channel_opening_fee_ppm as u64) / 1_000_000; let expected_received_amount_msat = jit_amount_msat - service_fee_msat; - let claimable_amount_msat = expect_payment_claimable_event!( + let (client_payment_id, claimable_amount_msat) = expect_payment_claimable_event!( client_node, - payment_id, manual_payment_hash, expected_received_amount_msat ); + assert_ne!(client_payment_id.0, manual_payment_hash.0); + assert_eq!(client_node.payment(&client_payment_id).unwrap().amount_msat, Some(jit_amount_msat)); println!("Claiming payment!"); client_node .bolt11_payment() - .claim_for_hash(manual_payment_hash, claimable_amount_msat, manual_preimage) + .claim_for_id(client_payment_id, claimable_amount_msat, manual_preimage) .unwrap(); expect_event!(service_node, PaymentForwarded); - expect_payment_successful_event!(payer_node, payment_id, None); - let client_payment_id = + expect_payment_successful_event!(payer_node, payer_payment_id, None); + let received_payment_id = expect_payment_received_event!(client_node, expected_received_amount_msat); + assert_eq!(received_payment_id, client_payment_id); let client_payment = client_node.payment(&client_payment_id).unwrap(); match client_payment.kind { PaymentKind::Bolt11 { counterparty_skimmed_fee_msat, .. } => { @@ -2958,7 +2956,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { } //////////////////////////////////////////////////////////////////////////// - // receive_via_jit_channel_for_hash and fail_for_hash + // receive_via_jit_channel_for_hash and fail_for_id //////////////////////////////////////////////////////////////////////////// println!("Generating JIT invoice!"); // Increase the amount to make sure it does not fit into the existing channels. @@ -2978,7 +2976,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { // Have the payer_node pay the invoice, therby triggering channel open service_node -> client_node. println!("Paying JIT invoice!"); - let payment_id = payer_node.bolt11_payment().send(&jit_invoice, None).unwrap(); + let _payer_payment_id = payer_node.bolt11_payment().send(&jit_invoice, None).unwrap(); expect_channel_pending_event!(service_node, client_node.node_id()); expect_channel_ready_event!(service_node, client_node.node_id()); expect_channel_pending_event!(client_node, service_node.node_id()); @@ -2986,17 +2984,17 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { let service_fee_msat = (jit_amount_msat * channel_opening_fee_ppm as u64) / 1_000_000; let expected_received_amount_msat = jit_amount_msat - service_fee_msat; - expect_payment_claimable_event!( + let (client_payment_id, _) = expect_payment_claimable_event!( client_node, - payment_id, manual_payment_hash, expected_received_amount_msat ); + assert_ne!(client_payment_id.0, manual_payment_hash.0); println!("Failing payment!"); - client_node.bolt11_payment().fail_for_hash(manual_payment_hash).unwrap(); + client_node.bolt11_payment().fail_for_id(client_payment_id).unwrap(); expect_event!(payer_node, PaymentFailed); - assert_eq!(client_node.payment(&payment_id).unwrap().status, PaymentStatus::Failed); + assert_eq!(client_node.payment(&client_payment_id).unwrap().status, PaymentStatus::Failed); } #[tokio::test(flavor = "multi_thread", worker_threads = 1)] @@ -3188,8 +3186,8 @@ async fn lsps2_client_trusts_lsp() { // Have the payer_node pay the invoice, therby triggering channel open service_node -> client_node. println!("Paying JIT invoice!"); - let payment_id = payer_node.bolt11_payment().send(&res, None).unwrap(); - println!("Payment ID: {:?}", payment_id); + let payer_payment_id = payer_node.bolt11_payment().send(&res, None).unwrap(); + println!("Payment ID: {:?}", payer_payment_id); let funding_txo = expect_channel_pending_event!(service_node, client_node.node_id()); expect_channel_ready_event!(service_node, client_node.node_id()); expect_channel_pending_event!(client_node, service_node.node_id()); @@ -3227,21 +3225,22 @@ async fn lsps2_client_trusts_lsp() { let service_fee_msat = (jit_amount_msat * channel_opening_fee_ppm as u64) / 1_000_000; let expected_received_amount_msat = jit_amount_msat - service_fee_msat; - let _ = expect_payment_claimable_event!( + let (client_payment_id, _) = expect_payment_claimable_event!( client_node, - payment_id, manual_payment_hash, expected_received_amount_msat ); client_node .bolt11_payment() - .claim_for_hash(manual_payment_hash, jit_amount_msat, manual_preimage) + .claim_for_id(client_payment_id, jit_amount_msat, manual_preimage) .unwrap(); - expect_payment_successful_event!(payer_node, payment_id, None); + expect_payment_successful_event!(payer_node, payer_payment_id, None); - let _ = expect_payment_received_event!(client_node, expected_received_amount_msat); + let received_payment_id = + expect_payment_received_event!(client_node, expected_received_amount_msat); + assert_eq!(received_payment_id, client_payment_id); // Check the nodes pick up on the confirmed funding tx now. wait_for_tx(&electrsd.client, funding_txo.txid).await; From 452c643c072a05040d70e936fa10fc71428c8a89 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jul 2026 14:12:55 +0200 Subject: [PATCH 13/13] f Queue claim events before cleanup Keep manual pending payments available until PaymentReceived is durably queued, so replay after queue persistence failure can still resolve the user-facing payment ID. Co-Authored-By: HAL 9000 --- src/event.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/event.rs b/src/event.rs index cc5586aeb6..722936ff81 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1380,18 +1380,6 @@ where }, } - if pending_payment.is_some() { - if let Err(e) = self.pending_payment_store.remove(&payment_id).await { - log_error!( - self.logger, - "Failed to remove pending payment with ID {}: {}", - payment_id, - e - ); - return Err(ReplayEvent()); - } - } - let event = Event::PaymentReceived { payment_id, payment_hash, @@ -1401,12 +1389,28 @@ where .unwrap_or_default(), }; match self.event_queue.add_event(event).await { - Ok(_) => return Ok(()), + Ok(_) => (), Err(e) => { log_error!(self.logger, "Failed to push to event queue: {}", e); return Err(ReplayEvent()); }, }; + + if pending_payment.is_some() { + if let Err(e) = self.pending_payment_store.remove(&payment_id).await { + log_error!( + self.logger, + "Failed to remove pending payment with ID {}: {}", + payment_id, + e + ); + // The user-visible PaymentReceived event has already been durably queued. + // Replaying the LDK event here would risk queuing a duplicate event. + return Ok(()); + } + } + + return Ok(()); }, LdkEvent::PaymentSent { payment_id,