Split sign_justice_transaction in two halves

To avoid caller data struct storing HTLC-related information when
a revokeable output is claimed on top of a commitment/second-stage
HTLC transactions, we split `keysinterface::sign_justice_transaction`
in two new halves `keysinterfaces::sign_justice_revoked_output` and
`keysinterfaces::sign_justice_revoked_htlc`.

Further, this split offers more flexibility to signer policy as a
commitment revokeable output might be of a value far more significant
than HTLC ones.

Author: Antoine Riard

lightning/src/chain/keysinterface.rs

@@ -279,12 +279,28 @@ pub trait BaseSign {
 	#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
 	fn unsafe_sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
 
-	/// Create a signature for the given input in a transaction spending an HTLC or commitment
-	/// transaction output when our counterparty broadcasts an old state.
+	/// Create a signature for 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 multiples outputs at the same time if timelocks are
+	/// A justice transaction may claim multiple outputs at the same time if timelocks are
 	/// similar, but only a signature for the input at index `input` should be signed for here.
-	/// It may be called multiples time for same output(s) if a fee-bump is needed with regards
+	/// It may be called multiple times for same output(s) if a fee-bump is needed with regards
+	/// to an upcoming timelock expiration.
+	///
+	/// Amount is 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).
+	fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
+
+	/// Create a signature for the given input in a transaction spending a commitment transaction
+	/// HTLC 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 a signature for the input at index `input` should be signed for here.
+	/// It may be called multiple times for same output(s) if a fee-bump is needed with regards
 	/// to an upcoming timelock expiration.
 	///
 	/// Amount is value of the output spent by this input, committed to in the BIP 143 signature.
@@ -297,7 +313,7 @@ pub trait BaseSign {
 	/// htlc holds HTLC elements (hash, timelock) if the output being spent is a HTLC output, thus
 	/// changing the format of the witness script (which is committed to in the BIP 143
 	/// signatures).
-	fn sign_justice_transaction(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &Option<HTLCOutputInCommitment>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
+	fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
 
 	/// Create a signature for a claiming transaction for a HTLC output on a counterparty's commitment
 	/// transaction, either offered or received.
@@ -624,7 +640,29 @@ impl BaseSign for InMemorySigner {
 		Ok((sig, htlc_sigs))
 	}
 
-	fn sign_justice_transaction(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &Option<HTLCOutputInCommitment>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+	fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+		let revocation_key = match chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_key, &self.revocation_base_key) {
+			Ok(revocation_key) => revocation_key,
+			Err(_) => return Err(())
+		};
+		let per_commitment_point = PublicKey::from_secret_key(secp_ctx, &per_commitment_key);
+		let revocation_pubkey = match chan_utils::derive_public_revocation_key(&secp_ctx, &per_commitment_point, &self.pubkeys().revocation_basepoint) {
+			Ok(revocation_pubkey) => revocation_pubkey,
+			Err(_) => return Err(())
+		};
+		let witness_script = {
+			let counterparty_delayedpubkey = match chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.counterparty_pubkeys().delayed_payment_basepoint) {
+				Ok(counterparty_delayedpubkey) => counterparty_delayedpubkey,
+				Err(_) => return Err(())
+			};
+			chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.holder_selected_contest_delay(), &counterparty_delayedpubkey)
+		};
+		let mut sighash_parts = bip143::SigHashCache::new(justice_tx);
+		let sighash = hash_to_message!(&sighash_parts.signature_hash(input, &witness_script, amount, SigHashType::All)[..]);
+		return Ok(secp_ctx.sign(&sighash, &revocation_key))
+	}
+
+	fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
 		let revocation_key = match chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_key, &self.revocation_base_key) {
 			Ok(revocation_key) => revocation_key,
 			Err(_) => return Err(())
@@ -634,7 +672,7 @@ impl BaseSign for InMemorySigner {
 			Ok(revocation_pubkey) => revocation_pubkey,
 			Err(_) => return Err(())
 		};
-		let witness_script = if let &Some(ref htlc) = htlc {
+		let witness_script = {
 			let counterparty_htlcpubkey = match chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.counterparty_pubkeys().htlc_basepoint) {
 				Ok(counterparty_htlcpubkey) => counterparty_htlcpubkey,
 				Err(_) => return Err(())
@@ -644,12 +682,6 @@ impl BaseSign for InMemorySigner {
 				Err(_) => return Err(())
 			};
 			chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &counterparty_htlcpubkey, &holder_htlcpubkey, &revocation_pubkey)
-		} else {
-			let counterparty_delayedpubkey = match chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.counterparty_pubkeys().delayed_payment_basepoint) {
-				Ok(counterparty_delayedpubkey) => counterparty_delayedpubkey,
-				Err(_) => return Err(())
-			};
-			chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.holder_selected_contest_delay(), &counterparty_delayedpubkey)
 		};
 		let mut sighash_parts = bip143::SigHashCache::new(justice_tx);
 		let sighash = hash_to_message!(&sighash_parts.signature_hash(input, &witness_script, amount, SigHashType::All)[..]);

lightning/src/ln/onchaintx.rs

@@ -628,7 +628,11 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 								chan_utils::get_revokeable_redeemscript(&tx_keys.revocation_key, *on_counterparty_tx_csv, &tx_keys.broadcaster_delayed_payment_key)
 							};
 
-							let sig = self.signer.sign_justice_transaction(&bumped_tx, i, *amount, &per_commitment_key, htlc, &self.secp_ctx).expect("sign justice tx");
+							let sig = if let Some(ref htlc) = *htlc {
+								self.signer.sign_justice_revoked_htlc(&bumped_tx, i, *amount, &per_commitment_key, &htlc, &self.secp_ctx).expect("sign justice tx")
+							} else {
+								self.signer.sign_justice_revoked_output(&bumped_tx, i, *amount, &per_commitment_key, &self.secp_ctx).expect("sign justice tx")
+							};
 							bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
 							bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
 							if htlc.is_some() {

lightning/src/util/enforcing_trait_impls.rs

@@ -140,8 +140,12 @@ impl BaseSign for EnforcingSigner {
 		Ok(self.inner.unsafe_sign_holder_commitment_and_htlcs(commitment_tx, secp_ctx).unwrap())
 	}
 
-	fn sign_justice_transaction(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &Option<HTLCOutputInCommitment>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
-		Ok(self.inner.sign_justice_transaction(justice_tx, input, amount, per_commitment_key, htlc, secp_ctx).unwrap())
+	fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+		Ok(self.inner.sign_justice_revoked_output(justice_tx, input, amount, per_commitment_key, secp_ctx).unwrap())
+	}
+
+	fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+		Ok(self.inner.sign_justice_revoked_htlc(justice_tx, input, amount, per_commitment_key, htlc, secp_ctx).unwrap())
 	}
 
 	fn sign_counterparty_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {