Introduce CommitmentTransaction, ChannelTransactionParameters

CommitmentTransaction maintains the per-commitment transaction fields needed to construct the associated bitcoin transactions (commitment, HTLC).  It replaces passing around of Bitcoin transactions.  Once ChannelKeys is modified accordingly, this will allow a higher level of assurance that all relevant aspects of the transactions were checked for policy violations.

ChannelTransactionParameters replaces passing around of individual per-channel fields that are needed to construct Bitcoin transactions.

Author: devrandom

lightning/src/chain/channelmonitor.rs

@@ -39,7 +39,7 @@ use bitcoin::secp256k1;
 
 use ln::msgs::DecodeError;
 use ln::chan_utils;
-use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, HTLCType};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCType, ChannelTransactionParameters, HolderCommitmentTransaction};
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
 use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
@@ -489,7 +489,7 @@ const MIN_SERIALIZATION_VERSION: u8 = 1;
 #[derive(Clone)]
 pub(crate) enum ChannelMonitorUpdateStep {
 	LatestHolderCommitmentTXInfo {
-		commitment_tx: HolderCommitmentTransaction,
+		commitment_info: HolderCommitmentTransaction,
 		htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
 	},
 	LatestCounterpartyCommitmentTXInfo {
@@ -517,9 +517,9 @@ pub(crate) enum ChannelMonitorUpdateStep {
 impl Writeable for ChannelMonitorUpdateStep {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
 		match self {
-			&ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { ref commitment_tx, ref htlc_outputs } => {
+			&ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { ref commitment_info, ref htlc_outputs } => {
 				0u8.write(w)?;
-				commitment_tx.write(w)?;
+				commitment_info.write(w)?;
 				(htlc_outputs.len() as u64).write(w)?;
 				for &(ref output, ref signature, ref source) in htlc_outputs.iter() {
 					output.write(w)?;
@@ -560,7 +560,7 @@ impl Readable for ChannelMonitorUpdateStep {
 		match Readable::read(r)? {
 			0u8 => {
 				Ok(ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo {
-					commitment_tx: Readable::read(r)?,
+					commitment_info: Readable::read(r)?,
 					htlc_outputs: {
 						let len: u64 = Readable::read(r)?;
 						let mut res = Vec::new();
@@ -948,35 +948,47 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
 
 impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	pub(crate) fn new(keys: ChanSigner, shutdown_pubkey: &PublicKey,
-			on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
-			counterparty_htlc_base_key: &PublicKey, counterparty_delayed_payment_base_key: &PublicKey,
-			on_holder_tx_csv: u16, funding_redeemscript: Script, channel_value_satoshis: u64,
-			commitment_transaction_number_obscure_factor: u64,
-			initial_holder_commitment_tx: HolderCommitmentTransaction) -> ChannelMonitor<ChanSigner> {
+	                  on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
+	                  channel_parameters: &ChannelTransactionParameters,
+	                  funding_redeemscript: Script, channel_value_satoshis: u64,
+	                  commitment_transaction_number_obscure_factor: u64,
+	                  initial_holder_commitment_info: HolderCommitmentTransaction) -> ChannelMonitor<ChanSigner> {
 
 		assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
 		let our_channel_close_key_hash = WPubkeyHash::hash(&shutdown_pubkey.serialize());
 		let shutdown_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_close_key_hash[..]).into_script();
 		let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize());
 		let counterparty_payment_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_key_hash[..]).into_script();
 
-		let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key: *counterparty_delayed_payment_base_key, counterparty_htlc_base_key: *counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() };
-
-		let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), on_holder_tx_csv);
-
-		let holder_tx_sequence = initial_holder_commitment_tx.unsigned_tx.input[0].sequence as u64;
-		let holder_tx_locktime = initial_holder_commitment_tx.unsigned_tx.lock_time as u64;
-		let holder_commitment_tx = HolderSignedTx {
-			txid: initial_holder_commitment_tx.txid(),
-			revocation_key: initial_holder_commitment_tx.keys.revocation_key,
-			a_htlc_key: initial_holder_commitment_tx.keys.broadcaster_htlc_key,
-			b_htlc_key: initial_holder_commitment_tx.keys.countersignatory_htlc_key,
-			delayed_payment_key: initial_holder_commitment_tx.keys.broadcaster_delayed_payment_key,
-			per_commitment_point: initial_holder_commitment_tx.keys.per_commitment_point,
-			feerate_per_kw: initial_holder_commitment_tx.feerate_per_kw,
-			htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
+		let counterparty_delayed_payment_base_key = channel_parameters.counterparty_pubkeys.delayed_payment_basepoint;
+		let counterparty_htlc_base_key = channel_parameters.counterparty_pubkeys.htlc_basepoint;
+		let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key, counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() };
+
+		let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), channel_parameters.clone());
+
+		let secp_ctx = Secp256k1::new();
+
+		let directed_channel_parameters = channel_parameters.to_directed(true);
+		let txid = initial_holder_commitment_info.calculate_txid(&directed_channel_parameters, &secp_ctx);
+
+		// block for Rust 1.34 compat
+		let (holder_commitment_tx, current_holder_commitment_number) = {
+			let info = &initial_holder_commitment_info.info;
+			let tx_keys = &info.keys;
+			let current_holder_commitment_number = info.commitment_number;
+			let holder_commitment_tx = HolderSignedTx {
+				txid,
+				revocation_key: tx_keys.revocation_key,
+				a_htlc_key: tx_keys.broadcaster_htlc_key,
+				b_htlc_key: tx_keys.countersignatory_htlc_key,
+				delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
+				per_commitment_point: tx_keys.per_commitment_point,
+				feerate_per_kw: info.feerate_per_kw,
+				htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
+			};
+			(holder_commitment_tx, current_holder_commitment_number)
 		};
-		onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_tx);
+		onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_info);
 
 		let mut outputs_to_watch = HashMap::new();
 		outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
@@ -1000,7 +1012,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			channel_value_satoshis,
 			their_cur_revocation_points: None,
 
-			on_holder_tx_csv,
+			on_holder_tx_csv: channel_parameters.counterparty_selected_contest_delay,
 
 			commitment_secrets: CounterpartyCommitmentSecrets::new(),
 			counterparty_claimable_outpoints: HashMap::new(),
@@ -1010,7 +1022,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			prev_holder_signed_commitment_tx: None,
 			current_holder_commitment_tx: holder_commitment_tx,
 			current_counterparty_commitment_number: 1 << 48,
-			current_holder_commitment_number: 0xffff_ffff_ffff - ((((holder_tx_sequence & 0xffffff) << 3*8) | (holder_tx_locktime as u64 & 0xffffff)) ^ commitment_transaction_number_obscure_factor),
+			current_holder_commitment_number,
 
 			payment_preimages: HashMap::new(),
 			pending_monitor_events: Vec::new(),
@@ -1025,7 +1037,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			holder_tx_signed: false,
 
 			last_block_hash: Default::default(),
-			secp_ctx: Secp256k1::new(),
+			secp_ctx,
 		}
 	}
 
@@ -1133,22 +1145,26 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
-	fn provide_latest_holder_commitment_tx_info(&mut self, commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
-		let txid = commitment_tx.txid();
-		let sequence = commitment_tx.unsigned_tx.input[0].sequence as u64;
-		let locktime = commitment_tx.unsigned_tx.lock_time as u64;
-		let mut new_holder_commitment_tx = HolderSignedTx {
-			txid,
-			revocation_key: commitment_tx.keys.revocation_key,
-			a_htlc_key: commitment_tx.keys.broadcaster_htlc_key,
-			b_htlc_key: commitment_tx.keys.countersignatory_htlc_key,
-			delayed_payment_key: commitment_tx.keys.broadcaster_delayed_payment_key,
-			per_commitment_point: commitment_tx.keys.per_commitment_point,
-			feerate_per_kw: commitment_tx.feerate_per_kw,
-			htlc_outputs,
+	fn provide_latest_holder_commitment_tx_info(&mut self, commitment_info: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
+		let txid = commitment_info.calculate_txid(&self.onchain_tx_handler.channel_transaction_parameters.to_directed(true), &self.secp_ctx);
+
+		// block for Rust 1.34 compat
+		let mut new_holder_commitment_tx = {
+			let info = &commitment_info.info;
+			let tx_keys = &info.keys;
+			self.current_holder_commitment_number = info.commitment_number;
+			HolderSignedTx {
+				txid,
+				revocation_key: tx_keys.revocation_key,
+				a_htlc_key: tx_keys.broadcaster_htlc_key,
+				b_htlc_key: tx_keys.countersignatory_htlc_key,
+				delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
+				per_commitment_point: tx_keys.per_commitment_point,
+				feerate_per_kw: info.feerate_per_kw,
+				htlc_outputs,
+			}
 		};
-		self.onchain_tx_handler.provide_latest_holder_tx(commitment_tx);
-		self.current_holder_commitment_number = 0xffff_ffff_ffff - ((((sequence & 0xffffff) << 3*8) | (locktime as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
+		self.onchain_tx_handler.provide_latest_holder_tx(commitment_info);
 		mem::swap(&mut new_holder_commitment_tx, &mut self.current_holder_commitment_tx);
 		self.prev_holder_signed_commitment_tx = Some(new_holder_commitment_tx);
 		if self.holder_tx_signed {
@@ -1236,10 +1252,10 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		}
 		for update in updates.updates.iter() {
 			match update {
-				ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs } => {
+				ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_info, htlc_outputs } => {
 					log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
 					if self.lockdown_from_offchain { panic!(); }
-					self.provide_latest_holder_commitment_tx_info(commitment_tx.clone(), htlc_outputs.clone())?
+					self.provide_latest_holder_commitment_tx_info(commitment_info.clone(), htlc_outputs.clone())?
 				},
 				ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { unsigned_commitment_tx, htlc_outputs, commitment_number, their_revocation_point } => {
 					log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -2589,7 +2605,7 @@ mod tests {
 	use ln::channelmanager::{PaymentPreimage, PaymentHash};
 	use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 	use ln::chan_utils;
-	use ln::chan_utils::{HTLCOutputInCommitment, HolderCommitmentTransaction};
+	use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction};
 	use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
 	use bitcoin::secp256k1::key::{SecretKey,PublicKey};
 	use bitcoin::secp256k1::Secp256k1;
@@ -2662,14 +2678,29 @@ mod tests {
 			(0, 0)
 		);
 
+		let counterparty_pubkeys = ChannelPublicKeys {
+			funding_pubkey: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
+			revocation_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
+			payment_point: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()),
+			delayed_payment_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[47; 32]).unwrap()),
+			htlc_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[48; 32]).unwrap())
+		};
+		let channel_parameters = ChannelTransactionParameters {
+			holder_pubkeys: keys.holder_channel_pubkeys.clone(),
+			counterparty_pubkeys,
+			holder_selected_contest_delay: 66,
+			counterparty_selected_contest_delay: 67,
+			funding_outpoint: Default::default(),
+			is_outbound_from_holder: true
+		};
 		// Prune with one old state and a holder commitment tx holding a few overlaps with the
 		// old state.
 		let mut monitor = ChannelMonitor::new(keys,
-			&PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
-			(OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
-			&PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
-			&PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
-			10, Script::new(), 46, 0, HolderCommitmentTransaction::dummy());
+		                                      &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
+		                                      (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
+		                                      &channel_parameters,
+		                                      Script::new(), 46, 0,
+		                                      HolderCommitmentTransaction::dummy());
 
 		monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap();
 		monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);

lightning/src/chain/keysinterface.rs

@@ -33,7 +33,7 @@ use util::ser::{Writeable, Writer, Readable};
 
 use chain::transaction::OutPoint;
 use ln::chan_utils;
-use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, PreCalculatedTxCreationKeys};
+use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransactionForSigning, PreCalculatedTxCreationKeys};
 use ln::msgs::UnsignedChannelAnnouncement;
 
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -242,14 +242,14 @@ 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
-	fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
+	fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransactionForSigning, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
 
 	/// Same as sign_holder_commitment, but exists only for tests to get access to holder commitment
 	/// transactions which will be broadcasted later, after the channel has moved on to a newer
 	/// state. Thus, needs its own method as sign_holder_commitment may enforce that we only ever
 	/// get called once.
 	#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
-	fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
+	fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransactionForSigning, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
 
 	/// Create a signature for each HTLC transaction spending a holder's commitment transaction.
 	///
@@ -264,7 +264,7 @@ pub trait ChannelKeys : Send+Clone {
 	/// (implying they were considered dust at the time the commitment transaction was negotiated),
 	/// a corresponding None should be included in the return value. All other positions in the
 	/// return value must contain a signature.
-	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()>;
+	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransactionForSigning, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()>;
 
 	/// Create a signature for the given input in a transaction spending an HTLC or commitment
 	/// transaction output when our counterparty broadcasts an old state.
@@ -499,7 +499,7 @@ impl ChannelKeys for InMemoryChannelKeys {
 		Ok((commitment_sig, htlc_sigs))
 	}
 
-	fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+	fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransactionForSigning, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let counterparty_channel_data = self.accepted_channel_data.as_ref().expect("must accept before signing");
 		let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_channel_data.counterparty_channel_pubkeys.funding_pubkey);
@@ -508,15 +508,15 @@ impl ChannelKeys for InMemoryChannelKeys {
 	}
 
 	#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
-	fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+	fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransactionForSigning, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let counterparty_channel_pubkeys = &self.accepted_channel_data.as_ref().expect("must accept before signing").counterparty_channel_pubkeys;
 		let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_channel_pubkeys.funding_pubkey);
 
 		Ok(holder_commitment_tx.get_holder_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
 	}
 
-	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
+	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransactionForSigning, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
 		let counterparty_selected_contest_delay = self.accepted_channel_data.as_ref().unwrap().counterparty_selected_contest_delay;
 		holder_commitment_tx.get_htlc_sigs(&self.htlc_base_key, counterparty_selected_contest_delay, secp_ctx)
 	}