Use external key signer to generate closing transaction signatures 2019-12-simple-signer-api-step
authorMatt Corallo <git@bluematt.me>
Fri, 13 Dec 2019 06:57:45 +0000 (01:57 -0500)
committerMatt Corallo <git@bluematt.me>
Fri, 13 Dec 2019 21:16:33 +0000 (16:16 -0500)
lightning/src/chain/keysinterface.rs
lightning/src/ln/channel.rs
lightning/src/util/enforcing_trait_impls.rs

index 2a68299c248caabc0045244d3e8cb03a5fa6d7f1..2a8910b03789a424469f8b9f5db0a0c297ca71ae 100644 (file)
@@ -142,7 +142,13 @@ pub trait ChannelKeys : Send {
        /// TODO: Document the things someone using this interface should enforce before signing.
        /// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
        /// making the callee generate it via some util function we expose)!
-       fn sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_script: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
+       fn sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, absolute_fee: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
+
+       /// Create a signature for a (proposed) closing transaction.
+       ///
+       /// Note that, due to rounding, there may be one "missing" satoshi, and either party may have
+       /// chosen to forgo their output as dust.
+       fn sign_closing_transaction<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
 
        /// Signs a channel announcement message with our funding key, proving it comes from one
        /// of the channel participants.
@@ -178,9 +184,9 @@ impl ChannelKeys for InMemoryChannelKeys {
        fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
        fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
 
-       fn sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_script: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
+       fn sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
                if commitment_tx.input.len() != 1 { return Err(()); }
-               let commitment_sighash = hash_to_message!(&bip143::SighashComponents::new(&commitment_tx).sighash_all(&commitment_tx.input[0], &channel_funding_script, channel_value_satoshis)[..]);
+               let commitment_sighash = hash_to_message!(&bip143::SighashComponents::new(&commitment_tx).sighash_all(&commitment_tx.input[0], &channel_funding_redeemscript, channel_value_satoshis)[..]);
                let commitment_sig = secp_ctx.sign(&commitment_sighash, &self.funding_key);
 
                let commitment_txid = commitment_tx.txid();
@@ -202,6 +208,16 @@ impl ChannelKeys for InMemoryChannelKeys {
                Ok((commitment_sig, htlc_sigs))
        }
 
+       fn sign_closing_transaction<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, 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(()); }
+               if closing_tx.output.len() > 2 { return Err(()); }
+
+               let sighash = hash_to_message!(&bip143::SighashComponents::new(closing_tx)
+                       .sighash_all(&closing_tx.input[0], &channel_funding_redeemscript, channel_value_satoshis)[..]);
+               Ok(secp_ctx.sign(&sighash, &self.funding_key))
+       }
+
        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))
index c7f074ffccce5104f4d9886c780fe8cf23bacbb9..f1330fb92029e22b9be5e53a99f29c2c9725df2f 100644 (file)
@@ -303,7 +303,7 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        #[cfg(not(test))]
        last_local_commitment_txn: Vec<Transaction>,
 
-       last_sent_closing_fee: Option<(u64, u64)>, // (feerate, fee)
+       last_sent_closing_fee: Option<(u64, u64, Signature)>, // (feerate, fee, our_sig)
 
        /// The hash of the block in which the funding transaction reached our CONF_TARGET. We use this
        /// to detect unconfirmation after a serialize-unserialize roundtrip where we may not see a full
@@ -2665,14 +2665,16 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let proposed_total_fee_satoshis = proposed_feerate * tx_weight / 1000;
 
                let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
-               let funding_redeemscript = self.get_funding_redeemscript();
-               let sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
+               let our_sig = self.local_keys
+                       .sign_closing_transaction(self.channel_value_satoshis, &self.get_funding_redeemscript(), &closing_tx, &self.secp_ctx)
+                       .ok();
+               if our_sig.is_none() { return None; }
 
-               self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis));
+               self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis, our_sig.clone().unwrap()));
                Some(msgs::ClosingSigned {
                        channel_id: self.channel_id,
                        fee_satoshis: total_fee_satoshis,
-                       signature: self.secp_ctx.sign(&sighash, &self.local_keys.funding_key()),
+                       signature: our_sig.unwrap(),
                })
        }
 
@@ -2749,6 +2751,28 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok((our_shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs))
        }
 
+       fn build_signed_closing_transaction(&self, tx: &mut Transaction, their_sig: &Signature, our_sig: &Signature) {
+               if tx.input.len() != 1 { panic!("Tried to sign closing transaction that had input count != 1!"); }
+               if tx.input[0].witness.len() != 0 { panic!("Tried to re-sign closing transaction"); }
+               if tx.output.len() > 2 { panic!("Tried to sign bogus closing transaction"); }
+
+               tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
+
+               let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
+               let their_funding_key = self.their_funding_pubkey.unwrap().serialize();
+               if our_funding_key[..] < their_funding_key[..] {
+                       tx.input[0].witness.push(our_sig.serialize_der().to_vec());
+                       tx.input[0].witness.push(their_sig.serialize_der().to_vec());
+               } else {
+                       tx.input[0].witness.push(their_sig.serialize_der().to_vec());
+                       tx.input[0].witness.push(our_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);
+
+               tx.input[0].witness.push(self.get_funding_redeemscript().into_bytes());
+       }
+
        pub fn closing_signed(&mut self, fee_estimator: &FeeEstimator, msg: &msgs::ClosingSigned) -> Result<(Option<msgs::ClosingSigned>, Option<Transaction>), ChannelError> {
                if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK != BOTH_SIDES_SHUTDOWN_MASK {
                        return Err(ChannelError::Close("Remote end sent us a closing_signed before both sides provided a shutdown"));
@@ -2781,9 +2805,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        },
                };
 
-               if let Some((_, last_fee)) = self.last_sent_closing_fee {
+               if let Some((_, last_fee, our_sig)) = self.last_sent_closing_fee {
                        if last_fee == msg.fee_satoshis {
-                               self.sign_commitment_transaction(&mut closing_tx, &msg.signature);
+                               self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig);
                                self.channel_state = ChannelState::ShutdownComplete as u32;
                                self.channel_update_count += 1;
                                return Ok((None, Some(closing_tx)));
@@ -2794,9 +2818,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        ($new_feerate: expr) => {
                                let closing_tx_max_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap());
                                let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate * closing_tx_max_weight / 1000, false);
-                               sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
-                               let our_sig = self.secp_ctx.sign(&sighash, &self.local_keys.funding_key());
-                               self.last_sent_closing_fee = Some(($new_feerate, used_total_fee));
+                               let our_sig = self.local_keys
+                                       .sign_closing_transaction(self.channel_value_satoshis, &funding_redeemscript, &closing_tx, &self.secp_ctx)
+                                       .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction"))?;
+                               self.last_sent_closing_fee = Some(($new_feerate, used_total_fee, our_sig.clone()));
                                return Ok((Some(msgs::ClosingSigned {
                                        channel_id: self.channel_id,
                                        fee_satoshis: used_total_fee,
@@ -2809,7 +2834,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if self.channel_outbound {
                        let our_max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
                        if proposed_sat_per_kw > our_max_feerate {
-                               if let Some((last_feerate, _)) = self.last_sent_closing_fee {
+                               if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
                                        if our_max_feerate <= last_feerate {
                                                return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something higher than our Normal feerate"));
                                        }
@@ -2819,7 +2844,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                } else {
                        let our_min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
                        if proposed_sat_per_kw < our_min_feerate {
-                               if let Some((last_feerate, _)) = self.last_sent_closing_fee {
+                               if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
                                        if our_min_feerate >= last_feerate {
                                                return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something lower than our Background feerate"));
                                        }
@@ -2828,7 +2853,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                }
 
-               let our_sig = self.sign_commitment_transaction(&mut closing_tx, &msg.signature);
+               let our_sig = self.local_keys
+                       .sign_closing_transaction(self.channel_value_satoshis, &funding_redeemscript, &closing_tx, &self.secp_ctx)
+                       .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction"))?;
+               self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig);
+
                self.channel_state = ChannelState::ShutdownComplete as u32;
                self.channel_update_count += 1;
 
@@ -3855,10 +3884,11 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                }
 
                match self.last_sent_closing_fee {
-                       Some((feerate, fee)) => {
+                       Some((feerate, fee, sig)) => {
                                1u8.write(writer)?;
                                feerate.write(writer)?;
                                fee.write(writer)?;
+                               sig.write(writer)?;
                        },
                        None => 0u8.write(writer)?,
                }
@@ -4022,7 +4052,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
 
                let last_sent_closing_fee = match <u8 as Readable<R>>::read(reader)? {
                        0 => None,
-                       1 => Some((Readable::read(reader)?, Readable::read(reader)?)),
+                       1 => Some((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)),
                        _ => return Err(DecodeError::InvalidValue),
                };
 
index 0415351c9cb82e713ca2a59752f9731638b15709..e9c82c1bd2128e1889fe996c704d31f4ec0ac7a3 100644 (file)
@@ -52,6 +52,10 @@ impl ChannelKeys for EnforcingChannelKeys {
                Ok(self.inner.sign_remote_commitment(channel_value_satoshis, channel_funding_script, feerate_per_kw, commitment_tx, keys, htlcs, to_self_delay, secp_ctx).unwrap())
        }
 
+       fn sign_closing_transaction<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+               Ok(self.inner.sign_closing_transaction(channel_value_satoshis, channel_funding_redeemscript, closing_tx, secp_ctx).unwrap())
+       }
+
        fn sign_channel_announcement<T: secp256k1::Signing>(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
                self.inner.sign_channel_announcement(msg, secp_ctx)
        }