Integrate OnchainTxHandler::HTLCache inside LocalCommitmentTransaction

Provide a safe API from which to withdraw local HTLC transaction built
against a parent local commitment transaction all in one place.

Author: Antoine Riard

lightning/src/chain/keysinterface.rs

@@ -2,7 +2,7 @@
 //! spendable on-chain outputs which the user owns and is responsible for using just as any other
 //! on-chain output which is theirs.
 
-use bitcoin::blockdata::transaction::{Transaction, OutPoint, TxOut, SigHashType};
+use bitcoin::blockdata::transaction::{Transaction, OutPoint, TxOut};
 use bitcoin::blockdata::script::{Script, Builder};
 use bitcoin::blockdata::opcodes;
 use bitcoin::network::constants::Network;
@@ -225,7 +225,7 @@ pub trait ChannelKeys : Send+Clone {
 
 	/// Signs a transaction created by build_htlc_transaction. If the transaction is an
 	/// HTLC-Success transaction (ie htlc.offered is false), preimage must be set!
-	fn sign_htlc_transaction<T: secp256k1::Signing>(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option<PaymentPreimage>, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<T>);
+	fn sign_htlc_transaction<T: secp256k1::Signing>(&self, local_commitment_tx: &mut LocalCommitmentTransaction, htlc_index: u32, preimage: Option<PaymentPreimage>, local_csv: u16, secp_ctx: &Secp256k1<T>);
 
 	/// Create a signature for a (proposed) closing transaction.
 	///
@@ -358,38 +358,8 @@ impl ChannelKeys for InMemoryChannelKeys {
 		local_commitment_tx.add_local_sig(&self.funding_key, funding_redeemscript, channel_value_satoshis, secp_ctx);
 	}
 
-	fn sign_htlc_transaction<T: secp256k1::Signing>(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option<PaymentPreimage>, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1<T>) {
-		if htlc_tx.input.len() != 1 { return; }
-		if htlc_tx.input[0].witness.len() != 0 { return; }
-
-		let htlc_redeemscript = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, a_htlc_key, b_htlc_key, revocation_key);
-
-		if let Ok(our_htlc_key) = chan_utils::derive_private_key(secp_ctx, per_commitment_point, &self.htlc_base_key) {
-			let sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
-			let local_tx = PublicKey::from_secret_key(&secp_ctx, &our_htlc_key) == *a_htlc_key;
-			let our_sig = secp_ctx.sign(&sighash, &our_htlc_key);
-
-			htlc_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
-
-			if local_tx { // b, then a
-				htlc_tx.input[0].witness.push(their_sig.serialize_der().to_vec());
-				htlc_tx.input[0].witness.push(our_sig.serialize_der().to_vec());
-			} else {
-				htlc_tx.input[0].witness.push(our_sig.serialize_der().to_vec());
-				htlc_tx.input[0].witness.push(their_sig.serialize_der().to_vec());
-			}
-			htlc_tx.input[0].witness[1].push(SigHashType::All as u8);
-			htlc_tx.input[0].witness[2].push(SigHashType::All as u8);
-
-			if htlc.offered {
-				htlc_tx.input[0].witness.push(Vec::new());
-				assert!(preimage.is_none());
-			} else {
-				htlc_tx.input[0].witness.push(preimage.unwrap().0.to_vec());
-			}
-
-			htlc_tx.input[0].witness.push(htlc_redeemscript.as_bytes().to_vec());
-		} else { return; }
+	fn sign_htlc_transaction<T: secp256k1::Signing>(&self, local_commitment_tx: &mut LocalCommitmentTransaction, htlc_index: u32, preimage: Option<PaymentPreimage>, local_csv: u16, secp_ctx: &Secp256k1<T>) {
+		local_commitment_tx.add_htlc_sig(&self.htlc_base_key, htlc_index, preimage, local_csv, secp_ctx);
 	}
 
 	fn sign_closing_transaction<T: secp256k1::Signing>(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {

lightning/src/ln/chan_utils.rs

@@ -14,7 +14,7 @@ use bitcoin_hashes::ripemd160::Hash as Ripemd160;
 use bitcoin_hashes::hash160::Hash as Hash160;
 use bitcoin_hashes::sha256d::Hash as Sha256dHash;
 
-use ln::channelmanager::PaymentHash;
+use ln::channelmanager::{PaymentHash, PaymentPreimage};
 use ln::msgs::DecodeError;
 use util::ser::{Readable, Writeable, Writer, WriterWriteAdaptor};
 use util::byte_utils;
@@ -23,6 +23,11 @@ use secp256k1::key::{SecretKey, PublicKey};
 use secp256k1::{Secp256k1, Signature};
 use secp256k1;
 
+use std::collections::HashMap;
+use std::cmp;
+
+const MAX_ALLOC_SIZE: usize = 64*1024;
+
 pub(super) const HTLC_SUCCESS_TX_WEIGHT: u64 = 703;
 pub(super) const HTLC_TIMEOUT_TX_WEIGHT: u64 = 663;
 
@@ -480,7 +485,11 @@ pub fn build_htlc_transaction(prev_hash: &Sha256dHash, feerate_per_kw: u64, to_s
 /// to broadcast. Eventually this will require a signer which is possibly external, but for now we
 /// just pass in the SecretKeys required.
 pub struct LocalCommitmentTransaction {
-	tx: Transaction
+	tx: Transaction,
+	//TODO: modify Channel methods to integrate HTLC material at LocalCommitmentTransaction generation to drop Option here
+	local_keys: Option<TxCreationKeys>,
+	feerate_per_kw: Option<u64>,
+	per_htlc: HashMap<u32, (HTLCOutputInCommitment, Option<Signature>, Option<Transaction>)>
 }
 impl LocalCommitmentTransaction {
 	#[cfg(test)]
@@ -495,7 +504,11 @@ impl LocalCommitmentTransaction {
 			input: vec![dummy_input],
 			output: Vec::new(),
 			lock_time: 0,
-		} }
+		},
+			local_keys: None,
+			feerate_per_kw: None,
+			per_htlc: HashMap::new()
+		}
 	}
 
 	/// Generate a new LocalCommitmentTransaction based on a raw transaction, both parties funding pubkeys and remote signatures
@@ -515,7 +528,11 @@ impl LocalCommitmentTransaction {
 			tx.input[0].witness.push(Vec::new());
 		}
 
-		Self { tx }
+		Self { tx,
+			local_keys: None,
+			feerate_per_kw: None,
+			per_htlc: HashMap::new()
+		}
 	}
 
 	/// Expose transaction id of LocalCommitmentTransaction
