Merge pull request #1064 from lightning-signer/2021-08-closing-tx-phase2
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Thu, 9 Sep 2021 19:31:47 +0000 (19:31 +0000)
committerGitHub <noreply@github.com>
Thu, 9 Sep 2021 19:31:47 +0000 (19:31 +0000)
lightning/src/chain/keysinterface.rs
lightning/src/ln/chan_utils.rs
lightning/src/ln/channel.rs
lightning/src/util/enforcing_trait_impls.rs

index 44a03d09f8e717019c19c070159a7b7556b24723..2da9e575047f68ec805fe0089642cdd90169ce92 100644 (file)
@@ -34,7 +34,7 @@ use util::ser::{Writeable, Writer, Readable};
 
 use chain::transaction::OutPoint;
 use ln::chan_utils;
-use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, ChannelTransactionParameters, CommitmentTransaction};
+use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, ChannelTransactionParameters, CommitmentTransaction, ClosingTransaction};
 use ln::msgs::UnsignedChannelAnnouncement;
 use ln::script::ShutdownScript;
 
@@ -322,7 +322,7 @@ pub trait BaseSign {
        ///
        /// 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(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
+       fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
 
        /// Signs a channel announcement message with our funding key, proving it comes from one
        /// of the channel participants.
@@ -671,17 +671,10 @@ impl BaseSign for InMemorySigner {
                Err(())
        }
 
-       fn sign_closing_transaction(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<secp256k1::All>) -> 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(()); }
-
+       fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
                let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
-
-               let sighash = hash_to_message!(&bip143::SigHashCache::new(closing_tx)
-                       .signature_hash(0, &channel_funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]);
-               Ok(secp_ctx.sign(&sighash, &self.funding_key))
+               Ok(closing_tx.trust().sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
        }
 
        fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
index 6bdef4307350d17c158ef2fe88dac121532db532..994e1c276c4427168bca70a6aa1369bc7f1a741f 100644 (file)
@@ -23,7 +23,7 @@ use bitcoin::hash_types::{Txid, PubkeyHash};
 use ln::{PaymentHash, PaymentPreimage};
 use ln::msgs::DecodeError;
 use util::ser::{Readable, Writeable, Writer};
-use util::byte_utils;
+use util::{byte_utils, transaction_utils};
 
 use bitcoin::hash_types::WPubkeyHash;
 use bitcoin::secp256k1::key::{SecretKey, PublicKey};
@@ -80,6 +80,50 @@ pub fn build_commitment_secret(commitment_seed: &[u8; 32], idx: u64) -> [u8; 32]
        res
 }
 
+/// Build a closing transaction
+pub fn build_closing_transaction(to_holder_value_sat: u64, to_counterparty_value_sat: u64, to_holder_script: Script, to_counterparty_script: Script, funding_outpoint: OutPoint) -> Transaction {
+       let txins = {
+               let mut ins: Vec<TxIn> = Vec::new();
+               ins.push(TxIn {
+                       previous_output: funding_outpoint,
+                       script_sig: Script::new(),
+                       sequence: 0xffffffff,
+                       witness: Vec::new(),
+               });
+               ins
+       };
+
+       let mut txouts: Vec<(TxOut, ())> = Vec::new();
+
+       if to_counterparty_value_sat > 0 {
+               txouts.push((TxOut {
+                       script_pubkey: to_counterparty_script,
+                       value: to_counterparty_value_sat
+               }, ()));
+       }
+
+       if to_holder_value_sat > 0 {
+               txouts.push((TxOut {
+                       script_pubkey: to_holder_script,
+                       value: to_holder_value_sat
+               }, ()));
+       }
+
+       transaction_utils::sort_outputs(&mut txouts, |_, _| { cmp::Ordering::Equal }); // Ordering doesnt matter if they used our pubkey...
+
+       let mut outputs: Vec<TxOut> = Vec::new();
+       for out in txouts.drain(..) {
+               outputs.push(out.0);
+       }
+
+       Transaction {
+               version: 2,
+               lock_time: 0,
+               input: txins,
+               output: outputs,
+       }
+}
+
 /// Implements the per-commitment secret storage scheme from
 /// [BOLT 3](https://github.com/lightningnetwork/lightning-rfc/blob/dcbf8583976df087c79c3ce0b535311212e6812d/03-transactions.md#efficient-per-commitment-secret-storage).
 ///
