Merge pull request #1072 from TheBlueMatt/2021-09-tighter-max_fee-constant
[rust-lightning] / lightning / src / ln / channel.rs
index 767cf130db162f70c3e391eb2365120a89a3734a..8ed7072dd8aafe1ec79871255676084f797cd089 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;
@@ -422,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,
@@ -1286,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 {
@@ -3610,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
 
@@ -3630,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>
@@ -3662,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(_) => {},
@@ -3670,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());
                        },
                };
@@ -3678,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)));
                        }
                }
 
@@ -3689,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 };
 
@@ -3877,6 +3846,8 @@ impl<Signer: Sign> Channel<Signer> {
                // 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);
                }
@@ -5125,9 +5096,10 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                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)?;