From a38b1803706c4650d0c5d74042d671e50f58bbd4 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 13 Feb 2025 15:03:51 +0100 Subject: [PATCH 1/2] Fix amount calculation for tracked onchain payments We recently started tracking onchain payments in the store. It turns out the calculated amount was slightly off as in the outbound case it would include the paid fees. Here, we fix this minor bug. --- src/wallet/mod.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index f623cfc2c..1b22ade17 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -187,14 +187,21 @@ where // 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 fee = locked_wallet.calculate_fee(&wtx.tx_node.tx).unwrap_or(Amount::ZERO); 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 = Some( + sent.to_sat().saturating_sub(fee.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 = Some( + received.to_sat().saturating_sub(sent.to_sat().saturating_sub(fee.to_sat())) + * 1000, + ); (direction, amount_msat) }; From 0b7af80ab517630cbd6197f75a63197913de4705 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 13 Feb 2025 15:05:02 +0100 Subject: [PATCH 2/2] Track paid fees in `PaymentDetails` Since we split-out the paid fees anways, we optionally start tracking them in `PaymentDetails` for all payment types. --- bindings/ldk_node.udl | 1 + src/event.rs | 3 ++ src/payment/bolt11.rs | 6 ++++ src/payment/bolt12.rs | 6 ++++ src/payment/spontaneous.rs | 2 ++ src/payment/store.rs | 42 +++++++++++++++++++++++---- src/wallet/mod.rs | 16 ++++------- tests/common/mod.rs | 6 ++++ tests/integration_tests_rust.rs | 50 +++++++++++++++++++++++++++++++-- 9 files changed, 113 insertions(+), 19 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index 5c4133c46..5c5238e0a 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -412,6 +412,7 @@ dictionary PaymentDetails { PaymentId id; PaymentKind kind; u64? amount_msat; + u64? fee_paid_msat; PaymentDirection direction; PaymentStatus status; u64 latest_update_timestamp; diff --git a/src/event.rs b/src/event.rs index 40e236793..8cd01c5af 100644 --- a/src/event.rs +++ b/src/event.rs @@ -725,6 +725,7 @@ where payment_id, kind, Some(amount_msat), + None, PaymentDirection::Inbound, PaymentStatus::Pending, ); @@ -765,6 +766,7 @@ where payment_id, kind, Some(amount_msat), + None, PaymentDirection::Inbound, PaymentStatus::Pending, ); @@ -931,6 +933,7 @@ where let update = PaymentDetailsUpdate { hash: Some(Some(payment_hash)), preimage: Some(Some(payment_preimage)), + fee_paid_msat: Some(fee_paid_msat), status: Some(PaymentStatus::Succeeded), ..PaymentDetailsUpdate::new(payment_id) }; diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index 2c1b19143..5c6ce35f8 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -160,6 +160,7 @@ impl Bolt11Payment { payment_id, kind, invoice.amount_milli_satoshis(), + None, PaymentDirection::Outbound, PaymentStatus::Pending, ); @@ -182,6 +183,7 @@ impl Bolt11Payment { payment_id, kind, invoice.amount_milli_satoshis(), + None, PaymentDirection::Outbound, PaymentStatus::Failed, ); @@ -293,6 +295,7 @@ impl Bolt11Payment { payment_id, kind, Some(amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Pending, ); @@ -315,6 +318,7 @@ impl Bolt11Payment { payment_id, kind, Some(amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Failed, ); @@ -531,6 +535,7 @@ impl Bolt11Payment { id, kind, amount_msat, + None, PaymentDirection::Inbound, PaymentStatus::Pending, ); @@ -666,6 +671,7 @@ impl Bolt11Payment { id, kind, amount_msat, + None, PaymentDirection::Inbound, PaymentStatus::Pending, ); diff --git a/src/payment/bolt12.rs b/src/payment/bolt12.rs index 1ff8739be..dbeee0ab8 100644 --- a/src/payment/bolt12.rs +++ b/src/payment/bolt12.rs @@ -113,6 +113,7 @@ impl Bolt12Payment { payment_id, kind, Some(offer_amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Pending, ); @@ -137,6 +138,7 @@ impl Bolt12Payment { payment_id, kind, Some(offer_amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Failed, ); @@ -217,6 +219,7 @@ impl Bolt12Payment { payment_id, kind, Some(amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Pending, ); @@ -241,6 +244,7 @@ impl Bolt12Payment { payment_id, kind, Some(amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Failed, ); @@ -338,6 +342,7 @@ impl Bolt12Payment { payment_id, kind, Some(refund.amount_msats()), + None, PaymentDirection::Inbound, PaymentStatus::Pending, ); @@ -402,6 +407,7 @@ impl Bolt12Payment { payment_id, kind, Some(amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Pending, ); diff --git a/src/payment/spontaneous.rs b/src/payment/spontaneous.rs index 984619855..f33ea15cc 100644 --- a/src/payment/spontaneous.rs +++ b/src/payment/spontaneous.rs @@ -140,6 +140,7 @@ impl SpontaneousPayment { payment_id, kind, Some(amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Pending, ); @@ -161,6 +162,7 @@ impl SpontaneousPayment { payment_id, kind, Some(amount_msat), + None, PaymentDirection::Outbound, PaymentStatus::Failed, ); diff --git a/src/payment/store.rs b/src/payment/store.rs index 7e677c02d..8a9222912 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -42,6 +42,13 @@ pub struct PaymentDetails { pub kind: PaymentKind, /// The amount transferred. pub amount_msat: Option, + /// The fees that were paid for this payment. + /// + /// For Lightning payments, this will only be updated for outbound payments once they + /// succeeded. + /// + /// Will be `None` for Lightning payments made with LDK Node v0.4.x and earlier. + pub fee_paid_msat: Option, /// The direction of the payment. pub direction: PaymentDirection, /// The status of the payment. @@ -52,14 +59,14 @@ pub struct PaymentDetails { impl PaymentDetails { pub(crate) fn new( - id: PaymentId, kind: PaymentKind, amount_msat: Option, direction: PaymentDirection, - status: PaymentStatus, + id: PaymentId, kind: PaymentKind, amount_msat: Option, fee_paid_msat: Option, + direction: PaymentDirection, status: PaymentStatus, ) -> Self { let latest_update_timestamp = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or(Duration::from_secs(0)) .as_secs(); - Self { id, kind, amount_msat, direction, status, latest_update_timestamp } + Self { id, kind, amount_msat, fee_paid_msat, direction, status, latest_update_timestamp } } pub(crate) fn update(&mut self, update: &PaymentDetailsUpdate) -> bool { @@ -154,6 +161,10 @@ impl PaymentDetails { update_if_necessary!(self.amount_msat, amount_opt); } + if let Some(fee_paid_msat_opt) = update.fee_paid_msat { + update_if_necessary!(self.fee_paid_msat, fee_paid_msat_opt); + } + if let Some(status) = update.status { update_if_necessary!(self.status, status); } @@ -192,6 +203,7 @@ impl Writeable for PaymentDetails { (4, None::>, required), (5, self.latest_update_timestamp, required), (6, self.amount_msat, required), + (7, self.fee_paid_msat, option), (8, self.direction, required), (10, self.status, required) }); @@ -213,6 +225,7 @@ impl Readable for PaymentDetails { (4, secret, required), (5, latest_update_timestamp, (default_value, unix_time_secs)), (6, amount_msat, required), + (7, fee_paid_msat, option), (8, direction, required), (10, status, required) }); @@ -253,7 +266,15 @@ impl Readable for PaymentDetails { } }; - Ok(PaymentDetails { id, kind, amount_msat, direction, status, latest_update_timestamp }) + Ok(PaymentDetails { + id, + kind, + amount_msat, + fee_paid_msat, + direction, + status, + latest_update_timestamp, + }) } } @@ -479,6 +500,7 @@ pub(crate) struct PaymentDetailsUpdate { pub preimage: Option>, pub secret: Option>, pub amount_msat: Option>, + pub fee_paid_msat: Option>, pub direction: Option, pub status: Option, pub confirmation_status: Option, @@ -492,6 +514,7 @@ impl PaymentDetailsUpdate { preimage: None, secret: None, amount_msat: None, + fee_paid_msat: None, direction: None, status: None, confirmation_status: None, @@ -521,6 +544,7 @@ impl From<&PaymentDetails> for PaymentDetailsUpdate { preimage: Some(preimage), secret: Some(secret), amount_msat: Some(value.amount_msat), + fee_paid_msat: Some(value.fee_paid_msat), direction: Some(value.direction), status: Some(value.status), confirmation_status, @@ -708,8 +732,14 @@ mod tests { .is_err()); let kind = PaymentKind::Bolt11 { hash, preimage: None, secret: None }; - let payment = - PaymentDetails::new(id, kind, None, PaymentDirection::Inbound, PaymentStatus::Pending); + let payment = PaymentDetails::new( + id, + kind, + None, + None, + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); assert_eq!(Ok(false), payment_store.insert(payment.clone())); assert!(payment_store.get(&id).is_some()); diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 1b22ade17..58a28308d 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -49,7 +49,6 @@ 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 }, @@ -146,11 +145,6 @@ where 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; @@ -205,14 +199,16 @@ where (direction, amount_msat) }; - let payment = PaymentDetails { + let fee_paid_msat = Some(fee.to_sat() * 1000); + + let payment = PaymentDetails::new( id, kind, amount_msat, + fee_paid_msat, direction, - status: payment_status, - latest_update_timestamp, - }; + payment_status, + ); self.payment_store.insert_or_update(&payment)?; } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 0c0c24b7c..2ad736823 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -103,6 +103,10 @@ 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(); + if !matches!(payment.kind, PaymentKind::Onchain { .. }) { + assert_eq!(payment.fee_paid_msat, None); + } $node.event_handled(); payment_id }, @@ -148,6 +152,8 @@ 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(); + assert_eq!(payment.fee_paid_msat, fee_paid_msat); assert_eq!(payment_id, $payment_id); $node.event_handled(); }, diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 2dc74cea9..19e7806d7 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -26,6 +26,7 @@ use lightning::util::persist::KVStore; use bitcoincore_rpc::RpcApi; +use bitcoin::hashes::Hash; use bitcoin::Amount; use lightning_invoice::{Bolt11InvoiceDescription, Description}; @@ -281,7 +282,7 @@ fn start_stop_reinit() { } #[test] -fn onchain_spend_receive() { +fn onchain_send_receive() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); let chain_source = TestChainSource::Esplora(&electrsd); let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false); @@ -352,12 +353,37 @@ fn onchain_spend_receive() { node_a.onchain_payment().send_to_address(&addr_b, expected_node_a_balance + 1, None) ); - let amount_to_send_sats = 1000; + let amount_to_send_sats = 54321; let txid = node_b.onchain_payment().send_to_address(&addr_a, amount_to_send_sats, None).unwrap(); - generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); wait_for_tx(&electrsd.client, txid); + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + let payment_id = PaymentId(txid.to_byte_array()); + let payment_a = node_a.payment(&payment_id).unwrap(); + assert_eq!(payment_a.status, PaymentStatus::Pending); + match payment_a.kind { + PaymentKind::Onchain { status, .. } => { + assert!(matches!(status, ConfirmationStatus::Unconfirmed)); + }, + _ => panic!("Unexpected payment kind"), + } + assert!(payment_a.fee_paid_msat > Some(0)); + let payment_b = node_b.payment(&payment_id).unwrap(); + assert_eq!(payment_b.status, PaymentStatus::Pending); + match payment_a.kind { + PaymentKind::Onchain { status, .. } => { + assert!(matches!(status, ConfirmationStatus::Unconfirmed)); + }, + _ => panic!("Unexpected payment kind"), + } + assert!(payment_b.fee_paid_msat > Some(0)); + assert_eq!(payment_a.amount_msat, Some(amount_to_send_sats * 1000)); + assert_eq!(payment_a.amount_msat, payment_b.amount_msat); + assert_eq!(payment_a.fee_paid_msat, payment_b.fee_paid_msat); + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); @@ -375,6 +401,24 @@ fn onchain_spend_receive() { node_b.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Onchain { .. })); assert_eq!(node_b_payments.len(), 3); + let payment_a = node_a.payment(&payment_id).unwrap(); + match payment_a.kind { + PaymentKind::Onchain { txid: _txid, status } => { + assert_eq!(_txid, txid); + assert!(matches!(status, ConfirmationStatus::Confirmed { .. })); + }, + _ => panic!("Unexpected payment kind"), + } + + let payment_b = node_a.payment(&payment_id).unwrap(); + match payment_b.kind { + PaymentKind::Onchain { txid: _txid, status } => { + assert_eq!(_txid, txid); + assert!(matches!(status, ConfirmationStatus::Confirmed { .. })); + }, + _ => panic!("Unexpected payment kind"), + } + 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);