@@ -846,7 +890,130 @@ impl BuiltCommitmentTransaction {
        }
 }
 
-/// This class tracks the per-transaction information needed to build a commitment transaction and to
+/// This class tracks the per-transaction information needed to build a closing transaction and will
+/// actually build it and sign.
+///
+/// This class can be used inside a signer implementation to generate a signature given the relevant
+/// secret key.
+pub struct ClosingTransaction {
+       to_holder_value_sat: u64,
+       to_counterparty_value_sat: u64,
+       to_holder_script: Script,
+       to_counterparty_script: Script,
+       built: Transaction,
+}
+
+impl ClosingTransaction {
+       /// Construct an object of the class
+       pub fn new(
+               to_holder_value_sat: u64,
+               to_counterparty_value_sat: u64,
+               to_holder_script: Script,
+               to_counterparty_script: Script,
+               funding_outpoint: OutPoint,
+       ) -> Self {
+               let built = build_closing_transaction(
+                       to_holder_value_sat, to_counterparty_value_sat,
+                       to_holder_script.clone(), to_counterparty_script.clone(),
+                       funding_outpoint
+               );
+               ClosingTransaction {
+                       to_holder_value_sat,
+                       to_counterparty_value_sat,
+                       to_holder_script,
+                       to_counterparty_script,
+                       built
+               }
+       }
+
+       /// Trust our pre-built transaction.
+       ///
+       /// Applies a wrapper which allows access to the transaction.
+       ///
+       /// This should only be used if you fully trust the builder of this object. It should not
+       /// be used by an external signer - instead use the verify function.
+       pub fn trust(&self) -> TrustedClosingTransaction {
+               TrustedClosingTransaction { inner: self }
+       }
+
+       /// Verify our pre-built transaction.
+       ///
+       /// Applies a wrapper which allows access to the transaction.
+       ///
+       /// An external validating signer must call this method before signing
+       /// or using the built transaction.
+       pub fn verify(&self, funding_outpoint: OutPoint) -> Result<TrustedClosingTransaction, ()> {
+               let built = build_closing_transaction(
+                       self.to_holder_value_sat, self.to_counterparty_value_sat,
+                       self.to_holder_script.clone(), self.to_counterparty_script.clone(),
+                       funding_outpoint
+               );
+               if self.built != built {
+                       return Err(())
+               }
+               Ok(TrustedClosingTransaction { inner: self })
+       }
+
+       /// The value to be sent to the holder, or zero if the output will be omitted
+       pub fn to_holder_value_sat(&self) -> u64 {
+               self.to_holder_value_sat
+       }
+
+       /// The value to be sent to the counterparty, or zero if the output will be omitted
+       pub fn to_counterparty_value_sat(&self) -> u64 {
+               self.to_counterparty_value_sat
+       }
+
+       /// The destination of the holder's output
+       pub fn to_holder_script(&self) -> &Script {
+               &self.to_holder_script
+       }
+
+       /// The destination of the counterparty's output
+       pub fn to_counterparty_script(&self) -> &Script {
+               &self.to_counterparty_script
+       }
+}
+
+/// A wrapper on ClosingTransaction indicating that the built bitcoin
+/// transaction is trusted.
+///
+/// See trust() and verify() functions on CommitmentTransaction.
+///
+/// This structure implements Deref.
+pub struct TrustedClosingTransaction<'a> {
+       inner: &'a ClosingTransaction,
+}
+
+impl<'a> Deref for TrustedClosingTransaction<'a> {
+       type Target = ClosingTransaction;
+
+       fn deref(&self) -> &Self::Target { self.inner }
+}
+
+impl<'a> TrustedClosingTransaction<'a> {
+       /// The pre-built Bitcoin commitment transaction
+       pub fn built_transaction(&self) -> &Transaction {
+               &self.inner.built
+       }
+
+       /// Get the SIGHASH_ALL sighash value of the transaction.
+       ///
+       /// This can be used to verify a signature.
+       pub fn get_sighash_all(&self, funding_redeemscript: &Script, channel_value_satoshis: u64) -> Message {
+               let sighash = &bip143::SigHashCache::new(&self.inner.built).signature_hash(0, funding_redeemscript, channel_value_satoshis, SigHashType::All)[..];
+               hash_to_message!(sighash)
+       }
+
+       /// Sign a transaction, either because we are counter-signing the counterparty's transaction or
+       /// because we are about to broadcast a holder transaction.
+       pub fn sign<T: secp256k1::Signing>(&self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>) -> Signature {
+               let sighash = self.get_sighash_all(funding_redeemscript, channel_value_satoshis);
+               secp_ctx.sign(&sighash, funding_key)
+       }
+}
+
+/// This class tracks the per-transaction information needed to build a commitment transaction and will
 /// actually build it and sign.  It is used for holder transactions that we sign only when needed
 /// and for transactions we sign for the counterparty.
 ///
