Merge pull request #734 from TheBlueMatt/2020-10-ci-no-git-conf
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 5 Oct 2020 17:27:35 +0000 (10:27 -0700)
committerGitHub <noreply@github.com>
Mon, 5 Oct 2020 17:27:35 +0000 (10:27 -0700)
Don't let CI fail to check commits b/c git isn't configured in CI

lightning/src/chain/chainmonitor.rs
lightning/src/chain/channelmonitor.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs

index d858c12673daf1e24365bd20d79558f540a9ff86..179d0edb55fd7942eda7b819d16c57f53989fbd8 100644 (file)
@@ -80,32 +80,27 @@ impl<ChanSigner: ChannelKeys, C: Deref, T: Deref, F: Deref, L: Deref> ChainMonit
        /// [`ChannelMonitor::block_connected`] for details. Any HTLCs that were resolved on chain will
        /// be returned by [`chain::Watch::release_pending_monitor_events`].
        ///
-       /// Calls back to [`chain::Filter`] if any monitor indicated new outputs to watch, returning
-       /// `true` if so. Subsequent calls must not exclude any transactions matching the new outputs
-       /// nor any in-block descendants of such transactions. It is not necessary to re-fetch the block
-       /// to obtain updated `txdata`.
+       /// Calls back to [`chain::Filter`] if any monitor indicated new outputs to watch. Subsequent
+       /// calls must not exclude any transactions matching the new outputs nor any in-block
+       /// descendants of such transactions. It is not necessary to re-fetch the block to obtain
+       /// updated `txdata`.
        ///
        /// [`ChannelMonitor::block_connected`]: ../channelmonitor/struct.ChannelMonitor.html#method.block_connected
        /// [`chain::Watch::release_pending_monitor_events`]: ../trait.Watch.html#tymethod.release_pending_monitor_events
        /// [`chain::Filter`]: ../trait.Filter.html
