f Generally track fee_paid_msat as part of PaymentDetails

tnull · tnull · commit 68bf90c3a2bb · 2025-02-13T11:04:15.000+01:00
diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl
@@ -371,7 +371,7 @@ interface ClosureReason {
 
 [Enum]
 interface PaymentKind {
-	Onchain(Txid txid, ConfirmationStatus status, u64 fee_msat);
+	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);
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -42,6 +42,13 @@ pub struct PaymentDetails {
 	pub kind: PaymentKind,
 	/// The amount transferred.
 	pub amount_msat: Option<u64>,
+	/// 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<u64>,
 	/// 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<u64>, direction: PaymentDirection,
-		status: PaymentStatus,
+		id: PaymentId, kind: PaymentKind, amount_msat: Option<u64>, fee_paid_msat: Option<u64>,
+		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 {
@@ -192,6 +199,7 @@ impl Writeable for PaymentDetails {
 			(4, None::<Option<PaymentSecret>>, 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 +221,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 +262,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,
+		})
 	}
 }
 
@@ -302,8 +319,6 @@ pub enum PaymentKind {
 		txid: Txid,
 		/// The confirmation status of this payment.
 		status: ConfirmationStatus,
-		/// The on-chain fees that were paid for this payment.
-		fee_msat: u64,
 	},
 	/// A [BOLT 11] payment.
 	///
@@ -396,7 +411,6 @@ impl_writeable_tlv_based_enum!(PaymentKind,
 	(0, Onchain) => {
 		(0, txid, required),
 		(2, status, required),
-		(4, fee_msat, required),
 	},
 	(2, Bolt11) => {
 		(0, hash, required),
@@ -482,6 +496,7 @@ pub(crate) struct PaymentDetailsUpdate {
 	pub preimage: Option<Option<PaymentPreimage>>,
 	pub secret: Option<Option<PaymentSecret>>,
 	pub amount_msat: Option<Option<u64>>,
+	pub fee_paid_msat: Option<Option<u64>>,
 	pub direction: Option<PaymentDirection>,
 	pub status: Option<PaymentStatus>,
 	pub confirmation_status: Option<ConfirmationStatus>,
@@ -495,6 +510,7 @@ impl PaymentDetailsUpdate {
 			preimage: None,
 			secret: None,
 			amount_msat: None,
+			fee_paid_msat: None,
 			direction: None,
 			status: None,
 			confirmation_status: None,
@@ -524,6 +540,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,
@@ -711,8 +728,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
@@ -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<KVStoreWalletPersister>,
 	) -> 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;
@@ -187,7 +181,6 @@ 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 fee = locked_wallet.calculate_fee(&wtx.tx_node.tx).unwrap_or(Amount::ZERO);
-			let fee_msat = fee.to_sat() * 1000;
 			let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);
 			let (direction, amount_msat) = if sent > received {
 				let direction = PaymentDirection::Outbound;
@@ -204,20 +197,18 @@ where
 				);
 				(direction, amount_msat)
 			};
-			let kind = crate::payment::PaymentKind::Onchain {
-				txid,
-				status: confirmation_status,
-				fee_msat,
-			};
+			let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+
+			let fee_paid_msat = Some(fee.to_sat() * 1000);
 
-			let payment = PaymentDetails {
+			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
@@ -148,6 +148,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
@@ -376,22 +376,22 @@ fn onchain_send_receive() {
 	let payment_id = PaymentId(txid.to_byte_array());
 	let payment_a = node_a.payment(&payment_id).unwrap();
 	assert_eq!(payment_a.amount_msat, Some(amount_to_send_sats * 1000));
+	assert!(payment_a.fee_paid_msat > Some(0));
 	match payment_a.kind {
-		PaymentKind::Onchain { txid: _txid, status, fee_msat } => {
+		PaymentKind::Onchain { txid: _txid, status } => {
 			assert_eq!(_txid, txid);
 			assert!(matches!(status, ConfirmationStatus::Confirmed { .. }));
-			assert!(fee_msat != 0);
 		},
 		_ => panic!("Unexpected payment kind"),
 	}
 
 	let payment_b = node_a.payment(&payment_id).unwrap();
 	assert_eq!(payment_b.amount_msat, Some(amount_to_send_sats * 1000));
+	assert!(payment_b.fee_paid_msat > Some(0));
 	match payment_b.kind {
-		PaymentKind::Onchain { txid: _txid, status, fee_msat } => {
+		PaymentKind::Onchain { txid: _txid, status } => {
 			assert_eq!(_txid, txid);
 			assert!(matches!(status, ConfirmationStatus::Confirmed { .. }));
-			assert!(fee_msat != 0);
 		},
 		_ => panic!("Unexpected payment kind"),
 	}

Original file line number	Diff line number	Diff line change
`@@ -148,6 +148,8 @@ macro_rules! expect_payment_successful_event {`
`148`	`148`	`if let Some(fee_msat) = $fee_paid_msat {`
`149`	`149`	`assert_eq!(fee_paid_msat, fee_msat);`
`150`	`150`	`}`
	`151`	`+ let payment = $node.payment(&$payment_id.unwrap()).unwrap();`
	`152`	`+ assert_eq!(payment.fee_paid_msat, fee_paid_msat);`
`151`	`153`	`assert_eq!(payment_id, $payment_id);`
`152`	`154`	`$node.event_handled();`
`153`	`155`	`},`