From cd8958f4f8c0c46e8ff62c7dd19adc910cbfa9d2 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jan 2025 15:19:36 +0100 Subject: [PATCH 1/6] Prefactor: Only update `PaymentDetails` fields if necessary Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits. --- src/payment/store.rs | 154 ++++++++++++++++++++++++++++--------------- 1 file changed, 101 insertions(+), 53 deletions(-) diff --git a/src/payment/store.rs b/src/payment/store.rs index 9ae137de9..df35555ae 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -59,6 +59,104 @@ impl PaymentDetails { .as_secs(); Self { id, kind, amount_msat, direction, status, latest_update_timestamp } } + + pub(crate) fn update(&mut self, update: &PaymentDetailsUpdate) -> bool { + debug_assert_eq!( + self.id, update.id, + "We should only ever override payment data for the same payment id" + ); + + let mut updated = false; + + macro_rules! update_if_necessary { + ($val: expr, $update: expr) => { + if $val != $update { + $val = $update; + updated = true; + } + }; + } + + if let Some(hash_opt) = update.hash { + match self.kind { + PaymentKind::Bolt12Offer { ref mut hash, .. } => { + debug_assert_eq!( + self.direction, + PaymentDirection::Outbound, + "We should only ever override payment hash for outbound BOLT 12 payments" + ); + update_if_necessary!(*hash, hash_opt); + }, + PaymentKind::Bolt12Refund { ref mut hash, .. } => { + debug_assert_eq!( + self.direction, + PaymentDirection::Outbound, + "We should only ever override payment hash for outbound BOLT 12 payments" + ); + update_if_necessary!(*hash, hash_opt); + }, + _ => { + // We can omit updating the hash for BOLT11 payments as the payment hash + // will always be known from the beginning. + }, + } + } + if let Some(preimage_opt) = update.preimage { + match self.kind { + PaymentKind::Bolt11 { ref mut preimage, .. } => { + update_if_necessary!(*preimage, preimage_opt) + }, + PaymentKind::Bolt11Jit { ref mut preimage, .. } => { + update_if_necessary!(*preimage, preimage_opt) + }, + PaymentKind::Bolt12Offer { ref mut preimage, .. } => { + update_if_necessary!(*preimage, preimage_opt) + }, + PaymentKind::Bolt12Refund { ref mut preimage, .. } => { + update_if_necessary!(*preimage, preimage_opt) + }, + PaymentKind::Spontaneous { ref mut preimage, .. } => { + update_if_necessary!(*preimage, preimage_opt) + }, + _ => {}, + } + } + + if let Some(secret_opt) = update.secret { + match self.kind { + PaymentKind::Bolt11 { ref mut secret, .. } => { + update_if_necessary!(*secret, secret_opt) + }, + PaymentKind::Bolt11Jit { ref mut secret, .. } => { + update_if_necessary!(*secret, secret_opt) + }, + PaymentKind::Bolt12Offer { ref mut secret, .. } => { + update_if_necessary!(*secret, secret_opt) + }, + PaymentKind::Bolt12Refund { ref mut secret, .. } => { + update_if_necessary!(*secret, secret_opt) + }, + _ => {}, + } + } + + if let Some(amount_opt) = update.amount_msat { + update_if_necessary!(self.amount_msat, amount_opt); + } + + if let Some(status) = update.status { + update_if_necessary!(self.status, status); + } + + if updated { + self.latest_update_timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or(Duration::from_secs(0)) + .as_secs(); + } + + updated + } } impl Writeable for PaymentDetails { @@ -401,60 +499,10 @@ where let mut locked_payments = self.payments.lock().unwrap(); if let Some(payment) = locked_payments.get_mut(&update.id) { - if let Some(hash_opt) = update.hash { - match payment.kind { - PaymentKind::Bolt12Offer { ref mut hash, .. } => { - debug_assert_eq!(payment.direction, PaymentDirection::Outbound, - "We should only ever override payment hash for outbound BOLT 12 payments"); - *hash = hash_opt - }, - PaymentKind::Bolt12Refund { ref mut hash, .. } => { - debug_assert_eq!(payment.direction, PaymentDirection::Outbound, - "We should only ever override payment hash for outbound BOLT 12 payments"); - *hash = hash_opt - }, - _ => { - // We can omit updating the hash for BOLT11 payments as the payment hash - // will always be known from the beginning. - }, - } + updated = payment.update(update); + if updated { + self.persist_info(&update.id, payment)?; } - if let Some(preimage_opt) = update.preimage { - match payment.kind { - PaymentKind::Bolt11 { ref mut preimage, .. } => *preimage = preimage_opt, - PaymentKind::Bolt11Jit { ref mut preimage, .. } => *preimage = preimage_opt, - PaymentKind::Bolt12Offer { ref mut preimage, .. } => *preimage = preimage_opt, - PaymentKind::Bolt12Refund { ref mut preimage, .. } => *preimage = preimage_opt, - PaymentKind::Spontaneous { ref mut preimage, .. } => *preimage = preimage_opt, - _ => {}, - } - } - - if let Some(secret_opt) = update.secret { - match payment.kind { - PaymentKind::Bolt11 { ref mut secret, .. } => *secret = secret_opt, - PaymentKind::Bolt11Jit { ref mut secret, .. } => *secret = secret_opt, - PaymentKind::Bolt12Offer { ref mut secret, .. } => *secret = secret_opt, - PaymentKind::Bolt12Refund { ref mut secret, .. } => *secret = secret_opt, - _ => {}, - } - } - - if let Some(amount_opt) = update.amount_msat { - payment.amount_msat = amount_opt; - } - - if let Some(status) = update.status { - payment.status = status; - } - - payment.latest_update_timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or(Duration::from_secs(0)) - .as_secs(); - - self.persist_info(&update.id, payment)?; - updated = true; } Ok(updated) } From d92a5684f33a443471a93f368dd79a789308d872 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 28 Jan 2025 10:26:13 +0100 Subject: [PATCH 2/6] Add more `debug_assert`s for updating BOLT12 payment hashes --- src/payment/store.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/payment/store.rs b/src/payment/store.rs index df35555ae..291050181 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -85,6 +85,10 @@ impl PaymentDetails { PaymentDirection::Outbound, "We should only ever override payment hash for outbound BOLT 12 payments" ); + debug_assert!( + hash.is_none() || *hash == hash_opt, + "We should never change a payment hash after being initially set" + ); update_if_necessary!(*hash, hash_opt); }, PaymentKind::Bolt12Refund { ref mut hash, .. } => { @@ -93,6 +97,10 @@ impl PaymentDetails { PaymentDirection::Outbound, "We should only ever override payment hash for outbound BOLT 12 payments" ); + debug_assert!( + hash.is_none() || *hash == hash_opt, + "We should never change a payment hash after being initially set" + ); update_if_necessary!(*hash, hash_opt); }, _ => { From c9ddfaed8a8009fc11c75f71c5474c096cd43e13 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jan 2025 15:45:47 +0100 Subject: [PATCH 3/6] Prefactor: Implement `PaymentStore::insert_or_update` We implement an inserting/updating method that will only persist entries that have been changed. --- src/payment/store.rs | 47 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/src/payment/store.rs b/src/payment/store.rs index 291050181..d3352c714 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -25,8 +25,8 @@ use lightning::{ use lightning_types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; +use std::collections::hash_map; use std::collections::HashMap; -use std::iter::FromIterator; use std::ops::Deref; use std::sync::{Arc, Mutex}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -448,6 +448,29 @@ impl PaymentDetailsUpdate { } } +impl From<&PaymentDetails> for PaymentDetailsUpdate { + fn from(value: &PaymentDetails) -> Self { + let (hash, preimage, secret) = match value.kind { + PaymentKind::Bolt11 { hash, preimage, secret, .. } => (Some(hash), preimage, secret), + PaymentKind::Bolt11Jit { hash, preimage, secret, .. } => (Some(hash), preimage, secret), + PaymentKind::Bolt12Offer { hash, preimage, secret, .. } => (hash, preimage, secret), + PaymentKind::Bolt12Refund { hash, preimage, secret, .. } => (hash, preimage, secret), + PaymentKind::Spontaneous { hash, preimage, .. } => (Some(hash), preimage, None), + _ => (None, None, None), + }; + + Self { + id: value.id, + hash: Some(hash), + preimage: Some(preimage), + secret: Some(secret), + amount_msat: Some(value.amount_msat), + direction: Some(value.direction), + status: Some(value.status), + } + } +} + pub(crate) struct PaymentStore where L::Target: LdkLogger, @@ -476,6 +499,28 @@ where Ok(updated) } + pub(crate) fn insert_or_update(&self, payment: &PaymentDetails) -> Result { + let mut locked_payments = self.payments.lock().unwrap(); + + let updated; + match locked_payments.entry(payment.id) { + hash_map::Entry::Occupied(mut e) => { + let update = payment.into(); + updated = e.get_mut().update(&update); + if updated { + self.persist_info(&payment.id, e.get())?; + } + }, + hash_map::Entry::Vacant(e) => { + e.insert(payment.clone()); + self.persist_info(&payment.id, payment)?; + updated = true; + }, + } + + Ok(updated) + } + pub(crate) fn remove(&self, id: &PaymentId) -> Result<(), Error> { let store_key = hex_utils::to_string(&id.0); self.kv_store From 62ff60c2795c245072d988681a7dc1c670ab001b Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jan 2025 13:44:23 +0100 Subject: [PATCH 4/6] Add required fields to `PaymentKind::Onchain` Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful. --- bindings/ldk_node.udl | 8 ++++++- src/payment/mod.rs | 4 +++- src/payment/store.rs | 56 +++++++++++++++++++++++++++++++++++++++++-- src/uniffi_types.rs | 4 +++- 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index b0ff44b0a..c4fe6bfbb 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -358,7 +358,7 @@ interface ClosureReason { [Enum] interface PaymentKind { - Onchain(); + Onchain(Txid txid, ConfirmationStatus status); Bolt11(PaymentHash hash, PaymentPreimage? preimage, PaymentSecret? secret); Bolt11Jit(PaymentHash hash, PaymentPreimage? preimage, PaymentSecret? secret, LSPFeeLimits lsp_fee_limits); Bolt12Offer(PaymentHash? hash, PaymentPreimage? preimage, PaymentSecret? secret, OfferId offer_id, UntrustedString? payer_note, u64? quantity); @@ -389,6 +389,12 @@ dictionary LSPFeeLimits { u64? max_proportional_opening_fee_ppm_msat; }; +[Enum] +interface ConfirmationStatus { + Confirmed (BlockHash block_hash, u32 height, u64 timestamp); + Unconfirmed (); +}; + dictionary PaymentDetails { PaymentId id; PaymentKind kind; diff --git a/src/payment/mod.rs b/src/payment/mod.rs index 5c99cfcf8..b031e37fd 100644 --- a/src/payment/mod.rs +++ b/src/payment/mod.rs @@ -18,7 +18,9 @@ pub use bolt11::Bolt11Payment; pub use bolt12::Bolt12Payment; pub use onchain::OnchainPayment; pub use spontaneous::SpontaneousPayment; -pub use store::{LSPFeeLimits, PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus}; +pub use store::{ + ConfirmationStatus, LSPFeeLimits, PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus, +}; pub use unified_qr::{QrPaymentResult, UnifiedQrPayment}; /// Represents information used to send a payment. diff --git a/src/payment/store.rs b/src/payment/store.rs index d3352c714..2c968dc96 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -25,6 +25,8 @@ use lightning::{ use lightning_types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; +use bitcoin::{BlockHash, Txid}; + use std::collections::hash_map; use std::collections::HashMap; use std::ops::Deref; @@ -156,6 +158,15 @@ impl PaymentDetails { update_if_necessary!(self.status, status); } + if let Some(confirmation_status) = update.confirmation_status { + match self.kind { + PaymentKind::Onchain { ref mut status, .. } => { + update_if_necessary!(*status, confirmation_status); + }, + _ => {}, + } + } + if updated { self.latest_update_timestamp = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -281,7 +292,12 @@ impl_writeable_tlv_based_enum!(PaymentStatus, #[derive(Clone, Debug, PartialEq, Eq)] pub enum PaymentKind { /// An on-chain payment. - Onchain, + Onchain { + /// The transaction identifier of this payment. + txid: Txid, + /// The confirmation status of this payment. + status: ConfirmationStatus, + }, /// A [BOLT 11] payment. /// /// [BOLT 11]: https://github.com/lightning/bolts/blob/master/11-payment-encoding.md @@ -370,7 +386,10 @@ pub enum PaymentKind { } impl_writeable_tlv_based_enum!(PaymentKind, - (0, Onchain) => {}, + (0, Onchain) => { + (0, txid, required), + (2, status, required), + }, (2, Bolt11) => { (0, hash, required), (2, preimage, option), @@ -403,6 +422,31 @@ impl_writeable_tlv_based_enum!(PaymentKind, } ); +/// Represents the confirmation status of a transaction. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum ConfirmationStatus { + /// The transaction is confirmed in the best chain. + Confirmed { + /// The hash of the block in which the transaction was confirmed. + block_hash: BlockHash, + /// The height under which the block was confirmed. + height: u32, + /// The timestamp, in seconds since start of the UNIX epoch, when this entry was last updated. + timestamp: u64, + }, + /// The transaction is unconfirmed. + Unconfirmed, +} + +impl_writeable_tlv_based_enum!(ConfirmationStatus, + (0, Confirmed) => { + (0, block_hash, required), + (2, height, required), + (4, timestamp, required), + }, + (2, Unconfirmed) => {}, +); + /// Limits applying to how much fee we allow an LSP to deduct from the payment amount. /// /// See [`LdkChannelConfig::accept_underpaying_htlcs`] for more information. @@ -432,6 +476,7 @@ pub(crate) struct PaymentDetailsUpdate { pub amount_msat: Option>, pub direction: Option, pub status: Option, + pub confirmation_status: Option, } impl PaymentDetailsUpdate { @@ -444,6 +489,7 @@ impl PaymentDetailsUpdate { amount_msat: None, direction: None, status: None, + confirmation_status: None, } } } @@ -459,6 +505,11 @@ impl From<&PaymentDetails> for PaymentDetailsUpdate { _ => (None, None, None), }; + let confirmation_status = match value.kind { + PaymentKind::Onchain { status, .. } => Some(status), + _ => None, + }; + Self { id: value.id, hash: Some(hash), @@ -467,6 +518,7 @@ impl From<&PaymentDetails> for PaymentDetailsUpdate { amount_msat: Some(value.amount_msat), direction: Some(value.direction), status: Some(value.status), + confirmation_status, } } } diff --git a/src/uniffi_types.rs b/src/uniffi_types.rs index 2747503aa..a5168dee6 100644 --- a/src/uniffi_types.rs +++ b/src/uniffi_types.rs @@ -14,7 +14,9 @@ pub use crate::config::{ default_config, AnchorChannelsConfig, EsploraSyncConfig, MaxDustHTLCExposure, }; pub use crate::graph::{ChannelInfo, ChannelUpdateInfo, NodeAnnouncementInfo, NodeInfo}; -pub use crate::payment::store::{LSPFeeLimits, PaymentDirection, PaymentKind, PaymentStatus}; +pub use crate::payment::store::{ + ConfirmationStatus, LSPFeeLimits, PaymentDirection, PaymentKind, PaymentStatus, +}; pub use crate::payment::{MaxTotalRoutingFeeLimit, QrPaymentResult, SendingParameters}; pub use lightning::chain::channelmonitor::BalanceSource; From e46e4078926b14bc331c398d57584f79574bcfac Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 27 Jan 2025 16:37:02 +0100 Subject: [PATCH 5/6] Have `Wallet` take `PaymentStore` ref --- src/builder.rs | 21 +++++++++++---------- src/wallet/mod.rs | 9 ++++++--- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index bcd91eeb8..d13b62abd 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -855,11 +855,22 @@ fn build_with_store_internal( let tx_broadcaster = Arc::new(TransactionBroadcaster::new(Arc::clone(&logger))); let fee_estimator = Arc::new(OnchainFeeEstimator::new()); + + let payment_store = match io::utils::read_payments(Arc::clone(&kv_store), Arc::clone(&logger)) { + Ok(payments) => { + Arc::new(PaymentStore::new(payments, Arc::clone(&kv_store), Arc::clone(&logger))) + }, + Err(_) => { + return Err(BuildError::ReadFailed); + }, + }; + let wallet = Arc::new(Wallet::new( bdk_wallet, wallet_persister, Arc::clone(&tx_broadcaster), Arc::clone(&fee_estimator), + Arc::clone(&payment_store), Arc::clone(&logger), )); @@ -1249,16 +1260,6 @@ fn build_with_store_internal( }, } - // Init payment info storage - let payment_store = match io::utils::read_payments(Arc::clone(&kv_store), Arc::clone(&logger)) { - Ok(payments) => { - Arc::new(PaymentStore::new(payments, Arc::clone(&kv_store), Arc::clone(&logger))) - }, - Err(_) => { - return Err(BuildError::ReadFailed); - }, - }; - let event_queue = match io::utils::read_event_queue(Arc::clone(&kv_store), Arc::clone(&logger)) { Ok(event_queue) => Arc::new(event_queue), diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 755328379..4cb2f88b2 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -7,9 +7,10 @@ use persist::KVStoreWalletPersister; -use crate::logger::{log_debug, log_error, log_info, log_trace, LdkLogger}; +use crate::logger::{log_debug, log_error, log_info, log_trace, Logger, LdkLogger}; use crate::fee_estimator::{ConfirmationTarget, FeeEstimator}; +use crate::payment::store::PaymentStore; use crate::Error; use lightning::chain::chaininterface::BroadcasterInterface; @@ -66,6 +67,7 @@ where persister: Mutex, broadcaster: B, fee_estimator: E, + payment_store: Arc>>, logger: L, } @@ -77,11 +79,12 @@ where { pub(crate) fn new( wallet: bdk_wallet::PersistedWallet, - wallet_persister: KVStoreWalletPersister, broadcaster: B, fee_estimator: E, logger: L, + wallet_persister: KVStoreWalletPersister, broadcaster: B, fee_estimator: E, + payment_store: Arc>>, logger: L, ) -> Self { let inner = Mutex::new(wallet); let persister = Mutex::new(wallet_persister); - Self { inner, persister, broadcaster, fee_estimator, logger } + Self { inner, persister, broadcaster, fee_estimator, payment_store, logger } } pub(crate) fn get_full_scan_request(&self) -> FullScanRequest { From 071043480b06ab209ea5135944011532bd96280e Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jan 2025 16:32:19 +0100 Subject: [PATCH 6/6] Update `PaymentStore` after each `Wallet` sync We update the payment store whenever syncing the wallet state finished. --- .../lightningdevkit/ldknode/LibraryTest.kt | 4 +- src/payment/store.rs | 5 + src/wallet/mod.rs | 88 ++++++++++++++++- tests/common/mod.rs | 99 ++++++++++++++++--- tests/integration_tests_rust.rs | 78 +++++++++++++-- 5 files changed, 251 insertions(+), 23 deletions(-) diff --git a/bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt b/bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt index c82a5d92e..c8c43c49c 100644 --- a/bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt +++ b/bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt @@ -296,8 +296,8 @@ class LibraryTest { assert(paymentReceivedEvent is Event.PaymentReceived) node2.eventHandled() - assert(node1.listPayments().size == 1) - assert(node2.listPayments().size == 1) + assert(node1.listPayments().size == 3) + assert(node2.listPayments().size == 2) node2.closeChannel(userChannelId, nodeId1) diff --git a/src/payment/store.rs b/src/payment/store.rs index 2c968dc96..edae27329 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -292,6 +292,11 @@ impl_writeable_tlv_based_enum!(PaymentStatus, #[derive(Clone, Debug, PartialEq, Eq)] pub enum PaymentKind { /// An on-chain payment. + /// + /// Payments of this kind will be considered pending until the respective transaction has + /// reached [`ANTI_REORG_DELAY`] confirmations on-chain. + /// + /// [`ANTI_REORG_DELAY`]: lightning::chain::channelmonitor::ANTI_REORG_DELAY Onchain { /// The transaction identifier of this payment. txid: Txid, diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 4cb2f88b2..6f8b7b185 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -10,13 +10,16 @@ use persist::KVStoreWalletPersister; use crate::logger::{log_debug, log_error, log_info, log_trace, Logger, LdkLogger}; use crate::fee_estimator::{ConfirmationTarget, FeeEstimator}; -use crate::payment::store::PaymentStore; +use crate::payment::store::{ConfirmationStatus, PaymentStore}; +use crate::payment::{PaymentDetails, PaymentDirection, PaymentStatus}; use crate::Error; use lightning::chain::chaininterface::BroadcasterInterface; +use lightning::chain::channelmonitor::ANTI_REORG_DELAY; use lightning::chain::{BestBlock, Listen}; use lightning::events::bump_transaction::{Utxo, WalletSource}; +use lightning::ln::channelmanager::PaymentId; use lightning::ln::inbound_payment::ExpandedKey; use lightning::ln::msgs::{DecodeError, UnsignedGossipMessage}; use lightning::ln::script::ShutdownScript; @@ -46,6 +49,7 @@ use bitcoin::{ use std::ops::Deref; use std::sync::{Arc, Mutex}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; pub(crate) enum OnchainSendAmount { ExactRetainingReserve { amount_sats: u64, cur_anchor_reserve_sats: u64 }, @@ -110,6 +114,11 @@ where Error::PersistenceFailed })?; + self.update_payment_store(&mut *locked_wallet).map_err(|e| { + log_error!(self.logger, "Failed to update payment store: {}", e); + Error::PersistenceFailed + })?; + Ok(()) }, Err(e) => { @@ -134,6 +143,76 @@ where Ok(()) } + fn update_payment_store<'a>( + &self, locked_wallet: &'a mut PersistedWallet, + ) -> Result<(), Error> { + let latest_update_timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or(Duration::from_secs(0)) + .as_secs(); + + for wtx in locked_wallet.transactions() { + let id = PaymentId(wtx.tx_node.txid.to_byte_array()); + let txid = wtx.tx_node.txid; + let (payment_status, confirmation_status) = match wtx.chain_position { + bdk_chain::ChainPosition::Confirmed { anchor, .. } => { + let confirmation_height = anchor.block_id.height; + let cur_height = locked_wallet.latest_checkpoint().height(); + let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1 + { + PaymentStatus::Succeeded + } else { + PaymentStatus::Pending + }; + let confirmation_status = ConfirmationStatus::Confirmed { + block_hash: anchor.block_id.hash, + height: confirmation_height, + timestamp: anchor.confirmation_time, + }; + (payment_status, confirmation_status) + }, + bdk_chain::ChainPosition::Unconfirmed { .. } => { + (PaymentStatus::Pending, ConfirmationStatus::Unconfirmed) + }, + }; + // TODO: It would be great to introduce additional variants for + // `ChannelFunding` and `ChannelClosing`. For the former, we could just + // take a reference to `ChannelManager` here and check against + // `list_channels`. But for the latter the best approach is much less + // clear: for force-closes/HTLC spends we should be good querying + // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes + // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly + // spent to a wallet address. The only solution I can come up with is to + // create and persist a list of 'static pending outputs' that we could use + // here to determine the `PaymentKind`, but that's not really satisfactory, so + // we're punting on it until we can come up with a better solution. + let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status }; + let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx); + let (direction, amount_msat) = if sent > received { + let direction = PaymentDirection::Outbound; + let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000); + (direction, amount_msat) + } else { + let direction = PaymentDirection::Inbound; + let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000); + (direction, amount_msat) + }; + + let payment = PaymentDetails { + id, + kind, + amount_msat, + direction, + status: payment_status, + latest_update_timestamp, + }; + + self.payment_store.insert_or_update(&payment)?; + } + + Ok(()) + } + pub(crate) fn create_funding_transaction( &self, output_script: ScriptBuf, amount: Amount, confirmation_target: ConfirmationTarget, locktime: LockTime, @@ -481,7 +560,12 @@ where } match locked_wallet.apply_block(block, height) { - Ok(()) => (), + Ok(()) => { + if let Err(e) = self.update_payment_store(&mut *locked_wallet) { + log_error!(self.logger, "Failed to update payment store: {}", e); + return; + } + }, Err(e) => { log_error!( self.logger, diff --git a/tests/common/mod.rs b/tests/common/mod.rs index cb58a28cd..31a08eb07 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -484,6 +484,36 @@ pub(crate) fn do_channel_full_cycle( assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, premine_amount_sat); assert_eq!(node_b.list_balances().spendable_onchain_balance_sats, premine_amount_sat); + // Check we saw the node funding transactions. + assert_eq!( + node_a + .list_payments_with_filter(|p| p.direction == PaymentDirection::Inbound + && matches!(p.kind, PaymentKind::Onchain { .. })) + .len(), + 1 + ); + assert_eq!( + node_a + .list_payments_with_filter(|p| p.direction == PaymentDirection::Outbound + && matches!(p.kind, PaymentKind::Onchain { .. })) + .len(), + 0 + ); + assert_eq!( + node_b + .list_payments_with_filter(|p| p.direction == PaymentDirection::Inbound + && matches!(p.kind, PaymentKind::Onchain { .. })) + .len(), + 1 + ); + assert_eq!( + node_b + .list_payments_with_filter(|p| p.direction == PaymentDirection::Outbound + && matches!(p.kind, PaymentKind::Onchain { .. })) + .len(), + 0 + ); + // Check we haven't got any events yet assert_eq!(node_a.next_event(), None); assert_eq!(node_b.next_event(), None); @@ -516,6 +546,15 @@ pub(crate) fn do_channel_full_cycle( node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); + // Check we now see the channel funding transaction as outbound. + assert_eq!( + node_a + .list_payments_with_filter(|p| p.direction == PaymentDirection::Outbound + && matches!(p.kind, PaymentKind::Onchain { .. })) + .len(), + 1 + ); + let onchain_fee_buffer_sat = 5000; let node_a_anchor_reserve_sat = if expect_anchor_channel { 25_000 } else { 0 }; let node_a_upper_bound_sat = @@ -565,22 +604,26 @@ pub(crate) fn do_channel_full_cycle( let payment_id = node_a.bolt11_payment().send(&invoice, None).unwrap(); assert_eq!(node_a.bolt11_payment().send(&invoice, None), Err(NodeError::DuplicatePayment)); - assert_eq!(node_a.list_payments().first().unwrap().id, payment_id); + assert!(!node_a.list_payments_with_filter(|p| p.id == payment_id).is_empty()); - let outbound_payments_a = - node_a.list_payments_with_filter(|p| p.direction == PaymentDirection::Outbound); + let outbound_payments_a = node_a.list_payments_with_filter(|p| { + p.direction == PaymentDirection::Outbound && matches!(p.kind, PaymentKind::Bolt11 { .. }) + }); assert_eq!(outbound_payments_a.len(), 1); - let inbound_payments_a = - node_a.list_payments_with_filter(|p| p.direction == PaymentDirection::Inbound); + let inbound_payments_a = node_a.list_payments_with_filter(|p| { + p.direction == PaymentDirection::Inbound && matches!(p.kind, PaymentKind::Bolt11 { .. }) + }); assert_eq!(inbound_payments_a.len(), 0); - let outbound_payments_b = - node_b.list_payments_with_filter(|p| p.direction == PaymentDirection::Outbound); + let outbound_payments_b = node_b.list_payments_with_filter(|p| { + p.direction == PaymentDirection::Outbound && matches!(p.kind, PaymentKind::Bolt11 { .. }) + }); assert_eq!(outbound_payments_b.len(), 0); - let inbound_payments_b = - node_b.list_payments_with_filter(|p| p.direction == PaymentDirection::Inbound); + 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); expect_event!(node_a, PaymentSuccessful); @@ -814,8 +857,26 @@ pub(crate) fn do_channel_full_cycle( node_b.payment(&keysend_payment_id).unwrap().kind, PaymentKind::Spontaneous { .. } )); - assert_eq!(node_a.list_payments().len(), 6); - assert_eq!(node_b.list_payments().len(), 7); + 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 + ); + assert_eq!( + node_a + .list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Spontaneous { .. })) + .len(), + 1 + ); + assert_eq!( + node_b + .list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Spontaneous { .. })) + .len(), + 1 + ); println!("\nB close_channel (force: {})", force_close); if force_close { @@ -936,6 +997,22 @@ pub(crate) fn do_channel_full_cycle( assert_eq!(node_a.list_balances().total_anchor_channels_reserve_sats, 0); assert_eq!(node_b.list_balances().total_anchor_channels_reserve_sats, 0); + // Now we should have seen the channel closing transaction on-chain. + assert_eq!( + node_a + .list_payments_with_filter(|p| p.direction == PaymentDirection::Inbound + && matches!(p.kind, PaymentKind::Onchain { .. })) + .len(), + 2 + ); + assert_eq!( + node_b + .list_payments_with_filter(|p| p.direction == PaymentDirection::Inbound + && matches!(p.kind, PaymentKind::Onchain { .. })) + .len(), + 2 + ); + // Check we handled all events assert_eq!(node_a.next_event(), None); assert_eq!(node_b.next_event(), None); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 8dd39133b..2dc74cea9 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -15,7 +15,10 @@ use common::{ }; use ldk_node::config::EsploraSyncConfig; -use ldk_node::payment::{PaymentKind, QrPaymentResult, SendingParameters}; +use ldk_node::payment::{ + ConfirmationStatus, PaymentDirection, PaymentKind, PaymentStatus, QrPaymentResult, + SendingParameters, +}; use ldk_node::{Builder, Event, NodeError}; use lightning::ln::channelmanager::PaymentId; @@ -299,6 +302,24 @@ fn onchain_spend_receive() { assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, premine_amount_sat); assert_eq!(node_b.list_balances().spendable_onchain_balance_sats, premine_amount_sat); + let node_a_payments = node_a.list_payments(); + let node_b_payments = node_b.list_payments(); + for payments in [&node_a_payments, &node_b_payments] { + assert_eq!(payments.len(), 1) + } + for p in [node_a_payments.first().unwrap(), node_b_payments.first().unwrap()] { + assert_eq!(p.amount_msat, Some(premine_amount_sat * 1000)); + assert_eq!(p.direction, PaymentDirection::Inbound); + // We got only 1-conf here, so we're only pending for now. + assert_eq!(p.status, PaymentStatus::Pending); + match p.kind { + PaymentKind::Onchain { status, .. } => { + assert!(matches!(status, ConfirmationStatus::Confirmed { .. })); + }, + _ => panic!("Unexpected payment kind"), + } + } + let channel_amount_sat = 1_000_000; let reserve_amount_sat = 25_000; open_channel(&node_b, &node_a, channel_amount_sat, true, &electrsd); @@ -309,6 +330,13 @@ fn onchain_spend_receive() { expect_channel_ready_event!(node_a, node_b.node_id()); expect_channel_ready_event!(node_b, node_a.node_id()); + let node_a_payments = + node_a.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); + assert_eq!(node_a_payments.len(), 1); + let node_b_payments = + node_b.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); + assert_eq!(node_b_payments.len(), 2); + let onchain_fee_buffer_sat = 1000; let expected_node_a_balance = premine_amount_sat - reserve_amount_sat; let expected_node_b_balance_lower = @@ -340,6 +368,13 @@ fn onchain_spend_receive() { assert!(node_b.list_balances().spendable_onchain_balance_sats > expected_node_b_balance_lower); assert!(node_b.list_balances().spendable_onchain_balance_sats < expected_node_b_balance_upper); + let node_a_payments = + node_a.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); + assert_eq!(node_a_payments.len(), 2); + let node_b_payments = + node_b.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); + assert_eq!(node_b_payments.len(), 3); + let addr_b = node_b.onchain_payment().new_address().unwrap(); let txid = node_a.onchain_payment().send_all_to_address(&addr_b, true, None).unwrap(); generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); @@ -356,6 +391,13 @@ fn onchain_spend_receive() { assert!(node_b.list_balances().spendable_onchain_balance_sats > expected_node_b_balance_lower); assert!(node_b.list_balances().spendable_onchain_balance_sats < expected_node_b_balance_upper); + let node_a_payments = + node_a.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); + assert_eq!(node_a_payments.len(), 3); + let node_b_payments = + node_b.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); + assert_eq!(node_b_payments.len(), 4); + let addr_b = node_b.onchain_payment().new_address().unwrap(); let txid = node_a.onchain_payment().send_all_to_address(&addr_b, false, None).unwrap(); generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); @@ -372,6 +414,13 @@ fn onchain_spend_receive() { assert_eq!(node_a.list_balances().total_onchain_balance_sats, expected_node_a_balance); assert!(node_b.list_balances().spendable_onchain_balance_sats > expected_node_b_balance_lower); assert!(node_b.list_balances().spendable_onchain_balance_sats < expected_node_b_balance_upper); + + let node_a_payments = + node_a.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); + assert_eq!(node_a_payments.len(), 4); + let node_b_payments = + node_b.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); + assert_eq!(node_b_payments.len(), 5); } #[test] @@ -613,7 +662,8 @@ fn simple_bolt12_send_receive() { .unwrap(); expect_payment_successful_event!(node_a, Some(payment_id), None); - let node_a_payments = node_a.list_payments(); + let node_a_payments = + node_a.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Bolt12Offer { .. })); assert_eq!(node_a_payments.len(), 1); match node_a_payments.first().unwrap().kind { PaymentKind::Bolt12Offer { @@ -639,7 +689,8 @@ 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_payments = node_b.list_payments(); + let node_b_payments = + node_b.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Bolt12Offer { .. })); assert_eq!(node_b_payments.len(), 1); match node_b_payments.first().unwrap().kind { PaymentKind::Bolt12Offer { hash, preimage, secret, offer_id, .. } => { @@ -676,7 +727,9 @@ fn simple_bolt12_send_receive() { .unwrap(); expect_payment_successful_event!(node_a, Some(payment_id), None); - let node_a_payments = node_a.list_payments_with_filter(|p| p.id == payment_id); + let node_a_payments = node_a.list_payments_with_filter(|p| { + 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 { PaymentKind::Bolt12Offer { @@ -704,7 +757,9 @@ fn simple_bolt12_send_receive() { expect_payment_received_event!(node_b, expected_amount_msat); let node_b_payment_id = PaymentId(payment_hash.0); - let node_b_payments = node_b.list_payments_with_filter(|p| p.id == node_b_payment_id); + let node_b_payments = node_b.list_payments_with_filter(|p| { + matches!(p.kind, PaymentKind::Bolt12Offer { .. }) && p.id == node_b_payment_id + }); assert_eq!(node_b_payments.len(), 1); match node_b_payments.first().unwrap().kind { PaymentKind::Bolt12Offer { hash, preimage, secret, offer_id, .. } => { @@ -731,13 +786,18 @@ fn simple_bolt12_send_receive() { expect_payment_received_event!(node_a, overpaid_amount); let node_b_payment_id = node_b - .list_payments_with_filter(|p| p.amount_msat == Some(overpaid_amount)) + .list_payments_with_filter(|p| { + matches!(p.kind, PaymentKind::Bolt12Refund { .. }) + && p.amount_msat == Some(overpaid_amount) + }) .first() .unwrap() .id; expect_payment_successful_event!(node_b, Some(node_b_payment_id), None); - let node_b_payments = node_b.list_payments_with_filter(|p| p.id == node_b_payment_id); + let node_b_payments = node_b.list_payments_with_filter(|p| { + matches!(p.kind, PaymentKind::Bolt12Refund { .. }) && p.id == node_b_payment_id + }); assert_eq!(node_b_payments.len(), 1); match node_b_payments.first().unwrap().kind { PaymentKind::Bolt12Refund { @@ -761,7 +821,9 @@ 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| p.id == node_a_payment_id); + let node_a_payments = node_a.list_payments_with_filter(|p| { + matches!(p.kind, PaymentKind::Bolt12Refund { .. }) && p.id == node_a_payment_id + }); assert_eq!(node_a_payments.len(), 1); match node_a_payments.first().unwrap().kind { PaymentKind::Bolt12Refund { hash, preimage, secret, .. } => {