diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index ec76262b15a..73c22684c54 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; @@ -185,11 +184,11 @@ pub trait KeysInterface: Send + Sync { /// Readable/Writable to serialize out a unique reference to this set of keys so /// that you can serialize the full ChannelManager object. /// -/// (TODO: We shouldn't require that, and should have an API to get them at deser time, due mostly -/// to the possibility of reentrancy issues by calling the user's code during our deserialization -/// routine). -/// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create -/// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors. +// (TODO: We shouldn't require that, and should have an API to get them at deser time, due mostly +// to the possibility of reentrancy issues by calling the user's code during our deserialization +// routine). +// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create +// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors. pub trait ChannelKeys : Send+Clone { /// Gets the private key for the anchor tx fn funding_key<'a>(&'a self) -> &'a SecretKey; @@ -210,32 +209,42 @@ pub trait ChannelKeys : Send+Clone { /// Create a signature for a remote commitment transaction and associated HTLC transactions. /// /// Note that if signing fails or is rejected, the channel will be force-closed. - /// - /// TODO: Document the things someone using this interface should enforce before signing. - /// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and - /// making the callee generate it via some util function we expose)! + // + // TODO: Document the things someone using this interface should enforce before signing. + // TODO: Add more input vars to enable better checking (preferably removing commitment_tx and + // making the callee generate it via some util function we expose)! fn sign_remote_commitment(&self, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; - /// Create a signature for a local commitment transaction - /// - /// TODO: Document the things someone using this interface should enforce before signing. - /// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and - /// TODO: Ensure test-only version doesn't enforce uniqueness of signature when it's enforced in this method - /// making the callee generate it via some util function we expose)! - fn sign_local_commitment(&self, local_commitment_tx: &mut LocalCommitmentTransaction, secp_ctx: &Secp256k1); + /// Create a signature for a local commitment transaction. This will only ever be called with + /// the same local_commitment_tx (or a copy thereof), though there are currently no guarantees + /// that it will not be called multiple times. + // + // TODO: Document the things someone using this interface should enforce before signing. + // TODO: Add more input vars to enable better checking (preferably removing commitment_tx and + fn sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result; + + /// Same as sign_local_commitment, but exists only for tests to get access to local commitment + /// transactions which will be broadcasted later, after the channel has moved on to a newer + /// state. Thus, needs its own method as sign_local_commitment may enforce that we only ever + /// get called once. + #[cfg(test)] + fn unsafe_sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result; - /// Create a signature for a local commitment transaction without enforcing one-time signing. + /// Create a signature for each HTLC transaction spending a local commitment transaction. /// - /// Testing revocation logic by our test framework needs to sign multiple local commitment - /// transactions. This unsafe test-only version doesn't enforce one-time signing security - /// requirement. - #[cfg(test)] - fn unsafe_sign_local_commitment(&self, local_commitment_tx: &mut LocalCommitmentTransaction, secp_ctx: &Secp256k1); + /// 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>, ()>; - /// 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 a (proposed) closing transaction. /// /// Note that, due to rounding, there may be one "missing" satoshi, and either party may have @@ -363,25 +372,25 @@ impl ChannelKeys for InMemoryChannelKeys { Ok((commitment_sig, htlc_sigs)) } - fn sign_local_commitment(&self, local_commitment_tx: &mut LocalCommitmentTransaction, secp_ctx: &Secp256k1) { + fn sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key); let remote_channel_pubkeys = self.remote_channel_pubkeys.as_ref().expect("must set remote channel pubkeys before signing"); let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &remote_channel_pubkeys.funding_pubkey); - local_commitment_tx.add_local_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx); + Ok(local_commitment_tx.get_local_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx)) } #[cfg(test)] - fn unsafe_sign_local_commitment(&self, local_commitment_tx: &mut LocalCommitmentTransaction, secp_ctx: &Secp256k1) { + fn unsafe_sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key); let remote_channel_pubkeys = self.remote_channel_pubkeys.as_ref().expect("must set remote channel pubkeys before signing"); let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &remote_channel_pubkeys.funding_pubkey); - local_commitment_tx.add_local_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx); + 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 43f567933fd..62d3103337d 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -484,10 +484,30 @@ 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, - pub(crate) local_keys: TxCreationKeys, - pub(crate) feerate_per_kw: u64, - per_htlc: Vec<(HTLCOutputInCommitment, Option, Option)> + // TODO: We should migrate away from providing the transaction, instead providing enough to + // allow the ChannelKeys to construct it from scratch. Luckily we already have HTLC data here, + // so we're probably most of the way there. + /// The commitment transaction itself, in unsigned form. + pub unsigned_tx: Transaction, + /// Our counterparty's signature for the transaction, above. + pub their_sig: Signature, + // Which order the signatures should go in when constructing the final commitment tx witness. + // The user should be able to reconstruc this themselves, so we don't bother to expose it. + our_sig_first: bool, + /// The key derivation parameters for this commitment transaction + pub local_keys: TxCreationKeys, + /// The feerate paid per 1000-weight-unit in this commitment transaction. This value is + /// controlled by the channel initiator. + pub feerate_per_kw: u64, + /// The HTLCs and remote htlc signatures which were included in this commitment transaction. + /// + /// Note that this includes all HTLCs, including ones which were considered dust and not + /// actually included in the transaction as it appears on-chain, but who's value is burned as + /// fees and not included in the to_local or to_remote outputs. + /// + /// The remote HTLC signatures in the second element will always be set for non-dust HTLCs, ie + /// those for which transaction_output_index.is_some(). + pub per_htlc: Vec<(HTLCOutputInCommitment, Option)>, } impl LocalCommitmentTransaction { #[cfg(test)] @@ -499,16 +519,19 @@ impl LocalCommitmentTransaction { }, script_sig: Default::default(), sequence: 0, - witness: vec![vec![], vec![], vec![]] + witness: vec![] }; let dummy_key = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&[42; 32]).unwrap()); + let dummy_sig = Secp256k1::new().sign(&secp256k1::Message::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[42; 32]).unwrap()); Self { - tx: Transaction { + unsigned_tx: Transaction { version: 2, input: vec![dummy_input], output: Vec::new(), lock_time: 0, }, + their_sig: dummy_sig, + our_sig_first: false, local_keys: TxCreationKeys { per_commitment_point: dummy_key.clone(), revocation_key: dummy_key.clone(), @@ -524,53 +547,27 @@ 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 { - 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?"); } - - tx.input[0].witness.push(Vec::new()); // First is the multisig dummy - - if our_funding_key.serialize()[..] < their_funding_key.serialize()[..] { - tx.input[0].witness.push(Vec::new()); - tx.input[0].witness.push(their_sig.serialize_der().to_vec()); - tx.input[0].witness[2].push(SigHashType::All as u8); - } else { - tx.input[0].witness.push(their_sig.serialize_der().to_vec()); - tx.input[0].witness[1].push(SigHashType::All as u8); - tx.input[0].witness.push(Vec::new()); - } + pub(crate) fn new_missing_local_sig(unsigned_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 unsigned_tx.input.len() != 1 { panic!("Tried to store a commitment transaction that had input count != 1!"); } + if unsigned_tx.input[0].witness.len() != 0 { panic!("Tried to store a signed commitment transaction?"); } - Self { tx, + Self { + unsigned_tx, + their_sig, + our_sig_first: our_funding_key.serialize()[..] < their_funding_key.serialize()[..], 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, } } /// Get the txid of the local commitment transaction contained in this /// LocalCommitmentTransaction pub fn txid(&self) -> Sha256dHash { - self.tx.txid() + self.unsigned_tx.txid() } - /// Check if LocalCommitmentTransaction has already been signed by us - pub fn has_local_sig(&self) -> bool { - if self.tx.input.len() != 1 { panic!("Commitment transactions must have input count == 1!"); } - if self.tx.input[0].witness.len() == 4 { - assert!(!self.tx.input[0].witness[1].is_empty()); - assert!(!self.tx.input[0].witness[2].is_empty()); - true - } else { - assert_eq!(self.tx.input[0].witness.len(), 3); - assert!(self.tx.input[0].witness[0].is_empty()); - assert!(self.tx.input[0].witness[1].is_empty() || self.tx.input[0].witness[2].is_empty()); - false - } - } - - /// Add local signature for LocalCommitmentTransaction, do nothing if signature is already - /// present + /// Gets our signature for the contained commitment transaction given our funding private key. /// /// Funding key is your key included in the 2-2 funding_outpoint lock. Should be provided /// by your ChannelKeys. @@ -578,80 +575,91 @@ impl LocalCommitmentTransaction { /// between your own funding key and your counterparty's. Currently, this is provided in /// ChannelKeys::sign_local_commitment() calls directly. /// Channel value is amount locked in funding_outpoint. - pub fn add_local_sig(&mut self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1) { - if self.has_local_sig() { return; } - let sighash = hash_to_message!(&bip143::SighashComponents::new(&self.tx) - .sighash_all(&self.tx.input[0], funding_redeemscript, channel_value_satoshis)[..]); - let our_sig = secp_ctx.sign(&sighash, funding_key); - - if self.tx.input[0].witness[1].is_empty() { - self.tx.input[0].witness[1] = our_sig.serialize_der().to_vec(); - self.tx.input[0].witness[1].push(SigHashType::All as u8); + pub fn get_local_sig(&self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1) -> Signature { + let sighash = hash_to_message!(&bip143::SighashComponents::new(&self.unsigned_tx) + .sighash_all(&self.unsigned_tx.input[0], funding_redeemscript, channel_value_satoshis)[..]); + secp_ctx.sign(&sighash, funding_key) + } + + pub(crate) fn add_local_sig(&self, funding_redeemscript: &Script, our_sig: Signature) -> Transaction { + let mut tx = self.unsigned_tx.clone(); + // First push the multisig dummy, note that due to BIP147 (NULLDUMMY) it must be a zero-length element. + tx.input[0].witness.push(Vec::new()); + + if self.our_sig_first { + tx.input[0].witness.push(our_sig.serialize_der().to_vec()); + tx.input[0].witness.push(self.their_sig.serialize_der().to_vec()); } else { - self.tx.input[0].witness[2] = our_sig.serialize_der().to_vec(); - self.tx.input[0].witness[2].push(SigHashType::All as u8); + tx.input[0].witness.push(self.their_sig.serialize_der().to_vec()); + tx.input[0].witness.push(our_sig.serialize_der().to_vec()); } + tx.input[0].witness[1].push(SigHashType::All as u8); + tx.input[0].witness[2].push(SigHashType::All as u8); - self.tx.input[0].witness.push(funding_redeemscript.as_bytes().to_vec()); - } - - /// Get raw transaction without asserting if witness is complete - pub(crate) fn without_valid_witness(&self) -> &Transaction { &self.tx } - /// Get raw transaction with panics if witness is incomplete - pub fn with_valid_witness(&self) -> &Transaction { - assert!(self.has_local_sig()); - &self.tx + tx.input[0].witness.push(funding_redeemscript.as_bytes().to_vec()); + 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); + 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(|_| ())?; - 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); - - 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); - 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()); + + 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 { @@ -662,49 +670,53 @@ impl PartialEq for LocalCommitmentTransaction { } impl Writeable for LocalCommitmentTransaction { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { - if let Err(e) = self.tx.consensus_encode(&mut WriterWriteAdaptor(writer)) { + if let Err(e) = self.unsigned_tx.consensus_encode(&mut WriterWriteAdaptor(writer)) { match e { encode::Error::Io(e) => return Err(e), _ => panic!("local tx must have been well-formed!"), } } + self.their_sig.write(writer)?; + self.our_sig_first.write(writer)?; 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(()) } } impl Readable for LocalCommitmentTransaction { fn read(reader: &mut R) -> Result { - let tx = match Transaction::consensus_decode(reader.by_ref()) { + let unsigned_tx = match Transaction::consensus_decode(reader.by_ref()) { Ok(tx) => tx, Err(e) => match e { encode::Error::Io(ioe) => return Err(DecodeError::Io(ioe)), _ => return Err(DecodeError::InvalidValue), }, }; + let their_sig = Readable::read(reader)?; + let our_sig_first = Readable::read(reader)?; 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 { + if unsigned_tx.input.len() != 1 { // Ensure tx didn't hit the 0-input ambiguity case. return Err(DecodeError::InvalidValue); } Ok(Self { - tx, + unsigned_tx, + their_sig, + our_sig_first, local_keys, feerate_per_kw, per_htlc, diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 35f10ef2a91..35e688a0d76 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1460,7 +1460,7 @@ impl Channel { // They sign the "local" commitment transaction... secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer"); - let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new()); + let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new()); let remote_keys = self.build_remote_transaction_keys()?; let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0; @@ -1574,7 +1574,7 @@ impl Channel { let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); macro_rules! create_monitor { () => { { - let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new()); + let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new()); let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(), &self.shutdown_pubkey, self.our_to_self_delay, &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()), @@ -1902,7 +1902,7 @@ impl Channel { let mut monitor_update = ChannelMonitorUpdate { update_id: self.latest_monitor_update_id, updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo { - commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source), + commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source), htlc_outputs: htlcs_and_sigs }] }; @@ -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(..) @@ -4516,12 +4516,15 @@ mod tests { })* assert_eq!(unsigned_tx.1.len(), per_htlc.len()); - 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(), keys.clone(), chan.feerate_per_kw, per_htlc); - chan_keys.sign_local_commitment(&mut localtx, &chan.secp_ctx); + localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc); + let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap(); - assert_eq!(serialize(localtx.with_valid_witness())[..], + assert_eq!(serialize(&localtx.add_local_sig(&redeemscript, local_sig))[..], 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(); @@ -4543,12 +4546,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 c8e6b8260ad..c417671eca0 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -443,7 +443,7 @@ pub(crate) enum InputMaterial { amount: u64, }, Funding { - channel_value: u64, + funding_redeemscript: Script, } } @@ -471,9 +471,9 @@ impl Writeable for InputMaterial { preimage.write(writer)?; writer.write_all(&byte_utils::be64_to_array(*amount))?; }, - &InputMaterial::Funding { ref channel_value } => { + &InputMaterial::Funding { ref funding_redeemscript } => { writer.write_all(&[3; 1])?; - channel_value.write(writer)?; + funding_redeemscript.write(writer)?; } } Ok(()) @@ -520,9 +520,8 @@ impl Readable for InputMaterial { } }, 3 => { - let channel_value = Readable::read(reader)?; InputMaterial::Funding { - channel_value + funding_redeemscript: Readable::read(reader)?, } } _ => return Err(DecodeError::InvalidValue), @@ -790,9 +789,16 @@ pub struct ChannelMonitor { #[cfg(not(test))] onchain_tx_handler: OnchainTxHandler, - // Used to detect programming bug due to unsafe monitor update sequence { ChannelForceClosed, LatestLocalCommitmentTXInfo } + // This is set when the Channel[Manager] generated a ChannelMonitorUpdate which indicated the + // channel has been force-closed. After this is set, no further local commitment transaction + // updates may occur, and we panic!() if one is provided. lockdown_from_offchain: bool, + // Set once we've signed a local commitment transaction and handed it over to our + // OnchainTxHandler. After this is set, no future updates to our local commitment transactions + // may occur, and we fail any such monitor updates. + local_tx_signed: bool, + // We simply modify last_block_hash in Channel's block_connected so that serialization is // consistent but hopefully the users' copy handles block_connected in a consistent way. // (we do *not*, however, update them in update_monitor to ensure any local user copies keep @@ -836,7 +842,9 @@ impl PartialEq for ChannelMonitor { self.pending_htlcs_updated != other.pending_htlcs_updated || self.pending_events.len() != other.pending_events.len() || // We trust events to round-trip properly self.onchain_events_waiting_threshold_conf != other.onchain_events_waiting_threshold_conf || - self.outputs_to_watch != other.outputs_to_watch + self.outputs_to_watch != other.outputs_to_watch || + self.lockdown_from_offchain != other.lockdown_from_offchain || + self.local_tx_signed != other.local_tx_signed { false } else { @@ -1037,6 +1045,7 @@ impl ChannelMonitor { self.onchain_tx_handler.write(writer)?; self.lockdown_from_offchain.write(writer)?; + self.local_tx_signed.write(writer)?; Ok(()) } @@ -1057,8 +1066,8 @@ impl ChannelMonitor { let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), their_to_self_delay, logger.clone()); - let local_tx_sequence = initial_local_commitment_tx.without_valid_witness().input[0].sequence as u64; - let local_tx_locktime = initial_local_commitment_tx.without_valid_witness().lock_time as u64; + let local_tx_sequence = initial_local_commitment_tx.unsigned_tx.input[0].sequence as u64; + let local_tx_locktime = initial_local_commitment_tx.unsigned_tx.lock_time as u64; let local_commitment_tx = LocalSignedTx { txid: initial_local_commitment_tx.txid(), revocation_key: initial_local_commitment_tx.local_keys.revocation_key, @@ -1119,6 +1128,7 @@ impl ChannelMonitor { onchain_tx_handler, lockdown_from_offchain: false, + local_tx_signed: false, last_block_hash: Default::default(), secp_ctx: Secp256k1::new(), @@ -1235,9 +1245,12 @@ impl ChannelMonitor { /// 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, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>) -> Result<(), MonitorUpdateError> { + if self.local_tx_signed { + return Err(MonitorUpdateError("A local commitment tx has already been signed, no new local commitment txn can be sent to our counterparty")); + } 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 sequence = commitment_tx.unsigned_tx.input[0].sequence as u64; + let locktime = commitment_tx.unsigned_tx.lock_time as u64; let mut new_local_commitment_tx = LocalSignedTx { txid, revocation_key: commitment_tx.local_keys.revocation_key, @@ -1664,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()); } } @@ -1762,13 +1785,20 @@ impl ChannelMonitor { /// In any-case, choice is up to the user. pub fn get_latest_local_commitment_txn(&mut self) -> Vec { log_trace!(self, "Getting signed latest local commitment transaction!"); - if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx() { + self.local_tx_signed = true; + if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx(&self.funding_redeemscript) { 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); } } @@ -1786,13 +1816,19 @@ impl ChannelMonitor { #[cfg(test)] pub fn unsafe_get_latest_local_commitment_txn(&mut self) -> Vec { log_trace!(self, "Getting signed copy of latest local commitment transaction!"); - if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_copy_local_tx() { + if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_copy_local_tx(&self.funding_redeemscript) { 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); } } @@ -1864,10 +1900,10 @@ impl ChannelMonitor { } let should_broadcast = self.would_broadcast_at_height(height); if should_broadcast { - claimable_outpoints.push(ClaimRequest { absolute_timelock: height, aggregable: false, outpoint: BitcoinOutPoint { txid: self.funding_info.0.txid.clone(), vout: self.funding_info.0.index as u32 }, witness_data: InputMaterial::Funding { channel_value: self.channel_value_satoshis }}); + claimable_outpoints.push(ClaimRequest { absolute_timelock: height, aggregable: false, outpoint: BitcoinOutPoint { txid: self.funding_info.0.txid.clone(), vout: self.funding_info.0.index as u32 }, witness_data: InputMaterial::Funding { funding_redeemscript: self.funding_redeemscript.clone() }}); } if should_broadcast { - if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx() { + if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx(&self.funding_redeemscript) { let (mut new_outpoints, new_outputs, _) = self.broadcast_by_local_state(&commitment_tx, &self.current_local_commitment_tx); if !new_outputs.is_empty() { watch_outputs.push((self.current_local_commitment_tx.txid.clone(), new_outputs)); @@ -2421,6 +2457,7 @@ impl ReadableArgs> for (Sha256dH let onchain_tx_handler = ReadableArgs::read(reader, logger.clone())?; let lockdown_from_offchain = Readable::read(reader)?; + let local_tx_signed = Readable::read(reader)?; Ok((last_block_hash.clone(), ChannelMonitor { latest_update_id, @@ -2465,6 +2502,7 @@ impl ReadableArgs> for (Sha256dH onchain_tx_handler, lockdown_from_offchain, + local_tx_signed, last_block_hash, secp_ctx: Secp256k1::new(), diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index cb76dcb2a55..49572f2cb8b 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 @@ -531,16 +577,11 @@ impl OnchainTxHandler { } return None; }, - &InputMaterial::Funding { ref channel_value } => { - let signed_tx = self.get_fully_signed_local_tx().unwrap(); - let mut amt_outputs = 0; - for outp in signed_tx.output.iter() { - amt_outputs += outp.value; - } - let feerate = (channel_value - amt_outputs) * 1000 / signed_tx.get_weight() as u64; + &InputMaterial::Funding { ref funding_redeemscript } => { + let signed_tx = self.get_fully_signed_local_tx(funding_redeemscript).unwrap(); // Timer set to $NEVER given we can't bump tx without anchor outputs log_trace!(self, "Going to broadcast Local Transaction {} claiming funding output {} from {}...", signed_tx.txid(), outp.vout, outp.txid); - return Some((None, feerate, signed_tx)); + return Some((None, self.local_commitment.as_ref().unwrap().feerate_per_kw, signed_tx)); } _ => unreachable!() } @@ -773,44 +814,113 @@ impl OnchainTxHandler { // HTLC-timeout or channel force-closure), don't allow any further update of local // commitment transaction view to avoid delivery of revocation secret to counterparty // for the aformentionned signed transaction. - 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 // to monitor before. - pub(super) fn get_fully_signed_local_tx(&mut self) -> Option { + pub(super) fn get_fully_signed_local_tx(&mut self, funding_redeemscript: &Script) -> Option { if let Some(ref mut local_commitment) = self.local_commitment { - self.key_storage.sign_local_commitment(local_commitment, &self.secp_ctx); - return Some(local_commitment.with_valid_witness().clone()); + match self.key_storage.sign_local_commitment(local_commitment, &self.secp_ctx) { + Ok(sig) => Some(local_commitment.add_local_sig(funding_redeemscript, sig)), + Err(_) => return None, + } + } else { + None } - None } #[cfg(test)] - pub(super) fn get_fully_signed_copy_local_tx(&mut self) -> Option { + pub(super) fn get_fully_signed_copy_local_tx(&mut self, funding_redeemscript: &Script) -> Option { if let Some(ref mut local_commitment) = self.local_commitment { - let mut local_commitment = local_commitment.clone(); - self.key_storage.unsafe_sign_local_commitment(&mut local_commitment, &self.secp_ctx); - return Some(local_commitment.with_valid_witness().clone()); + let local_commitment = local_commitment.clone(); + match self.key_storage.sign_local_commitment(&local_commitment, &self.secp_ctx) { + Ok(sig) => Some(local_commitment.add_local_sig(funding_redeemscript, sig)), + Err(_) => return None, + } + } else { + None } - 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 1e5dc707c44..3e1ffd04446 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}; @@ -79,17 +79,30 @@ impl ChannelKeys for EnforcingChannelKeys { Ok(self.inner.sign_remote_commitment(feerate_per_kw, commitment_tx, keys, htlcs, to_self_delay, secp_ctx).unwrap()) } - fn sign_local_commitment(&self, local_commitment_tx: &mut LocalCommitmentTransaction, secp_ctx: &Secp256k1) { - self.inner.sign_local_commitment(local_commitment_tx, secp_ctx) + fn sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { + Ok(self.inner.sign_local_commitment(local_commitment_tx, secp_ctx).unwrap()) } #[cfg(test)] - fn unsafe_sign_local_commitment(&self, local_commitment_tx: &mut LocalCommitmentTransaction, secp_ctx: &Secp256k1) { - self.inner.unsafe_sign_local_commitment(local_commitment_tx, secp_ctx); + fn unsafe_sign_local_commitment(&self, local_commitment_tx: &LocalCommitmentTransaction, secp_ctx: &Secp256k1) -> Result { + 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 8c94ac5ca22..42c59b4536b 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;