Merge pull request #722 from TheBlueMatt/2020-09-broken-fn
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 5 Oct 2020 16:50:08 +0000 (09:50 -0700)
committerGitHub <noreply@github.com>
Mon, 5 Oct 2020 16:50:08 +0000 (09:50 -0700)
Drop the redundant/broken `ChannelMonitor::get_monitored_outpoints`

lightning/src/chain/channelmonitor.rs
lightning/src/ln/channelmanager.rs

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 b5aa30896dd22acdf3c3b451fd6c25623fb75d65..6b2f50e9704b0a16312eb9f10134dcee0f9b6882 100644 (file)
@@ -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.