Add ChannelSigner::punish_revokeable_output

All LN-Penalty channel signers need to be able to punish the
counterparty in case they broadcast an old state. In this commit, we
ask implementers of `ChannelSigner` to produce the full transaction with
the given input finalized to punish the corresponding previous output.
Consumers of the `ChannelSigner` trait can now be agnostic to the
specific scripts used in revokeable outputs.

We leave passing to the `ChannelSigner` all the previous `TxOut`'s
needed to produce valid schnorr signatures under BIP 341 spending rules
to a later patch.

lightning/src/chain/chainmonitor.rs

@@ -96,7 +96,7 @@ use bitcoin::secp256k1::PublicKey;
 /// provided for bulding transactions for a watchtower:
 /// [`ChannelMonitor::initial_counterparty_commitment_tx`],
 /// [`ChannelMonitor::counterparty_commitment_txs_from_update`],
-/// [`ChannelMonitor::sign_to_local_justice_tx`], [`TrustedCommitmentTransaction::revokeable_output_index`],
+/// [`ChannelMonitor::punish_revokeable_output`], [`TrustedCommitmentTransaction::revokeable_output_index`],
 /// [`TrustedCommitmentTransaction::build_to_local_justice_tx`].
 ///
 /// [`TrustedCommitmentTransaction::revokeable_output_index`]: crate::ln::chan_utils::TrustedCommitmentTransaction::revokeable_output_index

lightning/src/chain/channelmonitor.rs

@@ -29,7 +29,6 @@ use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hash_types::{Txid, BlockHash};
 
-use bitcoin::ecdsa::Signature as BitcoinSignature;
 use bitcoin::secp256k1::{self, SecretKey, PublicKey, Secp256k1, ecdsa::Signature};
 
 use crate::ln::channel::INITIAL_COMMITMENT_NUMBER;
@@ -1675,8 +1674,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// This is provided so that watchtower clients in the persistence pipeline are able to build
 	/// justice transactions for each counterparty commitment upon each update. It's intended to be
 	/// used within an implementation of [`Persist::update_persisted_channel`], which is provided
-	/// with a monitor and an update. Once revoked, signing a justice transaction can be done using
-	/// [`Self::sign_to_local_justice_tx`].
+	/// with a monitor and an update. Once revoked, punishing a revokeable output can be done using
+	/// [`Self::punish_revokeable_output`].
 	///
 	/// It is expected that a watchtower client may use this method to retrieve the latest counterparty
 	/// commitment transaction(s), and then hold the necessary data until a later update in which
@@ -1692,12 +1691,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		self.inner.lock().unwrap().counterparty_commitment_txs_from_update(update)
 	}
 
-	/// Wrapper around [`EcdsaChannelSigner::sign_justice_revoked_output`] to make
-	/// signing the justice transaction easier for implementors of
+	/// Wrapper around [`ChannelSigner::punish_revokeable_output`] to make
+	/// punishing a revokeable output easier for implementors of
 	/// [`chain::chainmonitor::Persist`]. On success this method returns the provided transaction
-	/// signing the input at `input_idx`. This method will only produce a valid signature for
+	/// finalizing the input at `input_idx`. This method will only produce a valid transaction for
 	/// a transaction spending the `to_local` output of a commitment transaction, i.e. this cannot
-	/// be used for revoked HTLC outputs.
+	/// be used for revoked HTLC outputs of a commitment transaction.
 	///
 	/// `Value` is the value of the output being spent by the input at `input_idx`, committed
 	/// in the BIP 143 signature.
@@ -1707,10 +1706,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// to the commitment transaction being revoked, this will return a signed transaction, but
 	/// the signature will not be valid.
 	///
-	/// [`EcdsaChannelSigner::sign_justice_revoked_output`]: crate::sign::ecdsa::EcdsaChannelSigner::sign_justice_revoked_output
+	/// [`ChannelSigner::punish_revokeable_output`]: crate::sign::ChannelSigner::punish_revokeable_output
 	/// [`Persist`]: crate::chain::chainmonitor::Persist
