rm counterparty_claimable_outpoints from check_spend & use option<vec>

Author: G8XSU

lightning/src/chain/channelmonitor.rs

@@ -2771,8 +2771,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		if let Some(txid) = self.current_counterparty_commitment_txid {
 			if txid == confirmed_spend_txid {
 				if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
-					let htlc_outputs: Vec<HTLCOutputInCommitment> = self.counterparty_claimable_outpoints.get(&txid).unwrap_or(&vec![]).iter().map(|(htlc, _)| htlc.clone()).collect();
-					claim_htlcs!(*commitment_number, txid, htlc_outputs.as_ref());
+					claim_htlcs!(*commitment_number, txid, self.counterparty_claimable_outpoints.get(&txid));
 				} else {
 					debug_assert!(false);
 					log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
@@ -2783,8 +2782,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		if let Some(txid) = self.prev_counterparty_commitment_txid {
 			if txid == confirmed_spend_txid {
 				if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
-					let htlc_outputs: Vec<HTLCOutputInCommitment> = self.counterparty_claimable_outpoints.get(&txid).unwrap_or(&vec![]).iter().map(|(htlc, _)| htlc.clone()).collect();
-					claim_htlcs!(*commitment_number, txid, htlc_outputs.as_ref());
+					claim_htlcs!(*commitment_number, txid, self.counterparty_claimable_outpoints.get(&txid));
 				} else {
 					debug_assert!(false);
 					log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
@@ -3237,8 +3235,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// Returns packages to claim the revoked output(s), as well as additional outputs to watch and
 	/// general information about the output that is to the counterparty in the commitment
 	/// transaction.
-	fn check_spend_counterparty_transaction<L: Deref>(&mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, per_commitment_claimable_outpoints: &Vec<HTLCOutputInCommitment>,  logger: &L)
-		-> (Vec<PackageTemplate>, TransactionOutputs, CommitmentTxCounterpartyOutputInfo)
+	fn check_spend_counterparty_transaction<L: Deref>(&mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>, logger: &L)
+													  -> (Vec<PackageTemplate>, TransactionOutputs, CommitmentTxCounterpartyOutputInfo)
 	where L::Target: Logger {
 		// Most secp and related errors trying to create keys means we have no hope of constructing
 		// a spend transaction...so we return no transactions to broadcast
@@ -3247,7 +3245,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		let mut to_counterparty_output_info = None;
 
 		let commitment_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
-		let per_commitment_option = self.counterparty_claimable_outpoints.get(&commitment_txid);
 
 		macro_rules! ignore_error {
 			( $thing : expr ) => {
@@ -3281,32 +3278,34 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			}
 
 			// Then, try to find revoked htlc outputs
-			for htlc in per_commitment_claimable_outpoints {
-				if let Some(transaction_output_index) = htlc.transaction_output_index {
-					if transaction_output_index as usize >= tx.output.len() ||
+			if let Some(per_commitment_claimable_data) = per_commitment_option {
+				for (htlc, _) in per_commitment_claimable_data {
+					if let Some(transaction_output_index) = htlc.transaction_output_index {
+						if transaction_output_index as usize >= tx.output.len() ||
 							tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
-						// per_commitment_data is corrupt or our commitment signing key leaked!
-						return (claimable_outpoints, (commitment_txid, watch_outputs),
-							to_counterparty_output_info);
+							// per_commitment_data is corrupt or our commitment signing key leaked!
+							return (claimable_outpoints, (commitment_txid, watch_outputs),
+									to_counterparty_output_info);
+						}
+						let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
+						let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp), htlc.cltv_expiry, height);
+						claimable_outpoints.push(justice_package);
 					}
-					let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
-					let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp), htlc.cltv_expiry, height);
-					claimable_outpoints.push(justice_package);
 				}
 			}
 
 			// Last, track onchain revoked commitment transaction and fail backward outgoing HTLCs as payment path is broken
-			if !claimable_outpoints.is_empty() || !per_commitment_claimable_outpoints.is_empty() { // ie we're confident this is actually ours
+			if !claimable_outpoints.is_empty() || per_commitment_option.is_some() { // ie we're confident this is actually ours
 				// We're definitely a counterparty commitment transaction!
 				log_error!(logger, "Got broadcast of revoked counterparty commitment transaction, going to generate general spend tx with {} inputs", claimable_outpoints.len());
 				for (idx, outp) in tx.output.iter().enumerate() {
 					watch_outputs.push((idx as u32, outp.clone()));
 				}
 				self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
-				if let Some(per_commitment_data) = per_commitment_option {
+				if let Some(per_commitment_claimable_data) = per_commitment_option {
 					fail_unbroadcast_htlcs!(self, "revoked_counterparty", commitment_txid, tx, height,
-						block_hash, per_commitment_data.iter().map(|(htlc, htlc_source)|
+						block_hash, per_commitment_claimable_data.iter().map(|(htlc, htlc_source)|
 							(htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
 						), logger);
 				} else {
@@ -3319,7 +3318,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						block_hash, [].iter().map(|reference| *reference), logger);
 				}
 			}
-		} else if let Some(per_commitment_data) = per_commitment_option {
+		} else if let Some(per_commitment_claimable_data) = per_commitment_option {
 			// While this isn't useful yet, there is a potential race where if a counterparty
 			// revokes a state at the same time as the commitment transaction for that state is
 			// confirmed, and the watchtower receives the block before the user, the user could
@@ -3334,13 +3333,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 			log_info!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid);
 			fail_unbroadcast_htlcs!(self, "counterparty", commitment_txid, tx, height, block_hash,
-				per_commitment_data.iter().map(|(htlc, htlc_source)|
+				per_commitment_claimable_data.iter().map(|(htlc, htlc_source)|
 					(htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
 				), logger);
-			let per_commitment_htlc_outputs : Vec<HTLCOutputInCommitment>  = per_commitment_data.iter().map(|(a, _)| a.clone()).collect();
-			// let per_commitment_htlc_outputs = per_commitment_data.iter().map(|&(ref htlc, _)| htlc.clone()).collect();
 			let (htlc_claim_reqs, counterparty_output_info) =
-				self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx), per_commitment_htlc_outputs.as_ref());
+				self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx), per_commitment_option);
 			to_counterparty_output_info = counterparty_output_info;
 			for req in htlc_claim_reqs {
 				claimable_outpoints.push(req);
@@ -3351,15 +3348,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	}
 
 	/// Returns the HTLC claim package templates and the counterparty output info
-	fn get_counterparty_output_claim_info(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>, per_commitment_claimable_outpoints: &Vec<HTLCOutputInCommitment>)
-		-> (Vec<PackageTemplate>, CommitmentTxCounterpartyOutputInfo) {
+	fn get_counterparty_output_claim_info(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>)
+										  -> (Vec<PackageTemplate>, CommitmentTxCounterpartyOutputInfo) {
 		let mut claimable_outpoints = Vec::new();
 		let mut to_counterparty_output_info: CommitmentTxCounterpartyOutputInfo = None;
 
-		if per_commitment_claimable_outpoints.is_empty() || self.their_cur_per_commitment_points.is_none(){
+		if per_commitment_option.is_none() || self.their_cur_per_commitment_points.is_none() {
 			return (claimable_outpoints, to_counterparty_output_info);
 		}
 		let per_commitment_points = self.their_cur_per_commitment_points.as_ref().unwrap();
+		let per_commitment_claimable_data =  per_commitment_option.unwrap();
 
 		let per_commitment_point =
 			// If the counterparty commitment tx is the latest valid state, use their latest
@@ -3391,7 +3389,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			}
 		}
 
-		for  &ref htlc in per_commitment_claimable_outpoints.iter() {
+		for  &(ref htlc, _) in per_commitment_claimable_data.iter() {
 			if let Some(transaction_output_index) = htlc.transaction_output_index {
 				if let Some(transaction) = tx {
 					if transaction_output_index as usize >= transaction.output.len() ||
@@ -3574,14 +3572,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	/// Cancels any existing pending claims for a commitment that previously confirmed and has now
 	/// been replaced by another.
 	pub fn cancel_prev_commitment_claims<L: Deref>(
-		&mut self, logger: &L, confirmed_commitment_txid: &Txid, per_commitment_claimable_outpoints: &Vec<HTLCOutputInCommitment>
+		&mut self, logger: &L, confirmed_commitment_txid: &Txid, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>
 	) where L::Target: Logger {
 		for (counterparty_commitment_txid, _) in &self.counterparty_commitment_txn_on_chain {
 			// Cancel any pending claims for counterparty commitments we've seen confirm.
 			if counterparty_commitment_txid == confirmed_commitment_txid {
 				continue;
 			}
-			for htlc in per_commitment_claimable_outpoints {
+			for (htlc, _) in per_commitment_option.unwrap_or(&vec![]) {
 				log_trace!(logger, "Canceling claims for previously confirmed counterparty commitment {}",
 					counterparty_commitment_txid);
 				let mut outpoint = BitcoinOutPoint { txid: *counterparty_commitment_txid, vout: 0 };
@@ -3766,16 +3764,20 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				// (except for HTLC transactions for channels with anchor outputs), which is an easy
 				// way to filter out any potential non-matching txn for lazy filters.
 				let prevout = &tx.input[0].previous_output;
-				let per_commitment_claimable_outpoints: Vec<HTLCOutputInCommitment> = self.counterparty_claimable_outpoints.get(&tx.txid()).unwrap_or(&vec![]).iter().map(|(htlc, _)| htlc.clone()).collect();
+
 				if prevout.txid == self.funding_info.0.txid && prevout.vout == self.funding_info.0.index as u32 {
 					let mut balance_spendable_csv = None;
 					log_info!(logger, "Channel {} closed by funding output spend in txid {}.",
 						&self.channel_id(), txid);
 					self.funding_spend_seen = true;
 					let mut commitment_tx_to_counterparty_output = None;
-					if (tx.input[0].sequence.0 >> 8*3) as u8 == 0x80 && (tx.lock_time.to_consensus_u32() >> 8*3) as u8 == 0x20 {
+
+					let per_commitment_option = self.counterparty_claimable_outpoints.get(&tx.txid()).map(|x| (x.iter()
+						.map(|&(ref a, ref b)| (a.clone(), b.clone())).collect::<Vec<_>>()));
+
+					if (tx.input[0].sequence.0 >> 8 * 3) as u8 == 0x80 && (tx.lock_time.to_consensus_u32() >> 8 * 3) as u8 == 0x20 {
 						let (mut new_outpoints, new_outputs, counterparty_output_idx_sats) =
-							self.check_spend_counterparty_transaction(&tx, height, &block_hash, &per_commitment_claimable_outpoints, &logger);
+							self.check_spend_counterparty_transaction(&tx, height, &block_hash, per_commitment_option.as_ref(), &logger);
 						commitment_tx_to_counterparty_output = counterparty_output_idx_sats;
 						if !new_outputs.1.is_empty() {
 							watch_outputs.push(new_outputs);
@@ -3785,7 +3787,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 							if let Some((mut new_outpoints, new_outputs)) = self.check_spend_holder_transaction(&tx, height, &block_hash, &logger) {
 								#[cfg(not(fuzzing))]
 								debug_assert!(commitment_tx_to_counterparty_output.is_none(),
-									"A commitment transaction matched as both a counterparty and local commitment tx?");
+											  "A commitment transaction matched as both a counterparty and local commitment tx?");
 								if !new_outputs.1.is_empty() {
 									watch_outputs.push(new_outputs);
 								}
@@ -3794,6 +3796,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 							}
 						}
 					}
+
 					self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry {
 						txid,
 						transaction: Some((*tx).clone()),
@@ -3807,7 +3810,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					// Now that we've detected a confirmed commitment transaction, attempt to cancel
 					// pending claims for any commitments that were previously confirmed such that
 					// we don't continue claiming inputs that no longer exist.
-					self.cancel_prev_commitment_claims(&logger, &txid, &per_commitment_claimable_outpoints);
+					self.cancel_prev_commitment_claims(&logger, &txid, per_commitment_option.as_ref());
 				}
 			}
 			if tx.input.len() >= 1 {
@@ -4287,7 +4290,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				}
 			}
 			if let Some(ref htlc_outputs) = self.counterparty_claimable_outpoints.get(&input.previous_output.txid) {
-				scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, (b.as_ref().clone()).map(|boxed| &**boxed))),
+				scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, (b.as_ref()).map(|boxed| &**boxed))),
 					"counterparty commitment tx", false);
 			}