Fix wallet UTXO reuse for funding transactions and onchain spends

src/event.rs

@@ -1054,6 +1054,23 @@ where
 					.await;
 				match funding_transaction {
 					Ok(final_tx) => {
+						// LDK reports a transaction it took and then lets go, but not one it
+						// never took. For the latter the wallet state held for the transaction
+						// is returned here.
+						let release_unused_funding = |tx: bitcoin::Transaction| async move {
+							let inputs =
+								tx.input.into_iter().map(|txin| txin.previous_output).collect();
+							if let Err(e) =
+								self.wallet.release_inputs_and_outputs(inputs, tx.output).await
+							{
+								log_error!(
+									self.logger,
+									"Failed to release the inputs of an unused funding transaction: {}",
+									e
+								);
+							}
+						};
+
 						// Record what the transaction is before handing it to LDK, which is what
 						// authorizes either party to broadcast it. LDK identifies the funding
 						// output by the same script and value, and derives the channel's id from
@@ -1074,17 +1091,22 @@ where
 								counterparty_node_id,
 								channel_id: ChannelId::v1_from_funding_outpoint(funding_txo),
 							};
-							let facts = ChannelTxFacts::new(txid).with_outputs(
-								&channel,
-								Some(UserChannelId(user_channel_id)),
-								ChannelOutputRole::Funding,
-								[vout as u32],
-							);
+							let facts = ChannelTxFacts::new(txid)
+								.with_outputs(
+									&channel,
+									Some(UserChannelId(user_channel_id)),
+									ChannelOutputRole::Funding,
+									[vout as u32],
+								)
+								.with_locked_inputs(
+									final_tx.input.iter().map(|txin| txin.previous_output),
+								);
 							match self.wallet.record_own_funding_tx_facts(facts).await {
 								Ok(FactsRecordOutcome::Recorded) => {},
 								// Admitted however full the store is, the report can only be
-								// refused as too large a record, which one funding output does
-								// not make. Replaying would meet the same refusal, so the
+								// refused as too large a record, which one funding output and
+								// the inputs of a transaction do not make. Replaying would meet
+								// the same refusal, so the
 								// channel is funded with a transaction this node will report
 								// without a classification.
 								Ok(FactsRecordOutcome::Incomplete) => {
@@ -1101,6 +1123,8 @@ where
 										temporary_channel_id,
 										e,
 									);
+									// The replayed event builds another transaction.
+									release_unused_funding(final_tx).await;
 									return Err(ReplayEvent());
 								},
 							}
@@ -1112,6 +1136,7 @@ where
 							);
 						}
 
+						let unused_tx = final_tx.clone();
 						let needs_manual_broadcast = self
 							.liquidity_source
 							.lsps2_service()
@@ -1140,6 +1165,9 @@ where
 							)
 						};
 
