Merge pull request #3004 from TheBlueMatt/2024-04-2761-followups

#2761 Followups

ci/rustfmt.sh

@@ -4,10 +4,16 @@ set -eox pipefail
 # Generate initial exclusion list
 #find . -name '*.rs' -type f |sort >rustfmt_excluded_files
 
+# The +rustversion syntax only works with rustup-installed rust toolchains,
+# not with any distro-provided ones. Thus, we check for a rustup install and
+# only pass +1.63.0 if we find one.
+VERS=""
+[ "$(which rustup)" != "" ] && VERS="+1.63.0"
+
 # Run fmt
 TMP_FILE=$(mktemp)
 find . -name '*.rs' -type f |sort >$TMP_FILE
 for file in $(comm -23 $TMP_FILE rustfmt_excluded_files); do
 	echo "Checking formatting of $file"
-	rustfmt +1.63.0 --check $file
+	rustfmt $VERS --check $file
 done

lightning/src/ln/channel_keys.rs

@@ -31,26 +31,30 @@ macro_rules! doc_comment {
 	};
 }
 macro_rules! basepoint_impl {
-	($BasepointT:ty) => {
+	($BasepointT:ty $(, $KeyName: expr)?) => {
 		impl $BasepointT {
 			/// Get inner Public Key
 			pub fn to_public_key(&self) -> PublicKey {
 				self.0
 			}
 
-			/// Derives a per-commitment-transaction (eg an htlc key or delayed_payment key) private key addition tweak
-			/// from a basepoint and a per_commitment_point:
-			/// `privkey = basepoint_secret + SHA256(per_commitment_point || basepoint)`
-			/// This calculates the hash part in the tweak derivation process, which is used to ensure
-			/// that each key is unique and cannot be guessed by an external party. It is equivalent
-			/// to the `from_basepoint` method, but without the addition operation, providing just the
-			/// tweak from the hash of the per_commitment_point and the basepoint.
-			pub fn derive_add_tweak(&self, per_commitment_point: &PublicKey) -> [u8; 32] {
-				let mut sha = Sha256::engine();
-				sha.input(&per_commitment_point.serialize());
-				sha.input(&self.to_public_key().serialize());
-				Sha256::from_engine(sha).to_byte_array()
-			}
+			$(doc_comment!(
+				concat!(
+				"Derives the \"tweak\" used in calculate [`", $KeyName, "::from_basepoint`].\n",
+				"\n",
+				"[`", $KeyName, "::from_basepoint`] calculates a private key as:\n",
+				"`privkey = basepoint_secret + SHA256(per_commitment_point || basepoint)`\n",
+				"\n",
+				"This calculates the hash part in the tweak derivation process, which is used to\n",
+				"ensure that each key is unique and cannot be guessed by an external party."
+				),
+				pub fn derive_add_tweak(&self, per_commitment_point: &PublicKey) -> Sha256 {
+					let mut sha = Sha256::engine();
+					sha.input(&per_commitment_point.serialize());
+					sha.input(&self.to_public_key().serialize());
+					Sha256::from_engine(sha)
+				});
+			)?
 		}
 
 		impl From<PublicKey> for $BasepointT {
@@ -110,7 +114,7 @@ macro_rules! key_read_write {
 /// state broadcasted was previously revoked.
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
 pub struct DelayedPaymentBasepoint(pub PublicKey);
-basepoint_impl!(DelayedPaymentBasepoint);
+basepoint_impl!(DelayedPaymentBasepoint, "DelayedPaymentKey");
 key_read_write!(DelayedPaymentBasepoint);
 
 /// A derived key built from a [`DelayedPaymentBasepoint`] and `per_commitment_point`.
@@ -137,7 +141,7 @@ key_read_write!(DelayedPaymentKey);
 /// Thus, both channel counterparties' HTLC keys will appears in each HTLC output's script.
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
 pub struct HtlcBasepoint(pub PublicKey);
-basepoint_impl!(HtlcBasepoint);
+basepoint_impl!(HtlcBasepoint, "HtlcKey");
 key_read_write!(HtlcBasepoint);
 
 /// A derived key built from a [`HtlcBasepoint`] and `per_commitment_point`.
@@ -166,18 +170,20 @@ fn derive_public_key<T: secp256k1::Signing>(
 	let mut sha = Sha256::engine();
 	sha.input(&per_commitment_point.serialize());
 	sha.input(&base_point.serialize());
-	let res = Sha256::from_engine(sha).to_byte_array();
+	let res = Sha256::from_engine(sha);
 
 	add_public_key_tweak(secp_ctx, base_point, &res)
 }
 
 /// Adds a tweak to a public key to derive a new public key.
+///
+/// May panic if `tweak` is not the output of a SHA-256 hash.
 pub fn add_public_key_tweak<T: secp256k1::Signing>(
-	secp_ctx: &Secp256k1<T>, base_point: &PublicKey, tweak: &[u8; 32],
+	secp_ctx: &Secp256k1<T>, base_point: &PublicKey, tweak: &Sha256,
 ) -> PublicKey {
 	let hashkey = PublicKey::from_secret_key(
 		&secp_ctx,
-		&SecretKey::from_slice(tweak)
+		&SecretKey::from_slice(tweak.as_byte_array())
 			.expect("Hashes should always be valid keys unless SHA-256 is broken"),
 	);
 	base_point.combine(&hashkey)

lightning/src/sign/mod.rs

@@ -110,8 +110,8 @@ pub struct DelayedPaymentOutputDescriptor {
 	pub channel_keys_id: [u8; 32],
 	/// The value of the channel which this output originated from, possibly indirectly.
 	pub channel_value_satoshis: u64,
-	/// The channel public keys and other parameters needed to generate a spending transaction or to provide to a re-derived signer through
-	/// [`ChannelSigner::provide_channel_parameters`].
+	/// The channel public keys and other parameters needed to generate a spending transaction or
+	/// to provide to a re-derived signer through [`ChannelSigner::provide_channel_parameters`].
 	///
 	/// Added as optional, but always `Some` if the descriptor was produced in v0.0.123 or later.
 	pub channel_transaction_parameters: Option<ChannelTransactionParameters>,
@@ -405,7 +405,7 @@ impl SpendableOutputDescriptor {
 									subtype: 0,
 									key: "add_tweak".as_bytes().to_vec(),
 								},
-								add_tweak.to_vec(),
+								add_tweak.as_byte_array().to_vec(),
 							)]
 							.into_iter()
 							.collect()

rustfmt_excluded_files

@@ -190,7 +190,6 @@
 ./lightning/src/ln/chanmon_update_fail_tests.rs
 ./lightning/src/ln/channel.rs
 ./lightning/src/ln/channel_id.rs
-./lightning/src/ln/channel_keys.rs
 ./lightning/src/ln/channelmanager.rs
 ./lightning/src/ln/features.rs
 ./lightning/src/ln/functional_test_utils.rs