f - cache compute_txid

Author: jkczyz

lightning/src/ln/channel.rs

@@ -1861,6 +1861,30 @@ struct PendingSplice {
 	received_funding_txid: Option<Txid>,
 }
 
+/// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`].
+struct ConfirmedTransaction<'a> {
+	tx: &'a Transaction,
+	txid: Option<Txid>,
+}
+
+impl<'a> ConfirmedTransaction<'a> {
+	/// Returns the underlying [`Transaction`].
+	pub fn tx(&self) -> &'a Transaction {
+		self.tx
+	}
+
+	/// Returns the [`Txid`], computing and caching it if necessary.
+	pub fn txid(&mut self) -> Txid {
+		*self.txid.get_or_insert_with(|| self.tx.compute_txid())
+	}
+}
+
+impl<'a> From<&'a Transaction> for ConfirmedTransaction<'a> {
+	fn from(tx: &'a Transaction) -> Self {
+		ConfirmedTransaction { tx, txid: None }
+	}
+}
+
 /// Contains everything about the channel including state, and various flags.
 pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
 	config: LegacyChannelConfig,
@@ -4929,83 +4953,83 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		return true;
 	}
 
-	fn check_for_funding_tx<'a, L: Deref>(
+	fn check_for_funding_tx<L: Deref>(
 		&mut self, funding: &mut FundingScope, block_hash: &BlockHash, height: u32,
-		txdata: &'a TransactionData, logger: &L,
-	) -> Result<Option<&'a Transaction>, ClosureReason>
+		index_in_block: usize, tx: &mut ConfirmedTransaction, logger: &L,
+	) -> Result<bool, ClosureReason>
 	where
 		L::Target: Logger
 	{
 		let funding_txo = match funding.get_funding_txo() {
 			Some(funding_txo) => funding_txo,
 			None => {
 				debug_assert!(false);
-				return Ok(None);
+				return Ok(false);
 			},
 		};
 
-		let mut confirmed_funding_tx = None;
-		for &(index_in_block, tx) in txdata.iter() {
-			// Check if the transaction is the expected funding transaction, and if it is,
-			// check that it pays the right amount to the right script.
-			if funding.funding_tx_confirmation_height == 0 {
-				if tx.compute_txid() == funding_txo.txid {
-					let txo_idx = funding_txo.index as usize;
-					if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != funding.get_funding_redeemscript().to_p2wsh() ||
-							tx.output[txo_idx].value.to_sat() != funding.get_value_satoshis() {
-						if funding.is_outbound() {
-							// If we generated the funding transaction and it doesn't match what it
-							// should, the client is really broken and we should just panic and
-							// tell them off. That said, because hash collisions happen with high
-							// probability in fuzzing mode, if we're fuzzing we just close the
-							// channel and move on.
-							#[cfg(not(fuzzing))]
-							panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
-						}
-						self.update_time_counter += 1;
-						let err_reason = "funding tx had wrong script/value or output index";
-						return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
-					} else {
-						if funding.is_outbound() {
-							if !tx.is_coinbase() {
-								for input in tx.input.iter() {
-									if input.witness.is_empty() {
-										// We generated a malleable funding transaction, implying we've
-										// just exposed ourselves to funds loss to our counterparty.
-										#[cfg(not(fuzzing))]
-										panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
-									}
+		let mut is_confirmed_funding_tx = false;
+
+		// Check if the transaction is the expected funding transaction, and if it is,
+		// check that it pays the right amount to the right script.
+		if funding.funding_tx_confirmation_height == 0 {
+			if tx.txid() == funding_txo.txid {
+				let tx = tx.tx();
+				let txo_idx = funding_txo.index as usize;
+				if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != funding.get_funding_redeemscript().to_p2wsh() ||
+						tx.output[txo_idx].value.to_sat() != funding.get_value_satoshis() {
+					if funding.is_outbound() {
+						// If we generated the funding transaction and it doesn't match what it
+						// should, the client is really broken and we should just panic and
+						// tell them off. That said, because hash collisions happen with high
+						// probability in fuzzing mode, if we're fuzzing we just close the
+						// channel and move on.
+						#[cfg(not(fuzzing))]
+						panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
+					}
+					self.update_time_counter += 1;
+					let err_reason = "funding tx had wrong script/value or output index";
+					return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
+				} else {
+					if funding.is_outbound() {
+						if !tx.is_coinbase() {
+							for input in tx.input.iter() {
+								if input.witness.is_empty() {
+									// We generated a malleable funding transaction, implying we've
+									// just exposed ourselves to funds loss to our counterparty.
+									#[cfg(not(fuzzing))]
+									panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
 								}
 							}
 						}
+					}
 
-						// The acceptor of v1-established channels doesn't have the funding
-						// transaction until it is seen on chain. Set it so that minimum_depth
-						// checks can tell if the coinbase transaction was used.
-						if funding.funding_transaction.is_none() {
-							funding.funding_transaction = Some(tx.clone());
-						}
-
-						funding.funding_tx_confirmation_height = height;
-						funding.funding_tx_confirmed_in = Some(*block_hash);
-						funding.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) {
-							Ok(scid) => Some(scid),
-							Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"),
-						};
+					// The acceptor of v1-established channels doesn't have the funding
+					// transaction until it is seen on chain. Set it so that minimum_depth
+					// checks can tell if the coinbase transaction was used.
+					if funding.funding_transaction.is_none() {
+						funding.funding_transaction = Some(tx.clone());
 					}
 
-					confirmed_funding_tx = Some(tx);
+					funding.funding_tx_confirmation_height = height;
+					funding.funding_tx_confirmed_in = Some(*block_hash);
+					funding.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) {
+						Ok(scid) => Some(scid),
+						Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"),
+					};
 				}
+
+				is_confirmed_funding_tx = true;
 			}
-			for inp in tx.input.iter() {
-				if inp.previous_output == funding_txo.into_bitcoin_outpoint() {
-					log_info!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.compute_txid(), inp.previous_output.txid, inp.previous_output.vout, &self.channel_id());
-					return Err(ClosureReason::CommitmentTxConfirmed);
-				}
+		}
+		for inp in tx.tx().input.iter() {
+			if inp.previous_output == funding_txo.into_bitcoin_outpoint() {
+				log_info!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, &self.channel_id());
+				return Err(ClosureReason::CommitmentTxConfirmed);
 			}
 		}
 
-		Ok(confirmed_funding_tx)
+		Ok(is_confirmed_funding_tx)
 	}
 }
 
@@ -8327,59 +8351,63 @@ impl<SP: Deref> FundedChannel<SP> where
 		NS::Target: NodeSigner,
 		L::Target: Logger
 	{
-		// If we allow 1-conf funding, we may need to check for channel_ready or splice_locked here
-		// and send it immediately instead of waiting for a best_block_updated call (which may have
-		// already happened for this block).
-		let confirmed_funding_tx = self.context.check_for_funding_tx(
-			&mut self.funding, block_hash, height, txdata, logger,
-		)?;
+		for &(index_in_block, tx) in txdata.iter() {
+			let mut confirmed_tx = ConfirmedTransaction::from(tx);
 
-		if let Some(funding_tx) = confirmed_funding_tx {
-			if let Some(channel_ready) = self.check_get_channel_ready(height, logger) {
-				log_info!(logger, "Sending a channel_ready to our peer for channel {}", &self.context.channel_id);
-				let announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger);
-				return Ok((Some(FundingConfirmedMessage::Establishment(channel_ready)), announcement_sigs));
-			}
-		}
+			// If we allow 1-conf funding, we may need to check for channel_ready or splice_locked here
+			// and send it immediately instead of waiting for a best_block_updated call (which may have
+			// already happened for this block).
+			let is_confirmed_funding_tx = self.context.check_for_funding_tx(
+				&mut self.funding, block_hash, height, index_in_block, &mut confirmed_tx, logger,
+			)?;
 
-		#[cfg(splicing)]
-		let mut confirmed_funding = None;
-		#[cfg(splicing)]
-		for funding in self.pending_funding.iter_mut() {
-			if self.context.check_for_funding_tx(funding, block_hash, height, txdata, logger)?.is_some() {
-				if confirmed_funding.is_some() {
-					let err_reason = "splice tx of another pending funding already confirmed";
-					return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
+			if is_confirmed_funding_tx {
+				if let Some(channel_ready) = self.check_get_channel_ready(height, logger) {
+					log_info!(logger, "Sending a channel_ready to our peer for channel {}", &self.context.channel_id);
+					let announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger);
+					return Ok((Some(FundingConfirmedMessage::Establishment(channel_ready)), announcement_sigs));
 				}
+			}
+
+			#[cfg(splicing)]
+			let mut confirmed_funding = None;
+			#[cfg(splicing)]
+			for funding in self.pending_funding.iter_mut() {
+				if self.context.check_for_funding_tx(funding, block_hash, height, index_in_block, &mut confirmed_tx, logger)? {
+					if confirmed_funding.is_some() {
+						let err_reason = "splice tx of another pending funding already confirmed";
+						return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
+					}
 
-				confirmed_funding = Some(funding);
+					confirmed_funding = Some(funding);
+				}
 			}
-		}
 
-		#[cfg(splicing)]
-		if let Some(funding) = confirmed_funding {
-			let pending_splice = match self.pending_splice.as_mut() {
-				Some(pending_splice) => pending_splice,
-				None => {
-					// TODO: Move pending_funding into pending_splice?
-					debug_assert!(false);
-					// TODO: Error instead?
-					return Ok((None, None));
-				},
-			};
+			#[cfg(splicing)]
+			if let Some(funding) = confirmed_funding {
+				let pending_splice = match self.pending_splice.as_mut() {
+					Some(pending_splice) => pending_splice,
+					None => {
+						// TODO: Move pending_funding into pending_splice?
+						debug_assert!(false);
+						// TODO: Error instead?
+						return Ok((None, None));
+					},
+				};
 
-			if let Some(splice_locked) = self.context.check_get_splice_locked(pending_splice, funding, height, logger) {
-				log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id);
+				if let Some(splice_locked) = self.context.check_get_splice_locked(pending_splice, funding, height, logger) {
+					log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id);
 
-				pending_splice.sent_funding_txid = Some(splice_locked.splice_txid);
-				if pending_splice.sent_funding_txid == pending_splice.received_funding_txid {
-					promote_splice_funding!(self, funding);
+					pending_splice.sent_funding_txid = Some(splice_locked.splice_txid);
+					if pending_splice.sent_funding_txid == pending_splice.received_funding_txid {
+						promote_splice_funding!(self, funding);
 
-					let announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger);
-					return Ok((Some(FundingConfirmedMessage::Splice(splice_locked)), announcement_sigs));
-				}
+						let announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger);
+						return Ok((Some(FundingConfirmedMessage::Splice(splice_locked)), announcement_sigs));
+					}
 
-				return Ok((Some(FundingConfirmedMessage::Splice(splice_locked)), None));
+					return Ok((Some(FundingConfirmedMessage::Splice(splice_locked)), None));
+				}
 			}
 		}