From: Matt Corallo <git@bluematt.me>
Date: Fri, 15 Sep 2023 19:17:34 +0000 (+0000)
Subject: Avoid unnecessarily cloning unsigned Transaction when broadcasting
X-Git-Tag: v0.0.117-alpha1~7^2
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=53c8f89ba9c0506fb138f57ffa5d0754be4b238c;p=rust-lightning

Avoid unnecessarily cloning unsigned Transaction when broadcasting

Our `Trusted*` wrappers in `chan_utils` expose additional inner
fields by reference. However, because they were not explicitly
marked as returning a reference with the wrapped struct's
lifetimes, rustc was considering them to return a reference with
the wrapper struct's lifetime.

This is unnecessarily restrictive, and resulted in the addition of
a clone in 9850c5814abdf69883b51a4afe73912ff5436fd9 which we remove
here.
---

diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
index 1d874d43e..5daa2463d 100644
--- a/lightning/src/chain/onchaintx.rs
+++ b/lightning/src/chain/onchaintx.rs
@@ -1123,8 +1123,8 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 		ret
 	}
 
-	pub(crate) fn get_unsigned_holder_commitment_tx(&self) -> Transaction {
-		self.holder_commitment.trust().built_transaction().transaction.clone()
+	pub(crate) fn get_unsigned_holder_commitment_tx(&self) -> &Transaction {
+		&self.holder_commitment.trust().built_transaction().transaction
 	}
 
 	//TODO: getting lastest holder transactions should be infallible and result in us "force-closing the channel", but we may
diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs
index 968de7b43..7521da01f 100644
--- a/lightning/src/ln/chan_utils.rs
+++ b/lightning/src/ln/chan_utils.rs
@@ -982,7 +982,7 @@ pub struct DirectedChannelTransactionParameters<'a> {
 
 impl<'a> DirectedChannelTransactionParameters<'a> {
 	/// Get the channel pubkeys for the broadcaster
-	pub fn broadcaster_pubkeys(&self) -> &ChannelPublicKeys {
+	pub fn broadcaster_pubkeys(&self) -> &'a ChannelPublicKeys {
 		if self.holder_is_broadcaster {
 			&self.inner.holder_pubkeys
 		} else {
@@ -991,7 +991,7 @@ impl<'a> DirectedChannelTransactionParameters<'a> {
 	}
 
 	/// Get the channel pubkeys for the countersignatory
-	pub fn countersignatory_pubkeys(&self) -> &ChannelPublicKeys {
+	pub fn countersignatory_pubkeys(&self) -> &'a ChannelPublicKeys {
 		if self.holder_is_broadcaster {
 			&self.inner.counterparty_parameters.as_ref().unwrap().pubkeys
 		} else {
@@ -1020,7 +1020,7 @@ impl<'a> DirectedChannelTransactionParameters<'a> {
 	}
 
 	/// Whether to use anchors for this channel
-	pub fn channel_type_features(&self) -> &ChannelTypeFeatures {
+	pub fn channel_type_features(&self) -> &'a ChannelTypeFeatures {
 		&self.inner.channel_type_features
 	}
 }
@@ -1279,7 +1279,7 @@ impl<'a> Deref for TrustedClosingTransaction<'a> {
 
 impl<'a> TrustedClosingTransaction<'a> {
 	/// The pre-built Bitcoin commitment transaction
-	pub fn built_transaction(&self) -> &Transaction {
+	pub fn built_transaction(&self) -> &'a Transaction {
 		&self.inner.built
 	}
 
@@ -1668,17 +1668,17 @@ impl<'a> TrustedCommitmentTransaction<'a> {
 	}
 
 	/// The pre-built Bitcoin commitment transaction
-	pub fn built_transaction(&self) -> &BuiltCommitmentTransaction {
+	pub fn built_transaction(&self) -> &'a BuiltCommitmentTransaction {
 		&self.inner.built
 	}
 
 	/// The pre-calculated transaction creation public keys.
-	pub fn keys(&self) -> &TxCreationKeys {
+	pub fn keys(&self) -> &'a TxCreationKeys {
 		&self.inner.keys
 	}
 
 	/// Should anchors be used.
-	pub fn channel_type_features(&self) -> &ChannelTypeFeatures {
+	pub fn channel_type_features(&self) -> &'a ChannelTypeFeatures {
 		&self.inner.channel_type_features
 	}