From c2347d61b442e6ea78b48a18d04dda7171a6c6e8 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Sat, 21 Mar 2020 15:39:19 -0400 Subject: [PATCH] Remove signing htlc transaction from ChannelMonitor Extend external signer interface to sign HTLC transactions on its behalf without seckey passing. This move will allow us to remove key access access from ChannelMonitor hot memory in further work. HTLC transactions should stay half-signed by remote until we need to broadcast them for timing-out/claiming HTLCs onchain. --- lightning/src/chain/keysinterface.rs | 42 ++++++++++++++++++++- lightning/src/ln/chan_utils.rs | 41 +------------------- lightning/src/ln/channel.rs | 2 +- lightning/src/ln/channelmonitor.rs | 12 +----- lightning/src/util/enforcing_trait_impls.rs | 5 +++ 5 files changed, 51 insertions(+), 51 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 5f1a307bf..e3e2d9294 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -2,7 +2,7 @@ //! spendable on-chain outputs which the user owns and is responsible for using just as any other //! on-chain output which is theirs. -use bitcoin::blockdata::transaction::{Transaction, OutPoint, TxOut}; +use bitcoin::blockdata::transaction::{Transaction, OutPoint, TxOut, SigHashType}; use bitcoin::blockdata::script::{Script, Builder}; use bitcoin::blockdata::opcodes; use bitcoin::network::constants::Network; @@ -25,6 +25,7 @@ use util::ser::{Writeable, Writer, Readable}; use ln::chan_utils; use ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, LocalCommitmentTransaction}; +use ln::channelmanager::PaymentPreimage; use ln::msgs; use std::sync::Arc; @@ -231,6 +232,11 @@ pub trait ChannelKeys : Send+Clone { #[cfg(test)] fn unsafe_sign_local_commitment(&self, local_commitment_tx: &mut LocalCommitmentTransaction, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1); + /// Signs a transaction created by build_htlc_transaction. If the transaction is an + /// HTLC-Success transaction, preimage must be set! + /// TODO: should be merged with sign_local_commitment as a slice of HTLC transactions to sign + fn sign_htlc_transaction(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1); + /// Create a signature for a (proposed) closing transaction. /// /// Note that, due to rounding, there may be one "missing" satoshi, and either party may have @@ -367,6 +373,40 @@ impl ChannelKeys for InMemoryChannelKeys { local_commitment_tx.add_local_sig(&self.funding_key, funding_redeemscript, channel_value_satoshis, secp_ctx); } + fn sign_htlc_transaction(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1) { + if htlc_tx.input.len() != 1 { return; } + if htlc_tx.input[0].witness.len() != 0 { return; } + + let htlc_redeemscript = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, a_htlc_key, b_htlc_key, revocation_key); + + if let Ok(our_htlc_key) = chan_utils::derive_private_key(secp_ctx, per_commitment_point, &self.htlc_base_key) { + let sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]); + let local_tx = PublicKey::from_secret_key(&secp_ctx, &our_htlc_key) == *a_htlc_key; + let our_sig = secp_ctx.sign(&sighash, &our_htlc_key); + + htlc_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy + + if local_tx { // b, then a + htlc_tx.input[0].witness.push(their_sig.serialize_der().to_vec()); + htlc_tx.input[0].witness.push(our_sig.serialize_der().to_vec()); + } else { + htlc_tx.input[0].witness.push(our_sig.serialize_der().to_vec()); + htlc_tx.input[0].witness.push(their_sig.serialize_der().to_vec()); + } + htlc_tx.input[0].witness[1].push(SigHashType::All as u8); + htlc_tx.input[0].witness[2].push(SigHashType::All as u8); + + if htlc.offered { + htlc_tx.input[0].witness.push(Vec::new()); + assert!(preimage.is_none()); + } else { + htlc_tx.input[0].witness.push(preimage.unwrap().0.to_vec()); + } + + htlc_tx.input[0].witness.push(htlc_redeemscript.as_bytes().to_vec()); + } else { return; } + } + fn sign_closing_transaction(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1) -> Result { if closing_tx.input.len() != 1 { return Err(()); } if closing_tx.input[0].witness.len() != 0 { return Err(()); } diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index a5e0c1460..4be3b3151 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -14,7 +14,7 @@ use bitcoin_hashes::ripemd160::Hash as Ripemd160; use bitcoin_hashes::hash160::Hash as Hash160; use bitcoin_hashes::sha256d::Hash as Sha256dHash; -use ln::channelmanager::{PaymentHash, PaymentPreimage}; +use ln::channelmanager::PaymentHash; use ln::msgs::DecodeError; use util::ser::{Readable, Writeable, Writer, WriterWriteAdaptor}; use util::byte_utils; @@ -355,7 +355,7 @@ impl_writeable!(HTLCOutputInCommitment, 1 + 8 + 4 + 32 + 5, { }); #[inline] -pub(super) fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey) -> Script { +pub(crate) fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey) -> Script { let payment_hash160 = Ripemd160::hash(&htlc.payment_hash.0[..]).into_inner(); if htlc.offered { Builder::new().push_opcode(opcodes::all::OP_DUP) @@ -475,43 +475,6 @@ pub fn build_htlc_transaction(prev_hash: &Sha256dHash, feerate_per_kw: u64, to_s } } -/// Signs a transaction created by build_htlc_transaction. If the transaction is an -/// HTLC-Success transaction (ie htlc.offered is false), preimage must be set! -pub(crate) fn sign_htlc_transaction(tx: &mut Transaction, their_sig: &Signature, preimage: &Option, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, htlc_base_key: &SecretKey, secp_ctx: &Secp256k1) -> Result<(Signature, Script), ()> { - if tx.input.len() != 1 { return Err(()); } - if tx.input[0].witness.len() != 0 { return Err(()); } - - let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&htlc, a_htlc_key, b_htlc_key, revocation_key); - - let our_htlc_key = derive_private_key(secp_ctx, per_commitment_point, htlc_base_key).map_err(|_| ())?; - let sighash = hash_to_message!(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]); - let local_tx = PublicKey::from_secret_key(&secp_ctx, &our_htlc_key) == *a_htlc_key; - let our_sig = secp_ctx.sign(&sighash, &our_htlc_key); - - tx.input[0].witness.push(Vec::new()); // First is the multisig dummy - - if local_tx { // b, then a - tx.input[0].witness.push(their_sig.serialize_der().to_vec()); - tx.input[0].witness.push(our_sig.serialize_der().to_vec()); - } else { - tx.input[0].witness.push(our_sig.serialize_der().to_vec()); - tx.input[0].witness.push(their_sig.serialize_der().to_vec()); - } - tx.input[0].witness[1].push(SigHashType::All as u8); - tx.input[0].witness[2].push(SigHashType::All as u8); - - if htlc.offered { - tx.input[0].witness.push(Vec::new()); - assert!(preimage.is_none()); - } else { - tx.input[0].witness.push(preimage.unwrap().0.to_vec()); - } - - tx.input[0].witness.push(htlc_redeemscript.as_bytes().to_vec()); - - Ok((our_sig, htlc_redeemscript)) -} - #[derive(Clone)] /// We use this to track local commitment transactions and put off signing them until we are ready /// to broadcast. Eventually this will require a signer which is possibly external, but for now we diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 43aba4201..815280017 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4519,7 +4519,7 @@ mod tests { assert!(preimage.is_some()); } - chan_utils::sign_htlc_transaction(&mut htlc_tx, &remote_signature, &preimage, &htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key, &keys.per_commitment_point, chan.local_keys.htlc_base_key(), &chan.secp_ctx).unwrap(); + chan_keys.sign_htlc_transaction(&mut htlc_tx, &remote_signature, &preimage, &htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key, &keys.per_commitment_point, &chan.secp_ctx); assert_eq!(serialize(&htlc_tx)[..], hex::decode($tx_hex).unwrap()[..]); }; diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index fa5e35902..560ace94b 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -1702,11 +1702,7 @@ impl ChannelMonitor { if htlc.offered { log_trace!(self, "Broadcasting HTLC-Timeout transaction against local commitment transactions"); let mut htlc_timeout_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key); - let (our_sig, htlc_script) = match - chan_utils::sign_htlc_transaction(&mut htlc_timeout_tx, their_sig, &None, htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.onchain_detection.keys.htlc_base_key(), &self.secp_ctx) { - Ok(res) => res, - Err(_) => continue, - }; + self.onchain_detection.keys.sign_htlc_transaction(&mut htlc_timeout_tx, their_sig, &None, htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.secp_ctx); log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_timeout_tx.input[0].previous_output.vout, htlc_timeout_tx.input[0].previous_output.txid); res.push(htlc_timeout_tx); @@ -1714,11 +1710,7 @@ impl ChannelMonitor { if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { log_trace!(self, "Broadcasting HTLC-Success transaction against local commitment transactions"); let mut htlc_success_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key); - let (our_sig, htlc_script) = match - chan_utils::sign_htlc_transaction(&mut htlc_success_tx, their_sig, &Some(*payment_preimage), htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.onchain_detection.keys.htlc_base_key(), &self.secp_ctx) { - Ok(res) => res, - Err(_) => continue, - }; + self.onchain_detection.keys.sign_htlc_transaction(&mut htlc_success_tx, their_sig, &Some(*payment_preimage), htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.secp_ctx); log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_success_tx.input[0].previous_output.vout, htlc_success_tx.input[0].previous_output.txid); res.push(htlc_success_tx); diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index bc3a1a0da..5e3aba431 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -1,4 +1,5 @@ use ln::chan_utils::{HTLCOutputInCommitment, TxCreationKeys, ChannelPublicKeys, LocalCommitmentTransaction}; +use ln::channelmanager::PaymentPreimage; use ln::msgs; use chain::keysinterface::{ChannelKeys, InMemoryChannelKeys}; @@ -88,6 +89,10 @@ impl ChannelKeys for EnforcingChannelKeys { self.inner.unsafe_sign_local_commitment(local_commitment_tx, funding_redeemscript, channel_value_satoshis, secp_ctx); } + fn sign_htlc_transaction(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1) { + self.inner.sign_htlc_transaction(htlc_tx, their_sig, preimage, htlc, a_htlc_key, b_htlc_key, revocation_key, per_commitment_point, secp_ctx); + } + fn sign_closing_transaction(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1) -> Result { Ok(self.inner.sign_closing_transaction(closing_tx, secp_ctx).unwrap()) } -- 2.39.5