temp please review

lightning/src/chain/channelmonitor.rs

@@ -3432,11 +3432,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			&self.holder_revocation_basepoint, &their_per_commitment_point);
 		let delayed_key = DelayedPaymentKey::from_basepoint(&self.onchain_tx_handler.secp_ctx,
 			&self.counterparty_commitment_params.counterparty_delayed_payment_base_key, &their_per_commitment_point);
-		let witness = chan_utils::get_justice_witness(
+		let witness = self.onchain_tx_handler.signer.get_justice_witness(
 			&revocation_pubkey,
 			self.counterparty_commitment_params.on_counterparty_tx_csv,
 			&delayed_key,
-			&self.onchain_tx_handler.signer,
 			&justice_tx,
 			input_idx,
 			value,
@@ -3498,7 +3497,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			let per_commitment_point = PublicKey::from_secret_key(&self.onchain_tx_handler.secp_ctx, &per_commitment_key);
 			let revocation_pubkey = RevocationKey::from_basepoint(&self.onchain_tx_handler.secp_ctx,  &self.holder_revocation_basepoint, &per_commitment_point,);
 			let delayed_key = DelayedPaymentKey::from_basepoint(&self.onchain_tx_handler.secp_ctx, &self.counterparty_commitment_params.counterparty_delayed_payment_base_key, &PublicKey::from_secret_key(&self.onchain_tx_handler.secp_ctx, &per_commitment_key));
-			let revokeable_spk = chan_utils::get_revokeable_spk(&revocation_pubkey, self.counterparty_commitment_params.on_counterparty_tx_csv, &delayed_key);
+			let revokeable_spk = self.onchain_tx_handler.signer.get_revokeable_spk(&revocation_pubkey, self.counterparty_commitment_params.on_counterparty_tx_csv, &delayed_key);
 
 			// First, process non-htlc outputs (to_holder & to_counterparty)
 			for (idx, outp) in tx.output.iter().enumerate() {
@@ -3617,7 +3616,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 			let delayed_key = DelayedPaymentKey::from_basepoint(&self.onchain_tx_handler.secp_ctx, &self.counterparty_commitment_params.counterparty_delayed_payment_base_key, &per_commitment_point);
 
-			let revokeable_spk = chan_utils::get_revokeable_spk(&revocation_pubkey,
+			let revokeable_spk = self.onchain_tx_handler.signer.get_revokeable_spk(&revocation_pubkey,
 				self.counterparty_commitment_params.on_counterparty_tx_csv,
 				&delayed_key);
 			for (idx, outp) in transaction.output.iter().enumerate() {
@@ -3714,7 +3713,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	fn get_broadcasted_holder_claims(&self, holder_tx: &HolderSignedTx, conf_height: u32) -> (Vec<PackageTemplate>, Option<(ScriptBuf, PublicKey, RevocationKey)>) {
 		let mut claim_requests = Vec::with_capacity(holder_tx.htlc_outputs.len());
 
-		let revokeable_spk = chan_utils::get_revokeable_spk(&holder_tx.revocation_key, self.on_holder_tx_csv, &holder_tx.delayed_payment_key);
+		let revokeable_spk = self.onchain_tx_handler.signer.get_revokeable_spk(&holder_tx.revocation_key, self.on_holder_tx_csv, &holder_tx.delayed_payment_key);
 		let broadcasted_holder_revokable_script = Some((revokeable_spk, holder_tx.per_commitment_point.clone(), holder_tx.revocation_key.clone()));
 
 		for &(ref htlc, _, _) in holder_tx.htlc_outputs.iter() {

lightning/src/chain/package.rs

@@ -592,7 +592,8 @@ impl PackageSolvingData {
 		match self {
 			PackageSolvingData::RevokedOutput(ref outp) => {
 				let chan_keys = TxCreationKeys::derive_new(&onchain_handler.secp_ctx, &outp.per_commitment_point, &outp.counterparty_delayed_payment_base_key, &outp.counterparty_htlc_base_key, &onchain_handler.signer.pubkeys().revocation_basepoint, &onchain_handler.signer.pubkeys().htlc_basepoint);
-				let witness = chan_utils::get_justice_witness(&chan_keys.revocation_key, outp.on_counterparty_tx_csv, &chan_keys.broadcaster_delayed_payment_key, &onchain_handler.signer, &bumped_tx, i, outp.amount.to_sat(), &outp.per_commitment_key, &onchain_handler.secp_ctx);
+				//TODO: What if the signer fails here? Previously, we just returned false.
+				let witness = onchain_handler.signer.get_justice_witness(&chan_keys.revocation_key, outp.on_counterparty_tx_csv, &chan_keys.broadcaster_delayed_payment_key, &bumped_tx, i, outp.amount.to_sat(), &outp.per_commitment_key, &onchain_handler.secp_ctx);
 				bumped_tx.input[i].witness = witness;
 			},
 			PackageSolvingData::RevokedHTLCOutput(ref outp) => {

lightning/src/ln/chan_utils.rs

@@ -24,10 +24,11 @@ use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::ripemd160::Hash as Ripemd160;
 use bitcoin::hash_types::Txid;
 
+#[cfg(test)]
 use crate::chain::chaininterface::fee_for_weight;
+#[cfg(test)]
 use crate::chain::package::WEIGHT_REVOKED_OUTPUT;
 use crate::sign::{ChannelSigner, EntropySource};
-use crate::sign::ecdsa::EcdsaChannelSigner;
 use crate::types::payment::{PaymentHash, PaymentPreimage};
 use crate::ln::msgs::DecodeError;
 use crate::util::ser::{Readable, RequiredWrapper, Writeable, Writer};
@@ -36,7 +37,7 @@ use crate::util::transaction_utils;
 use bitcoin::locktime::absolute::LockTime;
 use bitcoin::ecdsa::Signature as BitcoinSignature;
 use bitcoin::secp256k1::{SecretKey, PublicKey, Scalar};
-use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature, Message, All};
+use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature, Message};
 use bitcoin::{secp256k1, Sequence, Witness};
 
 use crate::io;
@@ -554,17 +555,6 @@ pub fn get_revokeable_spk(revocation_key: &RevocationKey, contest_delay: u16, br
 	revokeable_redeemscript.to_p2wsh()
 }
 
-/// Document this please
-pub fn get_justice_witness<S: EcdsaChannelSigner>(revocation_key: &RevocationKey, contest_delay: u16, broadcaster_delayed_payment_key: &DelayedPaymentKey, signer: &S, justice_tx: &Transaction, input_idx: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<All>) -> Witness {
-	let sig = signer.sign_justice_revoked_output(justice_tx, input_idx, amount, per_commitment_key, secp_ctx).unwrap();
-	let revokeable_redeemscript = get_revokeable_redeemscript(revocation_key, contest_delay, broadcaster_delayed_payment_key);
-	let mut witness = Witness::new();
-	witness.push_ecdsa_signature(&BitcoinSignature::sighash_all(sig));
-	witness.push(&[1u8]);
-	witness.push(revokeable_redeemscript.as_bytes());
-	witness
-}
-
 /// Document this please
 pub fn get_to_local_witness<C: secp256k1::Signing, ES: Deref>(revocation_key: &RevocationKey, contest_delay: u16, delayed_payment_key: &SecretKey, spend_tx: &Transaction, input_idx: usize, amount: Amount, secp_ctx: &Secp256k1<C>, entropy_source: &ES) -> Witness
 	where ES::Target: crate::sign::EntropySource
@@ -1573,7 +1563,7 @@ impl CommitmentTransaction {
 		}
 
 		if to_broadcaster_value_sat > Amount::ZERO {
-			let script_pubkey = get_revokeable_spk(
+			let script_pubkey = signer.get_revokeable_spk(
 				&keys.revocation_key,
 				contest_delay,
 				&keys.broadcaster_delayed_payment_key,
@@ -1849,6 +1839,7 @@ impl<'a> TrustedCommitmentTransaction<'a> {
 	/// - This commitment was created before LDK 0.0.117. In this case, the
 	/// commitment transaction previously didn't contain enough information to locate the
 	/// revokeable output.
+	#[cfg(test)]
 	pub fn revokeable_output_index(&self) -> Option<usize> {
 		let revokeable_scriptpubkey = get_revokeable_spk(
 			&self.keys.revocation_key,
@@ -1872,6 +1863,7 @@ impl<'a> TrustedCommitmentTransaction<'a> {
 	/// The built transaction will allow fee bumping with RBF, and this method takes
 	/// `feerate_per_kw` as an input such that multiple copies of a justice transaction at different
 	/// fee rates may be built.
+	#[cfg(test)]
 	pub fn build_to_local_justice_tx(&self, feerate_per_kw: u64, destination_script: ScriptBuf)
 	-> Result<Transaction, ()> {
 		let output_idx = self.revokeable_output_index().ok_or(())?;

lightning/src/sign/ecdsa.rs

@@ -4,18 +4,20 @@ use bitcoin::transaction::Transaction;
 
 use bitcoin::secp256k1;
 use bitcoin::secp256k1::ecdsa::Signature;
-use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey, All};
+use bitcoin::Witness;
+use bitcoin::ecdsa::Signature as BitcoinSignature;
 
 use crate::ln::chan_utils::{
-	ClosingTransaction, CommitmentTransaction, HTLCOutputInCommitment, HolderCommitmentTransaction,
+	self, ClosingTransaction, CommitmentTransaction, HTLCOutputInCommitment, HolderCommitmentTransaction,
 };
 use crate::ln::msgs::UnsignedChannelAnnouncement;
 use crate::types::payment::PaymentPreimage;
 
 #[allow(unused_imports)]
 use crate::prelude::*;
 
-use crate::sign::{ChannelSigner, HTLCDescriptor};
+use crate::sign::{ChannelSigner, HTLCDescriptor, RevocationKey, DelayedPaymentKey};
 
 /// A trait to sign Lightning channel transactions as described in
 /// [BOLT 3](https://github.com/lightning/bolts/blob/master/03-transactions.md).
@@ -25,6 +27,16 @@ use crate::sign::{ChannelSigner, HTLCDescriptor};
 /// Controls](https://gitlab.com/lightning-signer/validating-lightning-signer/-/blob/main/docs/policy-controls.md)
 /// for an example of such policies.
 pub trait EcdsaChannelSigner: ChannelSigner {
+	/// Document this next please
+	fn get_justice_witness(&self, revocation_key: &RevocationKey, contest_delay: u16, broadcaster_delayed_payment_key: &DelayedPaymentKey, justice_tx: &Transaction, input_idx: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<All>) -> Witness {
+		let sig = self.sign_justice_revoked_output(justice_tx, input_idx, amount, per_commitment_key, secp_ctx).unwrap();
+		let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(revocation_key, contest_delay, broadcaster_delayed_payment_key);
+		let mut witness = Witness::new();
+		witness.push_ecdsa_signature(&BitcoinSignature::sighash_all(sig));
+		witness.push(&[1u8]);
+		witness.push(revokeable_redeemscript.as_bytes());
+		witness
+	}
 	/// Create a signature for a counterparty's commitment transaction and associated HTLC transactions.
 	///
 	/// Note that if signing fails or is rejected, the channel will be force-closed.

lightning/src/sign/mod.rs

@@ -30,8 +30,6 @@ use bitcoin::hashes::{Hash, HashEngine};
 use bitcoin::secp256k1::ecdh::SharedSecret;
 use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature};
 use bitcoin::secp256k1::schnorr;
-#[cfg(taproot)]
-use bitcoin::secp256k1::All;
 use bitcoin::secp256k1::{Keypair, PublicKey, Scalar, Secp256k1, SecretKey, Signing};
 use bitcoin::{secp256k1, Psbt, Sequence, Txid, WPubkeyHash, Witness};
 
@@ -728,6 +726,11 @@ impl HTLCDescriptor {
 /// is not yet complete, and panics may occur in certain situations when returning errors
 /// for these methods.
 pub trait ChannelSigner {
+	/// Should this go on a channel signer? We'll see later. See above eas well.
+	fn get_revokeable_spk(&self, revocation_key: &RevocationKey, contest_delay: u16, broadcaster_delayed_payment_key: &DelayedPaymentKey) -> ScriptBuf {
+		let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(revocation_key, contest_delay, broadcaster_delayed_payment_key);
+		revokeable_redeemscript.to_p2wsh()
+	}
 	/// Document this next
 	fn get_counterparty_payment_script(
 		&self, channel_type_features: &ChannelTypeFeatures, payment_key: &PublicKey,