@@ -1110,7 +1277,7 @@ impl CommitmentTransaction {
        /// Applies a wrapper which allows access to these fields.
        ///
        /// This should only be used if you fully trust the builder of this object.  It should not
-       ///     be used by an external signer - instead use the verify function.
+       /// be used by an external signer - instead use the verify function.
        pub fn trust(&self) -> TrustedCommitmentTransaction {
                TrustedCommitmentTransaction { inner: self }
        }
index 58384f3855e6ceddd7a5a4da646a5e1454e0c45c..bd97326ac12b7102568ff0e31f124aad62959c0f 100644 (file)
@@ -8,7 +8,7 @@
 // licenses.
 
 use bitcoin::blockdata::script::{Script,Builder};
-use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType};
+use bitcoin::blockdata::transaction::{Transaction, SigHashType};
 use bitcoin::util::bip143;
 use bitcoin::consensus::encode;
 
@@ -28,14 +28,13 @@ use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
 use ln::script::ShutdownScript;
 use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
-use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction};
 use ln::chan_utils;
 use chain::BestBlock;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER};
 use chain::transaction::{OutPoint, TransactionData};
 use chain::keysinterface::{Sign, KeysInterface};
-use util::transaction_utils;
 use util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use util::logger::Logger;
 use util::errors::APIError;