@@ -563,6 +580,63 @@ impl LocalCommitmentTransaction {
 		assert!(self.has_local_sig());
 		&self.tx
 	}
+
+	/// Set HTLC cache to generate any local HTLC transaction spending one of htlc ouput
+	/// from this local commitment transaction
+	pub fn set_htlc_cache(&mut self, local_keys: TxCreationKeys, feerate_per_kw: u64, htlc_outputs: HashMap<u32, (HTLCOutputInCommitment, Option<Signature>, Option<Transaction>)>) {
+		self.local_keys = Some(local_keys);
+		self.feerate_per_kw = Some(feerate_per_kw);
+		self.per_htlc = htlc_outputs;
+	}
+
+	/// Add local signature for a htlc transaction, do nothing if a cached signed transaction is
+	/// already present
+	pub fn add_htlc_sig<T: secp256k1::Signing>(&mut self, htlc_base_key: &SecretKey, htlc_index: u32, preimage: Option<PaymentPreimage>, local_csv: u16, secp_ctx: &Secp256k1<T>) {
+		if self.local_keys.is_none() || self.feerate_per_kw.is_none() { return; }
+		let local_keys = self.local_keys.as_ref().unwrap();
+		let txid = self.txid();
+		if let Some(ref mut this_htlc) = self.per_htlc.get_mut(&htlc_index) {
+			if this_htlc.2.is_some() { return; } // we already have a cached htlc transaction at provided index
+			let mut htlc_tx = build_htlc_transaction(&txid, self.feerate_per_kw.unwrap(), local_csv, &this_htlc.0, &local_keys.a_delayed_payment_key, &local_keys.revocation_key);
+			if !this_htlc.0.offered && preimage.is_none() { return; } // if we don't have preimage for HTLC-Success, don't try to generate
+			let htlc_secret = if !this_htlc.0.offered { preimage } else { None }; // if we have a preimage for HTLC-Timeout, don't use it that's likely a duplicate HTLC hash
+			if this_htlc.1.is_none() { return; } // we don't have any remote signature for this htlc
+			if htlc_tx.input.len() != 1 { return; }
+			if htlc_tx.input[0].witness.len() != 0 { return; }
+
+			let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc.0, &local_keys.a_htlc_key, &local_keys.b_htlc_key, &local_keys.revocation_key);
+
+			if let Ok(our_htlc_key) = derive_private_key(secp_ctx, &local_keys.per_commitment_point, htlc_base_key) {
+				let sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, this_htlc.0.amount_msat / 1000)[..]);
+				let our_sig = secp_ctx.sign(&sighash, &our_htlc_key);
+
+				htlc_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
+
+				htlc_tx.input[0].witness.push(this_htlc.1.unwrap().serialize_der().to_vec());
+				htlc_tx.input[0].witness.push(our_sig.serialize_der().to_vec());
+				htlc_tx.input[0].witness[1].push(SigHashType::All as u8);
+				htlc_tx.input[0].witness[2].push(SigHashType::All as u8);
+
+				if this_htlc.0.offered {
+					htlc_tx.input[0].witness.push(Vec::new());
+					assert!(htlc_secret.is_none());
+				} else {
+					htlc_tx.input[0].witness.push(htlc_secret.unwrap().0.to_vec());
+				}
+
+				htlc_tx.input[0].witness.push(htlc_redeemscript.as_bytes().to_vec());
+
+				this_htlc.2 = Some(htlc_tx);
+			} else { return; }
+		}
+	}
+	/// Expose raw htlc transaction, guarante witness is complete if non-empty
+	pub fn htlc_with_valid_witness(&self, htlc_index: u32) -> &Option<Transaction> {
+		if let Some(ref this_htlc) = self.per_htlc.get(&htlc_index) {
+			return &this_htlc.2;
+		}
+		&None
+	}
 }
 impl PartialEq for LocalCommitmentTransaction {
 	// We dont care whether we are signed in equality comparison
@@ -578,6 +652,14 @@ impl Writeable for LocalCommitmentTransaction {
 				_ => panic!("local tx must have been well-formed!"),
 			}
 		}
