Remove SecretKey from DynamicOutputP2WSH descriptor

Add sign_delayed_transaction in ChanSigner to be able to spend
SpendableOutputDescriptor in test framework.

Author: Antoine Riard

lightning/src/chain/keysinterface.rs

@@ -65,8 +65,8 @@ pub enum SpendableOutputDescriptor {
 	DynamicOutputP2WSH {
 		/// The outpoint which is spendable
 		outpoint: OutPoint,
-		/// The secret key which must be used to sign the spending transaction
-		key: SecretKey,
+		/// Per commitment point to derive delayed_payment_key by key holder
+		per_commitment_point: PublicKey,
 		/// The witness redeemScript which is hashed to create the script_pubkey in the given output
 		witness_script: Script,
 		/// The nSequence value which must be set in the spending input to satisfy the OP_CSV in
@@ -98,10 +98,10 @@ impl Writeable for SpendableOutputDescriptor {
 				outpoint.write(writer)?;
 				output.write(writer)?;
 			},
-			&SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref key, ref witness_script, ref to_self_delay, ref output } => {
+			&SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref witness_script, ref to_self_delay, ref output } => {
 				1u8.write(writer)?;
 				outpoint.write(writer)?;
-				key.write(writer)?;
+				per_commitment_point.write(writer)?;
 				witness_script.write(writer)?;
 				to_self_delay.write(writer)?;
 				output.write(writer)?;
@@ -126,7 +126,7 @@ impl Readable for SpendableOutputDescriptor {
 			}),
 			1u8 => Ok(SpendableOutputDescriptor::DynamicOutputP2WSH {
 				outpoint: Readable::read(reader)?,
-				key: Readable::read(reader)?,
+				per_commitment_point: Readable::read(reader)?,
 				witness_script: Readable::read(reader)?,
 				to_self_delay: Readable::read(reader)?,
 				output: Readable::read(reader)?,
@@ -239,6 +239,9 @@ pub trait ChannelKeys : Send+Clone {
 	/// chosen to forgo their output as dust.
 	fn sign_closing_transaction<T: secp256k1::Signing>(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
 
+	/// Create a signature for a delayed transaction.
+	fn sign_delayed_transaction<T: secp256k1::Signing>(&self, spend_tx: &mut Transaction, input: usize, witness_script: &Script, amount: u64, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<T>);
+
 	/// Signs a channel announcement message with our funding key, proving it comes from one
 	/// of the channel participants.
 	///
@@ -469,6 +472,18 @@ impl ChannelKeys for InMemoryChannelKeys {
 		Ok(secp_ctx.sign(&sighash, &self.funding_key))
 	}
 
+	fn sign_delayed_transaction<T: secp256k1::Signing>(&self, spend_tx: &mut Transaction, input: usize, witness_script: &Script, amount: u64, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<T>) {
+		if let Ok(htlc_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &self.delayed_payment_base_key) {
+			let sighash_parts = bip143::SighashComponents::new(&spend_tx);
+			let sighash = hash_to_message!(&sighash_parts.sighash_all(&spend_tx.input[input], witness_script, amount)[..]);
+			let local_delaysig = secp_ctx.sign(&sighash, &htlc_key);
+			spend_tx.input[0].witness.push(local_delaysig.serialize_der().to_vec());
+			spend_tx.input[0].witness[0].push(SigHashType::All as u8);
+			spend_tx.input[0].witness.push(vec!(0));
+			spend_tx.input[0].witness.push(witness_script.clone().into_bytes());
+		}
+	}
+
 	fn sign_channel_announcement<T: secp256k1::Signing>(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
 		let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
 		Ok(secp_ctx.sign(&msghash, &self.funding_key))

lightning/src/ln/channelmonitor.rs

@@ -722,7 +722,7 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
 	commitment_transaction_number_obscure_factor: u64,
 
 	destination_script: Script,
-	broadcasted_local_revokable_script: Option<(Script, SecretKey, Script)>,
+	broadcasted_local_revokable_script: Option<(Script, PublicKey, Script)>,
 	broadcasted_remote_payment_script: Option<(Script, SecretKey)>,
 	shutdown_script: Script,
 
@@ -1644,14 +1644,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		(claimable_outpoints, Some((htlc_txid, tx.output.clone())))
 	}
 
-	fn broadcast_by_local_state(&self, commitment_tx: &Transaction, local_tx: &LocalSignedTx) -> (Vec<ClaimRequest>, Vec<TxOut>, Option<(Script, SecretKey, Script)>) {
+	fn broadcast_by_local_state(&self, commitment_tx: &Transaction, local_tx: &LocalSignedTx) -> (Vec<ClaimRequest>, Vec<TxOut>, Option<(Script, PublicKey, Script)>) {
 		let mut claim_requests = Vec::with_capacity(local_tx.htlc_outputs.len());
 		let mut watch_outputs = Vec::with_capacity(local_tx.htlc_outputs.len());
 
 		let redeemscript = chan_utils::get_revokeable_redeemscript(&local_tx.revocation_key, self.their_to_self_delay.unwrap(), &local_tx.delayed_payment_key);
-		let broadcasted_local_revokable_script = if let Ok(local_delayedkey) = chan_utils::derive_private_key(&self.secp_ctx, &local_tx.per_commitment_point, self.onchain_detection.keys.delayed_payment_base_key()) {
-			Some((redeemscript.to_v0_p2wsh(), local_delayedkey, redeemscript))
-		} else { None };
+		let broadcasted_local_revokable_script = Some((redeemscript.to_v0_p2wsh(), local_tx.per_commitment_point.clone(), redeemscript));
 
 		for &(ref htlc, _, _) in local_tx.htlc_outputs.iter() {
 			if let Some(transaction_output_index) = htlc.transaction_output_index {
@@ -2126,7 +2124,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 				if broadcasted_local_revokable_script.0 == outp.script_pubkey {
 					spendable_output =  Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
 						outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
-						key: broadcasted_local_revokable_script.1,
+						per_commitment_point: broadcasted_local_revokable_script.1,
 						witness_script: broadcasted_local_revokable_script.2.clone(),
 						to_self_delay: self.their_to_self_delay.unwrap(),
 						output: outp.clone(),
@@ -2190,9 +2188,9 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
 		let broadcasted_local_revokable_script = match <u8 as Readable>::read(reader)? {
 			0 => {
 				let revokable_address = Readable::read(reader)?;
-				let local_delayedkey = Readable::read(reader)?;
+				let per_commitment_point = Readable::read(reader)?;
 				let revokable_script = Readable::read(reader)?;
-				Some((revokable_address, local_delayedkey, revokable_script))
+				Some((revokable_address, per_commitment_point, revokable_script))
 			},
 			1 => { None },
 			_ => return Err(DecodeError::InvalidValue),

lightning/src/ln/functional_tests.rs

@@ -3934,7 +3934,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 }
 
 macro_rules! check_spendable_outputs {
-	($node: expr, $der_idx: expr) => {
+	($node: expr, $der_idx: expr, $chan_signer: expr) => {
 		{
 			let events = $node.chan_monitor.simple_monitor.get_and_clear_pending_events();
 			let mut txn = Vec::new();
@@ -3970,7 +3970,7 @@ macro_rules! check_spendable_outputs {
 									spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
 									txn.push(spend_tx);
 								},
-								SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref key, ref witness_script, ref to_self_delay, ref output } => {
+								SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref witness_script, ref to_self_delay, ref output } => {
 									let input = TxIn {
 										previous_output: outpoint.clone(),
 										script_sig: Script::new(),
@@ -3988,12 +3988,7 @@ macro_rules! check_spendable_outputs {
 										output: vec![outp],
 									};
 									let secp_ctx = Secp256k1::new();
-									let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], witness_script, output.value)[..]).unwrap();
-									let local_delaysig = secp_ctx.sign(&sighash, key);
-									spend_tx.input[0].witness.push(local_delaysig.serialize_der().to_vec());
-									spend_tx.input[0].witness[0].push(SigHashType::All as u8);
-									spend_tx.input[0].witness.push(vec!(0));
-									spend_tx.input[0].witness.push(witness_script.clone().into_bytes());
+									$chan_signer.sign_delayed_transaction(&mut spend_tx, 0, &witness_script, output.value, per_commitment_point, &secp_ctx);
 									txn.push(spend_tx);
 								},
 								SpendableOutputDescriptor::StaticOutput { ref outpoint, ref output } => {
@@ -4066,7 +4061,8 @@ fn test_claim_sizeable_push_msat() {
 	nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()] }, 0);
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
-	let spend_txn = check_spendable_outputs!(nodes[1], 1);
+	let chan_signer = get_chan_signer!(nodes[1], chan.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 1);
 	check_spends!(spend_txn[0], node_txn[0]);
 }
@@ -4096,7 +4092,8 @@ fn test_claim_on_remote_sizeable_push_msat() {
 	check_added_monitors!(nodes[1], 1);
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
-	let spend_txn = check_spendable_outputs!(nodes[1], 1);
+	let chan_signer = get_chan_signer!(nodes[1], chan.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 2);
 	assert_eq!(spend_txn[0], spend_txn[1]);
 	check_spends!(spend_txn[0], node_txn[0]);
@@ -4129,7 +4126,8 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
 	nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
-	let spend_txn = check_spendable_outputs!(nodes[1], 1);
+	let chan_signer = get_chan_signer!(nodes[1], chan.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 3);
 	assert_eq!(spend_txn[0], spend_txn[1]); // to_remote output on revoked remote commitment_tx
 	check_spends!(spend_txn[0], revoked_local_txn[0]);
@@ -4180,7 +4178,8 @@ fn test_static_spendable_outputs_preimage_tx() {
 	nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
-	let spend_txn = check_spendable_outputs!(nodes[1], 1);
+	let chan_signer = get_chan_signer!(nodes[1], chan_1.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 1);
 	check_spends!(spend_txn[0], node_txn[0]);
 }
@@ -4234,7 +4233,8 @@ fn test_static_spendable_outputs_timeout_tx() {
 		_ => panic!("Unexpected event"),
 	}
 
-	let spend_txn = check_spendable_outputs!(nodes[1], 1);
+	let chan_signer = get_chan_signer!(nodes[1], chan_1.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 3); // SpendableOutput: remote_commitment_tx.to_remote (*2), timeout_tx.output (*1)
 	check_spends!(spend_txn[2], node_txn[0].clone());
 }
@@ -4270,7 +4270,8 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
 	nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
-	let spend_txn = check_spendable_outputs!(nodes[1], 1);
+	let chan_signer = get_chan_signer!(nodes[1], chan_1.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 1);
 	check_spends!(spend_txn[0], node_txn[0]);
 }
@@ -4325,7 +4326,8 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
 	// Check B's ChannelMonitor was able to generate the right spendable output descriptor
-	let spend_txn = check_spendable_outputs!(nodes[1], 1);
+	let chan_signer = get_chan_signer!(nodes[1], chan_1.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 2);
 	check_spends!(spend_txn[0], node_txn[0]);
 	check_spends!(spend_txn[1], node_txn[2]);
@@ -4375,7 +4377,8 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
 	connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
 
 	// Check A's ChannelMonitor was able to generate the right spendable output descriptor
-	let spend_txn = check_spendable_outputs!(nodes[0], 1);
+	let chan_signer = get_chan_signer!(nodes[0], chan_1.2);
+	let spend_txn = check_spendable_outputs!(nodes[0], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 5); // Duplicated SpendableOutput due to block rescan after revoked htlc output tracking
 	assert_eq!(spend_txn[0], spend_txn[1]);
 	assert_eq!(spend_txn[0], spend_txn[2]);
@@ -4651,7 +4654,8 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() {
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 201, true, header_201.bitcoin_hash());
 
 	// Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor
-	let spend_txn = check_spendable_outputs!(nodes[1], 1);
+	let chan_signer = get_chan_signer!(nodes[1], chan_1.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 2);
 	check_spends!(spend_txn[0], node_txn[0]);
 	check_spends!(spend_txn[1], node_txn[1]);
@@ -4952,7 +4956,8 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 	}
 
 	// Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
-	let spend_txn = check_spendable_outputs!(nodes[0], 1);
+	let chan_signer = get_chan_signer!(nodes[0], chan_1.2);
+	let spend_txn = check_spendable_outputs!(nodes[0], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 3);
 	assert_eq!(spend_txn[0], spend_txn[1]);
 	check_spends!(spend_txn[0], local_txn[0]);
@@ -4975,14 +4980,16 @@ fn test_static_output_closing_tx() {
 	nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0);
 	connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
 
-	let spend_txn = check_spendable_outputs!(nodes[0], 2);
+	let chan_signer = get_chan_signer!(nodes[0], chan.2);
+	let spend_txn = check_spendable_outputs!(nodes[0], 2, chan_signer);
 	assert_eq!(spend_txn.len(), 1);
 	check_spends!(spend_txn[0], closing_tx);
 
 	nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0);
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
 
-	let spend_txn = check_spendable_outputs!(nodes[1], 2);
+	let chan_signer = get_chan_signer!(nodes[1], chan.2);
+	let spend_txn = check_spendable_outputs!(nodes[1], 2, chan_signer);
 	assert_eq!(spend_txn.len(), 1);
 	check_spends!(spend_txn[0], closing_tx);
 }
@@ -6835,7 +6842,8 @@ fn test_data_loss_protect() {
 	let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
 	nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()]}, 0);
 	connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
-	let spend_txn = check_spendable_outputs!(nodes[0], 1);
+	let chan_signer = get_chan_signer!(nodes[0], chan.2);
+	let spend_txn = check_spendable_outputs!(nodes[0], 1, chan_signer);
 	assert_eq!(spend_txn.len(), 1);
 	check_spends!(spend_txn[0], node_txn[0]);
 }

lightning/src/util/enforcing_trait_impls.rs

@@ -97,6 +97,10 @@ impl ChannelKeys for EnforcingChannelKeys {
 		self.inner.sign_remote_htlc_transaction(bumped_tx, input, witness_script, amount, per_commitment_point, preimage, secp_ctx);
 	}
 
+	fn sign_delayed_transaction<T: secp256k1::Signing>(&self, spend_tx: &mut Transaction, input: usize, witness_script: &Script, amount: u64, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<T>) {
+		self.inner.sign_delayed_transaction(spend_tx, input, witness_script, amount, per_commitment_point, secp_ctx);
+	}
+
 	fn sign_closing_transaction<T: secp256k1::Signing>(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
 		Ok(self.inner.sign_closing_transaction(closing_tx, secp_ctx).unwrap())
 	}