Introduce OutputSpender trait and implement for (Phantom)KeysManager

.. we move `spend_spendable_outputs` to a new trait which we can easily
reuse in `OutputSweeper` later.

Author: tnull

lightning/src/ln/functional_tests.rs

@@ -17,7 +17,7 @@ use crate::chain::chaininterface::LowerBoundedFeeEstimator;
 use crate::chain::channelmonitor;
 use crate::chain::channelmonitor::{CLOSED_CHANNEL_UPDATE_ID, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
 use crate::chain::transaction::OutPoint;
-use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, SignerProvider};
+use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
 use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason};
 use crate::ln::{ChannelId, PaymentPreimage, PaymentSecret, PaymentHash};
 use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, ChannelPhase};
@@ -9951,9 +9951,9 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
 	let dust_outbound_htlc_on_holder_tx: u64 = max_dust_htlc_exposure_msat / dust_outbound_htlc_on_holder_tx_msat;
 
 	// Substract 3 sats for multiplier and 2 sats for fixed limit to make sure we are 50% below the dust limit.
-	// This is to make sure we fully use the dust limit. If we don't, we could end up with `dust_ibd_htlc_on_holder_tx` being 1 
+	// This is to make sure we fully use the dust limit. If we don't, we could end up with `dust_ibd_htlc_on_holder_tx` being 1
 	// while `max_dust_htlc_exposure_msat` is not equal to `dust_outbound_htlc_on_holder_tx_msat`.
-	let dust_inbound_htlc_on_holder_tx_msat: u64 = (dust_buffer_feerate * htlc_success_tx_weight(&channel_type_features) / 1000 + open_channel.common_fields.dust_limit_satoshis - if multiplier_dust_limit { 3 } else { 2 }) * 1000; 
+	let dust_inbound_htlc_on_holder_tx_msat: u64 = (dust_buffer_feerate * htlc_success_tx_weight(&channel_type_features) / 1000 + open_channel.common_fields.dust_limit_satoshis - if multiplier_dust_limit { 3 } else { 2 }) * 1000;
 	let dust_inbound_htlc_on_holder_tx: u64 = max_dust_htlc_exposure_msat / dust_inbound_htlc_on_holder_tx_msat;
 
 	let dust_htlc_on_counterparty_tx: u64 = 4;

lightning/src/ln/monitor_tests.rs

@@ -9,7 +9,7 @@
 
 //! Further functional tests which test blockchain reorganizations.
 
-use crate::sign::{ecdsa::EcdsaChannelSigner, SpendableOutputDescriptor};
+use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
 use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS, Balance};
 use crate::chain::transaction::OutPoint;
 use crate::chain::chaininterface::{LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};

lightning/src/ln/reorg_tests.rs

@@ -15,6 +15,7 @@ use crate::chain::transaction::OutPoint;
 use crate::chain::Confirm;
 use crate::events::{Event, MessageSendEventsProvider, ClosureReason, HTLCDestination, MessageSendEvent};
 use crate::ln::msgs::{ChannelMessageHandler, Init};
+use crate::sign::OutputSpender;
 use crate::util::test_utils;
 use crate::util::ser::Writeable;
 use crate::util::string::UntrustedString;

lightning/src/sign/mod.rs

@@ -805,6 +805,28 @@ pub trait NodeSigner {
 	fn sign_gossip_message(&self, msg: UnsignedGossipMessage) -> Result<Signature, ()>;
 }
 
+/// A trait that describes a wallet capable of creating a spending [`Transaction`] from a set of
+/// [`SpendableOutputDescriptor`]s.
+pub trait OutputSpender {
+	/// Creates a [`Transaction`] which spends the given descriptors to the given outputs, plus an
+	/// output to the given change destination (if sufficient change value remains). The
+	/// transaction will have a feerate, at least, of the given value.
+	///
+	/// The `locktime` argument is used to set the transaction's locktime. If `None`, the
+	/// transaction will have a locktime of 0. It it recommended to set this to the current block
+	/// height to avoid fee sniping, unless you have some specific reason to use a different
+	/// locktime.
+	///
+	/// Returns `Err(())` if the output value is greater than the input value minus required fee,
+	/// if a descriptor was duplicated, or if an output descriptor `script_pubkey`
+	/// does not match the one we can spend.
+	fn spend_spendable_outputs<C: Signing>(
+		&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>,
+		change_destination_script: ScriptBuf, feerate_sat_per_1000_weight: u32,
+		locktime: Option<LockTime>, secp_ctx: &Secp256k1<C>,
+	) -> Result<Transaction, ()>;
+}
+
 // Primarily needed in doctests because of https://github.com/rust-lang/rust/issues/67295
 /// A dynamic [`SignerProvider`] temporarily needed for doc tests.
 #[cfg(taproot)]
@@ -1991,50 +2013,6 @@ impl KeysManager {
 
 		Ok(psbt)
 	}
-
-	/// Creates a [`Transaction`] which spends the given descriptors to the given outputs, plus an
-	/// output to the given change destination (if sufficient change value remains). The
-	/// transaction will have a feerate, at least, of the given value.
-	///
-	/// The `locktime` argument is used to set the transaction's locktime. If `None`, the
-	/// transaction will have a locktime of 0. It it recommended to set this to the current block
-	/// height to avoid fee sniping, unless you have some specific reason to use a different
-	/// locktime.
-	///
-	/// Returns `Err(())` if the output value is greater than the input value minus required fee,
-	/// if a descriptor was duplicated, or if an output descriptor `script_pubkey`
-	/// does not match the one we can spend.
-	///
-	/// We do not enforce that outputs meet the dust limit or that any output scripts are standard.
-	///
-	/// May panic if the [`SpendableOutputDescriptor`]s were not generated by channels which used
-	/// this [`KeysManager`] or one of the [`InMemorySigner`] created by this [`KeysManager`].
-	pub fn spend_spendable_outputs<C: Signing>(
-		&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>,
-		change_destination_script: ScriptBuf, feerate_sat_per_1000_weight: u32,
-		locktime: Option<LockTime>, secp_ctx: &Secp256k1<C>,
-	) -> Result<Transaction, ()> {
-		let (mut psbt, expected_max_weight) =
-			SpendableOutputDescriptor::create_spendable_outputs_psbt(
-				descriptors,
-				outputs,
-				change_destination_script,
-				feerate_sat_per_1000_weight,
-				locktime,
-			)?;
-		psbt = self.sign_spendable_outputs_psbt(descriptors, psbt, secp_ctx)?;
-
-		let spend_tx = psbt.extract_tx();
-
-		debug_assert!(expected_max_weight >= spend_tx.weight().to_wu());
-		// Note that witnesses with a signature vary somewhat in size, so allow
-		// `expected_max_weight` to overshoot by up to 3 bytes per input.
-		debug_assert!(
-			expected_max_weight <= spend_tx.weight().to_wu() + descriptors.len() as u64 * 3
-		);
-
-		Ok(spend_tx)
-	}
 }
 
 impl EntropySource for KeysManager {
@@ -2106,6 +2084,44 @@ impl NodeSigner for KeysManager {
 	}
 }
 
+impl OutputSpender for KeysManager {
+	/// Creates a [`Transaction`] which spends the given descriptors to the given outputs, plus an
+	/// output to the given change destination (if sufficient change value remains).
+	///
+	/// See [`OutputSpender::spend_spendable_outputs`] documentation for more information.
+	///
+	/// We do not enforce that outputs meet the dust limit or that any output scripts are standard.
+	///
+	/// May panic if the [`SpendableOutputDescriptor`]s were not generated by channels which used
+	/// this [`KeysManager`] or one of the [`InMemorySigner`] created by this [`KeysManager`].
+	fn spend_spendable_outputs<C: Signing>(
+		&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>,
+		change_destination_script: ScriptBuf, feerate_sat_per_1000_weight: u32,
+		locktime: Option<LockTime>, secp_ctx: &Secp256k1<C>,
+	) -> Result<Transaction, ()> {
+		let (mut psbt, expected_max_weight) =
+			SpendableOutputDescriptor::create_spendable_outputs_psbt(
+				descriptors,
+				outputs,
+				change_destination_script,
+				feerate_sat_per_1000_weight,
+				locktime,
+			)?;
+		psbt = self.sign_spendable_outputs_psbt(descriptors, psbt, secp_ctx)?;
+
+		let spend_tx = psbt.extract_tx();
+
+		debug_assert!(expected_max_weight >= spend_tx.weight().to_wu());
+		// Note that witnesses with a signature vary somewhat in size, so allow
+		// `expected_max_weight` to overshoot by up to 3 bytes per input.
+		debug_assert!(
+			expected_max_weight <= spend_tx.weight().to_wu() + descriptors.len() as u64 * 3
+		);
+
+		Ok(spend_tx)
+	}
+}
+
 impl SignerProvider for KeysManager {
 	type EcdsaSigner = InMemorySigner;
 	#[cfg(taproot)]
@@ -2238,6 +2254,25 @@ impl NodeSigner for PhantomKeysManager {
 	}
 }
 
+impl OutputSpender for PhantomKeysManager {
+	/// See [`OutputSpender::spend_spendable_outputs`] and [`KeysManager::spend_spendable_outputs`]
+	/// for documentation on this method.
+	fn spend_spendable_outputs<C: Signing>(
+		&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>,
+		change_destination_script: ScriptBuf, feerate_sat_per_1000_weight: u32,
+		locktime: Option<LockTime>, secp_ctx: &Secp256k1<C>,
+	) -> Result<Transaction, ()> {
+		self.inner.spend_spendable_outputs(
+			descriptors,
+			outputs,
+			change_destination_script,
+			feerate_sat_per_1000_weight,
+			locktime,
+			secp_ctx,
+		)
+	}
+}
+
 impl SignerProvider for PhantomKeysManager {
 	type EcdsaSigner = InMemorySigner;
 	#[cfg(taproot)]
@@ -2299,22 +2334,6 @@ impl PhantomKeysManager {
 		}
 	}
 
-	/// See [`KeysManager::spend_spendable_outputs`] for documentation on this method.
-	pub fn spend_spendable_outputs<C: Signing>(
-		&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>,
-		change_destination_script: ScriptBuf, feerate_sat_per_1000_weight: u32,
-		locktime: Option<LockTime>, secp_ctx: &Secp256k1<C>,
-	) -> Result<Transaction, ()> {
-		self.inner.spend_spendable_outputs(
-			descriptors,
-			outputs,
-			change_destination_script,
-			feerate_sat_per_1000_weight,
-			locktime,
-			secp_ctx,
-		)
-	}
-
 	/// See [`KeysManager::derive_channel_keys`] for documentation on this method.
 	pub fn derive_channel_keys(
 		&self, channel_value_satoshis: u64, params: &[u8; 32],