+						if result.is_err() {
+							release_unused_funding(unused_tx).await;
+						}
 						match result {
 							Ok(()) => {},
 							Err(APIError::APIMisuseError { err }) => {
@@ -2532,24 +2560,36 @@ where
 					return Err(ReplayEvent());
 				}
 
-				// TODO(#1037): once inputs are locked at coin selection, `inputs` are locks this
-				// event returns: unlock them here.
-				if let FundingInfo::Contribution { inputs: _, outputs } = funding_info {
+				let (inputs, outputs) = match funding_info {
+					FundingInfo::Tx { transaction } => (
+						transaction.input.into_iter().map(|txin| txin.previous_output).collect(),
+						transaction.output,
+					),
+					FundingInfo::Contribution { inputs, outputs } => (
+						inputs,
+						outputs
+							.into_iter()
+							.map(|script_pubkey| bitcoin::TxOut {
+								value: bitcoin::Amount::ZERO,
+								script_pubkey,
+							})
+							.collect(),
+					),
+					// The funding transaction of a channel LDK left to this node to broadcast, or
+					// a splice round this node contributed nothing to. The event does not say what
+					// the transaction spent, but the wallet recorded that for one it built.
+					FundingInfo::OutPoint { outpoint } => {
+						(self.wallet.locked_funding_inputs(&outpoint.txid).await, Vec::new())
+					},
+				};
+				if !inputs.is_empty() || !outputs.is_empty() {
 					log_info!(
 						self.logger,
-						"Reclaiming unused addresses from channel {} funding",
+						"Reclaiming unused wallet state from channel {} funding",
 						channel_id,
 					);
-
-					let outputs = outputs
-						.into_iter()
-						.map(|script_pubkey| bitcoin::TxOut {
-							value: bitcoin::Amount::ZERO,
-							script_pubkey,
-						})
-						.collect();
-					if let Err(e) = self.wallet.release_outputs(outputs).await {
-						log_error!(self.logger, "Failed reclaiming unused addresses: {}", e);
+					if let Err(e) = self.wallet.release_inputs_and_outputs(inputs, outputs).await {
+						log_error!(self.logger, "Failed reclaiming unused wallet state: {}", e);
 						return Err(ReplayEvent());
 					}
 				}

src/io/mod.rs

@@ -90,6 +90,13 @@ pub(crate) const BDK_WALLET_TX_GRAPH_PRIMARY_NAMESPACE: &str = "bdk_wallet";
 pub(crate) const BDK_WALLET_TX_GRAPH_SECONDARY_NAMESPACE: &str = "";
 pub(crate) const BDK_WALLET_TX_GRAPH_KEY: &str = "tx_graph";
 
+/// The BDK wallet's [`ChangeSet::locked_outpoints`] will be persisted under this key.
+///
+/// [`ChangeSet::locked_outpoints`]: bdk_wallet::ChangeSet::locked_outpoints
+pub(crate) const BDK_WALLET_LOCKED_OUTPOINTS_PRIMARY_NAMESPACE: &str = "bdk_wallet";
+pub(crate) const BDK_WALLET_LOCKED_OUTPOINTS_SECONDARY_NAMESPACE: &str = "";
+pub(crate) const BDK_WALLET_LOCKED_OUTPOINTS_KEY: &str = "locked_outpoints";
+
 /// The BDK wallet's [`ChangeSet::indexer`] will be persisted under this key.
 ///
 /// [`ChangeSet::indexer`]: bdk_wallet::ChangeSet::indexer

src/io/utils.rs

@@ -19,6 +19,7 @@ use bdk_chain::local_chain::ChangeSet as BdkLocalChainChangeSet;
 use bdk_chain::miniscript::{Descriptor, DescriptorPublicKey};
 use bdk_chain::tx_graph::ChangeSet as BdkTxGraphChangeSet;
 use bdk_chain::ConfirmationBlockTime;
+use bdk_wallet::locked_outpoints::ChangeSet as BdkLockedOutpointsChangeSet;
 use bdk_wallet::ChangeSet as BdkWalletChangeSet;
 use bitcoin::Network;
 use lightning::ln::msgs::DecodeError;
@@ -721,6 +722,15 @@ impl_read_write_change_set_type!(
 	BDK_WALLET_TX_GRAPH_KEY
 );
 
+impl_read_write_change_set_type!(
+	read_bdk_wallet_locked_outpoints,
+	write_bdk_wallet_locked_outpoints,
+	BdkLockedOutpointsChangeSet,
+	BDK_WALLET_LOCKED_OUTPOINTS_PRIMARY_NAMESPACE,
+	BDK_WALLET_LOCKED_OUTPOINTS_SECONDARY_NAMESPACE,
+	BDK_WALLET_LOCKED_OUTPOINTS_KEY
+);
+
 impl_read_write_change_set_type!(
 	read_bdk_wallet_indexer,
 	write_bdk_wallet_indexer,
@@ -763,6 +773,9 @@ pub(crate) async fn read_bdk_wallet_change_set(
 	read_bdk_wallet_tx_graph(&*kv_store, logger)
 		.await?
 		.map(|tx_graph| change_set.tx_graph = tx_graph);
+	read_bdk_wallet_locked_outpoints(&*kv_store, logger)
+		.await?
+		.map(|locked_outpoints| change_set.locked_outpoints = locked_outpoints);
 	read_bdk_wallet_indexer(&*kv_store, logger).await?.map(|indexer| change_set.indexer = indexer);
 	Ok(Some(change_set))
 }

src/lib.rs

@@ -387,6 +387,34 @@ impl Node {
 			},
 		))?;
 
+		// See `Wallet::unlock_unreferenced_outpoints`. The locks still needed are those of a splice
+		// round LDK still tracks, which lists the inputs this node contributed to it, and those
+		// the wallet recorded for the funding transaction of a channel that is still around.
+		// Without that record for a channel awaiting its funding's confirmation, its locks cannot
+		// be told apart from leftovers, and every lock is kept. An output this releases although a
+		// broadcast transaction spends it is spent again in the wallet's view by the next sync.
+		let mut referenced: std::collections::HashSet<bitcoin::OutPoint> = channels
+			.iter()
+			.filter_map(|channel| channel.splice_details.as_ref())
+			.flat_map(|splice| splice.candidates.iter())
+			.filter_map(|candidate| candidate.contribution.as_ref())
+			.flat_map(|contribution| contribution.inputs().iter().map(|input| input.outpoint()))
+			.collect();
+		let mut has_untracked_funding = false;
+		for channel in channels.iter().filter(|channel| channel.is_outbound) {
+			let Some(funding_txo) = channel.funding_txo else {
+				continue;
+			};
+			let inputs =
+				self.runtime.block_on(self.wallet.locked_funding_inputs(&funding_txo.txid));
+			has_untracked_funding |= inputs.is_empty()
+				&& channel.confirmations.map_or(true, |confirmations| confirmations == 0);
+			referenced.extend(inputs);
+		}
+		self.runtime.block_on(self.wallet.unlock_unreferenced_outpoints(|outpoint| {
+			has_untracked_funding || referenced.contains(outpoint)
+		}))?;
+
 		// Synchronize Lightning chain state before peers or event processing can act on stale
 		// confirmations. Each restored ChannelMonitor may have been persisted at a different tip.
 		self.runtime.block_on(self.chain_source.initialize_wallets(
@@ -1957,6 +1985,13 @@ impl Node {
 			reserve_output.clone(),
 			feerate,
 		);
+		let built = built.and_then(|contribution| {
+			// These inputs were not chosen by the wallet's coin selection, which would have locked
+			// them. LDK reports them once it lets the contribution go.
+			let outpoints = contribution.inputs().iter().map(|input| input.outpoint()).collect();
+			self.runtime.block_on(self.wallet.lock_outpoints(outpoints))?;
+			Ok(contribution)
+		});
 		match built {
 			Ok(contribution) => Ok(contribution),
 			Err(e) => {