Merge pull request #742 from lightning-signer/tx-phase2

Introduce CommitmentTransactionInfo

Author: TheBlueMatt

lightning/Cargo.toml

@@ -12,6 +12,7 @@ Still missing tons of error-handling. See GitHub issues for suggested projects i
 
 [features]
 fuzztarget = ["bitcoin/fuzztarget"]
+# Internal test utilities exposed to other repo crates
 _test_utils = ["hex", "regex"]
 # Unlog messages superior at targeted level.
 max_level_off = []

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));
 				}
 			}
 		}