From 5d3e93ee0d1d9866bb765d2b7729ca8a9ec29bab Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jan 2025 15:19:36 +0100 Subject: [PATCH 1/7] 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 fbeba669b..5c1e3de39 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 6e5e9be07cfe701c4b70f9b2c7fc621240616fc2 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jan 2025 15:45:47 +0100 Subject: [PATCH 2/7] Prefactor: Implement `PaymentStore::batch_update` We implement a batch 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 5c1e3de39..393b12b7d 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}; @@ -440,6 +440,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: Logger, @@ -468,6 +491,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 4bcd4dcd29153ac4aec0ac0bcaf7a67a07b47c4c Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jan 2025 13:44:23 +0100 Subject: [PATCH 3/7] 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/store.rs | 56 +++++++++++++++++++++++++++++++++++++++++-- src/uniffi_types.rs | 4 +++- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index 2d93fb397..1de433874 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -326,7 +326,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); @@ -357,6 +357,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/store.rs b/src/payment/store.rs index 393b12b7d..067f6cd91 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; @@ -148,6 +150,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) @@ -273,7 +284,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 @@ -362,7 +378,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), @@ -395,6 +414,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 time at which the block was confirmed. + 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. @@ -424,6 +468,7 @@ pub(crate) struct PaymentDetailsUpdate { pub amount_msat: Option>, pub direction: Option, pub status: Option, + pub confirmation_status: Option, } impl PaymentDetailsUpdate { @@ -436,6 +481,7 @@ impl PaymentDetailsUpdate { amount_msat: None, direction: None, status: None, + confirmation_status: None, } } } @@ -451,6 +497,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), @@ -459,6 +510,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 0fe0f31b0..7f5d3e40f 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 9b33e52d891c2c95aafe6f6495ee27e548b5e79e Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 27 Jan 2025 16:37:02 +0100 Subject: [PATCH 4/7] 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 ceb3c0918..87a520ba9 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -782,11 +782,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), )); @@ -1176,16 +1187,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 3533e6fef..f2ae0a789 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, Logger}; +use crate::logger::{log_debug, log_error, log_info, log_trace, FilesystemLogger, Logger}; use crate::fee_estimator::{ConfirmationTarget, FeeEstimator}; +use crate::payment::store::PaymentStore; use crate::Error; use lightning::chain::chaininterface::BroadcasterInterface; @@ -65,6 +66,7 @@ where persister: Mutex, broadcaster: B, fee_estimator: E, + payment_store: Arc>>, logger: L, } @@ -76,11 +78,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 306a78ef1d004563827309264e9fca390826f924 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 15 Jan 2025 16:32:19 +0100 Subject: [PATCH 5/7] Update `PaymentStore` after each `Wallet` sync We update the payment store whenever syncing the wallet state finished. --- src/payment/store.rs | 5 +++ src/wallet/mod.rs | 88 ++++++++++++++++++++++++++++++++++++++- tests/common/mod.rs | 99 +++++++++++++++++++++++++++++++++++++++----- 3 files changed, 179 insertions(+), 13 deletions(-) diff --git a/src/payment/store.rs b/src/payment/store.rs index 067f6cd91..f75feef43 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -284,6 +284,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 f2ae0a789..ba0485ff1 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, FilesystemLogger, Logger}; 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; @@ -45,6 +48,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 }, @@ -109,6 +113,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) => { @@ -133,6 +142,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, @@ -477,7 +556,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 1e82fc60e..0fcdcc55f 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -483,6 +483,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); @@ -515,6 +545,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 = @@ -564,22 +603,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); @@ -813,8 +856,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 { @@ -935,6 +996,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); From a570108e3c50abfd2a65354ece46a9658ca3fec4 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 27 Jan 2025 16:37:12 +0100 Subject: [PATCH 6/7] Have `Wallet` take `EventQueue` and `ChannelManager` refs --- src/builder.rs | 26 ++++++++++++++------------ src/wallet/mod.rs | 22 ++++++++++++++++++++-- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 87a520ba9..e7d10ec29 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -792,12 +792,25 @@ fn build_with_store_internal( }, }; + let event_queue = match io::utils::read_event_queue(Arc::clone(&kv_store), Arc::clone(&logger)) + { + Ok(event_queue) => Arc::new(event_queue), + Err(e) => { + if e.kind() == std::io::ErrorKind::NotFound { + Arc::new(EventQueue::new(Arc::clone(&kv_store), Arc::clone(&logger))) + } else { + 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(&event_queue), Arc::clone(&logger), )); @@ -1006,6 +1019,7 @@ fn build_with_store_internal( }; let channel_manager = Arc::new(channel_manager); + wallet.set_channel_manager(Arc::clone(&channel_manager)); // Give ChannelMonitors to ChainMonitor for (_blockhash, channel_monitor) in channel_monitors.into_iter() { @@ -1187,18 +1201,6 @@ fn build_with_store_internal( }, } - let event_queue = match io::utils::read_event_queue(Arc::clone(&kv_store), Arc::clone(&logger)) - { - Ok(event_queue) => Arc::new(event_queue), - Err(e) => { - if e.kind() == std::io::ErrorKind::NotFound { - Arc::new(EventQueue::new(Arc::clone(&kv_store), Arc::clone(&logger))) - } else { - return Err(BuildError::ReadFailed); - } - }, - }; - let peer_store = match io::utils::read_peer_info(Arc::clone(&kv_store), Arc::clone(&logger)) { Ok(peer_store) => Arc::new(peer_store), Err(e) => { diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index ba0485ff1..25a53e4e3 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -9,9 +9,11 @@ use persist::KVStoreWalletPersister; use crate::logger::{log_debug, log_error, log_info, log_trace, FilesystemLogger, Logger}; +use crate::event::EventQueue; use crate::fee_estimator::{ConfirmationTarget, FeeEstimator}; use crate::payment::store::{ConfirmationStatus, PaymentStore}; use crate::payment::{PaymentDetails, PaymentDirection, PaymentStatus}; +use crate::types::ChannelManager; use crate::Error; use lightning::chain::chaininterface::BroadcasterInterface; @@ -68,9 +70,11 @@ where // A BDK on-chain wallet. inner: Mutex>, persister: Mutex, + channel_manager: Mutex>>, broadcaster: B, fee_estimator: E, payment_store: Arc>>, + event_queue: Arc>>, logger: L, } @@ -83,13 +87,27 @@ where pub(crate) fn new( wallet: bdk_wallet::PersistedWallet, wallet_persister: KVStoreWalletPersister, broadcaster: B, fee_estimator: E, - payment_store: Arc>>, logger: L, + payment_store: Arc>>, + event_queue: Arc>>, logger: L, ) -> Self { let inner = Mutex::new(wallet); let persister = Mutex::new(wallet_persister); - Self { inner, persister, broadcaster, fee_estimator, payment_store, logger } + let channel_manager = Mutex::new(None); + Self { + inner, + persister, + channel_manager, + broadcaster, + fee_estimator, + payment_store, + event_queue, + logger, + } } + pub(crate) fn set_channel_manager(&self, channel_manager: Arc) { + *self.channel_manager.lock().unwrap() = Some(channel_manager); + } pub(crate) fn get_full_scan_request(&self) -> FullScanRequest { self.inner.lock().unwrap().start_full_scan().build() } From b2cbeb063e716dd94fe7881a414246df321b8afb Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 16 Jan 2025 14:17:16 +0100 Subject: [PATCH 7/7] Add and emit `OnchainPaymentSuccessful` and `OnchainPaymentReceived` events .. two new events that we're emitting when we sent or received an onchain payment, i.e., the transactions reached ANTI_REORG_DELAY confirmations. --- bindings/ldk_node.udl | 2 ++ src/event.rs | 53 +++++++++++++++++++++++++++++++++++++++++-- src/wallet/mod.rs | 45 ++++++++++++++++++++++++++++++++---- 3 files changed, 93 insertions(+), 7 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index 1de433874..fe04dacef 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -291,6 +291,8 @@ interface Event { ChannelPending(ChannelId channel_id, UserChannelId user_channel_id, ChannelId former_temporary_channel_id, PublicKey counterparty_node_id, OutPoint funding_txo); ChannelReady(ChannelId channel_id, UserChannelId user_channel_id, PublicKey? counterparty_node_id); ChannelClosed(ChannelId channel_id, UserChannelId user_channel_id, PublicKey? counterparty_node_id, ClosureReason? reason); + OnchainPaymentSuccessful(PaymentId payment_id, Txid txid, u64 amount_msat, BlockHash block_hash, u32 block_height); + OnchainPaymentReceived(PaymentId payment_id, Txid txid, u64 amount_msat, BlockHash block_hash, u32 block_height); }; enum PaymentFailureReason { diff --git a/src/event.rs b/src/event.rs index 1de77e937..f479fdbc6 100644 --- a/src/event.rs +++ b/src/event.rs @@ -42,7 +42,7 @@ use lightning_liquidity::lsps2::utils::compute_opening_fee; use bitcoin::blockdata::locktime::absolute::LockTime; use bitcoin::secp256k1::PublicKey; -use bitcoin::{Amount, OutPoint}; +use bitcoin::{Amount, BlockHash, OutPoint, Txid}; use rand::{thread_rng, Rng}; @@ -221,6 +221,41 @@ pub enum Event { /// This will be `None` for events serialized by LDK Node v0.2.1 and prior. reason: Option, }, + /// A sent onchain payment was successful. + /// + /// It's guaranteed to have reached at least [`ANTI_REORG_DELAY`] delay confirmations. + /// + /// + /// [`ANTI_REORG_DELAY`]: lightning::chain::channelmonitor::ANTI_REORG_DELAY + OnchainPaymentSuccessful { + /// A local identifier used to track the payment. + payment_id: PaymentId, + /// The transaction identifier. + txid: Txid, + /// The value, in thousandths of a satoshi, that has been received. + amount_msat: u64, + /// The hash of the block in which the transaction was confirmed. + block_hash: BlockHash, + /// The height under which the block was confirmed. + block_height: u32, + }, + /// An onchain payment has been received. + /// + /// It's guaranteed to have reached at least [`ANTI_REORG_DELAY`] delay confirmations. + /// + /// [`ANTI_REORG_DELAY`]: lightning::chain::channelmonitor::ANTI_REORG_DELAY + OnchainPaymentReceived { + /// A local identifier used to track the payment. + payment_id: PaymentId, + /// The transaction identifier. + txid: Txid, + /// The value, in thousandths of a satoshi, that has been received. + amount_msat: u64, + /// The hash of the block in which the transaction was confirmed. + block_hash: BlockHash, + /// The height under which the block was confirmed. + block_height: u32, + }, } impl_writeable_tlv_based_enum!(Event, @@ -277,7 +312,21 @@ impl_writeable_tlv_based_enum!(Event, (10, skimmed_fee_msat, option), (12, claim_from_onchain_tx, required), (14, outbound_amount_forwarded_msat, option), - } + }, + (8, OnchainPaymentSuccessful) => { + (0, payment_id, required), + (2, txid, required), + (4, amount_msat, required), + (6, block_hash, required), + (8, block_height, required), + }, + (9, OnchainPaymentReceived) => { + (0, payment_id, required), + (2, txid, required), + (4, amount_msat, required), + (6, block_hash, required), + (8, block_height, required), + }, ); pub struct EventQueue diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 25a53e4e3..8715e8bb3 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -9,7 +9,7 @@ use persist::KVStoreWalletPersister; use crate::logger::{log_debug, log_error, log_info, log_trace, FilesystemLogger, Logger}; -use crate::event::EventQueue; +use crate::event::{Event, EventQueue}; use crate::fee_estimator::{ConfirmationTarget, FeeEstimator}; use crate::payment::store::{ConfirmationStatus, PaymentStore}; use crate::payment::{PaymentDetails, PaymentDirection, PaymentStatus}; @@ -207,24 +207,59 @@ where 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); + let amount_msat = 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); + let amount_msat = received.to_sat().saturating_sub(sent.to_sat()) * 1000; (direction, amount_msat) }; let payment = PaymentDetails { id, kind, - amount_msat, + amount_msat: Some(amount_msat), direction, status: payment_status, latest_update_timestamp, }; - self.payment_store.insert_or_update(&payment)?; + let updated = self.payment_store.insert_or_update(&payment)?; + + // If we just updated the entry and we deem the transaction successful (i.e., has seen + // at least ANTI_REORG_DELAY confirmations), issue an event. + if updated && payment_status == PaymentStatus::Succeeded { + match confirmation_status { + ConfirmationStatus::Confirmed { block_hash, height, .. } => { + let event; + if direction == PaymentDirection::Outbound { + event = Event::OnchainPaymentSuccessful { + payment_id: id, + txid, + block_hash, + block_height: height, + amount_msat, + }; + } else { + event = Event::OnchainPaymentReceived { + payment_id: id, + txid, + block_hash, + block_height: height, + amount_msat, + }; + } + self.event_queue.add_event(event).map_err(|e| { + log_error!(self.logger, "Failed to push to event queue: {}", e); + Error::PersistenceFailed + })?; + }, + _ => { + // We only issue events for transactions that have seen ANTI_REORG_DELAY + // confirmations. + }, + } + } } Ok(())