From 7159d1546ae92281a7e0533813a6e7558f16354a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 19 Apr 2020 22:59:53 -0400 Subject: [PATCH] Batch-sign local HTLC txn with a well-doc'd API, returning sigs 1107ab06c33bd360bdee7ee64f4b690e753003f6 introduced an API to have a ChannelKeys implementer sign HTLC transactions by calling into the LocalCommitmentTransaction object, which would then store the tx. This API was incredibly awkward, both because it required an external signer trust our own internal interfaces, but also because it didn't allow for any inspection of what was about to be signed. Further, it signed the HTLC transactions one-by-one in a somewhat inefficient way, and there isn't a clear way to resolve this (as the which-HTLC parameter has to refer to something in between the HTLC's arbitrary index, and its index in the commitment tx, which has "holes" for the non-HTLC outputs and skips some HTLCs). We replace it with a new function in ChannelKeys which allows us to sign all HTLCs in a given commitment transaction (which allows for a bit more effeciency on the signers' part, as well as sidesteps the which-HTLC issue). This may also simplify the signer implementation as we will always want to sign all HTLCs spending a given commitment transaction at once anyway. We also de-mut the LocalCommitmentTransaction passed to the ChanKeys, instead opting to make LocalCommitmentTransaction const and avoid storing any new HTLC-related data in it. --- lightning/src/chain/keysinterface.rs | 24 ++- lightning/src/ln/chan_utils.rs | 112 +++++++------- lightning/src/ln/channel.rs | 18 ++- lightning/src/ln/channelmonitor.rs | 38 ++++- lightning/src/ln/onchaintx.rs | 154 +++++++++++++++++--- lightning/src/util/enforcing_trait_impls.rs | 25 +++- lightning/src/util/mod.rs | 6 +- 7 files changed, 278 insertions(+), 99 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 037748844..44f347abc 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -25,7 +25,6 @@ use util::ser::{Writeable, Writer, Readable}; use ln::chan_utils; use ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, LocalCommitmentTransaction}; -use ln::channelmanager::PaymentPreimage; use ln::msgs; use std::sync::Arc; @@ -231,10 +230,21 @@ pub trait ChannelKeys : Send+Clone { #[cfg(test)] fn unsafe_sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result; - /// 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, local_commitment_tx: &mut LocalCommitmentTransaction, htlc_index: u32, preimage: Option, local_csv: u16, secp_ctx: &Secp256k1); + /// Create a signature for each HTLC transaction spending a local commitment transaction. + /// + /// Unlike sign_local_commitment, this may be called multiple times with *different* + /// local_commitment_tx values. While this will never be called with a revoked + /// local_commitment_tx, it is possible that it is called with the second-latest + /// local_commitment_tx (only if we haven't yet revoked it) if some watchtower/secondary + /// ChannelMonitor decided to broadcast before it had been updated to the latest. + /// + /// Either an Err should be returned, or a Vec with one entry for each HTLC which exists in + /// local_commitment_tx. For those HTLCs which have transaction_output_index set to None + /// (implying they were considered dust at the time the commitment transaction was negotiated), + /// a corresponding None should be included in the return value. All other positions in the + /// return value must contain a signature. + fn sign_local_commitment_htlc_transactions(&self, local_commitment_tx: &LocalCommitmentTransaction, local_csv: u16, secp_ctx: &Secp256k1) -> Result>, ()>; + /// Create a signature for a (proposed) closing transaction. /// /// Note that, due to rounding, there may be one "missing" satoshi, and either party may have @@ -379,8 +389,8 @@ impl ChannelKeys for InMemoryChannelKeys { Ok(local_commitment_tx.get_local_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx)) } - 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_local_commitment_htlc_transactions(&self, local_commitment_tx: &LocalCommitmentTransaction, local_csv: u16, secp_ctx: &Secp256k1) -> Result>, ()> { + local_commitment_tx.get_htlc_sigs(&self.htlc_base_key, 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 b899fe494..f6cbdc23b 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -487,7 +487,7 @@ pub struct LocalCommitmentTransaction { tx: Transaction, pub(crate) local_keys: TxCreationKeys, pub(crate) feerate_per_kw: u64, - per_htlc: Vec<(HTLCOutputInCommitment, Option, Option)> + pub(crate) per_htlc: Vec<(HTLCOutputInCommitment, Option)>, } impl LocalCommitmentTransaction { #[cfg(test)] @@ -524,7 +524,7 @@ impl LocalCommitmentTransaction { /// Generate a new LocalCommitmentTransaction based on a raw commitment transaction, /// remote signature and both parties keys - pub(crate) fn new_missing_local_sig(mut tx: Transaction, their_sig: &Signature, our_funding_key: &PublicKey, their_funding_key: &PublicKey, local_keys: TxCreationKeys, feerate_per_kw: u64, mut htlc_data: Vec<(HTLCOutputInCommitment, Option)>) -> LocalCommitmentTransaction { + pub(crate) fn new_missing_local_sig(mut tx: Transaction, their_sig: &Signature, our_funding_key: &PublicKey, their_funding_key: &PublicKey, local_keys: TxCreationKeys, feerate_per_kw: u64, htlc_data: Vec<(HTLCOutputInCommitment, Option)>) -> LocalCommitmentTransaction { if tx.input.len() != 1 { panic!("Tried to store a commitment transaction that had input count != 1!"); } if tx.input[0].witness.len() != 0 { panic!("Tried to store a signed commitment transaction?"); } @@ -543,8 +543,7 @@ impl LocalCommitmentTransaction { Self { tx, local_keys, feerate_per_kw, - // TODO: Avoid the conversion of a Vec created likely just for this: - per_htlc: htlc_data.drain(..).map(|(a, b)| (a, b, None)).collect(), + per_htlc: htlc_data, } } @@ -606,55 +605,70 @@ impl LocalCommitmentTransaction { &self.tx } - /// 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) { + /// Get a signature for each HTLC which was included in the commitment transaction (ie for + /// which HTLCOutputInCommitment::transaction_output_index.is_some()). + /// + /// The returned Vec has one entry for each HTLC, and in the same order. For HTLCs which were + /// considered dust and not included, a None entry exists, for all others a signature is + /// included. + pub fn get_htlc_sigs(&self, htlc_base_key: &SecretKey, local_csv: u16, secp_ctx: &Secp256k1) -> Result>, ()> { let txid = self.txid(); - for this_htlc in self.per_htlc.iter_mut() { - if this_htlc.0.transaction_output_index == Some(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, local_csv, &this_htlc.0, &self.local_keys.a_delayed_payment_key, &self.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, &self.local_keys.a_htlc_key, &self.local_keys.b_htlc_key, &self.local_keys.revocation_key); - - if let Ok(our_htlc_key) = derive_private_key(secp_ctx, &self.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); + let mut ret = Vec::with_capacity(self.per_htlc.len()); + let our_htlc_key = derive_private_key(secp_ctx, &self.local_keys.per_commitment_point, htlc_base_key).map_err(|_| ())?; - 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()); - } + for this_htlc in self.per_htlc.iter() { + if this_htlc.0.transaction_output_index.is_some() { + let htlc_tx = build_htlc_transaction(&txid, self.feerate_per_kw, local_csv, &this_htlc.0, &self.local_keys.a_delayed_payment_key, &self.local_keys.revocation_key); + assert_eq!(htlc_tx.input.len(), 1); + assert_eq!(htlc_tx.input[0].witness.len(), 0); - htlc_tx.input[0].witness.push(htlc_redeemscript.as_bytes().to_vec()); + let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc.0, &self.local_keys.a_htlc_key, &self.local_keys.b_htlc_key, &self.local_keys.revocation_key); - this_htlc.2 = Some(htlc_tx); - } else { return; } + let sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, this_htlc.0.amount_msat / 1000)[..]); + ret.push(Some(secp_ctx.sign(&sighash, &our_htlc_key))); + } else { + ret.push(None); } } + Ok(ret) } - /// 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; - } + + /// Gets a signed HTLC transaction given a preimage (for !htlc.offered) and the local HTLC transaction signature. + pub(crate) fn get_signed_htlc_tx(&self, htlc_index: usize, signature: &Signature, preimage: &Option, local_csv: u16) -> Transaction { + let txid = self.txid(); + let this_htlc = &self.per_htlc[htlc_index]; + assert!(this_htlc.0.transaction_output_index.is_some()); + // if we don't have preimage for an HTLC-Success, we can't generate an HTLC transaction. + if !this_htlc.0.offered && preimage.is_none() { unreachable!(); } + // Further, we should never be provided the preimage for an HTLC-Timeout transaction. + if this_htlc.0.offered && preimage.is_some() { unreachable!(); } + + let mut htlc_tx = build_htlc_transaction(&txid, self.feerate_per_kw, local_csv, &this_htlc.0, &self.local_keys.a_delayed_payment_key, &self.local_keys.revocation_key); + // Channel should have checked that we have a remote signature for this HTLC at + // creation, and we should have a sensible htlc transaction: + assert!(this_htlc.1.is_some()); + assert_eq!(htlc_tx.input.len(), 1); + assert_eq!(htlc_tx.input[0].witness.len(), 0); + + let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc.0, &self.local_keys.a_htlc_key, &self.local_keys.b_htlc_key, &self.local_keys.revocation_key); + + // First push the multisig dummy, note that due to BIP147 (NULLDUMMY) it must be a zero-length element. + htlc_tx.input[0].witness.push(Vec::new()); + + htlc_tx.input[0].witness.push(this_htlc.1.unwrap().serialize_der().to_vec()); + htlc_tx.input[0].witness.push(signature.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 { + // Due to BIP146 (MINIMALIF) this must be a zero-length element to relay. + htlc_tx.input[0].witness.push(Vec::new()); + } else { + htlc_tx.input[0].witness.push(preimage.unwrap().0.to_vec()); } - &None + + htlc_tx.input[0].witness.push(htlc_redeemscript.as_bytes().to_vec()); + htlc_tx } } impl PartialEq for LocalCommitmentTransaction { @@ -674,10 +688,9 @@ impl Writeable for LocalCommitmentTransaction { 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() { + for &(ref htlc, ref sig) in self.per_htlc.iter() { htlc.write(writer)?; sig.write(writer)?; - htlc_tx.write(writer)?; } Ok(()) } @@ -694,12 +707,11 @@ impl Readable for LocalCommitmentTransaction { 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)>())); + let mut per_htlc = Vec::with_capacity(cmp::min(htlcs_count as usize, MAX_ALLOC_SIZE / mem::size_of::<(HTLCOutputInCommitment, 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)); + per_htlc.push((htlc, sigs)); } if tx.input.len() != 1 { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9ea859ecb..c5908906a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4495,7 +4495,7 @@ mod tests { macro_rules! test_commitment { ( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr, { $( { $htlc_idx: expr, $their_htlc_sig_hex: expr, $our_htlc_sig_hex: expr, $htlc_tx_hex: expr } ), * - } ) => { + } ) => { { unsigned_tx = { let mut res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, chan.feerate_per_kw); let htlcs = res.2.drain(..) @@ -4523,6 +4523,9 @@ mod tests { assert_eq!(serialize(localtx.with_valid_witness())[..], hex::decode($tx_hex).unwrap()[..]); + let htlc_sigs = chan_keys.sign_local_commitment_htlc_transactions(&localtx, chan.their_to_self_delay, &chan.secp_ctx).unwrap(); + let mut htlc_sig_iter = localtx.per_htlc.iter().zip(htlc_sigs.iter().enumerate()); + $({ let remote_signature = Signature::from_der(&hex::decode($their_htlc_sig_hex).unwrap()[..]).unwrap(); @@ -4544,12 +4547,19 @@ mod tests { assert!(preimage.is_some()); } - chan_keys.sign_htlc_transaction(&mut localtx, $htlc_idx, preimage, chan.their_to_self_delay, &chan.secp_ctx); + let mut htlc_sig = htlc_sig_iter.next().unwrap(); + while (htlc_sig.1).1.is_none() { htlc_sig = htlc_sig_iter.next().unwrap(); } + assert_eq!((htlc_sig.0).0.transaction_output_index, Some($htlc_idx)); - assert_eq!(serialize(localtx.htlc_with_valid_witness($htlc_idx).as_ref().unwrap())[..], + assert_eq!(serialize(&localtx.get_signed_htlc_tx((htlc_sig.1).0, &(htlc_sig.1).1.unwrap(), &preimage, chan.their_to_self_delay))[..], hex::decode($htlc_tx_hex).unwrap()[..]); })* - } + loop { + let htlc_sig = htlc_sig_iter.next(); + if htlc_sig.is_none() { break; } + assert!((htlc_sig.unwrap().1).1.is_none()); + } + } } } { diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 4987f8f3b..0dbf98c72 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -1677,8 +1677,18 @@ impl ChannelMonitor { for &(ref htlc, _, _) in local_tx.htlc_outputs.iter() { if let Some(transaction_output_index) = htlc.transaction_output_index { - 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 }}); + 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: if !htlc.offered { + if let Some(preimage) = self.payment_preimages.get(&htlc.payment_hash) { + Some(preimage.clone()) + } else { + // We can't build an HTLC-Success transaction without the preimage + continue; + } + } else { None }, + amount: htlc.amount_msat, + }}); watch_outputs.push(commitment_tx.output[transaction_output_index as usize].clone()); } } @@ -1780,9 +1790,15 @@ impl ChannelMonitor { let txid = commitment_tx.txid(); let mut res = vec![commitment_tx]; for htlc in self.current_local_commitment_tx.htlc_outputs.iter() { - if let Some(htlc_index) = htlc.0.transaction_output_index { - let preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(*preimage) } else { None }; - if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(txid, htlc_index, preimage) { + if let Some(vout) = htlc.0.transaction_output_index { + let preimage = if !htlc.0.offered { + if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else { + // We can't build an HTLC-Success transaction without the preimage + continue; + } + } else { None }; + if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx( + &::bitcoin::OutPoint { txid, vout }, &preimage) { res.push(htlc_tx); } } @@ -1804,9 +1820,15 @@ impl ChannelMonitor { let txid = commitment_tx.txid(); let mut res = vec![commitment_tx]; for htlc in self.current_local_commitment_tx.htlc_outputs.iter() { - if let Some(htlc_index) = htlc.0.transaction_output_index { - let preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(*preimage) } else { None }; - if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(txid, htlc_index, preimage) { + if let Some(vout) = htlc.0.transaction_output_index { + let preimage = if !htlc.0.offered { + if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else { + // We can't build an HTLC-Success transaction without the preimage + continue; + } + } else { None }; + if let Some(htlc_tx) = self.onchain_tx_handler.unsafe_get_fully_signed_htlc_tx( + &::bitcoin::OutPoint { txid, vout }, &preimage) { res.push(htlc_tx); } } diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 77ce1bdc4..6b5da67c1 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -10,7 +10,7 @@ use bitcoin::util::bip143; use bitcoin_hashes::sha256d::Hash as Sha256dHash; -use secp256k1::Secp256k1; +use secp256k1::{Secp256k1, Signature}; use secp256k1; use ln::msgs::DecodeError; @@ -137,13 +137,63 @@ macro_rules! subtract_high_prio_fee { } } +impl Readable for Option>> { + fn read(reader: &mut R) -> Result { + match Readable::read(reader)? { + 0u8 => Ok(None), + 1u8 => { + let vlen: u64 = Readable::read(reader)?; + let mut ret = Vec::with_capacity(cmp::min(vlen as usize, MAX_ALLOC_SIZE / ::std::mem::size_of::>())); + for _ in 0..vlen { + ret.push(match Readable::read(reader)? { + 0u8 => None, + 1u8 => Some((::read(reader)? as usize, Readable::read(reader)?)), + _ => return Err(DecodeError::InvalidValue) + }); + } + Ok(Some(ret)) + }, + _ => Err(DecodeError::InvalidValue), + } + } +} + +impl Writeable for Option>> { + fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { + match self { + &Some(ref vec) => { + 1u8.write(writer)?; + (vec.len() as u64).write(writer)?; + for opt in vec.iter() { + match opt { + &Some((ref idx, ref sig)) => { + 1u8.write(writer)?; + (*idx as u64).write(writer)?; + sig.write(writer)?; + }, + &None => 0u8.write(writer)?, + } + } + }, + &None => 0u8.write(writer)?, + } + Ok(()) + } +} + /// OnchainTxHandler receives claiming requests, aggregates them if it's sound, broadcast and /// do RBF bumping if possible. pub struct OnchainTxHandler { destination_script: Script, local_commitment: Option, + // local_htlc_sigs and prev_local_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 LocalCommitmentTransaction (including those which do not appear in + // the commitment transaction). + local_htlc_sigs: Option>>, prev_local_commitment: Option, + prev_local_htlc_sigs: Option>>, local_csv: u16, key_storage: ChanSigner, @@ -185,7 +235,9 @@ impl OnchainTxHandler { pub(crate) fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { self.destination_script.write(writer)?; self.local_commitment.write(writer)?; + self.local_htlc_sigs.write(writer)?; self.prev_local_commitment.write(writer)?; + self.prev_local_htlc_sigs.write(writer)?; self.local_csv.write(writer)?; @@ -231,7 +283,9 @@ impl ReadableArgs> for OnchainTx let destination_script = Readable::read(reader)?; let local_commitment = Readable::read(reader)?; + let local_htlc_sigs = Readable::read(reader)?; let prev_local_commitment = Readable::read(reader)?; + let prev_local_htlc_sigs = Readable::read(reader)?; let local_csv = Readable::read(reader)?; @@ -283,7 +337,9 @@ impl ReadableArgs> for OnchainTx Ok(OnchainTxHandler { destination_script, local_commitment, + local_htlc_sigs, prev_local_commitment, + prev_local_htlc_sigs, local_csv, key_storage, claimable_outpoints, @@ -303,7 +359,9 @@ impl OnchainTxHandler { OnchainTxHandler { destination_script, local_commitment: None, + local_htlc_sigs: None, prev_local_commitment: None, + prev_local_htlc_sigs: None, local_csv, key_storage, pending_claim_requests: HashMap::new(), @@ -510,19 +568,7 @@ impl OnchainTxHandler { for (_, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() { match per_outp_material { &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(); - } - } + let htlc_tx = self.get_fully_signed_htlc_tx(outp, preimage); 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 @@ -771,11 +817,47 @@ impl OnchainTxHandler { if let Some(ref local_commitment) = self.local_commitment { if local_commitment.has_local_sig() { return Err(()) } } + if self.local_htlc_sigs.is_some() || self.prev_local_htlc_sigs.is_some() { + return Err(()); + } self.prev_local_commitment = self.local_commitment.take(); self.local_commitment = Some(tx); Ok(()) } + fn sign_latest_local_htlcs(&mut self) { + if let Some(ref local_commitment) = self.local_commitment { + if let Ok(sigs) = self.key_storage.sign_local_commitment_htlc_transactions(local_commitment, self.local_csv, &self.secp_ctx) { + self.local_htlc_sigs = Some(Vec::new()); + let ret = self.local_htlc_sigs.as_mut().unwrap(); + for (htlc_idx, (local_sig, &(ref htlc, _))) in sigs.iter().zip(local_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, local_sig.expect("Did not receive a signature for a non-dust HTLC"))); + } else { + assert!(local_sig.is_none(), "Received a signature for a dust HTLC"); + } + } + } + } + } + fn sign_prev_local_htlcs(&mut self) { + if let Some(ref local_commitment) = self.prev_local_commitment { + if let Ok(sigs) = self.key_storage.sign_local_commitment_htlc_transactions(local_commitment, self.local_csv, &self.secp_ctx) { + self.prev_local_htlc_sigs = Some(Vec::new()); + let ret = self.prev_local_htlc_sigs.as_mut().unwrap(); + for (htlc_idx, (local_sig, &(ref htlc, _))) in sigs.iter().zip(local_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, local_sig.expect("Did not receive a signature for a non-dust HTLC"))); + } else { + assert!(local_sig.is_none(), "Received a signature for a dust HTLC"); + } + } + } + } + } + //TODO: getting lastest local transactions should be infaillible and result in us "force-closing the channel", but we may // have empty local 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 @@ -804,14 +886,44 @@ impl OnchainTxHandler { None } - 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 mut local_commitment) = self.local_commitment { - if local_commitment.txid() == txid { - 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(); + pub(super) fn get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option) -> Option { + let mut htlc_tx = None; + if self.local_commitment.is_some() { + let commitment_txid = self.local_commitment.as_ref().unwrap().txid(); + if commitment_txid == outp.txid { + self.sign_latest_local_htlcs(); + if let &Some(ref htlc_sigs) = &self.local_htlc_sigs { + let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap(); + htlc_tx = Some(self.local_commitment.as_ref().unwrap() + .get_signed_htlc_tx(*htlc_idx, htlc_sig, preimage, self.local_csv)); + } } } - None + if self.prev_local_commitment.is_some() { + let commitment_txid = self.prev_local_commitment.as_ref().unwrap().txid(); + if commitment_txid == outp.txid { + self.sign_prev_local_htlcs(); + if let &Some(ref htlc_sigs) = &self.prev_local_htlc_sigs { + let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap(); + htlc_tx = Some(self.prev_local_commitment.as_ref().unwrap() + .get_signed_htlc_tx(*htlc_idx, htlc_sig, preimage, self.local_csv)); + } + } + } + htlc_tx + } + + #[cfg(test)] + pub(super) fn unsafe_get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option) -> Option { + let latest_had_sigs = self.local_htlc_sigs.is_some(); + let prev_had_sigs = self.prev_local_htlc_sigs.is_some(); + let ret = self.get_fully_signed_htlc_tx(outp, preimage); + if !latest_had_sigs { + self.local_htlc_sigs = None; + } + if !prev_had_sigs { + self.prev_local_htlc_sigs = None; + } + ret } } diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 989a16c37..3e1ffd044 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -1,12 +1,12 @@ use ln::chan_utils::{HTLCOutputInCommitment, TxCreationKeys, ChannelPublicKeys, LocalCommitmentTransaction}; -use ln::channelmanager::PaymentPreimage; -use ln::msgs; +use ln::{chan_utils, msgs}; use chain::keysinterface::{ChannelKeys, InMemoryChannelKeys}; use std::cmp; use std::sync::{Mutex, Arc}; use bitcoin::blockdata::transaction::Transaction; +use bitcoin::util::bip143; use secp256k1; use secp256k1::key::{SecretKey, PublicKey}; @@ -80,16 +80,29 @@ impl ChannelKeys for EnforcingChannelKeys { } fn sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { - self.inner.sign_local_commitment(local_commitment_tx, secp_ctx) + Ok(self.inner.sign_local_commitment(local_commitment_tx, secp_ctx).unwrap()) } #[cfg(test)] fn unsafe_sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { - self.inner.unsafe_sign_local_commitment(local_commitment_tx, secp_ctx) + Ok(self.inner.unsafe_sign_local_commitment(local_commitment_tx, secp_ctx).unwrap()) } - 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_local_commitment_htlc_transactions(&self, local_commitment_tx: &LocalCommitmentTransaction, local_csv: u16, secp_ctx: &Secp256k1) -> Result>, ()> { + let commitment_txid = local_commitment_tx.txid(); + + for this_htlc in local_commitment_tx.per_htlc.iter() { + if this_htlc.0.transaction_output_index.is_some() { + let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, local_commitment_tx.feerate_per_kw, local_csv, &this_htlc.0, &local_commitment_tx.local_keys.a_delayed_payment_key, &local_commitment_tx.local_keys.revocation_key); + + let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&this_htlc.0, &local_commitment_tx.local_keys); + + let sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, this_htlc.0.amount_msat / 1000)[..]); + secp_ctx.verify(&sighash, this_htlc.1.as_ref().unwrap(), &local_commitment_tx.local_keys.b_htlc_key).unwrap(); + } + } + + Ok(self.inner.sign_local_commitment_htlc_transactions(local_commitment_tx, local_csv, secp_ctx).unwrap()) } fn sign_closing_transaction(&self, closing_tx: &Transaction, secp_ctx: &Secp256k1) -> Result { diff --git a/lightning/src/util/mod.rs b/lightning/src/util/mod.rs index 8c94ac5ca..42c59b453 100644 --- a/lightning/src/util/mod.rs +++ b/lightning/src/util/mod.rs @@ -1,5 +1,8 @@ //! Some utility modules live here. See individual sub-modules for more info. +#[macro_use] +pub(crate) mod fuzz_wrappers; + pub mod events; pub mod errors; pub mod ser; @@ -27,6 +30,3 @@ pub(crate) mod test_utils; /// machine errors and used in fuzz targets and tests. #[cfg(any(test, feature = "fuzztarget"))] pub mod enforcing_trait_impls; - -#[macro_use] -pub(crate) mod fuzz_wrappers; -- 2.39.5