-	pub fn sign_to_local_justice_tx(&self, justice_tx: Transaction, input_idx: usize, value: u64, commitment_number: u64) -> Result<Transaction, ()> {
-		self.inner.lock().unwrap().sign_to_local_justice_tx(justice_tx, input_idx, value, commitment_number)
+	pub fn punish_revokeable_output(&self, justice_tx: &Transaction, input_idx: usize, value: u64, commitment_number: u64) -> Result<Transaction, ()> {
+		self.inner.lock().unwrap().punish_revokeable_output(justice_tx, input_idx, value, commitment_number)
 	}
 
 	pub(crate) fn get_min_seen_secret(&self) -> u64 {
@@ -3458,27 +3457,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		}).collect()
 	}
 
-	fn sign_to_local_justice_tx(
-		&self, mut justice_tx: Transaction, input_idx: usize, value: u64, commitment_number: u64
+	fn punish_revokeable_output(
+		&self, justice_tx: &Transaction, input_idx: usize, value: u64, commitment_number: u64
 	) -> Result<Transaction, ()> {
 		let secret = self.get_secret(commitment_number).ok_or(())?;
 		let per_commitment_key = SecretKey::from_slice(&secret).map_err(|_| ())?;
 		let their_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, &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 revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey,
-			self.counterparty_commitment_params.on_counterparty_tx_csv, &delayed_key);
-
-		let sig = self.onchain_tx_handler.signer.sign_justice_revoked_output(
-			&justice_tx, input_idx, value, &per_commitment_key, &self.onchain_tx_handler.secp_ctx)?;
-		justice_tx.input[input_idx].witness.push_ecdsa_signature(&BitcoinSignature::sighash_all(sig));
-		justice_tx.input[input_idx].witness.push(&[1u8]);
-		justice_tx.input[input_idx].witness.push(revokeable_redeemscript.as_bytes());
-		Ok(justice_tx)
+		self.onchain_tx_handler.signer.punish_revokeable_output(justice_tx, input_idx, value, &per_commitment_key, &self.onchain_tx_handler.secp_ctx, &their_per_commitment_point)
 	}
 
 	/// Can only fail if idx is < get_min_seen_secret

lightning/src/chain/package.rs

