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)
1  2 
lightning/src/ln/channel.rs

index 58384f3855e6ceddd7a5a4da646a5e1454e0c45c,4b5b9c867e58c4403e6fe7f0119ba5e376117035..bd97326ac12b7102568ff0e31f124aad62959c0f
@@@ -8,7 -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 +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;
@@@ -422,18 -421,22 +421,18 @@@ pub(super) struct Channel<Signer: Sign
        monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>,
        monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
  
 -      // pending_update_fee is filled when sending and receiving update_fee
 -      // For outbound channel, feerate_per_kw is updated with the value from
 -      // pending_update_fee when revoke_and_ack is received
 +      // pending_update_fee is filled when sending and receiving update_fee.
        //
 -      // For inbound channel, feerate_per_kw is updated when it receives
 -      // commitment_signed and revoke_and_ack is generated
 -      // The pending value is kept when another pair of update_fee and commitment_signed
 -      // is received during AwaitingRemoteRevoke and relieved when the expected
 -      // revoke_and_ack is received and new commitment_signed is generated to be
 -      // sent to the funder. Otherwise, the pending value is removed when receiving
 -      // commitment_signed.
 +      // Because it follows the same commitment flow as HTLCs, `FeeUpdateState` is either `Outbound`
 +      // or matches a subset of the `InboundHTLCOutput` variants. It is then updated/used when
 +      // generating new commitment transactions with exactly the same criteria as inbound/outbound
 +      // HTLCs with similar state.
        pending_update_fee: Option<(u32, FeeUpdateState)>,
 -      // update_fee() during ChannelState::AwaitingRemoteRevoke is hold in
 -      // holdina_cell_update_fee then moved to pending_udpate_fee when revoke_and_ack
 -      // is received. holding_cell_update_fee is updated when there are additional
 -      // update_fee() during ChannelState::AwaitingRemoteRevoke.
 +      // If a `send_update_fee()` call is made with ChannelState::AwaitingRemoteRevoke set, we place
 +      // it here instead of `pending_update_fee` in the same way as we place outbound HTLC updates in
 +      // `holding_cell_htlc_updates` instead of `pending_outbound_htlcs`. It is released into
 +      // `pending_update_fee` with the same criteria as outbound HTLC updates but can be updated by
 +      // further `send_update_fee` calls, dropping the previous holding cell update entirely.
        holding_cell_update_fee: Option<u32>,
        next_holder_htlc_id: u64,
        next_counterparty_htlc_id: u64,
@@@ -1282,63 -1285,38 +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 {
                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
  
                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>
                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(_) => {},
                                // 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());
                        },
                };
                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)));
                        }
                }
  
  
                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 };
  
                // more dust balance if the feerate increases when we have several HTLCs pending
                // which are near the dust limit.
                let mut feerate_per_kw = self.feerate_per_kw;
 +              // If there's a pending update fee, use it to ensure we aren't under-estimating
 +              // potential feerate updates coming soon.
                if let Some((feerate, _)) = self.pending_update_fee {
                        feerate_per_kw = cmp::max(feerate_per_kw, feerate);
                }
@@@ -5122,10 -5097,9 +5095,10 @@@ impl<Signer: Sign> Writeable for Channe
                if self.is_outbound() {
                        self.pending_update_fee.map(|(a, _)| a).write(writer)?;
                } else if let Some((feerate, FeeUpdateState::AwaitingRemoteRevokeToAnnounce)) = self.pending_update_fee {
 -                      // As for inbound HTLCs, if the update was only announced and never committed, drop it.
                        Some(feerate).write(writer)?;
                } else {
 +                      // As for inbound HTLCs, if the update was only announced and never committed in a
 +                      // commitment_signed, drop it.
                        None::<u32>.write(writer)?;
                }
                self.holding_cell_update_fee.write(writer)?;