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.

diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs
index 5f1a307bf27dc9f7bba705961b8b3646b4393b12..e3e2d929447b129cd4e9664b7d54a1b7c15bea70 100644 (file)
--- 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<T: secp256k1::Signing + secp256k1::Verification>(&self, local_commitment_tx: &mut LocalCommitmentTransaction, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>);
 
+       /// 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<T: secp256k1::Signing>(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option<PaymentPreimage>, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<T>);
+
        /// 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<T: secp256k1::Signing>(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option<PaymentPreimage>, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<T>) {
+               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<T: secp256k1::Signing>(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
                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 a5e0c1460be71b2281fb46e1abb528ac5fbecec5..4be3b3151df3d6e66e539585811d16fdfee07e69 100644 (file)
--- 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<T: secp256k1::Signing>(tx: &mut Transaction, their_sig: &Signature, preimage: &Option<PaymentPreimage>, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, htlc_base_key: &SecretKey, secp_ctx: &Secp256k1<T>) -> 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 43aba4201f43c419f542fef6319bb5e3fda12919..815280017761069100384186651be7bb18e4ae9d 100644 (file)
--- 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 fa5e359027a4ce1b1c30cb690b6dfc4380308410..560ace94b4215599313e7169cccc8af4e04e5d6c 100644 (file)
--- a/lightning/src/ln/channelmonitor.rs
+++ b/lightning/src/ln/channelmonitor.rs
@@ -1702,11 +1702,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                        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<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                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 bc3a1a0da1502a00e80946dd8c8c89e1bcb95498..5e3aba43193f050a7e38ce4ad2bbb7c1048e4f15 100644 (file)
--- 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<T: secp256k1::Signing>(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option<PaymentPreimage>, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<T>) {
+               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<T: secp256k1::Signing>(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
                Ok(self.inner.sign_closing_transaction(closing_tx, secp_ctx).unwrap())
        }