@@ -604,15 +604,8 @@ impl PackageSolvingData {
 	fn finalize_input<Signer: EcdsaChannelSigner>(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler<Signer>) -> bool {
 		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_script = chan_utils::get_revokeable_redeemscript(&chan_keys.revocation_key, outp.on_counterparty_tx_csv, &chan_keys.broadcaster_delayed_payment_key);
-				//TODO: should we panic on signer failure ?
-				if let Ok(sig) = onchain_handler.signer.sign_justice_revoked_output(&bumped_tx, i, outp.amount.to_sat(), &outp.per_commitment_key, &onchain_handler.secp_ctx) {
-					let mut ser_sig = sig.serialize_der().to_vec();
-					ser_sig.push(EcdsaSighashType::All as u8);
-					bumped_tx.input[i].witness.push(ser_sig);
-					bumped_tx.input[i].witness.push(vec!(1));
-					bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
+				if let Ok(tx) = onchain_handler.signer.punish_revokeable_output(bumped_tx, i, outp.amount.to_sat(), &outp.per_commitment_key, &onchain_handler.secp_ctx, &outp.per_commitment_point) {
+					*bumped_tx = tx;
 				} else { return false; }
 			},
 			PackageSolvingData::RevokedHTLCOutput(ref outp) => {

lightning/src/sign/mod.rs

@@ -795,6 +795,34 @@ pub trait ChannelSigner {
 		&self, to_self: bool, commitment_number: u64, per_commitment_point: &PublicKey,
 		secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> ScriptBuf;
+
+	/// Finalize the given input in a transaction spending an HTLC transaction output
+	/// or a commitment transaction `to_local` output when our counterparty broadcasts an old state.
+	///
+	/// A justice transaction may claim multiple outputs at the same time if timelocks are
+	/// similar, but only the input at index `input` should be finalized here.
+	/// It may be called multiple times for the same output(s) if a fee-bump is needed with regards
+	/// to an upcoming timelock expiration.
+	///
+	/// Amount is the value of the output spent by this input, committed to in the BIP 143 signature.
+	///
+	/// `per_commitment_key` is revocation secret which was provided by our counterparty when they
+	/// revoked the state which they eventually broadcast. It's not a _holder_ secret key and does
+	/// not allow the spending of any funds by itself (you need our holder `revocation_secret` to do
+	/// so).
+	///
+	/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
+	/// signature and should be retried later. Once the signer is ready to provide a signature after
+	/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
+	/// monitor.
+	///
+	/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
+	///
+	/// TODO(taproot): pass to the `ChannelSigner` all the `TxOut`'s spent by the justice transaction.
+	fn punish_revokeable_output(
+		&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey,
+		secp_ctx: &Secp256k1<secp256k1::All>, per_commitment_point: &PublicKey,
+	) -> Result<Transaction, ()>;
 }
 
 /// Specifies the recipient of an invoice.
@@ -1421,6 +1449,40 @@ impl ChannelSigner for InMemorySigner {
 		)
 		.to_p2wsh()
 	}
+
+	fn punish_revokeable_output(
+		&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey,
+		secp_ctx: &Secp256k1<secp256k1::All>, per_commitment_point: &PublicKey,
+	) -> Result<Transaction, ()> {
+		let params = self.channel_parameters.as_ref().unwrap().as_counterparty_broadcastable();
+		let contest_delay = params.contest_delay();
+		let keys = TxCreationKeys::from_channel_static_keys(
+			per_commitment_point,
+			params.broadcaster_pubkeys(),
+			params.countersignatory_pubkeys(),
+			secp_ctx,
+		);
+		let witness_script = get_revokeable_redeemscript(
+			&keys.revocation_key,
+			contest_delay,
+			&keys.broadcaster_delayed_payment_key,
+		);
+		let sig = EcdsaChannelSigner::sign_justice_revoked_output(
+			self,
+			justice_tx,
+			input,
+			amount,
+			per_commitment_key,
+			secp_ctx,
+		)?;
+		let mut justice_tx = justice_tx.clone();
+		let mut ser_sig = sig.serialize_der().to_vec();
+		ser_sig.push(EcdsaSighashType::All as u8);
+		justice_tx.input[input].witness.push(ser_sig);
+		justice_tx.input[input].witness.push(vec![1]);
+		justice_tx.input[input].witness.push(witness_script.into_bytes());
+		Ok(justice_tx)
+	}
 }
 
 const MISSING_PARAMS_ERR: &'static str =

lightning/src/util/test_channel_signer.rs

@@ -226,6 +226,13 @@ impl ChannelSigner for TestChannelSigner {
 	fn get_revokeable_spk(&self, to_self: bool, commitment_number: u64, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<secp256k1::All>) -> ScriptBuf {
 		self.inner.get_revokeable_spk(to_self, commitment_number, per_commitment_point, secp_ctx)
 	}
+
+	fn punish_revokeable_output(
+		&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey,
+		secp_ctx: &Secp256k1<secp256k1::All>, per_commitment_point: &PublicKey,
+	) -> Result<Transaction, ()> {
+		self.inner.punish_revokeable_output(justice_tx, input, amount, per_commitment_key, secp_ctx, per_commitment_point)
+	}
 }
 
 impl EcdsaChannelSigner for TestChannelSigner {

lightning/src/util/test_utils.rs

@@ -517,7 +517,7 @@ impl<Signer: sign::ecdsa::EcdsaChannelSigner> chainmonitor::Persist<Signer> for
 			while let Some(JusticeTxData { justice_tx, value, commitment_number }) = channel_state.front() {
 				let input_idx = 0;
 				let commitment_txid = justice_tx.input[input_idx].previous_output.txid;
-				match data.sign_to_local_justice_tx(justice_tx.clone(), input_idx, value.to_sat(), *commitment_number) {
+				match data.punish_revokeable_output(justice_tx, input_idx, value.to_sat(), *commitment_number) {
 					Ok(signed_justice_tx) => {
 						let dup = self.watchtower_state.lock().unwrap()
 							.get_mut(&funding_txo).unwrap()