From: Antoine Riard Date: Mon, 9 Mar 2020 22:15:35 +0000 (-0400) Subject: Move HTLC tx generation in OnchainTxHandler X-Git-Tag: v0.0.12~84^2~4 X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=1107ab06c33bd360bdee7ee64f4b690e753003f6;p=rust-lightning Move HTLC tx generation in OnchainTxHandler HTLC Transaction can't be bumped without sighash changes so their gneeration is one-time for nwo. We move them in OnchainTxHandler for simplifying ChannelMonitor and to prepare storage of keys material behind one external signer interface. Some tests break due to change in transaction broadcaster order. Number of transactions may vary because of temporary anti-duplicata tweak can't dissociate between 2- broadcast from different origins (ChannelMonitor, ChannelManager) and 2-broadcast from same component. --- diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index e3e2d9294..58bdd7226 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/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; @@ -235,8 +235,7 @@ pub trait ChannelKeys : Send+Clone { /// Signs a transaction created by build_htlc_transaction. If the transaction is an /// HTLC-Success transaction, preimage must be set! /// TODO: should be merged with sign_local_commitment as a slice of HTLC transactions to sign - fn sign_htlc_transaction(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1); - + fn sign_htlc_transaction(&self, local_commitment_tx: &mut LocalCommitmentTransaction, htlc_index: u32, preimage: Option, local_csv: u16, secp_ctx: &Secp256k1); /// Create a signature for a (proposed) closing transaction. /// /// Note that, due to rounding, there may be one "missing" satoshi, and either party may have @@ -373,38 +372,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(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1) { - 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(&self, local_commitment_tx: &mut LocalCommitmentTransaction, htlc_index: u32, preimage: Option, local_csv: u16, secp_ctx: &Secp256k1) { + local_commitment_tx.add_htlc_sig(&self.htlc_base_key, htlc_index, preimage, local_csv, secp_ctx); } fn sign_closing_transaction(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1) -> Result { diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 4be3b3151..566fd7b00 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/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,10 @@ use secp256k1::key::{SecretKey, PublicKey}; use secp256k1::{Secp256k1, Signature}; use secp256k1; +use std::{cmp, mem}; + +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 +484,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, + feerate_per_kw: Option, + per_htlc: Vec<(HTLCOutputInCommitment, Option, Option)> } impl LocalCommitmentTransaction { #[cfg(test)] @@ -499,7 +507,11 @@ impl LocalCommitmentTransaction { input: vec![dummy_input], output: Vec::new(), lock_time: 0, - } } + }, + local_keys: None, + feerate_per_kw: None, + per_htlc: Vec::new() + } } /// Generate a new LocalCommitmentTransaction based on a raw commitment transaction, @@ -520,7 +532,11 @@ impl LocalCommitmentTransaction { tx.input[0].witness.push(Vec::new()); } - Self { tx } + Self { tx, + local_keys: None, + feerate_per_kw: None, + per_htlc: Vec::new() + } } /// Get the txid of the local commitment transaction contained in this @@ -577,6 +593,67 @@ 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(crate) fn set_htlc_cache(&mut self, local_keys: TxCreationKeys, feerate_per_kw: u64, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>) { + 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(&mut self, htlc_base_key: &SecretKey, htlc_index: u32, preimage: Option, local_csv: u16, secp_ctx: &Secp256k1) { + 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(); + for this_htlc in self.per_htlc.iter_mut() { + if this_htlc.0.transaction_output_index.unwrap() == 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 { + for this_htlc in self.per_htlc.iter() { + if this_htlc.0.transaction_output_index.unwrap() == htlc_index { + return &this_htlc.2; + } + } + &None + } } impl PartialEq for LocalCommitmentTransaction { // We dont care whether we are signed in equality comparison @@ -592,6 +669,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 htlc, ref sig, ref htlc_tx) in self.per_htlc.iter() { + htlc.write(writer)?; + sig.write(writer)?; + htlc_tx.write(writer)?; + } Ok(()) } } @@ -604,12 +689,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 = Vec::with_capacity(cmp::min(htlcs_count as usize, MAX_ALLOC_SIZE / mem::size_of::<(HTLCOutputInCommitment, Option, Option)>())); + for _ in 0..htlcs_count { + let htlc: HTLCOutputInCommitment = Readable::read(reader)?; + let sigs = Readable::read(reader)?; + let htlc_tx = Readable::read(reader)?; + per_htlc.push((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, + }) } } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 815280017..47936ea61 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4475,6 +4475,7 @@ mod tests { let mut unsigned_tx: (Transaction, Vec); + let mut localtx; macro_rules! test_commitment { ( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => { unsigned_tx = { @@ -4489,7 +4490,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 +4499,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 +4520,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 = Vec::new(); + per_htlc.push((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).as_ref().unwrap())[..], hex::decode($tx_hex).unwrap()[..]); }; } diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index f4f03abc3..d180d3bbd 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -409,7 +409,6 @@ impl PartialEq for OnchainDetection { struct LocalSignedTx { /// txid of the transaction in tx, just used to make comparison faster txid: Sha256dHash, - tx: LocalCommitmentTransaction, revocation_key: PublicKey, a_htlc_key: PublicKey, b_htlc_key: PublicKey, @@ -439,8 +438,6 @@ pub(crate) enum InputMaterial { locktime: u32, }, LocalHTLC { - witness_script: Script, - sigs: (Signature, Signature), preimage: Option, amount: u64, }, @@ -468,11 +465,8 @@ impl Writeable for InputMaterial { writer.write_all(&byte_utils::be64_to_array(*amount))?; writer.write_all(&byte_utils::be32_to_array(*locktime))?; }, - &InputMaterial::LocalHTLC { ref witness_script, ref sigs, ref preimage, ref amount } => { + &InputMaterial::LocalHTLC { ref preimage, ref amount } => { writer.write_all(&[2; 1])?; - witness_script.write(writer)?; - sigs.0.write(writer)?; - sigs.1.write(writer)?; preimage.write(writer)?; writer.write_all(&byte_utils::be64_to_array(*amount))?; }, @@ -517,16 +511,11 @@ impl Readable for InputMaterial { } }, 2 => { - let witness_script = Readable::read(reader)?; - let their_sig = Readable::read(reader)?; - let our_sig = Readable::read(reader)?; let preimage = Readable::read(reader)?; let amount = Readable::read(reader)?; InputMaterial::LocalHTLC { - witness_script, - sigs: (their_sig, our_sig), preimage, - amount + amount, } }, 3 => { @@ -970,7 +959,7 @@ impl ChannelMonitor { macro_rules! serialize_local_tx { ($local_tx: expr) => { - $local_tx.tx.write(writer)?; + $local_tx.txid.write(writer)?; writer.write_all(&$local_tx.revocation_key.serialize())?; writer.write_all(&$local_tx.a_htlc_key.serialize())?; writer.write_all(&$local_tx.b_htlc_key.serialize())?; @@ -1261,23 +1250,32 @@ impl ChannelMonitor { /// 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, Option)>) -> 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, Option)>) -> 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")); } + let txid = commitment_tx.txid(); + let sequence = commitment_tx.without_valid_witness().input[0].sequence as u64; + let locktime = commitment_tx.without_valid_witness().lock_time as u64; + let mut htlcs = Vec::with_capacity(htlc_outputs.len()); + for htlc in htlc_outputs.clone() { + if let Some(_) = htlc.0.transaction_output_index { + htlcs.push((htlc.0, htlc.1, None)); + } + } + commitment_tx.set_htlc_cache(local_keys.clone(), feerate_per_kw, htlcs); // Returning a monitor error before updating tracking points means in case of using // a concurrent watchtower implementation for same channel, if this one doesn't // reject update as we do, you MAY have the latest local valid commitment tx onchain // for which you want to spend outputs. We're NOT robust again this scenario right // now but we should consider it later. - if let Err(_) = self.onchain_tx_handler.provide_latest_local_tx(commitment_tx.clone(), local_keys.clone(), feerate_per_kw, htlc_outputs.clone()) { + if let Err(_) = self.onchain_tx_handler.provide_latest_local_tx(commitment_tx) { return Err(MonitorUpdateError("Local commitment signed has already been signed, no further update of LOCAL commitment transaction is allowed")); } - 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.current_local_commitment_number = 0xffff_ffff_ffff - ((((sequence & 0xffffff) << 3*8) | (locktime as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor); self.prev_local_signed_commitment_tx = self.current_local_signed_commitment_tx.take(); self.current_local_signed_commitment_tx = Some(LocalSignedTx { - txid: commitment_tx.txid(), - tx: commitment_tx, + txid, revocation_key: local_keys.revocation_key, a_htlc_key: local_keys.a_htlc_key, b_htlc_key: local_keys.b_htlc_key, @@ -1687,8 +1685,8 @@ impl ChannelMonitor { (claimable_outpoints, Some((htlc_txid, tx.output.clone()))) } - fn broadcast_by_local_state(&self, commitment_tx: &Transaction, local_tx: &LocalSignedTx) -> (Vec, Vec, Option<(Script, SecretKey, Script)>) { - let mut res = Vec::with_capacity(local_tx.htlc_outputs.len()); + fn broadcast_by_local_state(&self, commitment_tx: &Transaction, local_tx: &LocalSignedTx) -> (Vec, Vec, Option<(Script, SecretKey, Script)>) { + let mut claim_requests = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut watch_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); let redeemscript = chan_utils::get_revokeable_redeemscript(&local_tx.revocation_key, self.their_to_self_delay.unwrap(), &local_tx.delayed_payment_key); @@ -1696,40 +1694,23 @@ impl ChannelMonitor { Some((redeemscript.to_v0_p2wsh(), local_delayedkey, redeemscript)) } else { None }; - for &(ref htlc, ref sigs, _) in local_tx.htlc_outputs.iter() { + for &(ref htlc, _, _) in local_tx.htlc_outputs.iter() { if let Some(transaction_output_index) = htlc.transaction_output_index { - if let &Some(ref their_sig) = sigs { - if htlc.offered { - log_trace!(self, "Broadcasting HTLC-Timeout transaction against local commitment transactions"); - let mut htlc_timeout_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key); - self.onchain_detection.keys.sign_htlc_transaction(&mut htlc_timeout_tx, their_sig, &None, htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.secp_ctx); - - log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_timeout_tx.input[0].previous_output.vout, htlc_timeout_tx.input[0].previous_output.txid); - res.push(htlc_timeout_tx); - } else { - if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { - log_trace!(self, "Broadcasting HTLC-Success transaction against local commitment transactions"); - let mut htlc_success_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key); - self.onchain_detection.keys.sign_htlc_transaction(&mut htlc_success_tx, their_sig, &Some(*payment_preimage), htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.secp_ctx); - - log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_success_tx.input[0].previous_output.vout, htlc_success_tx.input[0].previous_output.txid); - res.push(htlc_success_tx); - } - } - watch_outputs.push(commitment_tx.output[transaction_output_index as usize].clone()); - } else { panic!("Should have sigs for non-dust local tx outputs!") } + let preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.payment_hash) { Some(*preimage) } else { None }; + claim_requests.push(ClaimRequest { absolute_timelock: ::std::u32::MAX, aggregable: false, outpoint: BitcoinOutPoint { txid: local_tx.txid, vout: transaction_output_index as u32 }, witness_data: InputMaterial::LocalHTLC { preimage, amount: htlc.amount_msat / 1000 }}); + watch_outputs.push(commitment_tx.output[transaction_output_index as usize].clone()); } } - (res, watch_outputs, broadcasted_local_revokable_script) + (claim_requests, watch_outputs, broadcasted_local_revokable_script) } /// Attempts to claim any claimable HTLCs in a commitment transaction which was not (yet) /// revoked using data in local_claimable_outpoints. /// Should not be used if check_spend_revoked_transaction succeeds. - fn check_spend_local_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, (Sha256dHash, Vec)) { + fn check_spend_local_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, (Sha256dHash, Vec)) { let commitment_txid = tx.txid(); - let mut local_txn = Vec::new(); + let mut claim_requests = Vec::new(); let mut watch_outputs = Vec::new(); macro_rules! wait_threshold_conf { @@ -1757,7 +1738,7 @@ impl ChannelMonitor { macro_rules! append_onchain_update { ($updates: expr) => { - local_txn.append(&mut $updates.0); + claim_requests = $updates.0; watch_outputs.append(&mut $updates.1); self.broadcasted_local_revokable_script = $updates.2; } @@ -1804,7 +1785,7 @@ impl ChannelMonitor { } } - (local_txn, (commitment_txid, watch_outputs)) + (claim_requests, (commitment_txid, watch_outputs)) } /// Used by ChannelManager deserialization to broadcast the latest local state if its copy of @@ -1898,14 +1879,11 @@ impl ChannelMonitor { watch_outputs.push(new_outputs); } if new_outpoints.is_empty() { - let (local_txn, new_outputs) = self.check_spend_local_transaction(&tx, height); - for tx in local_txn.iter() { - log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); - broadcaster.broadcast_transaction(tx); - } + let (mut new_outpoints, new_outputs) = self.check_spend_local_transaction(&tx, height); if !new_outputs.1.is_empty() { watch_outputs.push(new_outputs); } + claimable_outpoints.append(&mut new_outpoints); } claimable_outpoints.append(&mut new_outpoints); } @@ -1935,14 +1913,11 @@ impl ChannelMonitor { if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx { if should_broadcast { if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx(self.channel_value_satoshis.unwrap()) { - let (txs, new_outputs, _) = self.broadcast_by_local_state(&commitment_tx, cur_local_tx); + let (mut new_outpoints, new_outputs, _) = self.broadcast_by_local_state(&commitment_tx, cur_local_tx); if !new_outputs.is_empty() { watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); } - for tx in txs { - log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); - broadcaster.broadcast_transaction(&tx); - } + claimable_outpoints.append(&mut new_outpoints); } } } @@ -2393,7 +2368,7 @@ impl ReadableArgs> for (Sha256dH macro_rules! read_local_tx { () => { { - let tx = ::read(reader)?; + let txid = Readable::read(reader)?; let revocation_key = Readable::read(reader)?; let a_htlc_key = Readable::read(reader)?; let b_htlc_key = Readable::read(reader)?; @@ -2414,8 +2389,7 @@ impl ReadableArgs> for (Sha256dH } LocalSignedTx { - txid: tx.txid(), - tx, + txid, revocation_key, a_htlc_key, b_htlc_key, delayed_payment_key, per_commitment_point, feerate_per_kw, htlc_outputs: htlcs } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 0c6bac59c..d45469194 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2342,25 +2342,25 @@ fn claim_htlc_outputs_single_tx() { // ChannelMonitor: local commitment + local HTLC-timeout (2) // Check the pair local commitment and HTLC-timeout broadcast due to HTLC expiration + assert_eq!(node_txn[2].input.len(), 1); + check_spends!(node_txn[2], chan_1.3); assert_eq!(node_txn[3].input.len(), 1); - check_spends!(node_txn[3], chan_1.3); - assert_eq!(node_txn[0].input.len(), 1); - let witness_script = node_txn[0].input[0].witness.last().unwrap(); + let witness_script = node_txn[3].input[0].witness.last().unwrap(); assert_eq!(witness_script.len(), OFFERED_HTLC_SCRIPT_WEIGHT); //Spending an offered htlc output - check_spends!(node_txn[0], node_txn[3]); + check_spends!(node_txn[3], node_txn[2]); // Justice transactions are indices 1-2-4 + assert_eq!(node_txn[0].input.len(), 1); assert_eq!(node_txn[1].input.len(), 1); - assert_eq!(node_txn[2].input.len(), 1); assert_eq!(node_txn[4].input.len(), 1); + check_spends!(node_txn[0], revoked_local_txn[0]); check_spends!(node_txn[1], revoked_local_txn[0]); - check_spends!(node_txn[2], revoked_local_txn[0]); check_spends!(node_txn[4], revoked_local_txn[0]); let mut witness_lens = BTreeSet::new(); + witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len()); witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len()); - witness_lens.insert(node_txn[2].input[0].witness.last().unwrap().len()); witness_lens.insert(node_txn[4].input[0].witness.last().unwrap().len()); assert_eq!(witness_lens.len(), 3); assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local @@ -2612,19 +2612,18 @@ fn test_htlc_on_chain_timeout() { { let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); assert_eq!(node_txn.len(), 5); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 2 (local commitment tx + HTLC-timeout), 1 timeout tx + assert_eq!(node_txn[1], node_txn[3]); + assert_eq!(node_txn[2], node_txn[4]); - assert_eq!(node_txn[2], node_txn[3]); - assert_eq!(node_txn[0], node_txn[4]); - - check_spends!(node_txn[1], commitment_tx[0]); - assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); + check_spends!(node_txn[0], commitment_tx[0]); + assert_eq!(node_txn[0].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); - check_spends!(node_txn[2], chan_2.3); - check_spends!(node_txn[0], node_txn[2]); - assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), 71); - assert_eq!(node_txn[0].clone().input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); + check_spends!(node_txn[1], chan_2.3); + check_spends!(node_txn[2], node_txn[1]); + assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), 71); + assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); - timeout_tx = node_txn[1].clone(); + timeout_tx = node_txn[0].clone(); node_txn.clear(); } @@ -4354,8 +4353,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() { check_added_monitors!(nodes[0], 1); let revoked_htlc_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(revoked_htlc_txn.len(), 3); - assert_eq!(revoked_htlc_txn[0], revoked_htlc_txn[2]); + assert_eq!(revoked_htlc_txn.len(), 2); assert_eq!(revoked_htlc_txn[0].input.len(), 1); assert_eq!(revoked_htlc_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]); @@ -4411,8 +4409,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() { check_added_monitors!(nodes[1], 1); let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(revoked_htlc_txn.len(), 3); - assert_eq!(revoked_htlc_txn[0], revoked_htlc_txn[2]); + assert_eq!(revoked_htlc_txn.len(), 2); assert_eq!(revoked_htlc_txn[0].input.len(), 1); assert_eq!(revoked_htlc_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]); @@ -7118,7 +7115,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { check_added_monitors!(nodes[1], 1); let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(revoked_htlc_txn.len(), 6); + assert_eq!(revoked_htlc_txn.len(), 4); if revoked_htlc_txn[0].input[0].witness.last().unwrap().len() == ACCEPTED_HTLC_SCRIPT_WEIGHT { assert_eq!(revoked_htlc_txn[0].input.len(), 1); check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]); @@ -7376,11 +7373,11 @@ fn test_set_outpoints_partial_claiming() { let partial_claim_tx = { let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); assert_eq!(node_txn.len(), 3); - check_spends!(node_txn[0], node_txn[2]); - check_spends!(node_txn[1], node_txn[2]); - assert_eq!(node_txn[0].input.len(), 1); + check_spends!(node_txn[1], node_txn[0]); + check_spends!(node_txn[2], node_txn[0]); assert_eq!(node_txn[1].input.len(), 1); - node_txn[0].clone() + assert_eq!(node_txn[2].input.len(), 1); + node_txn[1].clone() }; // Broadcast partial claim on node A, should regenerate a claiming tx with HTLC dropped diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 8cb9bfb39..b2115c0f8 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -10,21 +10,20 @@ use bitcoin::util::bip143; use bitcoin_hashes::sha256d::Hash as Sha256dHash; -use secp256k1::{Secp256k1, Signature}; +use secp256k1::Secp256k1; use secp256k1; use ln::msgs::DecodeError; use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest}; -use ln::channelmanager::{HTLCSource, PaymentPreimage}; -use ln::chan_utils; -use ln::chan_utils::{HTLCType, LocalCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment}; +use ln::channelmanager::PaymentPreimage; +use ln::chan_utils::{HTLCType, LocalCommitmentTransaction}; use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT}; use chain::keysinterface::ChannelKeys; use util::logger::Logger; use util::ser::{ReadableArgs, Readable, Writer, Writeable}; use util::byte_utils; -use std::collections::{HashMap, hash_map, HashSet}; +use std::collections::{HashMap, hash_map}; use std::sync::Arc; use std::cmp; use std::ops::Deref; @@ -49,15 +48,6 @@ enum OnchainEvent { } } -/// Cache public keys and feerate used to compute any HTLC transaction. -/// We only keep state for latest 2 commitment transactions as we should -/// never have to generate HTLC txn for revoked local commitment -struct HTLCTxCache { - local_keys: TxCreationKeys, - feerate_per_kw: u64, - per_htlc: HashMap)> -} - /// Higher-level cache structure needed to re-generate bumped claim txn if needed #[derive(Clone, PartialEq)] pub struct ClaimTxBumpMaterial { @@ -155,8 +145,6 @@ pub struct OnchainTxHandler { funding_redeemscript: Script, local_commitment: Option, prev_local_commitment: Option, - current_htlc_cache: Option, - prev_htlc_cache: Option, local_csv: u16, key_storage: ChanSigner, @@ -201,36 +189,6 @@ impl OnchainTxHandler { self.local_commitment.write(writer)?; self.prev_local_commitment.write(writer)?; - macro_rules! serialize_htlc_cache { - ($cache: expr) => { - $cache.local_keys.write(writer)?; - $cache.feerate_per_kw.write(writer)?; - writer.write_all(&byte_utils::be64_to_array($cache.per_htlc.len() as u64))?; - for (_, &(ref htlc, ref sig)) in $cache.per_htlc.iter() { - htlc.write(writer)?; - if let &Some(ref their_sig) = sig { - 1u8.write(writer)?; - writer.write_all(&their_sig.serialize_compact())?; - } else { - 0u8.write(writer)?; - } - } - } - } - - if let Some(ref current) = self.current_htlc_cache { - writer.write_all(&[1; 1])?; - serialize_htlc_cache!(current); - } else { - writer.write_all(&[0; 1])?; - } - - if let Some(ref prev) = self.prev_htlc_cache { - writer.write_all(&[1; 1])?; - serialize_htlc_cache!(prev); - } else { - writer.write_all(&[0; 1])?; - } self.local_csv.write(writer)?; self.key_storage.write(writer)?; @@ -274,49 +232,10 @@ impl ReadableArgs> for OnchainTx fn read(reader: &mut R, logger: Arc) -> Result { let destination_script = Readable::read(reader)?; let funding_redeemscript = Readable::read(reader)?; + let local_commitment = Readable::read(reader)?; let prev_local_commitment = Readable::read(reader)?; - macro_rules! read_htlc_cache { - () => { - { - 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 = match ::read(reader)? { - 0 => None, - 1 => Some(Readable::read(reader)?), - _ => return Err(DecodeError::InvalidValue), - }; - per_htlc.insert(htlc.transaction_output_index.unwrap(), (htlc, sigs)); - } - HTLCTxCache { - local_keys, - feerate_per_kw, - per_htlc - } - } - } - } - - let current_htlc_cache = match ::read(reader)? { - 0 => None, - 1 => { - Some(read_htlc_cache!()) - } - _ => return Err(DecodeError::InvalidValue), - }; - - let prev_htlc_cache = match ::read(reader)? { - 0 => None, - 1 => { - Some(read_htlc_cache!()) - } - _ => return Err(DecodeError::InvalidValue), - }; let local_csv = Readable::read(reader)?; let key_storage = Readable::read(reader)?; @@ -369,8 +288,6 @@ impl ReadableArgs> for OnchainTx funding_redeemscript, local_commitment, prev_local_commitment, - current_htlc_cache, - prev_htlc_cache, local_csv, key_storage, claimable_outpoints, @@ -392,8 +309,6 @@ impl OnchainTxHandler { funding_redeemscript, local_commitment: None, prev_local_commitment: None, - current_htlc_cache: None, - prev_htlc_cache: None, local_csv, key_storage, pending_claim_requests: HashMap::new(), @@ -451,7 +366,7 @@ impl OnchainTxHandler { /// Lightning security model (i.e being able to redeem/timeout HTLC or penalize coutnerparty onchain) lays on the assumption of claim transactions getting confirmed before timelock expiration /// (CSV or CLTV following cases). In case of high-fee spikes, claim tx may stuck in the mempool, so you need to bump its feerate quickly using Replace-By-Fee or Child-Pay-For-Parent. - fn generate_claim_tx(&self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F) -> Option<(Option, u64, Transaction)> + fn generate_claim_tx(&mut self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F) -> Option<(Option, u64, Transaction)> where F::Target: FeeEstimator { if cached_claim_datas.per_input_material.len() == 0 { return None } // But don't prune pending claiming request yet, we may have to resurrect HTLCs @@ -530,13 +445,14 @@ impl OnchainTxHandler { inputs_witnesses_weight += Self::get_witnesses_weight(if preimage.is_some() { &[InputDescriptors::OfferedHTLC] } else { &[InputDescriptors::ReceivedHTLC] }); amt += *amount; }, - &InputMaterial::LocalHTLC { .. } => { return None; } + &InputMaterial::LocalHTLC { .. } => { + dynamic_fee = false; + }, &InputMaterial::Funding { .. } => { dynamic_fee = false; } } } - if dynamic_fee { let predicted_weight = bumped_tx.get_weight() + inputs_witnesses_weight; let mut new_feerate; @@ -598,16 +514,31 @@ impl OnchainTxHandler { } else { for (_, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() { match per_outp_material { - &InputMaterial::LocalHTLC { .. } => { - //TODO : Given that Local Commitment Transaction and HTLC-Timeout/HTLC-Success are counter-signed by peer, we can't - // RBF them. Need a Lightning specs change and package relay modification : - // https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html + &InputMaterial::LocalHTLC { ref preimage, ref amount } => { + let mut htlc_tx = None; + if let Some(ref mut local_commitment) = self.local_commitment { + if local_commitment.txid() == outp.txid { + self.key_storage.sign_htlc_transaction(local_commitment, outp.vout, *preimage, self.local_csv, &self.secp_ctx); + htlc_tx = local_commitment.htlc_with_valid_witness(outp.vout).clone(); + } + } + if let Some(ref mut prev_local_commitment) = self.prev_local_commitment { + if prev_local_commitment.txid() == outp.txid { + self.key_storage.sign_htlc_transaction(prev_local_commitment, outp.vout, *preimage, self.local_csv, &self.secp_ctx); + htlc_tx = prev_local_commitment.htlc_with_valid_witness(outp.vout).clone(); + } + } + if let Some(htlc_tx) = htlc_tx { + let feerate = (amount - htlc_tx.output[0].value) * 1000 / htlc_tx.get_weight() as u64; + // Timer set to $NEVER given we can't bump tx without anchor outputs + log_trace!(self, "Going to broadcast Local HTLC-{} claiming HTLC output {} from {}...", if preimage.is_some() { "Success" } else { "Timeout" }, outp.vout, outp.txid); + return Some((None, feerate, htlc_tx)); + } return None; }, &InputMaterial::Funding { ref channel_value } => { - if self.local_commitment.is_some() { - let mut local_commitment = self.local_commitment.clone().unwrap(); - self.key_storage.sign_local_commitment(&mut local_commitment, &self.funding_redeemscript, *channel_value, &self.secp_ctx); + if let Some(ref mut local_commitment) = self.local_commitment { + self.key_storage.sign_local_commitment(local_commitment, &self.funding_redeemscript, *channel_value, &self.secp_ctx); let signed_tx = local_commitment.with_valid_witness().clone(); let mut amt_outputs = 0; for outp in signed_tx.output.iter() { @@ -673,7 +604,7 @@ impl OnchainTxHandler { } } - let mut bump_candidates = HashSet::new(); + let mut bump_candidates = HashMap::new(); for tx in txn_matched { // Scan all input to verify is one of the outpoint spent is of interest for us let mut claimed_outputs_material = Vec::new(); @@ -730,7 +661,7 @@ impl OnchainTxHandler { } //TODO: recompute soonest_timelock to avoid wasting a bit on fees if at_least_one_drop { - bump_candidates.insert(first_claim_txid_height.0.clone()); + bump_candidates.insert(first_claim_txid_height.0.clone(), claim_material.clone()); } } break; //No need to iterate further, either tx is our or their @@ -778,27 +709,25 @@ impl OnchainTxHandler { for (first_claim_txid, ref claim_data) in self.pending_claim_requests.iter() { if let Some(h) = claim_data.height_timer { if h == height { - bump_candidates.insert(*first_claim_txid); + bump_candidates.insert(*first_claim_txid, (*claim_data).clone()); } } } // Build, bump and rebroadcast tx accordingly log_trace!(self, "Bumping {} candidates", bump_candidates.len()); - for first_claim_txid in bump_candidates.iter() { - if let Some((new_timer, new_feerate)) = { - if let Some(claim_material) = self.pending_claim_requests.get(first_claim_txid) { - if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &claim_material, &*fee_estimator) { - log_trace!(self, "Broadcast onchain {}", log_tx!(bump_tx)); - broadcaster.broadcast_transaction(&bump_tx); - Some((new_timer, new_feerate)) - } else { None } - } else { unreachable!(); } - } { - if let Some(claim_material) = self.pending_claim_requests.get_mut(first_claim_txid) { - claim_material.height_timer = new_timer; - claim_material.feerate_previous = new_feerate; - } else { unreachable!(); } + let mut pending_claim_updates = Vec::with_capacity(bump_candidates.len()); + for (first_claim_txid, claim_material) in bump_candidates.iter() { + if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &claim_material, &*fee_estimator) { + log_trace!(self, "Broadcast onchain {}", log_tx!(bump_tx)); + broadcaster.broadcast_transaction(&bump_tx); + pending_claim_updates.push((*first_claim_txid, new_timer, new_feerate)); + } + } + for updates in pending_claim_updates { + if let Some(claim_material) = self.pending_claim_requests.get_mut(&updates.0) { + claim_material.height_timer = updates.1; + claim_material.feerate_previous = updates.2; } } } @@ -850,7 +779,7 @@ impl OnchainTxHandler { } } - pub(super) fn provide_latest_local_tx(&mut self, tx: LocalCommitmentTransaction, local_keys: chan_utils::TxCreationKeys, feerate_per_kw: u64, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>) -> Result<(), ()> { + pub(super) fn provide_latest_local_tx(&mut self, tx: LocalCommitmentTransaction) -> Result<(), ()> { // To prevent any unsafe state discrepancy between offchain and onchain, once local // commitment transaction has been signed due to an event (either block height for // HTLC-timeout or channel force-closure), don't allow any further update of local @@ -861,18 +790,6 @@ impl OnchainTxHandler { } self.prev_local_commitment = self.local_commitment.take(); self.local_commitment = Some(tx); - self.prev_htlc_cache = self.current_htlc_cache.take(); - let mut per_htlc = HashMap::with_capacity(htlc_outputs.len()); - for htlc in htlc_outputs { - if htlc.0.transaction_output_index.is_some() { // Discard dust HTLC as we will never have to generate onchain tx for them - per_htlc.insert(htlc.0.transaction_output_index.unwrap(), (htlc.0, htlc.1)); - } - } - self.current_htlc_cache = Some(HTLCTxCache { - local_keys, - feerate_per_kw, - per_htlc - }); Ok(()) } @@ -896,18 +813,10 @@ impl OnchainTxHandler { pub(super) fn get_fully_signed_htlc_tx(&mut self, txid: Sha256dHash, htlc_index: u32, preimage: Option) -> Option { //TODO: store preimage in OnchainTxHandler - if let Some(ref local_commitment) = self.local_commitment { + if let Some(ref mut local_commitment) = self.local_commitment { if local_commitment.txid() == txid { - if let Some(ref htlc_cache) = self.current_htlc_cache { - if let Some(htlc) = htlc_cache.per_htlc.get(&htlc_index) { - if !htlc.0.offered && preimage.is_none() { return None; }; // If we don't have preimage for HTLC-Success, don't try to generate - let htlc_secret = if !htlc.0.offered { preimage } else { None }; // If we have a preimage for a HTLC-Timeout, don't use it that's likely a duplicate HTLC hash - let mut htlc_tx = chan_utils::build_htlc_transaction(&txid, htlc_cache.feerate_per_kw, self.local_csv, &htlc.0, &htlc_cache.local_keys.a_delayed_payment_key, &htlc_cache.local_keys.revocation_key); - self.key_storage.sign_htlc_transaction(&mut htlc_tx, htlc.1.as_ref().unwrap(), &htlc_secret, &htlc.0, &htlc_cache.local_keys.a_htlc_key, &htlc_cache.local_keys.b_htlc_key, &htlc_cache.local_keys.revocation_key, &htlc_cache.local_keys.per_commitment_point, &self.secp_ctx); - return Some(htlc_tx); - - } - } + self.key_storage.sign_htlc_transaction(local_commitment, htlc_index, preimage, self.local_csv, &self.secp_ctx); + return local_commitment.htlc_with_valid_witness(htlc_index).clone(); } } None diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 5e3aba431..c48424f9d 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -89,8 +89,8 @@ impl ChannelKeys for EnforcingChannelKeys { self.inner.unsafe_sign_local_commitment(local_commitment_tx, funding_redeemscript, channel_value_satoshis, secp_ctx); } - fn sign_htlc_transaction(&self, htlc_tx: &mut Transaction, their_sig: &Signature, preimage: &Option, htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1) { - self.inner.sign_htlc_transaction(htlc_tx, their_sig, preimage, htlc, a_htlc_key, b_htlc_key, revocation_key, per_commitment_point, secp_ctx); + fn sign_htlc_transaction(&self, local_commitment_tx: &mut LocalCommitmentTransaction, htlc_index: u32, preimage: Option, local_csv: u16, secp_ctx: &Secp256k1) { + self.inner.sign_htlc_transaction(local_commitment_tx, htlc_index, preimage, local_csv, secp_ctx); } fn sign_closing_transaction(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1) -> Result {