-       pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) -> bool {
-               let mut has_new_outputs_to_watch = false;
-               {
-                       let mut monitors = self.monitors.lock().unwrap();
-                       for monitor in monitors.values_mut() {
-                               let mut txn_outputs = monitor.block_connected(header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
-                               has_new_outputs_to_watch |= !txn_outputs.is_empty();
-
-                               if let Some(ref chain_source) = self.chain_source {
-                                       for (txid, outputs) in txn_outputs.drain(..) {
-                                               for (idx, output) in outputs.iter().enumerate() {
-                                                       chain_source.register_output(&OutPoint { txid, index: idx as u16 }, &output.script_pubkey);
-                                               }
+       pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+               let mut monitors = self.monitors.lock().unwrap();
+               for monitor in monitors.values_mut() {
+                       let mut txn_outputs = monitor.block_connected(header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
+
+                       if let Some(ref chain_source) = self.chain_source {
+                               for (txid, outputs) in txn_outputs.drain(..) {
+                                       for (idx, output) in outputs.iter().enumerate() {
+                                               chain_source.register_output(&OutPoint { txid, index: idx as u16 }, &output.script_pubkey);
                                        }
                                }
                        }
                }
-               has_new_outputs_to_watch
        }
 
        /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view
index feff3978603bb3d535c8c4c8c6e2f89a02c005a9..0a9cf6997c99b9bda1e5e6217782f7813b79de63 100644 (file)
@@ -631,7 +631,7 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        /// spending. Thus, in order to claim them via revocation key, we track all the counterparty
        /// commitment transactions which we find on-chain, mapping them to the commitment number which
        /// can be used to derive the revocation key and claim the transactions.
-       counterparty_commitment_txn_on_chain: HashMap<Txid, (u64, Vec<Script>)>,
+       counterparty_commitment_txn_on_chain: HashMap<Txid, u64>,
        /// Cache used to make pruning of payment_preimages faster.
        /// Maps payment_hash values to commitment numbers for counterparty transactions for non-revoked
        /// counterparty transactions (ie should remain pretty small).
@@ -824,13 +824,9 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
                }
 
                writer.write_all(&byte_utils::be64_to_array(self.counterparty_commitment_txn_on_chain.len() as u64))?;
-               for (ref txid, &(commitment_number, ref txouts)) in self.counterparty_commitment_txn_on_chain.iter() {
+               for (ref txid, commitment_number) in self.counterparty_commitment_txn_on_chain.iter() {
                        writer.write_all(&txid[..])?;
-                       writer.write_all(&byte_utils::be48_to_array(commitment_number))?;
-                       (txouts.len() as u64).write(writer)?;
-                       for script in txouts.iter() {
-                               script.write(writer)?;
-                       }
+                       writer.write_all(&byte_utils::be48_to_array(*commitment_number))?;
                }
 
                writer.write_all(&byte_utils::be64_to_array(self.counterparty_hash_commitment_number.len() as u64))?;
@@ -1214,23 +1210,13 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        ///
        /// (C-not exported) because we have no HashMap bindings
        pub fn get_outputs_to_watch(&self) -> &HashMap<Txid, Vec<Script>> {
-               &self.outputs_to_watch
-       }
-
-       /// Gets the sets of all outpoints which this ChannelMonitor expects to hear about spends of.
-       /// Generally useful when deserializing as during normal operation the return values of
-       /// block_connected are sufficient to ensure all relevant outpoints are being monitored (note
-       /// that the get_funding_txo outpoint and transaction must also be monitored for!).
-       ///
-       /// (C-not exported) as there is no practical way to track lifetimes of returned values.
-       pub fn get_monitored_outpoints(&self) -> Vec<(Txid, u32, &Script)> {
-               let mut res = Vec::with_capacity(self.counterparty_commitment_txn_on_chain.len() * 2);
-               for (ref txid, &(_, ref outputs)) in self.counterparty_commitment_txn_on_chain.iter() {
-                       for (idx, output) in outputs.iter().enumerate() {
-                               res.push(((*txid).clone(), idx as u32, output));
-                       }
+               // If we've detected a counterparty commitment tx on chain, we must include it in the set
+               // of outputs to watch for spends of, otherwise we're likely to lose user funds. Because
+               // its trivial to do, double-check that here.
+               for (txid, _) in self.counterparty_commitment_txn_on_chain.iter() {
+                       self.outputs_to_watch.get(txid).expect("Counterparty commitment txn which have been broadcast should have outputs registered");
                }
-               res
+               &self.outputs_to_watch
        }
 
        /// Get the list of HTLCs who's status has been updated on chain. This should be called by
@@ -1334,7 +1320,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                // We're definitely a counterparty commitment transaction!
                                log_trace!(logger, "Got broadcast of revoked counterparty commitment transaction, going to generate general spend tx with {} inputs", claimable_outpoints.len());
                                watch_outputs.append(&mut tx.output.clone());
-                               self.counterparty_commitment_txn_on_chain.insert(commitment_txid, (commitment_number, tx.output.iter().map(|output| { output.script_pubkey.clone() }).collect()));
+                               self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
                                macro_rules! check_htlc_fails {
                                        ($txid: expr, $commitment_tx: expr) => {
@@ -1381,7 +1367,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        // not being generated by the above conditional. Thus, to be safe, we go ahead and
                        // insert it here.
                        watch_outputs.append(&mut tx.output.clone());
-                       self.counterparty_commitment_txn_on_chain.insert(commitment_txid, (commitment_number, tx.output.iter().map(|output| { output.script_pubkey.clone() }).collect()));
+                       self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
                        log_trace!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid);
 
@@ -1719,7 +1705,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                claimable_outpoints.append(&mut new_outpoints);
                                        }
                                } else {
-                                       if let Some(&(commitment_number, _)) = self.counterparty_commitment_txn_on_chain.get(&prevout.txid) {
+                                       if let Some(&commitment_number) = self.counterparty_commitment_txn_on_chain.get(&prevout.txid) {
                                                let (mut new_outpoints, new_outputs_option) = self.check_spend_counterparty_htlc(&tx, commitment_number, height, &logger);
                                                claimable_outpoints.append(&mut new_outpoints);
                                                if let Some(new_outputs) = new_outputs_option {
@@ -2211,12 +2197,7 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for (BlockHash, ChannelMonitor
                for _ in 0..counterparty_commitment_txn_on_chain_len {
                        let txid: Txid = Readable::read(reader)?;
                        let commitment_number = <U48 as Readable>::read(reader)?.0;
-                       let outputs_count = <u64 as Readable>::read(reader)?;
-                       let mut outputs = Vec::with_capacity(cmp::min(outputs_count as usize, MAX_ALLOC_SIZE / 8));
-                       for _ in 0..outputs_count {
-                               outputs.push(Readable::read(reader)?);
-                       }
-                       if let Some(_) = counterparty_commitment_txn_on_chain.insert(txid, (commitment_number, outputs)) {
+                       if let Some(_) = counterparty_commitment_txn_on_chain.insert(txid, commitment_number) {
                                return Err(DecodeError::InvalidValue);
                        }
                }
index 21917bb72967e0f17f1b4cd110a9a9cef558b734..934f155db2cc6afac33c3afd2f482d04bc9b0236 100644 (file)
@@ -1054,8 +1054,30 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        #[inline]
-       fn get_closing_transaction_weight(a_scriptpubkey: &Script, b_scriptpubkey: &Script) -> u64 {
-               (4 + 1 + 36 + 4 + 1 + 1 + 2*(8+1) + 4 + a_scriptpubkey.len() as u64 + b_scriptpubkey.len() as u64)*4 + 2 + 1 + 1 + 2*(1 + 72)
+       fn get_closing_transaction_weight(&self, a_scriptpubkey: Option<&Script>, b_scriptpubkey: Option<&Script>) -> u64 {
+               let mut ret =
+               (4 +                                           // version
+                1 +                                           // input count
+                36 +                                          // prevout
+                1 +                                           // script length (0)
+                4 +                                           // sequence
+                1 +                                           // output count
+                4                                             // lock time
+                )*4 +                                         // * 4 for non-witness parts
+               2 +                                            // witness marker and flag
+               1 +                                            // witness element count
+               4 +                                            // 4 element lengths (2 sigs, multisig dummy, and witness script)
+               self.get_funding_redeemscript().len() as u64 + // funding witness script
+               2*(1 + 71);                                    // two signatures + sighash type flags
+               if let Some(spk) = a_scriptpubkey {
+                       ret += ((8+1) +                            // output values and script length
+                               spk.len() as u64) * 4;                 // scriptpubkey and witness multiplier
+               }
+               if let Some(spk) = b_scriptpubkey {
+                       ret += ((8+1) +                            // output values and script length
+                               spk.len() as u64) * 4;                 // scriptpubkey and witness multiplier
+               }
+               ret
        }
 
        #[inline]
@@ -2880,13 +2902,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if self.feerate_per_kw > proposed_feerate {
                        proposed_feerate = self.feerate_per_kw;
                }
-               let tx_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.counterparty_shutdown_scriptpubkey.as_ref().unwrap());
+               let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap()));
                let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000;
 
                let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
                let sig = self.holder_keys
                        .sign_closing_transaction(&closing_tx, &self.secp_ctx)
                        .ok();
+               assert!(closing_tx.get_weight() as u64 <= tx_weight);
                if sig.is_none() { return None; }
 
                self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis, sig.clone().unwrap()));
@@ -3007,7 +3030,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() {
                        return Err(ChannelError::Close("Remote end sent us a closing_signed while there were still pending HTLCs".to_owned()));
                }
-               if msg.fee_satoshis > 21000000 * 10000000 { //this is required to stop potential overflow in build_closing_transaction
+               if msg.fee_satoshis > 21_000_000 * 1_0000_0000 { //this is required to stop potential overflow in build_closing_transaction
                        return Err(ChannelError::Close("Remote tried to send us a closing tx with > 21 million BTC fee".to_owned()));
                }
 
@@ -3031,9 +3054,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        },
                };
 
+               let closing_tx_max_weight = self.get_closing_transaction_weight(
+                       if let Some(oup) = closing_tx.output.get(0) { Some(&oup.script_pubkey) } else { None },
+                       if let Some(oup) = closing_tx.output.get(1) { Some(&oup.script_pubkey) } else { None });
                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);
+                               assert!(closing_tx.get_weight() as u64 <= closing_tx_max_weight);
+                               debug_assert!(closing_tx.get_weight() as u64 >= closing_tx_max_weight - 2);
                                self.channel_state = ChannelState::ShutdownComplete as u32;
                                self.update_time_counter += 1;
                                return Ok((None, Some(closing_tx)));
@@ -3042,11 +3070,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                macro_rules! propose_new_feerate {
                        ($new_feerate: expr) => {
-                               let closing_tx_max_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.counterparty_shutdown_scriptpubkey.as_ref().unwrap());
-                               let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * closing_tx_max_weight / 1000, false);
+                               let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap()));
+                               let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * tx_weight / 1000, false);
                                let sig = self.holder_keys
                                        .sign_closing_transaction(&closing_tx, &self.secp_ctx)
                                        .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
+                               assert!(closing_tx.get_weight() as u64 <= tx_weight);
                                self.last_sent_closing_fee = Some(($new_feerate, used_total_fee, sig.clone()));
                                return Ok((Some(msgs::ClosingSigned {
                                        channel_id: self.channel_id,
@@ -3056,10 +3085,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                }
 
-               let proposed_sat_per_kw = msg.fee_satoshis  * 1000 / closing_tx.get_weight() as u64;
+               let mut min_feerate = 253;
                if self.channel_outbound {
                        let max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
-                       if (proposed_sat_per_kw as u32) > max_feerate {
+                       if (msg.fee_satoshis as u64) > max_feerate as u64 * closing_tx_max_weight / 1000 {
                                if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
                                        if max_feerate <= last_feerate {
                                                return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something higher ({}) than our Normal feerate ({})", last_feerate, max_feerate)));
@@ -3068,21 +3097,23 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                propose_new_feerate!(max_feerate);
                        }
                } else {
-                       let min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-                       if (proposed_sat_per_kw as u32) < min_feerate {
-                               if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
-                                       if min_feerate >= last_feerate {
-                                               return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something lower ({}) than our Background feerate ({}).", last_feerate, min_feerate)));
-                                       }
+                       min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
+               }
+               if (msg.fee_satoshis as u64) < min_feerate as u64 * closing_tx_max_weight / 1000 {
+                       if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
+                               if min_feerate >= last_feerate {
+                                       return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something lower ({}) than our Background feerate ({}).", last_feerate, min_feerate)));
                                }
-                               propose_new_feerate!(min_feerate);
                        }
+                       propose_new_feerate!(min_feerate);
                }
 
                let sig = self.holder_keys
                        .sign_closing_transaction(&closing_tx, &self.secp_ctx)
                        .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
                self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig);
+               assert!(closing_tx.get_weight() as u64 <= closing_tx_max_weight);
+               debug_assert!(closing_tx.get_weight() as u64 >= closing_tx_max_weight - 2);
 
                self.channel_state = ChannelState::ShutdownComplete as u32;
                self.update_time_counter += 1;
index 353de9240cc3007a010b73d01f4853e9a2c6d909..6b2f50e9704b0a16312eb9f10134dcee0f9b6882 100644 (file)
@@ -775,7 +775,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                let channel = Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, channel_value_satoshis, push_msat, user_id, config)?;
                let res = channel.get_open_channel(self.genesis_hash.clone());
 
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let mut channel_state = self.channel_state.lock().unwrap();
                match channel_state.by_id.entry(channel.channel_id()) {
                        hash_map::Entry::Occupied(_) => {
@@ -847,7 +847,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        ///
        /// May generate a SendShutdown message event on success, which should be relayed.
        pub fn close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                let (mut failed_htlcs, chan_option) = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
@@ -907,7 +907,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        /// Force closes a channel, immediately broadcasting the latest local commitment transaction to
        /// the chain and rejecting new HTLCs on the given channel.
        pub fn force_close_channel(&self, channel_id: &[u8; 32]) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                let mut chan = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
@@ -1255,7 +1255,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                }
                let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash);
 
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                let err: Result<(), _> = loop {
                        let mut channel_lock = self.channel_state.lock().unwrap();
@@ -1423,7 +1423,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        /// May panic if the funding_txo is duplicative with some other channel (note that this should
        /// be trivially prevented by using unique funding transaction keys per-channel).
        pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_txo: OutPoint) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                let (chan, msg) = {
                        let (res, chan) = match self.channel_state.lock().unwrap().by_id.remove(temporary_channel_id) {
@@ -1506,7 +1506,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        ///
        /// Panics if addresses is absurdly large (more than 500).
        pub fn broadcast_node_announcement(&self, rgb: [u8; 3], alias: [u8; 32], addresses: Vec<NetAddress>) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                if addresses.len() > 500 {
                        panic!("More than half the message size was taken up by public addresses!");
@@ -1536,7 +1536,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        /// Should only really ever be called in response to a PendingHTLCsForwardable event.
        /// Will likely generate further events.
        pub fn process_pending_htlc_forwards(&self) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                let mut new_events = Vec::new();
                let mut failed_forwards = Vec::new();
@@ -1789,7 +1789,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        ///
        /// This method handles all the details, and must be called roughly once per minute.
        pub fn timer_chan_freshness_every_min(&self) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let mut channel_state_lock = self.channel_state.lock().unwrap();
                let channel_state = &mut *channel_state_lock;
                for (_, chan) in channel_state.by_id.iter_mut() {
@@ -1814,7 +1814,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        /// Returns false if no payment was found to fail backwards, true if the process of failing the
        /// HTLC backwards has been started.
        pub fn fail_htlc_backwards(&self, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>) -> bool {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                let mut channel_state = Some(self.channel_state.lock().unwrap());
                let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(&(*payment_hash, *payment_secret));
@@ -1993,7 +1993,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        pub fn claim_funds(&self, payment_preimage: PaymentPreimage, payment_secret: &Option<PaymentSecret>, expected_amount: u64) -> bool {
                let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                let mut channel_state = Some(self.channel_state.lock().unwrap());
                let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(&(payment_hash, *payment_secret));
@@ -2178,7 +2178,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        ///  4) once all remote copies are updated, you call this function with the update_id that
        ///     completed, and once it is the latest the Channel will be re-enabled.
        pub fn channel_monitor_updated(&self, funding_txo: &OutPoint, highest_applied_update_id: u64) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                let mut close_results = Vec::new();
                let mut htlc_forwards = Vec::new();
@@ -2922,7 +2922,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        /// (C-not exported) Cause its doc(hidden) anyway
        #[doc(hidden)]
        pub fn update_fee(&self, channel_id: [u8;32], feerate_per_kw: u32) -> Result<(), APIError> {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let counterparty_node_id;
                let err: Result<(), _> = loop {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
@@ -3062,7 +3062,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
                let header_hash = header.block_hash();
                log_trace!(self.logger, "Block {} at height {} connected", header_hash, height);
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let mut failed_channels = Vec::new();
                let mut timed_out_htlcs = Vec::new();
                {
@@ -3175,7 +3175,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
        /// If necessary, the channel may be force-closed without letting the counterparty participate
        /// in the shutdown.
        pub fn block_disconnected(&self, header: &BlockHeader) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let mut failed_channels = Vec::new();
                {
                        let mut channel_lock = self.channel_state.lock().unwrap();
@@ -3216,87 +3216,87 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
         L::Target: Logger,
 {
        fn handle_open_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_open_channel(counterparty_node_id, their_features, msg), *counterparty_node_id);
        }
 
        fn handle_accept_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::AcceptChannel) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_accept_channel(counterparty_node_id, their_features, msg), *counterparty_node_id);
        }
 
        fn handle_funding_created(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingCreated) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_funding_created(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_funding_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingSigned) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_funding_signed(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_funding_locked(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingLocked) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_funding_locked(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_shutdown(&self, counterparty_node_id: &PublicKey, msg: &msgs::Shutdown) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_shutdown(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_closing_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::ClosingSigned) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_closing_signed(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_update_add_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateAddHTLC) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_update_add_htlc(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_update_fulfill_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_update_fulfill_htlc(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_update_fail_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_update_fail_htlc(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_update_fail_malformed_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFailMalformedHTLC) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_update_fail_malformed_htlc(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_commitment_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::CommitmentSigned) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_commitment_signed(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_revoke_and_ack(&self, counterparty_node_id: &PublicKey, msg: &msgs::RevokeAndACK) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_revoke_and_ack(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_update_fee(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFee) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_update_fee(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_announcement_signatures(&self, counterparty_node_id: &PublicKey, msg: &msgs::AnnouncementSignatures) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_announcement_signatures(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn handle_channel_reestablish(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReestablish) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let _ = handle_error!(self, self.internal_channel_reestablish(counterparty_node_id, msg), *counterparty_node_id);
        }
 
        fn peer_disconnected(&self, counterparty_node_id: &PublicKey, no_connection_possible: bool) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
                let mut failed_channels = Vec::new();
                let mut failed_payments = Vec::new();
                let mut no_channels_remain = true;
@@ -3387,7 +3387,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
        fn peer_connected(&self, counterparty_node_id: &PublicKey, init_msg: &msgs::Init) {
                log_debug!(self.logger, "Generating channel_reestablish events for {}", log_pubkey!(counterparty_node_id));
 
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                {
                        let mut peer_state_lock = self.per_peer_state.write().unwrap();
@@ -3427,7 +3427,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
        }
 
        fn handle_error(&self, counterparty_node_id: &PublicKey, msg: &msgs::ErrorMessage) {
-               let _ = self.total_consistency_lock.read().unwrap();
+               let _consistency_lock = self.total_consistency_lock.read().unwrap();
 
                if msg.channel_id == [0; 32] {
                        for chan in self.list_channels() {
@@ -3659,7 +3659,7 @@ impl<ChanSigner: ChannelKeys + Writeable, M: Deref, T: Deref, K: Deref, F: Deref
         L::Target: Logger,
 {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-               let _ = self.total_consistency_lock.write().unwrap();
+               let _consistency_lock = self.total_consistency_lock.write().unwrap();
 
                writer.write_all(&[SERIALIZATION_VERSION; 1])?;
                writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
@@ -3730,7 +3730,7 @@ impl<ChanSigner: ChannelKeys + Writeable, M: Deref, T: Deref, K: Deref, F: Deref
 ///    This may result in closing some Channels if the ChannelMonitor is newer than the stored
 ///    ChannelManager state to ensure no loss of funds. Thus, transactions may be broadcasted.
 /// 3) Register all relevant ChannelMonitor outpoints with your chain watch mechanism using
-///    ChannelMonitor::get_monitored_outpoints and ChannelMonitor::get_funding_txo().
+///    ChannelMonitor::get_outputs_to_watch() and ChannelMonitor::get_funding_txo().
 /// 4) Reconnect blocks on your ChannelMonitors.
 /// 5) Move the ChannelMonitors into your local chain::Watch.
 /// 6) Disconnect/connect blocks on the ChannelManager.
index 4c3db606d9f96b56c773898268aab74279e9390e..7d54b6a92166d9c3378e4f0bf3431d157c28d37d 100644 (file)
@@ -81,7 +81,7 @@ pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32, he
 
 pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, height: u32) {
        let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
-       while node.chain_monitor.chain_monitor.block_connected(&block.header, &txdata, height) {}
+       node.chain_monitor.chain_monitor.block_connected(&block.header, &txdata, height);
        node.node.block_connected(&block.header, &txdata, height);
 }
 
@@ -505,14 +505,26 @@ pub fn create_announced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a
 macro_rules! check_spends {
        ($tx: expr, $($spends_txn: expr),*) => {
                {
-                       $tx.verify(|out_point| {
+                       let get_output = |out_point: &bitcoin::blockdata::transaction::OutPoint| {
                                $(
                                        if out_point.txid == $spends_txn.txid() {
                                                return $spends_txn.output.get(out_point.vout as usize).cloned()
                                        }
                                )*
                                None
-                       }).unwrap();
+                       };
+                       let mut total_value_in = 0;
+                       for input in $tx.input.iter() {
+                               total_value_in += get_output(&input.previous_output).unwrap().value;
+                       }
+                       let mut total_value_out = 0;
+                       for output in $tx.output.iter() {
+                               total_value_out += output.value;
+                       }
+                       let min_fee = ($tx.get_weight() as u64 + 3) / 4; // One sat per vbyte (ie per weight/4, rounded up)
+                       // Input amount - output amount = fee, so check that out + min_fee is smaller than input
+                       assert!(total_value_out + min_fee <= total_value_in);
+                       $tx.verify(get_output).unwrap();
                }
        }
 }
index 59b0d071f17c6f7728f8a46ee6b36e90bfa9bcfe..c94b220ce8702877b80a9912e39d9fbf9aa0b3f3 100644 (file)
@@ -4708,6 +4708,7 @@ macro_rules! check_spendable_outputs {
                                                                                input: vec![input],
                                                                                output: vec![outp],
                                                                        };
+                                                                       spend_tx.output[0].value -= (spend_tx.get_weight() + 2 + 1 + 73 + 35 + 3) as u64 / 4; // (Max weight + 3 (to round up)) / 4
                                                                        let secp_ctx = Secp256k1::new();
                                                                        let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
                                                                        let remotepubkey = keys.pubkeys().payment_point;
@@ -4742,6 +4743,7 @@ macro_rules! check_spendable_outputs {
 
                                                                                let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
                                                                                let witness_script = chan_utils::get_revokeable_redeemscript(revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
+                                                                               spend_tx.output[0].value -= (spend_tx.get_weight() + 2 + 1 + 73 + 1 + witness_script.len() + 1 + 3) as u64 / 4; // (Max weight + 3 (to round up)) / 4
                                                                                let sighash = Message::from_slice(&bip143::SigHashCache::new(&spend_tx).signature_hash(0, &witness_script, output.value, SigHashType::All)[..]).unwrap();
                                                                                let local_delayedsig = secp_ctx.sign(&sighash, &delayed_payment_key);
                                                                                spend_tx.input[0].witness.push(local_delayedsig.serialize_der().to_vec());
@@ -4769,6 +4771,7 @@ macro_rules! check_spendable_outputs {
                                                                                input: vec![input],
                                                                                output: vec![outp.clone()],
                                                                        };
+                                                                       spend_tx.output[0].value -= (spend_tx.get_weight() + 2 + 1 + 73 + 35 + 3) as u64 / 4; // (Max weight + 3 (to round up)) / 4
                                                                        let secret = {
                                                                                match ExtendedPrivKey::new_master(Network::Testnet, &$node.node_seed) {
                                                                                        Ok(master_key) => {
@@ -4852,8 +4855,7 @@ fn test_claim_on_remote_sizeable_push_msat() {
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1, 1, true, header.block_hash());
 
        let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
-       assert_eq!(spend_txn.len(), 2);
-       assert_eq!(spend_txn[0], spend_txn[1]);
+       assert_eq!(spend_txn.len(), 1);
        check_spends!(spend_txn[0], node_txn[0]);
 }
 
@@ -4885,10 +4887,9 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
        connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1, 1, true, header.block_hash());
 
        let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
-       assert_eq!(spend_txn.len(), 3);
-       assert_eq!(spend_txn[0], spend_txn[1]); // to_remote output on revoked remote commitment_tx
-       check_spends!(spend_txn[0], revoked_local_txn[0]);
-       check_spends!(spend_txn[2], node_txn[0]);
+       assert_eq!(spend_txn.len(), 2);
+       check_spends!(spend_txn[0], revoked_local_txn[0]); // to_remote output on revoked remote commitment_tx
+       check_spends!(spend_txn[1], node_txn[0]);
 }
 
 #[test]
@@ -4983,8 +4984,8 @@ fn test_static_spendable_outputs_timeout_tx() {
        expect_payment_failed!(nodes[1], our_payment_hash, true);
 
        let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000);
-       assert_eq!(spend_txn.len(), 3); // SpendableOutput: remote_commitment_tx.to_remote (*2), timeout_tx.output (*1)
-       check_spends!(spend_txn[2], node_txn[0].clone());
+       assert_eq!(spend_txn.len(), 2); // SpendableOutput: remote_commitment_tx.to_remote, timeout_tx.output
+       check_spends!(spend_txn[1], node_txn[0]);
 }
 
 #[test]
@@ -5159,12 +5160,11 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
 
        // Check A's ChannelMonitor was able to generate the right spendable output descriptor
        let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000);
-       assert_eq!(spend_txn.len(), 3); // Duplicated SpendableOutput due to block rescan after revoked htlc output tracking
-       assert_eq!(spend_txn[0], spend_txn[1]);
+       assert_eq!(spend_txn.len(), 2);
        assert_eq!(spend_txn[0].input.len(), 1);
        check_spends!(spend_txn[0], revoked_local_txn[0]); // spending to_remote output from revoked local tx
        assert_ne!(spend_txn[0].input[0].previous_output, revoked_htlc_txn[0].input[0].previous_output);
-       check_spends!(spend_txn[2], node_txn[1]); // spending justice tx output on the htlc success tx
+       check_spends!(spend_txn[1], node_txn[1]); // spending justice tx output on the htlc success tx
 }
 
 #[test]
@@ -5228,7 +5228,7 @@ fn test_onchain_to_onchain_claim() {
                assert_eq!(b_txn[2].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
                assert!(b_txn[2].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
                assert_ne!(b_txn[2].lock_time, 0); // Timeout tx
-               check_spends!(b_txn[0], c_txn[1]); // timeout tx on C remote commitment tx, issued by ChannelMonitor, * 2 due to block rescan
+               check_spends!(b_txn[0], c_txn[1]); // timeout tx on C remote commitment tx, issued by ChannelMonitor
                assert_eq!(b_txn[0].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
                assert!(b_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
                assert_ne!(b_txn[2].lock_time, 0); // Timeout tx
@@ -5726,10 +5726,9 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 
        // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
        let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000);
-       assert_eq!(spend_txn.len(), 3);
-       assert_eq!(spend_txn[0], spend_txn[1]);
+       assert_eq!(spend_txn.len(), 2);
        check_spends!(spend_txn[0], local_txn[0]);
-       check_spends!(spend_txn[2], htlc_timeout);
+       check_spends!(spend_txn[1], htlc_timeout);
 }
 
 #[test]
@@ -5797,10 +5796,9 @@ fn test_key_derivation_params() {
        // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
        let new_keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
        let spend_txn = check_spendable_outputs!(nodes[0], 1, new_keys_manager, 100000);
-       assert_eq!(spend_txn.len(), 3);
-       assert_eq!(spend_txn[0], spend_txn[1]);
+       assert_eq!(spend_txn.len(), 2);
        check_spends!(spend_txn[0], local_txn_1[0]);
-       check_spends!(spend_txn[2], htlc_timeout);
+       check_spends!(spend_txn[1], htlc_timeout);
 }
 
 #[test]