Merge pull request #2748 from TheBlueMatt/2023-11-2675-followups

Doc and style followups from #2675

lightning/src/ln/channel.rs

@@ -1833,8 +1833,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 	/// will sign and send to our counterparty.
 	/// If an Err is returned, it is a ChannelError::Close (for get_funding_created)
 	fn build_remote_transaction_keys(&self) -> TxCreationKeys {
-		//TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
-		//may see payments to it!
 		let revocation_basepoint = &self.get_holder_pubkeys().revocation_basepoint;
 		let htlc_basepoint = &self.get_holder_pubkeys().htlc_basepoint;
 		let counterparty_pubkeys = self.get_counterparty_pubkeys();

lightning/src/ln/channel_keys.rs

@@ -25,126 +25,125 @@ use bitcoin::secp256k1::PublicKey;
 use bitcoin::hashes::sha256::Hash as Sha256;
 
 macro_rules! doc_comment {
-    ($x:expr, $($tt:tt)*) => {
+	($x:expr, $($tt:tt)*) => {
-        #[doc = $x]
+		#[doc = $x]
-        $($tt)*
+		$($tt)*
-    };
+	};
 }
 macro_rules! basepoint_impl {
-    ($BasepointT:ty) => {
+	($BasepointT:ty) => {
-        impl $BasepointT {
+		impl $BasepointT {
-            /// Get inner Public Key
+			/// Get inner Public Key
-            pub fn to_public_key(&self) -> PublicKey {
+			pub fn to_public_key(&self) -> PublicKey {
-                self.0
+				self.0
-            }
+			}
-        }
+		}
-        
+
-        impl From<PublicKey> for $BasepointT {
+		impl From<PublicKey> for $BasepointT {
-            fn from(value: PublicKey) -> Self {
+			fn from(value: PublicKey) -> Self {
-                Self(value)
+				Self(value)
-            }
+			}
-        }
+		}
-        
+
-    }
+	}
 }
 macro_rules! key_impl {
-    ($BasepointT:ty, $KeyName:expr) => {
+	($BasepointT:ty, $KeyName:expr) => {
-        doc_comment! {
+		doc_comment! {
-            concat!("Generate ", $KeyName, " using per_commitment_point"),
+			concat!("Derive a public ", $KeyName, " using one node's `per_commitment_point` and its countersignatory's `basepoint`"),
-            pub fn from_basepoint<T: secp256k1::Signing>(
+			pub fn from_basepoint<T: secp256k1::Signing>(
-                secp_ctx: &Secp256k1<T>,
+				secp_ctx: &Secp256k1<T>,
-                basepoint: &$BasepointT,
+				countersignatory_basepoint: &$BasepointT,
-                per_commitment_point: &PublicKey,
+				per_commitment_point: &PublicKey,
-            ) -> Self {
+			) -> Self {
-                Self(derive_public_key(secp_ctx, per_commitment_point, &basepoint.0))
+				Self(derive_public_key(secp_ctx, per_commitment_point, &countersignatory_basepoint.0))
-            }
+			}
-        }
+		}
-        
+
-        doc_comment! {
+		doc_comment! {
-            concat!("Generate ", $KeyName, " from privkey"),
+			concat!("Build a ", $KeyName, " directly from an already-derived private key"),
-            pub fn from_secret_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, sk: &SecretKey) -> Self {
+			pub fn from_secret_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, sk: &SecretKey) -> Self {
-                Self(PublicKey::from_secret_key(&secp_ctx, &sk))
+				Self(PublicKey::from_secret_key(&secp_ctx, &sk))
-            }
+			}
-        }
+		}
-        
+
-        /// Get inner Public Key
+		/// Get inner Public Key
-        pub fn to_public_key(&self) -> PublicKey {
+		pub fn to_public_key(&self) -> PublicKey {
-            self.0
+			self.0
-        }
+		}
-    }
+	}
 }
 macro_rules! key_read_write {
-    ($SelfT:ty) => {
+	($SelfT:ty) => {
-        impl Writeable for $SelfT {
+		impl Writeable for $SelfT {
-            fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+			fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
-                self.0.serialize().write(w)
+				self.0.serialize().write(w)
-            }
+			}
-        }
+		}
-        
+
-        impl Readable for $SelfT {
+		impl Readable for $SelfT {
-            fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+			fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
-                let key: PublicKey = Readable::read(r)?;
+				let key: PublicKey = Readable::read(r)?;
-                Ok(Self(key))
+				Ok(Self(key))
-            }
+			}
-        }
+		}
-    }
+	}
 }
 
 
 
-/// Master key used in conjunction with per_commitment_point to generate [`local_delayedpubkey`](https://github.com/lightning/bolts/blob/master/03-transactions.md#key-derivation) for the latest state of a channel.
+/// Base key used in conjunction with a `per_commitment_point` to generate a [`DelayedPaymentKey`].
-/// A watcher can be given a [DelayedPaymentBasepoint] to generate per commitment [DelayedPaymentKey] to create justice transactions.
+///
+/// The delayed payment key is used to pay the commitment state broadcaster their
+/// non-HTLC-encumbered funds after a delay to give their counterparty a chance to punish if the
+/// state broadcasted was previously revoked.
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
 pub struct DelayedPaymentBasepoint(pub PublicKey);
 basepoint_impl!(DelayedPaymentBasepoint);
 key_read_write!(DelayedPaymentBasepoint);
 
-/// [delayedpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation)
+
-/// To allow a counterparty to contest a channel state published by a node, Lightning protocol sets delays for some of the outputs, before can be spend.
+/// A derived key built from a [`DelayedPaymentBasepoint`] and `per_commitment_point`.
-/// For example a commitment transaction has to_local output encumbered by a delay, negotiated at the channel establishment flow.
+///
-/// To spend from such output a node has to generate a script using, among others, a local delayed payment key.
+/// The delayed payment key is used to pay the commitment state broadcaster their
+/// non-HTLC-encumbered funds after a delay. This delay gives their counterparty a chance to
+/// punish and claim all the channel funds if the state broadcasted was previously revoked.
+///
+/// [See the BOLT specs]
+/// (https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation)
+/// for more information on key derivation details.
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub struct DelayedPaymentKey(pub PublicKey);
 
 impl DelayedPaymentKey {
-    key_impl!(DelayedPaymentBasepoint, "delayedpubkey");
+	key_impl!(DelayedPaymentBasepoint, "delayedpubkey");
 }
 key_read_write!(DelayedPaymentKey);
 
-/// Master key used in conjunction with per_commitment_point to generate a [localpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#key-derivation) for the latest state of a channel.
+/// Base key used in conjunction with a `per_commitment_point` to generate an [`HtlcKey`].
-/// Also used to generate a commitment number in a commitment transaction or as a Payment Key for a remote node (not us) in an anchor output if `option_static_remotekey` is enabled.
+///
-/// Shared by both nodes in a channel establishment message flow.
+/// HTLC keys are used to ensure only the recipient of an HTLC can claim it on-chain with the HTLC
-#[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
+/// preimage and that only the sender of an HTLC can claim it on-chain after it has timed out.
-pub struct PaymentBasepoint(pub PublicKey);
+/// Thus, both channel counterparties' HTLC keys will appears in each HTLC output's script.
-basepoint_impl!(PaymentBasepoint);
-key_read_write!(PaymentBasepoint);
-
-
-/// [localpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation) is a child key of a payment basepoint,
-/// that enables a secure hash-lock for off-chain payments without risk of funds getting stuck or stolen. A payment key is normally shared with a counterparty so that it can generate 
-/// a commitment transaction's to_remote ouput, which our node can claim in case the counterparty force closes the channel.
-#[derive(PartialEq, Eq, Clone, Copy, Debug)]
-pub struct PaymentKey(pub PublicKey);
-
-impl PaymentKey {
-    key_impl!(PaymentBasepoint, "localpubkey");
-}
-key_read_write!(PaymentKey);
-
-/// Master key used in conjunction with per_commitment_point to generate [htlcpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#key-derivation) for the latest state of a channel.
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
 pub struct HtlcBasepoint(pub PublicKey);
 basepoint_impl!(HtlcBasepoint);
 key_read_write!(HtlcBasepoint);
 