+		self.local_keys.write(writer)?;
+		self.feerate_per_kw.write(writer)?;
+		writer.write_all(&byte_utils::be64_to_array(self.per_htlc.len() as u64))?;
+		for (ref _index, &(ref htlc, ref sig, ref htlc_tx)) in self.per_htlc.iter() {
+			htlc.write(writer)?;
+			sig.write(writer)?;
+			htlc_tx.write(writer)?;
+		}
 		Ok(())
 	}
 }
@@ -590,12 +672,27 @@ impl Readable for LocalCommitmentTransaction {
 				_ => return Err(DecodeError::InvalidValue),
 			},
 		};
+		let local_keys = Readable::read(reader)?;
+		let feerate_per_kw = Readable::read(reader)?;
+		let htlcs_count: u64 = Readable::read(reader)?;
+		let mut per_htlc = HashMap::with_capacity(cmp::min(htlcs_count as usize, MAX_ALLOC_SIZE / 32));
+		for _ in 0..htlcs_count {
+			let htlc: HTLCOutputInCommitment = Readable::read(reader)?;
+			let sigs = Readable::read(reader)?;
+			let htlc_tx = Readable::read(reader)?;
+			per_htlc.insert(htlc.transaction_output_index.unwrap(), (htlc, sigs, htlc_tx));
+		}
 
 		if tx.input.len() != 1 {
 			// Ensure tx didn't hit the 0-input ambiguity case.
 			return Err(DecodeError::InvalidValue);
 		}
-		Ok(Self { tx })
+		Ok(Self {
+			tx,
+			local_keys,
+			feerate_per_kw,
+			per_htlc,
+		})
 	}
 }
 

lightning/src/ln/channel.rs

