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.  The ChannelKeys API is modified accordingly.

By regenerating the transaction when implementing a validating external signer, this allows 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.

Eliminate ChannelStaticData in favor of ChannelTransactionParameters.

Use counterparty txid instead of tx in channelmonitor update.

Author: devrandom

lightning/src/chain/channelmonitor.rs

@@ -32,13 +32,12 @@ use util::ser::{Writeable, ReadableArgs, Readable};
 use util::config::UserConfig;
 
 use bitcoin::hashes::sha256d::Hash as Sha256dHash;
-use bitcoin::hashes::HashEngine;
-use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
+use bitcoin::hash_types::{Txid, BlockHash};
 use bitcoin::util::bip143;
 use bitcoin::util::address::Address;
 use bitcoin::util::bip32::{ChildNumber, ExtendedPubKey, ExtendedPrivKey};
 use bitcoin::blockdata::block::{Block, BlockHeader};
-use bitcoin::blockdata::transaction::{Transaction, TxOut, TxIn, SigHashType, OutPoint as BitcoinOutPoint};
+use bitcoin::blockdata::transaction::{Transaction, TxOut, TxIn, SigHashType};
 use bitcoin::blockdata::script::{Builder, Script};
 use bitcoin::blockdata::opcodes;
 use bitcoin::blockdata::constants::genesis_block;
@@ -59,7 +58,7 @@ use std::sync::atomic::Ordering;
 use std::mem;
 
 use ln::functional_test_utils::*;
