Fix: Insert all LDK-registered transaction outputs into wallet

Previously, we'd selectively insert the funding outputs into the onchain
wallet to later allow calculating `fees_paid` when creating payment
store entries (only for splicing mostly). However, this didn't always work, and we might for
example end up with a missing funding output (and hence would fall back
to `fees_paid: Some(0)`) if it was a counterparty-initiated channel and
we synced via `bitcoind` RPC.

Here, we fix this by tracking all LDK-registered `txids` in
`ChainSource` and then in the `Wallet`'s `Listen` implementation insert
all outputs of all registered transactions into the `Wallet`, ensuring
we'd always have sufficient data for `calculate_fee` available.

Thereby we also fix the `onchain_send_receive` test which previously
failed when using `TestChainSource::Bitcoind`.

Signed-off-by: Elias Rohrer <dev@tnull.de>

Author: tnull

src/builder.rs

@@ -1294,6 +1294,7 @@ fn build_with_store_internal(
 		wallet_persister,
 		Arc::clone(&tx_broadcaster),
 		Arc::clone(&fee_estimator),
+		Arc::clone(&chain_source),
 		Arc::clone(&payment_store),
 		Arc::clone(&config),
 		Arc::clone(&logger),

src/chain/mod.rs

@@ -10,7 +10,7 @@ mod electrum;
 mod esplora;
 
 use std::collections::HashMap;
-use std::sync::{Arc, RwLock};
+use std::sync::{Arc, Mutex, RwLock};
 use std::time::Duration;
 
 use bitcoin::{Script, Txid};
@@ -84,6 +84,7 @@ impl WalletSyncStatus {
 
 pub(crate) struct ChainSource {
 	kind: ChainSourceKind,
+	registered_txids: Mutex<Vec<Txid>>,
 	tx_broadcaster: Arc<Broadcaster>,
 	logger: Arc<Logger>,
 }
@@ -112,7 +113,8 @@ impl ChainSource {
 			node_metrics,
 		);
 		let kind = ChainSourceKind::Esplora(esplora_chain_source);
-		(Self { kind, tx_broadcaster, logger }, None)
+		let registered_txids = Mutex::new(Vec::new());
+		(Self { kind, registered_txids, tx_broadcaster, logger }, None)
 	}
 
 	pub(crate) fn new_electrum(
@@ -131,7 +133,8 @@ impl ChainSource {
 			node_metrics,
 		);
 		let kind = ChainSourceKind::Electrum(electrum_chain_source);
-		(Self { kind, tx_broadcaster, logger }, None)
+		let registered_txids = Mutex::new(Vec::new());
+		(Self { kind, registered_txids, tx_broadcaster, logger }, None)
 	}
 
 	pub(crate) async fn new_bitcoind_rpc(
@@ -153,7 +156,8 @@ impl ChainSource {
 		);
 		let best_block = bitcoind_chain_source.poll_best_block().await.ok();
 		let kind = ChainSourceKind::Bitcoind(bitcoind_chain_source);
-		(Self { kind, tx_broadcaster, logger }, best_block)
+		let registered_txids = Mutex::new(Vec::new());
+		(Self { kind, registered_txids, tx_broadcaster, logger }, best_block)
 	}
 
 	pub(crate) async fn new_bitcoind_rest(
@@ -176,7 +180,8 @@ impl ChainSource {
 		);
 		let best_block = bitcoind_chain_source.poll_best_block().await.ok();
 		let kind = ChainSourceKind::Bitcoind(bitcoind_chain_source);
-		(Self { kind, tx_broadcaster, logger }, best_block)
+		let registered_txids = Mutex::new(Vec::new());
+		(Self { kind, registered_txids, tx_broadcaster, logger }, best_block)
 	}
 
 	pub(crate) fn start(&self, runtime: Arc<Runtime>) -> Result<(), Error> {
@@ -209,6 +214,10 @@ impl ChainSource {
 		}
 	}
 
+	pub(crate) fn registered_txids(&self) -> Vec<Txid> {
+		self.registered_txids.lock().unwrap().clone()
+	}
+
 	pub(crate) fn is_transaction_based(&self) -> bool {
 		match &self.kind {
 			ChainSourceKind::Esplora(_) => true,
@@ -463,6 +472,7 @@ impl ChainSource {
 
 impl Filter for ChainSource {
 	fn register_tx(&self, txid: &Txid, script_pubkey: &Script) {
+		self.registered_txids.lock().unwrap().push(*txid);
 		match &self.kind {
 			ChainSourceKind::Esplora(esplora_chain_source) => {
 				esplora_chain_source.register_tx(txid, script_pubkey)

src/lib.rs

@@ -1329,10 +1329,6 @@ impl Node {
 					Error::ChannelSplicingFailed
 				})?;
 
-			// insert channel's funding utxo into the wallet so we can later calculate fees
-			// correctly when viewing this splice-in.
-			self.wallet.insert_txo(funding_txo.into_bitcoin_outpoint(), funding_output)?;
-
 			let change_address = self.wallet.get_new_internal_address()?;
 
 			let contribution = SpliceContribution::splice_in(
@@ -1426,18 +1422,6 @@ impl Node {
 				},
 			};
 
-			let funding_txo = channel_details.funding_txo.ok_or_else(|| {
-				log_error!(self.logger, "Failed to splice channel: channel not yet ready",);
-				Error::ChannelSplicingFailed
-			})?;
-
-			let funding_output = channel_details.get_funding_output().ok_or_else(|| {
-				log_error!(self.logger, "Failed to splice channel: channel not yet ready");
-				Error::ChannelSplicingFailed
-			})?;
-
-			self.wallet.insert_txo(funding_txo.into_bitcoin_outpoint(), funding_output)?;
-
 			self.channel_manager
 				.splice_channel(
 					&channel_details.channel_id,

src/wallet/mod.rs

@@ -54,7 +54,7 @@ use crate::payment::{
 	PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus, PendingPaymentDetails,
 };
 use crate::types::{Broadcaster, PaymentStore, PendingPaymentStore};
-use crate::Error;
+use crate::{ChainSource, Error};
 
 pub(crate) enum OnchainSendAmount {
 	ExactRetainingReserve { amount_sats: u64, cur_anchor_reserve_sats: u64 },
@@ -71,6 +71,7 @@ pub(crate) struct Wallet {
 	persister: Mutex<KVStoreWalletPersister>,
 	broadcaster: Arc<Broadcaster>,
 	fee_estimator: Arc<OnchainFeeEstimator>,
+	chain_source: Arc<ChainSource>,
 	payment_store: Arc<PaymentStore>,
 	config: Arc<Config>,
 	logger: Arc<Logger>,
@@ -81,8 +82,9 @@ impl Wallet {
 	pub(crate) fn new(
 		wallet: bdk_wallet::PersistedWallet<KVStoreWalletPersister>,
 		wallet_persister: KVStoreWalletPersister, broadcaster: Arc<Broadcaster>,
-		fee_estimator: Arc<OnchainFeeEstimator>, payment_store: Arc<PaymentStore>,
-		config: Arc<Config>, logger: Arc<Logger>, pending_payment_store: Arc<PendingPaymentStore>,
+		fee_estimator: Arc<OnchainFeeEstimator>, chain_source: Arc<ChainSource>,
+		payment_store: Arc<PaymentStore>, config: Arc<Config>, logger: Arc<Logger>,
+		pending_payment_store: Arc<PendingPaymentStore>,
 	) -> Self {
 		let inner = Mutex::new(wallet);
 		let persister = Mutex::new(wallet_persister);
@@ -91,6 +93,7 @@ impl Wallet {
 			persister,
 			broadcaster,
 			fee_estimator,
+			chain_source,
 			payment_store,
 			config,
 			logger,
@@ -196,19 +199,6 @@ impl Wallet {
 		Ok(())
 	}
 
-	pub(crate) fn insert_txo(&self, outpoint: OutPoint, txout: TxOut) -> Result<(), Error> {
-		let mut locked_wallet = self.inner.lock().unwrap();
-		locked_wallet.insert_txout(outpoint, txout);
-
-		let mut locked_persister = self.persister.lock().unwrap();
-		locked_wallet.persist(&mut locked_persister).map_err(|e| {
-			log_error!(self.logger, "Failed to persist wallet: {}", e);
-			Error::PersistenceFailed
-		})?;
-
-		Ok(())
-	}
-
 	fn update_payment_store<'a>(
 		&self, locked_wallet: &'a mut PersistedWallet<KVStoreWalletPersister>,
 		mut events: Vec<WalletEvent>,
@@ -1040,6 +1030,25 @@ impl Listen for Wallet {
 			);
 		}
 
+		// In order to be able to reliably calculate fees the `Wallet` needs access to the previous
+		// ouput data. To this end, we here insert any ouputs of transactions that LDK is intersted
+		// in (e.g., funding transaction ouputs) into the wallet's transaction graph when we see
+		// them, so it is reliably able to calculate fees for subsequent spends.
+		//
+		// FIXME: technically, we should also do this for mempool transactions. However, at the
+		// current time fixing the edge case doesn't seem worth the additional conplexity /
+		// additional overhead..
+		let registered_txids = self.chain_source.registered_txids();
+		for tx in &block.txdata {
+			let txid = tx.compute_txid();
+			if registered_txids.contains(&txid) {
+				for (vout, txout) in tx.output.iter().enumerate() {
+					let outpoint = OutPoint { txid, vout: vout as u32 };
+					locked_wallet.insert_txout(outpoint, txout.clone());
+				}
+			}
+		}
+
 		match locked_wallet.apply_block_events(block, height) {
 			Ok(events) => {
 				if let Err(e) = self.update_payment_store(&mut *locked_wallet, events) {