Merge pull request #664 from lightning-signer/tx-creation-keys

Wrap transaction creation keys

Author: TheBlueMatt

lightning/src/chain/keysinterface.rs

@@ -23,7 +23,7 @@ use util::byte_utils;
 use util::ser::{Writeable, Writer, Readable};
 
 use ln::chan_utils;
-use ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, LocalCommitmentTransaction};
+use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, LocalCommitmentTransaction, PreCalculatedTxCreationKeys};
 use ln::msgs;
 
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -223,7 +223,7 @@ pub trait ChannelKeys : Send+Clone {
 	// TODO: Document the things someone using this interface should enforce before signing.
 	// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
 	// making the callee generate it via some util function we expose)!
-	fn sign_remote_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
+	fn sign_remote_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &PreCalculatedTxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
 
 	/// Create a signature for a local commitment transaction. This will only ever be called with
 	/// the same local_commitment_tx (or a copy thereof), though there are currently no guarantees
@@ -461,8 +461,9 @@ impl ChannelKeys for InMemoryChannelKeys {
 	fn pubkeys(&self) -> &ChannelPublicKeys { &self.local_channel_pubkeys }
 	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
 
-	fn sign_remote_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
+	fn sign_remote_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, feerate_per_kw: u32, commitment_tx: &Transaction, pre_keys: &PreCalculatedTxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
 		if commitment_tx.input.len() != 1 { return Err(()); }
+		let keys = pre_keys.trust_key_derivation();
 
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let accepted_data = self.accepted_channel_data.as_ref().expect("must accept before signing");

lightning/src/ln/chan_utils.rs

@@ -267,23 +267,52 @@ pub fn derive_public_revocation_key<T: secp256k1::Verification>(secp_ctx: &Secp2
 
 /// The set of public keys which are used in the creation of one commitment transaction.
 /// These are derived from the channel base keys and per-commitment data.
+///
+/// These keys are assumed to be good, either because the code derived them from
+/// channel basepoints via the new function, or they were obtained via
+/// PreCalculatedTxCreationKeys.trust_key_derivation because we trusted the source of the
+/// pre-calculated keys.
 #[derive(PartialEq, Clone)]
 pub struct TxCreationKeys {
 	/// The per-commitment public key which was used to derive the other keys.
 	pub per_commitment_point: PublicKey,
 	/// The revocation key which is used to allow the owner of the commitment transaction to
 	/// provide their counterparty the ability to punish them if they broadcast an old state.
-	pub(crate) revocation_key: PublicKey,
+	pub revocation_key: PublicKey,
 	/// A's HTLC Key
-	pub(crate) a_htlc_key: PublicKey,
+	pub a_htlc_key: PublicKey,
 	/// B's HTLC Key
-	pub(crate) b_htlc_key: PublicKey,
+	pub b_htlc_key: PublicKey,
 	/// A's Payment Key (which isn't allowed to be spent from for some delay)
-	pub(crate) a_delayed_payment_key: PublicKey,
+	pub a_delayed_payment_key: PublicKey,
 }
 impl_writeable!(TxCreationKeys, 33*6,
 	{ per_commitment_point, revocation_key, a_htlc_key, b_htlc_key, a_delayed_payment_key });
 
+/// The per-commitment point and a set of pre-calculated public keys used for transaction creation
+/// in the signer.
+/// The pre-calculated keys are an optimization, because ChannelKeys has enough
+/// information to re-derive them.
+pub struct PreCalculatedTxCreationKeys(TxCreationKeys);
+
+impl PreCalculatedTxCreationKeys {
+	/// Create a new PreCalculatedTxCreationKeys from TxCreationKeys
+	pub fn new(keys: TxCreationKeys) -> Self {
+		PreCalculatedTxCreationKeys(keys)
+	}
+
+	/// The pre-calculated transaction creation public keys.
+	/// An external validating signer should not trust these keys.
+	pub fn trust_key_derivation(&self) -> &TxCreationKeys {
+		&self.0
+	}
+
+	/// The transaction per-commitment point
+	pub fn per_comitment_point(&self) -> &PublicKey {
+		&self.0.per_commitment_point
+	}
+}
+
 /// One counterparty's public keys which do not change over the life of a channel.
 #[derive(Clone, PartialEq)]
 pub struct ChannelPublicKeys {
@@ -318,7 +347,8 @@ impl_writeable!(ChannelPublicKeys, 33*5, {
 
 
 impl TxCreationKeys {
-	pub(crate) fn new<T: secp256k1::Signing + secp256k1::Verification>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, a_delayed_payment_base: &PublicKey, a_htlc_base: &PublicKey, b_revocation_base: &PublicKey, b_htlc_base: &PublicKey) -> Result<TxCreationKeys, secp256k1::Error> {
+	/// Create a new TxCreationKeys from channel base points and the per-commitment point
+	pub fn new<T: secp256k1::Signing + secp256k1::Verification>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, a_delayed_payment_base: &PublicKey, a_htlc_base: &PublicKey, b_revocation_base: &PublicKey, b_htlc_base: &PublicKey) -> Result<TxCreationKeys, secp256k1::Error> {
 		Ok(TxCreationKeys {
 			per_commitment_point: per_commitment_point.clone(),
 			revocation_key: derive_public_revocation_key(&secp_ctx, &per_commitment_point, &b_revocation_base)?,
@@ -510,8 +540,7 @@ pub struct LocalCommitmentTransaction {
 	// Which order the signatures should go in when constructing the final commitment tx witness.
 	// The user should be able to reconstruc this themselves, so we don't bother to expose it.
 	our_sig_first: bool,
-	/// The key derivation parameters for this commitment transaction
-	pub local_keys: TxCreationKeys,
+	pub(crate) local_keys: TxCreationKeys,
 	/// The feerate paid per 1000-weight-unit in this commitment transaction. This value is
 	/// controlled by the channel initiator.
 	pub feerate_per_kw: u32,
@@ -576,6 +605,12 @@ impl LocalCommitmentTransaction {
 		}
 	}
 
+	/// The pre-calculated transaction creation public keys.
+	/// An external validating signer should not trust these keys.
+	pub fn trust_key_derivation(&self) -> &TxCreationKeys {
+		&self.local_keys
+	}
+
 	/// Get the txid of the local commitment transaction contained in this
 	/// LocalCommitmentTransaction
 	pub fn txid(&self) -> Txid {

lightning/src/ln/channel.rs

@@ -19,7 +19,7 @@ use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
 use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER};
 use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
-use ln::chan_utils::{CounterpartyCommitmentSecrets, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, PreCalculatedTxCreationKeys};
 use ln::chan_utils;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::transaction::OutPoint;
@@ -1484,7 +1484,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 		let remote_keys = self.build_remote_transaction_keys()?;
 		let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
-		let remote_signature = self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), &self.secp_ctx)
+		let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys);
+		let remote_signature = self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &pre_remote_keys, &Vec::new(), &self.secp_ctx)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0;
 
 		// We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
@@ -3532,7 +3533,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 	fn get_outbound_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
 		let remote_keys = self.build_remote_transaction_keys()?;
 		let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
-		Ok(self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), &self.secp_ctx)
+		let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys);
+		Ok(self.local_keys.sign_remote_commitment(self.feerate_per_kw, &remote_initial_commitment_tx, &pre_remote_keys, &Vec::new(), &self.secp_ctx)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0)
 	}
 
@@ -3885,10 +3887,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 				htlcs.push(htlc);
 			}
 
-			let res = self.local_keys.sign_remote_commitment(feerate_per_kw, &remote_commitment_tx.0, &remote_keys, &htlcs, &self.secp_ctx)
+			let pre_remote_keys = PreCalculatedTxCreationKeys::new(remote_keys);
+			let res = self.local_keys.sign_remote_commitment(feerate_per_kw, &remote_commitment_tx.0, &pre_remote_keys, &htlcs, &self.secp_ctx)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?;
 			signature = res.0;
 			htlc_signatures = res.1;
+			let remote_keys = pre_remote_keys.trust_key_derivation();
 
 			log_trace!(logger, "Signed remote commitment tx {} with redeemscript {} -> {}",
 				encode::serialize_hex(&remote_commitment_tx.0),
@@ -3898,7 +3902,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) {
 				log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {}",
 					encode::serialize_hex(&chan_utils::build_htlc_transaction(&remote_commitment_tx.0.txid(), feerate_per_kw, self.our_to_self_delay, htlc, &remote_keys.a_delayed_payment_key, &remote_keys.revocation_key)),
-					encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &remote_keys)),
+					encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, remote_keys)),
 					log_bytes!(remote_keys.a_htlc_key.serialize()),
 					log_bytes!(htlc_sig.serialize_compact()[..]));
 			}