-
+/// A derived key built from a [`HtlcBasepoint`] and `per_commitment_point`.
-/// [htlcpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation) is a child key of an htlc basepoint,
+///
-/// that enables secure routing of payments in onion scheme without a risk of them getting stuck or diverted. It is used to claim the funds in successful or timed out htlc outputs.
+/// HTLC keys are used to ensure only the recipient of an HTLC can claim it on-chain with the HTLC
+/// preimage and that only the sender of an HTLC can claim it on-chain after it has timed out.
+/// Thus, both channel counterparties' HTLC keys will appears in each HTLC output's script.
+///
+/// [See the BOLT specs]
+/// (https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation)
+/// for more information on key derivation details.
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub struct HtlcKey(pub PublicKey);
 
 impl HtlcKey {
-    key_impl!(HtlcBasepoint, "htlcpubkey");
+	key_impl!(HtlcBasepoint, "htlcpubkey");
 }
 key_read_write!(HtlcKey);
 
@@ -156,7 +155,6 @@ fn derive_public_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_commitm
 	sha.input(&per_commitment_point.serialize());
 	sha.input(&base_point.serialize());
 	let res = Sha256::from_engine(sha).to_byte_array();
-    
 
 	let hashkey = PublicKey::from_secret_key(&secp_ctx,
 		&SecretKey::from_slice(&res).expect("Hashes should always be valid keys unless SHA-256 is broken"));
@@ -172,68 +170,71 @@ basepoint_impl!(RevocationBasepoint);
 key_read_write!(RevocationBasepoint);
 
 
