From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon, 5 Oct 2020 16:50:08 +0000 (-0700) Subject: Merge pull request #722 from TheBlueMatt/2020-09-broken-fn X-Git-Tag: v0.0.12~16 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=856648614e6d995f1bce5bcccd4b23ee8d50bcae;hp=45a558b17013531f95f40f1211647ae677e84a23;p=rust-lightning Merge pull request #722 from TheBlueMatt/2020-09-broken-fn Drop the redundant/broken `ChannelMonitor::get_monitored_outpoints` --- diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index feff3978..0a9cf699 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -631,7 +631,7 @@ pub struct ChannelMonitor { /// 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)>, + counterparty_commitment_txn_on_chain: HashMap, /// 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 ChannelMonitor { } 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 ChannelMonitor { /// /// (C-not exported) because we have no HashMap bindings pub fn get_outputs_to_watch(&self) -> &HashMap> { - &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 ChannelMonitor { // 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 ChannelMonitor { // 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 ChannelMonitor { 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 Readable for (BlockHash, ChannelMonitor for _ in 0..counterparty_commitment_txn_on_chain_len { let txid: Txid = Readable::read(reader)?; let commitment_number = ::read(reader)?.0; - let outputs_count = ::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); } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b5aa3089..6b2f50e9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3730,7 +3730,7 @@ impl