Significantly clarify key derivation and expose methods referenced

TheBlueMatt · TheBlueMatt · commit fd2571dde42a · 2020-05-15T21:00:14.000-04:00
diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs
@@ -56,26 +56,30 @@ pub enum SpendableOutputDescriptor {
 	/// <BIP 143 signature> <empty vector> (MINIMALIF standard rule) <provided witnessScript>
 	///
 	/// Note that the nSequence field in the spending input must be set to to_self_delay
-	/// (which means the transaction not being broadcastable until at least to_self_delay
+	/// (which means the transaction is not broadcastable until at least to_self_delay
 	/// blocks after the outpoint confirms).
 	///
 	/// These are generally the result of a "revocable" output to us, spendable only by us unless
-	/// it is an output from us having broadcast an old state (which should never happen).
+	/// it is an output from an old state which we broadcast (which should never happen).
 	///
-	/// WitnessScript may be regenerated by passing the revocation_pubkey, to_self_delay and
-	/// delayed_payment_pubkey to chan_utils::get_revokeable_redeemscript.
+	/// To derive the delayed_payment key which is used to sign for this input, you must pass the
+	/// local delayed_payment_base_key (ie the private key which corresponds to the pubkey in
+	/// ChannelKeys::pubkeys().delayed_payment_basepoint) and the provided per_commitment_point to
+	/// chan_utils::derive_private_key. The public key can be generated without the secret key
+	/// using chan_utils::derive_public_key and only the delayed_payment_basepoint which appears in
+	/// ChannelKeys::pubkeys().
 	///
-	/// To derive the delayed_payment key corresponding to the channel state, you must pass the
-	/// local delayed_payment_base_key and the provided per_commitment_point to
-	/// chan_utils::derive_private_key. The resulting key should be used to sign the spending
-	/// transaction.
-	///
-	/// To derive the revocation_pubkey corresponding to the channel state, you must pass the
-	/// remote revocation_basepoint and the provided per_commitment point to
+	/// To derive the revocation_pubkey which is used in the witness script generation, you must
+	/// pass the remote revocation_basepoint (which appears in the call to
+	/// ChannelKeys::set_remote_channel_pubkeys) and the provided per_commitment point to
 	/// chan_utils::derive_public_revocation_key.
 	///
-	/// Both remote revocation_basepoint and local delayed_payment_base_key should be given
-	/// by ChannelKeys, either default implementation (InMemoryChannelKeys) or custom one.
+	/// The witness script which is hashed and included in the output script_pubkey may be
+	/// regenerated by passing the revocation_pubkey (derived as above), our delayed_payment pubkey
+	/// (derived as above), and the to_self_delay contained here to
+	/// chan_utils::get_revokeable_redeemscript.
+	//
+	// TODO: we need to expose utility methods in KeyManager to do all the relevant derivation.
 	DynamicOutputP2WSH {
 		/// The outpoint which is spendable
 		outpoint: OutPoint,
@@ -92,7 +96,8 @@ pub enum SpendableOutputDescriptor {
 		/// The remote_revocation_pubkey used to derive witnessScript
 		remote_revocation_pubkey: PublicKey
 	},
-	/// An output to a P2WPKH, spendable exclusively by our payment key.
+	/// An output to a P2WPKH, spendable exclusively by our payment key (ie the private key which
+	/// corresponds to the public key in ChannelKeys::pubkeys().payment_point).
 	/// The witness in the spending input, is, thus, simply:
 	/// <BIP 143 signature> <payment key>
 	///
@@ -272,47 +277,47 @@ pub trait ChannelKeys : Send+Clone {
 	/// return value must contain a signature.
 	fn sign_local_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, local_commitment_tx: &LocalCommitmentTransaction, local_csv: u16, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()>;
 
-	/// Create a signature for a transaction spending an HTLC or commitment transaction output
-	/// when our counterparty broadcast an old state.
+	/// Create a signature for the given input in a transaction spending an HTLC or commitment
+	/// transaction output when our counterparty broadcast an old state.
 	///
-	/// Justice transaction may claim multiples outputs at same time if timelock are similar.
+	/// A justice transaction may claim multiples outputs at same time if timelock are similar,
+	/// but only a signature for the input at index `input` should be signed for here.
 	/// It may be called multiples time for same output(s) if a fee-bump is needed with regards
 	/// to an upcoming timelock expiration.
 	///
-	/// Input index is a pointer towards outpoint spent, commited by sigs (BIP 143).
+	/// Amount is value of the output spent by this input, committed to in the BIP 143 signature.
 	///
-	/// Amount is value of the output spent by this input, committed by sigs (BIP 143).
+	/// per_commitment_key is revocation secret which was provided by our counterparty when they
+	/// revoked the state which they eventually broadcast. It's not a _local_ secret key and does
+	/// not allow the spending of any funds by itself (you need our local revocation_secret to do
+	/// so).
 	///
-	/// Per_commitment key is revocation secret such as provided by remote party while
-	/// revocating detected onchain transaction. It's not a _local_ secret key, therefore
-	/// it may cross interfaces, a node compromise won't allow to spend revoked output without
-	/// also compromissing revocation key.
+	/// htlc holds HTLC elements (hash, timelock) if the output being spent is a HTLC output, thus
+	/// changing the format of the witness script (which is committed to in the BIP 143
+	/// signatures).
 	///
-	/// htlc holds HTLC elements (hash, timelock) if output spent is a HTLC one, committed as
-	/// part of witnessScript by sigs (BIP 143).
-	///
-	/// on_remote_tx_csv is the relative lock-time challenge if output spent is on remote
-	/// balance or 2nd-stage HTLC transactions, committed as part of witnessScript by sigs
-	/// (BIP 143).
+	/// on_remote_tx_csv is the relative lock-time that that our counterparty would have to set on
+	/// their transaction were they to spend the same output. It is included in the witness script
+	/// and thus committed to in the BIP 143 signature.
 	fn sign_justice_transaction<T: secp256k1::Signing + secp256k1::Verification>(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &Option<HTLCOutputInCommitment>, on_remote_tx_csv: u16, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
 
 	/// Create a signature for a claiming transaction for a HTLC output on a remote commitment
 	/// transaction, either offered or received.
 	///
-	/// HTLC transaction may claim multiples offered outputs at same time if we know preimage
-	/// for each at detection. It may be called multtiples time for same output(s) if a fee-bump
-	/// is needed with regards to an upcoming timelock expiration.
+	/// Such a transaction may claim multiples offered outputs at same time if we know the preimage
+	/// for each when we create it, but only the input at index `input` should be signed for here.
+	/// It may be called multiple time for same output(s) if a fee-bump is needed with regards to
+	/// an upcoming timelock expiration.
 	///
 	/// Witness_script is either a offered or received script as defined in BOLT3 for HTLC
 	/// outputs.
 	///
-	/// Input index is a pointer towards outpoint spent, commited by sigs (BIP 143).
-	///
-	/// Amount is value of the output spent by this input, committed by sigs (BIP 143).
+	/// Amount is value of the output spent by this input, committed to in the BIP 143 signature.
 	///
 	/// Per_commitment_point is the dynamic point corresponding to the channel state
-	/// detected onchain. It has been generated by remote party and is used to derive
-	/// channel state keys, committed as part of witnessScript by sigs (BIP 143).
+	/// detected onchain. It has been generated by our counterparty and is used to derive
+	/// channel state keys, which are then included in the witness script and committed to in the
+	/// BIP 143 signature.
 	fn sign_remote_htlc_transaction<T: secp256k1::Signing + secp256k1::Verification>(&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
 
 	/// Create a signature for a (proposed) closing transaction.
diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs
@@ -171,9 +171,11 @@ impl Readable for CounterpartyCommitmentSecrets {
 	}
 }
 
-/// Derives a per-commitment-transaction private key (eg an htlc key, payment key or delayed_payment
-/// key) from the base.
-/// private key for that type of key and the per_commitment_point (available in TxCreationKeys)
+/// Derives a per-commitment-transaction private key (eg an htlc key or delayed_payment key)
+/// from the base secret and the per_commitment_point.
+///
+/// Note that this is infallible iff we trust that at least one of the two input keys are randomly
+/// generated (ie our own).
 pub fn derive_private_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, base_secret: &SecretKey) -> Result<SecretKey, secp256k1::Error> {
 	let mut sha = Sha256::engine();
 	sha.input(&per_commitment_point.serialize());
@@ -185,7 +187,13 @@ pub fn derive_private_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_co
 	Ok(key)
 }
 
-pub(crate) fn derive_public_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, base_point: &PublicKey) -> Result<PublicKey, secp256k1::Error> {
+/// Derives a per-commitment-transaction public key (eg an htlc key or a delayed_payment key)
+/// from the base point and the per_commitment_key. This is the public equivalent of
+/// derive_private_key - using only public keys to derive a public key instead of private keys.
+///
+/// Note that this is infallible iff we trust that at least one of the two input keys are randomly
+/// generated (ie our own).
+pub fn derive_public_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, base_point: &PublicKey) -> Result<PublicKey, secp256k1::Error> {
 	let mut sha = Sha256::engine();
 	sha.input(&per_commitment_point.serialize());
 	sha.input(&base_point.serialize());
@@ -195,7 +203,8 @@ pub(crate) fn derive_public_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>,
 	base_point.combine(&hashkey)
 }
 
-/// Derives a revocation key from its constituent parts.
+/// Derives a per-commitment-transaction revocation key from its constituent parts.
+///
 /// Note that this is infallible iff we trust that at least one of the two input keys are randomly
 /// generated (ie our own).
 pub fn derive_private_revocation_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_commitment_secret: &SecretKey, revocation_base_secret: &SecretKey) -> Result<SecretKey, secp256k1::Error> {
@@ -225,7 +234,13 @@ pub fn derive_private_revocation_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1
 	Ok(part_a)
 }
 
-pub(crate) fn derive_public_revocation_key<T: secp256k1::Verification>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, revocation_base_point: &PublicKey) -> Result<PublicKey, secp256k1::Error> {
+/// Derives a per-commitment-transaction revocation public key from its constituent parts. This is
+/// the public equivalend of derive_private_revocation_key - using only public keys to derive a
+/// public key instead of private keys.
+///
+/// Note that this is infallible iff we trust that at least one of the two input keys are randomly
+/// generated (ie our own).
+pub fn derive_public_revocation_key<T: secp256k1::Verification>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, revocation_base_point: &PublicKey) -> Result<PublicKey, secp256k1::Error> {
 	let rev_append_commit_hash_key = {
 		let mut sha = Sha256::engine();
 		sha.input(&revocation_base_point.serialize());
@@ -274,9 +289,9 @@ pub struct ChannelPublicKeys {
 	/// on-chain channel lock-in 2-of-2 multisig output.
 	pub funding_pubkey: PublicKey,
 	/// The base point which is used (with derive_public_revocation_key) to derive per-commitment
-	/// revocation keys. The per-commitment revocation private key is then revealed by the owner of
-	/// a commitment transaction so that their counterparty can claim all available funds if they
-	/// broadcast an old state.
+	/// revocation keys. This is combined with the per-commitment-secret generated by the
+	/// counterparty to create a secret which the counterparty can reveal to revoke previous
+	/// states.
 	pub revocation_basepoint: PublicKey,
 	/// The public key which receives our immediately spendable primary channel balance in
 	/// remote-broadcasted commitment transactions. This key is static across every commitment