-use ln::chan_utils::PreCalculatedTxCreationKeys;
+use ln::chan_utils::CommitmentTransaction;
 
 #[test]
 fn test_insane_channel_opens() {
@@ -1617,20 +1616,20 @@ fn test_fee_spike_violation_fails_htlc() {
 
 	// Get the EnforcingChannelKeys for each channel, which will be used to (1) get the keys
 	// needed to sign the new commitment tx and (2) sign the new commitment tx.
-	let (local_revocation_basepoint, local_htlc_basepoint, local_payment_point, local_secret, local_secret2) = {
+	let (local_revocation_basepoint, local_htlc_basepoint, local_secret, local_secret2) = {
 		let chan_lock = nodes[0].node.channel_state.lock().unwrap();
 		let local_chan = chan_lock.by_id.get(&chan.2).unwrap();
 		let chan_keys = local_chan.get_keys();
 		let pubkeys = chan_keys.pubkeys();
-		(pubkeys.revocation_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
+		(pubkeys.revocation_basepoint, pubkeys.htlc_basepoint,
 		 chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER), chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2))
 	};
-	let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_payment_point, remote_secret1) = {
+	let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_secret1) = {
 		let chan_lock = nodes[1].node.channel_state.lock().unwrap();
 		let remote_chan = chan_lock.by_id.get(&chan.2).unwrap();
 		let chan_keys = remote_chan.get_keys();
 		let pubkeys = chan_keys.pubkeys();
-		(pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
+		(pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint,
 		 chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1))
 	};
 
@@ -1643,70 +1642,31 @@ fn test_fee_spike_violation_fails_htlc() {
 	// Build the remote commitment transaction so we can sign it, and then later use the
 	// signature for the commitment_signed message.
 	let local_chan_balance = 1313;
-	let static_payment_pk = local_payment_point.serialize();
-	let remote_commit_tx_output = TxOut {
-				script_pubkey: Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
-				                             .push_slice(&WPubkeyHash::hash(&static_payment_pk)[..])
-				                             .into_script(),
-		value: local_chan_balance as u64
-	};
-
-	let local_commit_tx_output = TxOut {
-		script_pubkey: chan_utils::get_revokeable_redeemscript(&commit_tx_keys.revocation_key,
-				                                               BREAKDOWN_TIMEOUT,
-				                                               &commit_tx_keys.broadcaster_delayed_payment_key).to_v0_p2wsh(),
-				value: 95000,
-	};
 
 	let accepted_htlc_info = chan_utils::HTLCOutputInCommitment {
 		offered: false,
 		amount_msat: 3460001,
 		cltv_expiry: htlc_cltv,
-		payment_hash: payment_hash,
+		payment_hash,
 		transaction_output_index: Some(1),
 	};
 
-	let htlc_output = TxOut {
-		script_pubkey: chan_utils::get_htlc_redeemscript(&accepted_htlc_info, &commit_tx_keys).to_v0_p2wsh(),
-		value: 3460001 / 1000
-	};
-
-	let commit_tx_obscure_factor = {
-		let mut sha = Sha256::engine();
-		let remote_payment_point = &remote_payment_point.serialize();
-		sha.input(&local_payment_point.serialize());
-		sha.input(remote_payment_point);
-		let res = Sha256::from_engine(sha).into_inner();
-
-		((res[26] as u64) << 5*8) |
-		((res[27] as u64) << 4*8) |
-		((res[28] as u64) << 3*8) |
-		((res[29] as u64) << 2*8) |
-		((res[30] as u64) << 1*8) |
-		((res[31] as u64) << 0*8)
-	};
-	let commitment_number = 1;
-	let obscured_commitment_transaction_number = commit_tx_obscure_factor ^ commitment_number;
-	let lock_time = ((0x20 as u32) << 8*3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32);
-	let input = TxIn {
-		previous_output: BitcoinOutPoint { txid: chan.3.txid(), vout: 0 },
-		script_sig: Script::new(),
-		sequence: ((0x80 as u32) << 8*3) | ((obscured_commitment_transaction_number >> 3*8) as u32),
-		witness: Vec::new(),
-	};
+	let commitment_number = INITIAL_COMMITMENT_NUMBER - 1;
 
-	let commit_tx = Transaction {
-		version: 2,
-		lock_time,
-		input: vec![input],
-		output: vec![remote_commit_tx_output, htlc_output, local_commit_tx_output],
-	};
 	let res = {
 		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_keys();
-		let pre_commit_tx_keys = PreCalculatedTxCreationKeys::new(commit_tx_keys);
-		local_chan_keys.sign_counterparty_commitment(feerate_per_kw, &commit_tx, &pre_commit_tx_keys, &[&accepted_htlc_info], &secp_ctx).unwrap()
+		let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data(
+			commitment_number,
+			95000,
+			local_chan_balance,
+			commit_tx_keys.clone(),
+			feerate_per_kw,
+			&mut vec![(accepted_htlc_info, ())],
+			&local_chan.channel_transaction_parameters.as_counterparty_broadcastable()
+		);
+		local_chan_keys.sign_counterparty_commitment(&commitment_tx, &secp_ctx).unwrap()
 	};
 
 	let commit_signed_msg = msgs::CommitmentSigned {

lightning/src/chain/keysinterface.rs

@@ -9,7 +9,7 @@
 
 //! The logic to build claims and bump in-flight transactions until confirmations.
 //!
-//! OnchainTxHandler objetcs are fully-part of ChannelMonitor and encapsulates all
+//! OnchainTxHandler objects are fully-part of ChannelMonitor and encapsulates all
 //! building, tracking, bumping and notifications functions.
 
 use bitcoin::blockdata::transaction::{Transaction, TxIn, TxOut, SigHashType};
@@ -24,7 +24,7 @@ use bitcoin::secp256k1;
 use ln::msgs::DecodeError;
 use ln::channelmanager::PaymentPreimage;
 use ln::chan_utils;
-use ln::chan_utils::{TxCreationKeys, HolderCommitmentTransaction};
+use ln::chan_utils::{TxCreationKeys, ChannelTransactionParameters, HolderCommitmentTransaction};
 use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
 use chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest};
 use chain::keysinterface::ChannelKeys;
@@ -244,14 +244,13 @@ pub struct OnchainTxHandler<ChanSigner: ChannelKeys> {
 	holder_commitment: Option<HolderCommitmentTransaction>,
 	// holder_htlc_sigs and prev_holder_htlc_sigs are in the order as they appear in the commitment
 	// transaction outputs (hence the Option<>s inside the Vec). The first usize is the index in
-	// the set of HTLCs in the HolderCommitmentTransaction (including those which do not appear in
-	// the commitment transaction).
+	// the set of HTLCs in the HolderCommitmentTransaction.
 	holder_htlc_sigs: Option<Vec<Option<(usize, Signature)>>>,
 	prev_holder_commitment: Option<HolderCommitmentTransaction>,
 	prev_holder_htlc_sigs: Option<Vec<Option<(usize, Signature)>>>,
-	on_holder_tx_csv: u16,
 
 	key_storage: ChanSigner,
+	pub(crate) channel_transaction_parameters: ChannelTransactionParameters,
 
 	// Used to track claiming requests. If claim tx doesn't confirm before height timer expiration we need to bump
 	// it (RBF or CPFP). If an input has been part of an aggregate tx at first claim try, we need to keep it within
@@ -295,9 +294,8 @@ impl<ChanSigner: ChannelKeys + Writeable> OnchainTxHandler<ChanSigner> {
 		self.prev_holder_commitment.write(writer)?;
 		self.prev_holder_htlc_sigs.write(writer)?;
 
-		self.on_holder_tx_csv.write(writer)?;
-
 		self.key_storage.write(writer)?;
+		self.channel_transaction_parameters.write(writer)?;
 
 		writer.write_all(&byte_utils::be64_to_array(self.pending_claim_requests.len() as u64))?;
 		for (ref ancestor_claim_txid, claim_tx_data) in self.pending_claim_requests.iter() {
@@ -344,9 +342,8 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
 		let prev_holder_commitment = Readable::read(reader)?;
 		let prev_holder_htlc_sigs = Readable::read(reader)?;
 
-		let on_holder_tx_csv = Readable::read(reader)?;
-
 		let key_storage = Readable::read(reader)?;
+		let channel_parameters = Readable::read(reader)?;
 
 		let pending_claim_requests_len: u64 = Readable::read(reader)?;
 		let mut pending_claim_requests = HashMap::with_capacity(cmp::min(pending_claim_requests_len as usize, MAX_ALLOC_SIZE / 128));
@@ -398,8 +395,8 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
 			holder_htlc_sigs,
 			prev_holder_commitment,
 			prev_holder_htlc_sigs,
-			on_holder_tx_csv,
 			key_storage,
+			channel_transaction_parameters: channel_parameters,
 			claimable_outpoints,
 			pending_claim_requests,
 			onchain_events_waiting_threshold_conf,
@@ -410,7 +407,7 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
 }
 
 impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
-	pub(crate) fn new(destination_script: Script, keys: ChanSigner, on_holder_tx_csv: u16) -> Self {
+	pub(crate) fn new(destination_script: Script, keys: ChanSigner, channel_parameters: ChannelTransactionParameters) -> Self {
 
 		let key_storage = keys;
 
@@ -420,8 +417,8 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 			holder_htlc_sigs: None,
 			prev_holder_commitment: None,
 			prev_holder_htlc_sigs: None,
-			on_holder_tx_csv,
 			key_storage,
+			channel_transaction_parameters: channel_parameters,
 			pending_claim_requests: HashMap::new(),
 			claimable_outpoints: HashMap::new(),
 			onchain_events_waiting_threshold_conf: HashMap::new(),
@@ -654,7 +651,7 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 						let signed_tx = self.get_fully_signed_holder_tx(funding_redeemscript).unwrap();
 						// Timer set to $NEVER given we can't bump tx without anchor outputs
 						log_trace!(logger, "Going to broadcast Holder Transaction {} claiming funding output {} from {}...", signed_tx.txid(), outp.vout, outp.txid);
-						return Some((None, self.holder_commitment.as_ref().unwrap().feerate_per_kw, signed_tx));
+						return Some((None, self.holder_commitment.as_ref().unwrap().feerate_per_kw(), signed_tx));
 					}
 					_ => unreachable!()
 				}
@@ -899,44 +896,39 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 	fn sign_latest_holder_htlcs(&mut self) {
 		if let Some(ref holder_commitment) = self.holder_commitment {
 			if let Ok(sigs) = self.key_storage.sign_holder_commitment_htlc_transactions(holder_commitment, &self.secp_ctx) {
-				self.holder_htlc_sigs = Some(Vec::new());
-				let ret = self.holder_htlc_sigs.as_mut().unwrap();
-				for (htlc_idx, (holder_sig, &(ref htlc, _))) in sigs.iter().zip(holder_commitment.per_htlc.iter()).enumerate() {
-					if let Some(tx_idx) = htlc.transaction_output_index {
-						if ret.len() <= tx_idx as usize { ret.resize(tx_idx as usize + 1, None); }
-						ret[tx_idx as usize] = Some((htlc_idx, holder_sig.expect("Did not receive a signature for a non-dust HTLC")));
-					} else {
-						assert!(holder_sig.is_none(), "Received a signature for a dust HTLC");
-					}
-				}
+				self.holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
 			}
 		}
 	}
+
 	fn sign_prev_holder_htlcs(&mut self) {
 		if let Some(ref holder_commitment) = self.prev_holder_commitment {
 			if let Ok(sigs) = self.key_storage.sign_holder_commitment_htlc_transactions(holder_commitment, &self.secp_ctx) {
-				self.prev_holder_htlc_sigs = Some(Vec::new());
-				let ret = self.prev_holder_htlc_sigs.as_mut().unwrap();
-				for (htlc_idx, (holder_sig, &(ref htlc, _))) in sigs.iter().zip(holder_commitment.per_htlc.iter()).enumerate() {
-					if let Some(tx_idx) = htlc.transaction_output_index {
-						if ret.len() <= tx_idx as usize { ret.resize(tx_idx as usize + 1, None); }
-						ret[tx_idx as usize] = Some((htlc_idx, holder_sig.expect("Did not receive a signature for a non-dust HTLC")));
-					} else {
-						assert!(holder_sig.is_none(), "Received a signature for a dust HTLC");
-					}
-				}
+				self.prev_holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
 			}
 		}
 	}
 
-	//TODO: getting lastest holder transactions should be infaillible and result in us "force-closing the channel", but we may
+	fn extract_holder_sigs(holder_commitment: &HolderCommitmentTransaction, sigs: Vec<Signature>) -> Vec<Option<(usize, Signature)>> {
+		let mut ret = Vec::new();
+		for (htlc_idx, (holder_sig, htlc)) in sigs.iter().zip(holder_commitment.htlcs().iter()).enumerate() {
+			let tx_idx = htlc.transaction_output_index.unwrap();
+			if ret.len() <= tx_idx as usize { ret.resize(tx_idx as usize + 1, None); }
+			ret[tx_idx as usize] = Some((htlc_idx, holder_sig.clone()));
+		}
+		ret
+	}
+
+	//TODO: getting lastest holder transactions should be infallible and result in us "force-closing the channel", but we may
 	// have empty holder commitment transaction if a ChannelMonitor is asked to force-close just after Channel::get_outbound_funding_created,
 	// before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing
 	// to monitor before.
 	pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Option<Transaction> {
 		if let Some(ref mut holder_commitment) = self.holder_commitment {
-			match self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx) {
-				Ok(sig) => Some(holder_commitment.add_holder_sig(funding_redeemscript, sig)),
+			match self.key_storage.sign_holder_commitment(&holder_commitment, &self.secp_ctx) {
+				Ok(sig) => {
+					Some(holder_commitment.add_holder_sig(funding_redeemscript, sig))
+				},
 				Err(_) => return None,
 			}
 		} else {
@@ -947,9 +939,10 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 	#[cfg(any(test, feature="unsafe_revoked_tx_signing"))]
 	pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Option<Transaction> {
 		if let Some(ref mut holder_commitment) = self.holder_commitment {
-			let holder_commitment = holder_commitment.clone();
-			match self.key_storage.sign_holder_commitment(&holder_commitment, &self.secp_ctx) {
-				Ok(sig) => Some(holder_commitment.add_holder_sig(funding_redeemscript, sig)),
+			match self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx) {
+				Ok(sig) => {
+					Some(holder_commitment.add_holder_sig(funding_redeemscript, sig))
+				},
 				Err(_) => return None,
 			}
 		} else {
@@ -960,24 +953,30 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 	pub(crate) fn get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option<PaymentPreimage>) -> Option<Transaction> {
 		let mut htlc_tx = None;
 		if self.holder_commitment.is_some() {
-			let commitment_txid = self.holder_commitment.as_ref().unwrap().txid();
+			let commitment_txid = self.holder_commitment.as_ref().unwrap().trust().txid();
 			if commitment_txid == outp.txid {
 				self.sign_latest_holder_htlcs();
 				if let &Some(ref htlc_sigs) = &self.holder_htlc_sigs {
 					let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap();
-					htlc_tx = Some(self.holder_commitment.as_ref().unwrap()
-						.get_signed_htlc_tx(*htlc_idx, htlc_sig, preimage, self.on_holder_tx_csv));
+					let holder_commitment = self.holder_commitment.as_ref().unwrap();
+					let trusted_tx = holder_commitment.trust();
+					let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[*htlc_idx];
+					htlc_tx = Some(trusted_tx
+						.get_signed_htlc_tx(&self.channel_transaction_parameters.as_holder_broadcastable(), *htlc_idx, &counterparty_htlc_sig, htlc_sig, preimage));
 				}
 			}
 		}
 		if self.prev_holder_commitment.is_some() {
-			let commitment_txid = self.prev_holder_commitment.as_ref().unwrap().txid();
+			let commitment_txid = self.prev_holder_commitment.as_ref().unwrap().trust().txid();
 			if commitment_txid == outp.txid {
 				self.sign_prev_holder_htlcs();
 				if let &Some(ref htlc_sigs) = &self.prev_holder_htlc_sigs {
 					let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap();
-					htlc_tx = Some(self.prev_holder_commitment.as_ref().unwrap()
-						.get_signed_htlc_tx(*htlc_idx, htlc_sig, preimage, self.on_holder_tx_csv));
+					let holder_commitment = self.prev_holder_commitment.as_ref().unwrap();
+					let trusted_tx = holder_commitment.trust();
+					let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[*htlc_idx];
+					htlc_tx = Some(trusted_tx
+						.get_signed_htlc_tx(&self.channel_transaction_parameters.as_holder_broadcastable(), *htlc_idx, &counterparty_htlc_sig, htlc_sig, preimage));
 				}
 			}
 		}