-/// [htlcpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation) is a child key of a revocation basepoint,
+/// The revocation key is used to allow a channel party to revoke their state - giving their
-/// that enables a node to create a justice transaction punishing a counterparty for an attempt to steal funds. Used to in generation of commitment and htlc outputs.
+/// counterparty the required material to claim all of their funds if they broadcast that state.
+///
+/// Each commitment transaction has a revocation key based on the basepoint and
+/// per_commitment_point which is used in both commitment and HTLC transactions.
+///
+/// See [the BOLT spec for derivation details]
+/// (https://github.com/lightning/bolts/blob/master/03-transactions.md#revocationpubkey-derivation)
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
 pub struct RevocationKey(pub PublicKey);
 
 impl RevocationKey {
-    /// Derives a per-commitment-transaction revocation public key from its constituent parts. This is
+	/// Derives a per-commitment-transaction revocation public key from one party's per-commitment
-    /// the public equivalend of derive_private_revocation_key - using only public keys to derive a
+	/// point and the other party's [`RevocationBasepoint`]. This is the public equivalent of
-    /// public key instead of private keys.
+	/// [`chan_utils::derive_private_revocation_key`] - using only public keys to derive a public
-    ///
+	/// key instead of private keys.
-    /// Only the cheating participant owns a valid witness to propagate a revoked
+	///
-    /// commitment transaction, thus per_commitment_point always come from cheater
+	/// Note that this is infallible iff we trust that at least one of the two input keys are randomly
-    /// and revocation_base_point always come from punisher, which is the broadcaster
+	/// generated (ie our own).
-    /// of the transaction spending with this key knowledge.
+	///
-    ///
+	/// [`chan_utils::derive_private_revocation_key`]: crate::ln::chan_utils::derive_private_revocation_key
-    /// Note that this is infallible iff we trust that at least one of the two input keys are randomly
+	pub fn from_basepoint<T: secp256k1::Verification>(
-    /// generated (ie our own).
+		secp_ctx: &Secp256k1<T>,
-    pub fn from_basepoint<T: secp256k1::Verification>(
+		countersignatory_basepoint: &RevocationBasepoint,
-        secp_ctx: &Secp256k1<T>,
+		per_commitment_point: &PublicKey,
-        basepoint: &RevocationBasepoint,
+	) -> Self {
-        per_commitment_point: &PublicKey,
+		let rev_append_commit_hash_key = {
-    ) -> Self {
+			let mut sha = Sha256::engine();
-        let rev_append_commit_hash_key = {
+			sha.input(&countersignatory_basepoint.to_public_key().serialize());
-            let mut sha = Sha256::engine();
+			sha.input(&per_commitment_point.serialize());
-            sha.input(&basepoint.to_public_key().serialize());
-            sha.input(&per_commitment_point.serialize());
-    
-            Sha256::from_engine(sha).to_byte_array()
-        };
-        let commit_append_rev_hash_key = {
-            let mut sha = Sha256::engine();
-            sha.input(&per_commitment_point.serialize());
-            sha.input(&basepoint.to_public_key().serialize());
-    
-            Sha256::from_engine(sha).to_byte_array()
-        };
-    
-        let countersignatory_contrib = basepoint.to_public_key().mul_tweak(&secp_ctx, &Scalar::from_be_bytes(rev_append_commit_hash_key).unwrap())
-            .expect("Multiplying a valid public key by a hash is expected to never fail per secp256k1 docs");
-        let broadcaster_contrib = (&per_commitment_point).mul_tweak(&secp_ctx, &Scalar::from_be_bytes(commit_append_rev_hash_key).unwrap())
-            .expect("Multiplying a valid public key by a hash is expected to never fail per secp256k1 docs");
-        let pk = countersignatory_contrib.combine(&broadcaster_contrib)
-            .expect("Addition only fails if the tweak is the inverse of the key. This is not possible when the tweak commits to the key.");
-        Self(pk)
-    }
 
-    /// Get inner Public Key
+			Sha256::from_engine(sha).to_byte_array()
-    pub fn to_public_key(&self) -> PublicKey {
+		};
-        self.0
+		let commit_append_rev_hash_key = {
-    }
+			let mut sha = Sha256::engine();
+			sha.input(&per_commitment_point.serialize());
+			sha.input(&countersignatory_basepoint.to_public_key().serialize());
+
+			Sha256::from_engine(sha).to_byte_array()
+		};
+
+		let countersignatory_contrib = countersignatory_basepoint.to_public_key().mul_tweak(&secp_ctx, &Scalar::from_be_bytes(rev_append_commit_hash_key).unwrap())
+			.expect("Multiplying a valid public key by a hash is expected to never fail per secp256k1 docs");
+		let broadcaster_contrib = (&per_commitment_point).mul_tweak(&secp_ctx, &Scalar::from_be_bytes(commit_append_rev_hash_key).unwrap())
+			.expect("Multiplying a valid public key by a hash is expected to never fail per secp256k1 docs");
+		let pk = countersignatory_contrib.combine(&broadcaster_contrib)
+			.expect("Addition only fails if the tweak is the inverse of the key. This is not possible when the tweak commits to the key.");
+		Self(pk)
+	}
+
+	/// Get inner Public Key
+	pub fn to_public_key(&self) -> PublicKey {
+		self.0
+	}
 }
 key_read_write!(RevocationKey);
 
 
-
 #[cfg(test)]
 mod test {
-    use bitcoin::secp256k1::{Secp256k1, SecretKey, PublicKey};
+	use bitcoin::secp256k1::{Secp256k1, SecretKey, PublicKey};
-    use bitcoin::hashes::hex::FromHex;
+	use bitcoin::hashes::hex::FromHex;
-    use super::derive_public_key;
+	use super::derive_public_key;
 
-    #[test]
+	#[test]
 	fn test_key_derivation() {
 		// Test vectors from BOLT 3 Appendix E:
 		let secp_ctx = Secp256k1::new();
@@ -248,6 +249,6 @@ mod test {
 		assert_eq!(per_commitment_point.serialize()[..], <Vec<u8>>::from_hex("025f7117a78150fe2ef97db7cfc83bd57b2e2c0d0dd25eaf467a4a1c2a45ce1486").unwrap()[..]);
 
 		assert_eq!(derive_public_key(&secp_ctx, &per_commitment_point, &base_point).serialize()[..],
-				<Vec<u8>>::from_hex("0235f2dbfaa89b57ec7b055afe29849ef7ddfeb1cefdb9ebdc43f5494984db29e5").unwrap()[..]);
+			<Vec<u8>>::from_hex("0235f2dbfaa89b57ec7b055afe29849ef7ddfeb1cefdb9ebdc43f5494984db29e5").unwrap()[..]);
 	}
 }