@@ -1282,63 +1281,38 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        #[inline]
-       fn build_closing_transaction(&self, proposed_total_fee_satoshis: u64, skip_remote_output: bool) -> (Transaction, u64) {
-               let txins = {
-                       let mut ins: Vec<TxIn> = Vec::new();
-                       ins.push(TxIn {
-                               previous_output: self.funding_outpoint().into_bitcoin_outpoint(),
-                               script_sig: Script::new(),
-                               sequence: 0xffffffff,
-                               witness: Vec::new(),
-                       });
-                       ins
-               };
-
+       fn build_closing_transaction(&self, proposed_total_fee_satoshis: u64, skip_remote_output: bool) -> (ClosingTransaction, u64) {
                assert!(self.pending_inbound_htlcs.is_empty());
                assert!(self.pending_outbound_htlcs.is_empty());
                assert!(self.pending_update_fee.is_none());
-               let mut txouts: Vec<(TxOut, ())> = Vec::new();
 
                let mut total_fee_satoshis = proposed_total_fee_satoshis;
-               let value_to_self: i64 = (self.value_to_self_msat as i64) / 1000 - if self.is_outbound() { total_fee_satoshis as i64 } else { 0 };
-               let value_to_remote: i64 = ((self.channel_value_satoshis * 1000 - self.value_to_self_msat) as i64 / 1000) - if self.is_outbound() { 0 } else { total_fee_satoshis as i64 };
+               let mut value_to_holder: i64 = (self.value_to_self_msat as i64) / 1000 - if self.is_outbound() { total_fee_satoshis as i64 } else { 0 };
+               let mut value_to_counterparty: i64 = ((self.channel_value_satoshis * 1000 - self.value_to_self_msat) as i64 / 1000) - if self.is_outbound() { 0 } else { total_fee_satoshis as i64 };
 
-               if value_to_self < 0 {
+               if value_to_holder < 0 {
                        assert!(self.is_outbound());
-                       total_fee_satoshis += (-value_to_self) as u64;
-               } else if value_to_remote < 0 {
+                       total_fee_satoshis += (-value_to_holder) as u64;
+               } else if value_to_counterparty < 0 {
                        assert!(!self.is_outbound());
-                       total_fee_satoshis += (-value_to_remote) as u64;
+                       total_fee_satoshis += (-value_to_counterparty) as u64;
                }
 
-               if !skip_remote_output && value_to_remote as u64 > self.holder_dust_limit_satoshis {
-                       txouts.push((TxOut {
-                               script_pubkey: self.counterparty_shutdown_scriptpubkey.clone().unwrap(),
-                               value: value_to_remote as u64
-                       }, ()));
+               if skip_remote_output || value_to_counterparty as u64 <= self.holder_dust_limit_satoshis {
+                       value_to_counterparty = 0;
                }
 
-               assert!(self.shutdown_scriptpubkey.is_some());
-               if value_to_self as u64 > self.holder_dust_limit_satoshis {
-                       txouts.push((TxOut {
-                               script_pubkey: self.get_closing_scriptpubkey(),
-                               value: value_to_self as u64
-                       }, ()));
+               if value_to_holder as u64 <= self.holder_dust_limit_satoshis {
+                       value_to_holder = 0;
                }
 
-               transaction_utils::sort_outputs(&mut txouts, |_, _| { cmp::Ordering::Equal }); // Ordering doesnt matter if they used our pubkey...
-
-               let mut outputs: Vec<TxOut> = Vec::new();
-               for out in txouts.drain(..) {
-                       outputs.push(out.0);
-               }
+               assert!(self.shutdown_scriptpubkey.is_some());
+               let holder_shutdown_script = self.get_closing_scriptpubkey();
+               let counterparty_shutdown_script = self.counterparty_shutdown_scriptpubkey.clone().unwrap();
+               let funding_outpoint = self.funding_outpoint().into_bitcoin_outpoint();
 
-               (Transaction {
-                       version: 2,
-                       lock_time: 0,
-                       input: txins,
-                       output: outputs,
-               }, total_fee_satoshis)
+               let closing_transaction = ClosingTransaction::new(value_to_holder as u64, value_to_counterparty as u64, holder_shutdown_script, counterparty_shutdown_script, funding_outpoint);
+               (closing_transaction, total_fee_satoshis)
        }
 
        fn funding_outpoint(&self) -> OutPoint {
@@ -3606,10 +3580,8 @@ impl<Signer: Sign> Channel<Signer> {
                Ok((shutdown, monitor_update, dropped_outbound_htlcs))
        }
 
-       fn build_signed_closing_transaction(&self, tx: &mut Transaction, counterparty_sig: &Signature, 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"); }
+       fn build_signed_closing_transaction(&self, closing_tx: &ClosingTransaction, counterparty_sig: &Signature, sig: &Signature) -> Transaction {
+               let mut tx = closing_tx.trust().built_transaction().clone();
 
                tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
 
@@ -3626,6 +3598,7 @@ impl<Signer: Sign> Channel<Signer> {
                tx.input[0].witness[2].push(SigHashType::All as u8);
 
                tx.input[0].witness.push(self.get_funding_redeemscript().into_bytes());
+               tx
        }
 
        pub fn closing_signed<F: Deref>(&mut self, fee_estimator: &F, msg: &msgs::ClosingSigned) -> Result<(Option<msgs::ClosingSigned>, Option<Transaction>), ChannelError>
@@ -3658,7 +3631,7 @@ impl<Signer: Sign> Channel<Signer> {
                if used_total_fee != msg.fee_satoshis {
                        return Err(ChannelError::Close(format!("Remote sent us a closing_signed with a fee other than the value they can claim. Fee in message: {}. Actual closing tx fee: {}", msg.fee_satoshis, used_total_fee)));
                }
-               let mut sighash = hash_to_message!(&bip143::SigHashCache::new(&closing_tx).signature_hash(0, &funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]);
+               let sighash = closing_tx.trust().get_sighash_all(&funding_redeemscript, self.channel_value_satoshis);
 
                match self.secp_ctx.verify(&sighash, &msg.signature, &self.get_counterparty_pubkeys().funding_pubkey) {
                        Ok(_) => {},
@@ -3666,7 +3639,7 @@ impl<Signer: Sign> Channel<Signer> {
                                // The remote end may have decided to revoke their output due to inconsistent dust
                                // limits, so check for that case by re-checking the signature here.
                                closing_tx = self.build_closing_transaction(msg.fee_satoshis, true).0;
-                               sighash = hash_to_message!(&bip143::SigHashCache::new(&closing_tx).signature_hash(0, &funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]);
+                               let sighash = closing_tx.trust().get_sighash_all(&funding_redeemscript, self.channel_value_satoshis);
                                secp_check!(self.secp_ctx.verify(&sighash, &msg.signature, self.counterparty_funding_pubkey()), "Invalid closing tx signature from peer".to_owned());
                        },
                };
@@ -3674,10 +3647,10 @@ impl<Signer: Sign> Channel<Signer> {
                assert!(self.shutdown_scriptpubkey.is_some());
                if let Some((last_fee, sig)) = self.last_sent_closing_fee {
                        if last_fee == msg.fee_satoshis {
-                               self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig);
+                               let tx = self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig);
                                self.channel_state = ChannelState::ShutdownComplete as u32;
                                self.update_time_counter += 1;
-                               return Ok((None, Some(closing_tx)));
+                               return Ok((None, Some(tx)));
                        }
                }
 
@@ -3685,20 +3658,20 @@ impl<Signer: Sign> Channel<Signer> {
 
                macro_rules! propose_fee {
                        ($new_fee: expr) => {
-                               let (mut tx, used_fee) = if $new_fee == msg.fee_satoshis {
+                               let (closing_tx, used_fee) = if $new_fee == msg.fee_satoshis {
                                        (closing_tx, $new_fee)
                                } else {
                                        self.build_closing_transaction($new_fee, false)
                                };
 
                                let sig = self.holder_signer
-                                       .sign_closing_transaction(&tx, &self.secp_ctx)
+                                       .sign_closing_transaction(&closing_tx, &self.secp_ctx)
                                        .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
 
                                let signed_tx = if $new_fee == msg.fee_satoshis {
                                        self.channel_state = ChannelState::ShutdownComplete as u32;
                                        self.update_time_counter += 1;
-                                       self.build_signed_closing_transaction(&mut tx, &msg.signature, &sig);
+                                       let tx = self.build_signed_closing_transaction(&closing_tx, &msg.signature, &sig);
                                        Some(tx)
                                } else { None };
 
index b7dfe767057836cdd59dd61c5bae32d88c362c6a..b905665c98918bbb8f1fd9306174badb51752724 100644 (file)
@@ -7,7 +7,7 @@
 // You may not use this file except in accordance with one or both of these
 // licenses.
 
-use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, HolderCommitmentTransaction, CommitmentTransaction, ChannelTransactionParameters, TrustedCommitmentTransaction};
+use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, HolderCommitmentTransaction, CommitmentTransaction, ChannelTransactionParameters, TrustedCommitmentTransaction, ClosingTransaction};
 use ln::{chan_utils, msgs};
 use chain::keysinterface::{Sign, InMemorySigner, BaseSign};
 
@@ -182,7 +182,9 @@ impl BaseSign for EnforcingSigner {
                Ok(self.inner.sign_counterparty_htlc_transaction(htlc_tx, input, amount, per_commitment_point, htlc, secp_ctx).unwrap())
        }
 
-       fn sign_closing_transaction(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+       fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+               closing_tx.verify(self.inner.funding_outpoint().into_bitcoin_outpoint())
+                       .expect("derived different closing transaction");
                Ok(self.inner.sign_closing_transaction(closing_tx, secp_ctx).unwrap())
        }