@@ -4299,6 +4299,7 @@ mod tests {
 	use bitcoin_hashes::hash160::Hash as Hash160;
 	use bitcoin_hashes::Hash;
 	use std::sync::Arc;
+	use std::collections::HashMap;
 	use rand::{thread_rng,Rng};
 
 	struct TestFeeEstimator {
@@ -4475,6 +4476,7 @@ mod tests {
 
 		let mut unsigned_tx: (Transaction, Vec<HTLCOutputInCommitment>);
 
+		let mut localtx;
 		macro_rules! test_commitment {
 			( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => {
 				unsigned_tx = {
@@ -4489,7 +4491,7 @@ mod tests {
 				let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0.input[0], &redeemscript, chan.channel_value_satoshis)[..]).unwrap();
 				secp_ctx.verify(&sighash, &their_signature, chan.their_funding_pubkey()).unwrap();
 
-				let mut localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), &their_signature, &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey());
+				localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), &their_signature, &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey());
 				chan_keys.sign_local_commitment(&mut localtx, &redeemscript, chan.channel_value_satoshis, &chan.secp_ctx);
 
 				assert_eq!(serialize(localtx.with_valid_witness())[..],
@@ -4498,11 +4500,11 @@ mod tests {
 		}
 
 		macro_rules! test_htlc_output {
-			( $htlc_idx: expr, $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr ) => {
+			( $htlc_idx: expr, $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => {
 				let remote_signature = Signature::from_der(&hex::decode($their_sig_hex).unwrap()[..]).unwrap();
 
 				let ref htlc = unsigned_tx.1[$htlc_idx];
-				let mut htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys, chan.feerate_per_kw);
+				let htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys, chan.feerate_per_kw);
 				let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
 				let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
 				secp_ctx.verify(&htlc_sighash, &remote_signature, &keys.b_htlc_key).unwrap();
@@ -4519,8 +4521,12 @@ mod tests {
 					assert!(preimage.is_some());
 				}
 
-				chan_keys.sign_htlc_transaction(&mut htlc_tx, &remote_signature, &preimage, &htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key, &keys.per_commitment_point, &chan.secp_ctx);
-				assert_eq!(serialize(&htlc_tx)[..],
+				let mut per_htlc = HashMap::new();
+				per_htlc.insert($htlc_idx, (htlc.clone(), Some(remote_signature), None));
+				localtx.set_htlc_cache(keys.clone(), chan.feerate_per_kw, per_htlc);
+				chan_keys.sign_htlc_transaction(&mut localtx, $htlc_idx, preimage, chan.their_to_self_delay, &chan.secp_ctx);
+
+				assert_eq!(serialize(&localtx.htlc_with_valid_witness($htlc_idx).clone().unwrap())[..],
 						hex::decode($tx_hex).unwrap()[..]);
 			};
 		}

lightning/src/ln/channelmonitor.rs

@@ -1251,7 +1251,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// is important that any clones of this channel monitor (including remote clones) by kept
 	/// up-to-date as our local commitment transaction is updated.
 	/// Panics if set_their_to_self_delay has never been called.
-	pub(super) fn provide_latest_local_commitment_tx_info(&mut self, commitment_tx: LocalCommitmentTransaction, local_keys: chan_utils::TxCreationKeys, feerate_per_kw: u64, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
+	pub(super) fn provide_latest_local_commitment_tx_info(&mut self, mut commitment_tx: LocalCommitmentTransaction, local_keys: chan_utils::TxCreationKeys, feerate_per_kw: u64, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
 		if self.their_to_self_delay.is_none() {
 			return Err(MonitorUpdateError("Got a local commitment tx info update before we'd set basic information about the channel"));
 		}
@@ -1268,7 +1268,14 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			htlc_outputs: htlc_outputs.clone(),
 		});
 		self.current_local_commitment_number = 0xffff_ffff_ffff - ((((commitment_tx.without_valid_witness().input[0].sequence as u64 & 0xffffff) << 3*8) | (commitment_tx.without_valid_witness().lock_time as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
-		self.onchain_tx_handler.provide_latest_local_tx(commitment_tx, local_keys, feerate_per_kw, htlc_outputs);
+		let mut htlcs = HashMap::with_capacity(htlc_outputs.len());
+		for htlc in htlc_outputs {
+			if let Some(htlc_index) = htlc.0.transaction_output_index {
+				htlcs.insert(htlc_index as u32, (htlc.0, htlc.1, None));
+			}
+		}
+		commitment_tx.set_htlc_cache(local_keys, feerate_per_kw, htlcs);
+		self.onchain_tx_handler.provide_latest_local_tx(commitment_tx);
 		Ok(())
 	}