lightning/src/ln/functional_tests.rs

@@ -52,6 +52,7 @@ use std::sync::atomic::Ordering;
 use std::{mem, io};
 
 use ln::functional_test_utils::*;
+use ln::chan_utils::PreCalculatedTxCreationKeys;
 
 #[test]
 fn test_insane_channel_opens() {
@@ -1694,7 +1695,8 @@ fn test_fee_spike_violation_fails_htlc() {
 		let local_chan_lock = nodes[0].node.channel_state.lock().unwrap();
 		let local_chan = local_chan_lock.by_id.get(&chan.2).unwrap();
 		let local_chan_keys = local_chan.get_local_keys();
-		local_chan_keys.sign_remote_commitment(feerate_per_kw, &commit_tx, &commit_tx_keys, &[&accepted_htlc_info], &secp_ctx).unwrap()
+		let pre_commit_tx_keys = PreCalculatedTxCreationKeys::new(commit_tx_keys);
+		local_chan_keys.sign_remote_commitment(feerate_per_kw, &commit_tx, &pre_commit_tx_keys, &[&accepted_htlc_info], &secp_ctx).unwrap()
 	};
 
 	let commit_signed_msg = msgs::CommitmentSigned {

lightning/src/util/enforcing_trait_impls.rs

@@ -1,4 +1,4 @@
-use ln::chan_utils::{HTLCOutputInCommitment, TxCreationKeys, ChannelPublicKeys, LocalCommitmentTransaction};
+use ln::chan_utils::{HTLCOutputInCommitment, TxCreationKeys, ChannelPublicKeys, LocalCommitmentTransaction, PreCalculatedTxCreationKeys};
 use ln::{chan_utils, msgs};
 use chain::keysinterface::{ChannelKeys, InMemoryChannelKeys};
 
@@ -60,9 +60,9 @@ impl ChannelKeys for EnforcingChannelKeys {
 	fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
 	fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() }
 
-	fn sign_remote_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
+	fn sign_remote_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, feerate_per_kw: u32, commitment_tx: &Transaction, pre_keys: &PreCalculatedTxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
 		if commitment_tx.input.len() != 1 { panic!("lightning commitment transactions have a single input"); }
-		self.check_keys(secp_ctx, keys);
+		self.check_keys(secp_ctx, pre_keys.trust_key_derivation());
 		let obscured_commitment_transaction_number = (commitment_tx.lock_time & 0xffffff) as u64 | ((commitment_tx.input[0].sequence as u64 & 0xffffff) << 3*8);
 
 		{
@@ -75,7 +75,7 @@ impl ChannelKeys for EnforcingChannelKeys {
 			commitment_data.1 = cmp::max(commitment_number, commitment_data.1)
 		}
 
-		Ok(self.inner.sign_remote_commitment(feerate_per_kw, commitment_tx, keys, htlcs, secp_ctx).unwrap())
+		Ok(self.inner.sign_remote_commitment(feerate_per_kw, commitment_tx, pre_keys, htlcs, secp_ctx).unwrap())
 	}
 
 	fn sign_local_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {