From 1cee5671c373c9417a02e74ad77a6777dce9fbad Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 18 Mar 2020 18:02:31 -0400 Subject: [PATCH] Move SpendableOutputDescriptor::DynamicOutputP2WPKH in is_paying_spendable_output Add ChannelMonitor::broadcasted_remote_payment_script to detect onchain remote txn paying back to us. --- lightning/src/ln/channelmonitor.rs | 134 ++++++++++++----------------- 1 file changed, 54 insertions(+), 80 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 70e41ea5..2fff92c4 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -761,6 +761,7 @@ pub struct ChannelMonitor { destination_script: Script, broadcasted_local_revokable_script: Option<(Script, SecretKey, Script)>, + broadcasted_remote_payment_script: Option<(Script, SecretKey)>, key_storage: Storage, their_htlc_base_key: Option, @@ -803,11 +804,6 @@ pub struct ChannelMonitor { pending_htlcs_updated: Vec, pending_events: Vec, - // Thanks to data loss protection, we may be able to claim our non-htlc funds - // back, this is the script we have to spend from but we need to - // scan every commitment transaction for that - to_remote_rescue: Option<(Script, SecretKey)>, - // Used to track onchain events, i.e transactions parts of channels confirmed on chain, on which // we have to take actions once they reach enough confs. Key is a block height timer, i.e we enforce // actions when we receive a block with given height. Actions depend on OnchainEvent type. @@ -843,6 +839,7 @@ impl PartialEq for ChannelMonitor { self.commitment_transaction_number_obscure_factor != other.commitment_transaction_number_obscure_factor || self.destination_script != other.destination_script || self.broadcasted_local_revokable_script != other.broadcasted_local_revokable_script || + self.broadcasted_remote_payment_script != other.broadcasted_remote_payment_script || self.key_storage != other.key_storage || self.their_htlc_base_key != other.their_htlc_base_key || self.their_delayed_payment_base_key != other.their_delayed_payment_base_key || @@ -861,7 +858,6 @@ impl PartialEq for ChannelMonitor { self.payment_preimages != other.payment_preimages || self.pending_htlcs_updated != other.pending_htlcs_updated || self.pending_events.len() != other.pending_events.len() || // We trust events to round-trip properly - self.to_remote_rescue != other.to_remote_rescue || self.onchain_events_waiting_threshold_conf != other.onchain_events_waiting_threshold_conf || self.outputs_to_watch != other.outputs_to_watch { @@ -895,6 +891,14 @@ impl ChannelMonitor { writer.write_all(&[1; 1])?; } + if let Some(ref broadcasted_remote_payment_script) = self.broadcasted_remote_payment_script { + writer.write_all(&[0; 1])?; + broadcasted_remote_payment_script.0.write(writer)?; + broadcasted_remote_payment_script.1.write(writer)?; + } else { + writer.write_all(&[1; 1])?; + } + match self.key_storage { Storage::Local { ref keys, ref funding_key, ref revocation_base_key, ref htlc_base_key, ref delayed_payment_base_key, ref payment_base_key, ref shutdown_pubkey, ref funding_info, ref current_remote_commitment_txid, ref prev_remote_commitment_txid } => { writer.write_all(&[0; 1])?; @@ -1049,13 +1053,6 @@ impl ChannelMonitor { } self.last_block_hash.write(writer)?; - if let Some((ref to_remote_script, ref local_key)) = self.to_remote_rescue { - writer.write_all(&[1; 1])?; - to_remote_script.write(writer)?; - local_key.write(writer)?; - } else { - writer.write_all(&[0; 1])?; - } writer.write_all(&byte_utils::be64_to_array(self.onchain_events_waiting_threshold_conf.len() as u64))?; for (ref target, ref events) in self.onchain_events_waiting_threshold_conf.iter() { @@ -1128,6 +1125,7 @@ impl ChannelMonitor { destination_script: destination_script.clone(), broadcasted_local_revokable_script: None, + broadcasted_remote_payment_script: None, key_storage: Storage::Local { keys, @@ -1163,8 +1161,6 @@ impl ChannelMonitor { pending_htlcs_updated: Vec::new(), pending_events: Vec::new(), - to_remote_rescue: None, - onchain_events_waiting_threshold_conf: HashMap::new(), outputs_to_watch: HashMap::new(), @@ -1280,7 +1276,7 @@ impl ChannelMonitor { .push_slice(&Hash160::hash(&payment_key.serialize())[..]) .into_script(); if let Ok(to_remote_key) = chan_utils::derive_private_key(&self.secp_ctx, &their_revocation_point, &payment_base_key) { - self.to_remote_rescue = Some((to_remote_script, to_remote_key)); + self.broadcasted_remote_payment_script = Some((to_remote_script, to_remote_key)); } } }, @@ -1468,12 +1464,11 @@ impl ChannelMonitor { /// HTLC-Success/HTLC-Timeout transactions. /// Return updates for HTLC pending in the channel and failed automatically by the broadcast of /// revoked remote commitment tx - fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, (Sha256dHash, Vec), Vec) { + fn check_spend_remote_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, (Sha256dHash, Vec)) { // Most secp and related errors trying to create keys means we have no hope of constructing // a spend transaction...so we return no transactions to broadcast let mut claimable_outpoints = Vec::new(); let mut watch_outputs = Vec::new(); - let mut spendable_outputs = Vec::new(); let commitment_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers! let per_commitment_option = self.remote_claimable_outpoints.get(&commitment_txid); @@ -1482,7 +1477,7 @@ impl ChannelMonitor { ( $thing : expr ) => { match $thing { Ok(a) => a, - Err(_) => return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_outputs) + Err(_) => return (claimable_outpoints, (commitment_txid, watch_outputs)) } }; } @@ -1497,7 +1492,7 @@ impl ChannelMonitor { (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &keys.pubkeys().revocation_basepoint)), ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key)), ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &keys.pubkeys().htlc_basepoint)), - Some(ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &payment_base_key)))) + ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &payment_base_key))) }, Storage::Watchtower { .. } => { unimplemented!() @@ -1505,31 +1500,25 @@ impl ChannelMonitor { }; let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap())); let a_htlc_key = match self.their_htlc_base_key { - None => return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_outputs), + None => return (claimable_outpoints, (commitment_txid, watch_outputs)), Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &their_htlc_base_key)), }; let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.our_to_self_delay, &delayed_key); let revokeable_p2wsh = revokeable_redeemscript.to_v0_p2wsh(); - let local_payment_p2wpkh = if let Some(payment_key) = local_payment_key { + self.broadcasted_remote_payment_script = { // Note that the Network here is ignored as we immediately drop the address for the - // script_pubkey version. - let payment_hash160 = Hash160::hash(&PublicKey::from_secret_key(&self.secp_ctx, &payment_key).serialize()); - Some(Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()) - } else { None }; + // script_pubkey version + let payment_hash160 = Hash160::hash(&PublicKey::from_secret_key(&self.secp_ctx, &local_payment_key).serialize()); + Some((Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script(), local_payment_key)) + }; // First, process non-htlc outputs (to_local & to_remote) for (idx, outp) in tx.output.iter().enumerate() { if outp.script_pubkey == revokeable_p2wsh { let witness_data = InputMaterial::Revoked { witness_script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: outp.value }; claimable_outpoints.push(ClaimRequest { absolute_timelock: height + self.our_to_self_delay as u32, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, witness_data}); - } else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() { - spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH { - outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, - key: local_payment_key.unwrap(), - output: outp.clone(), - }); } } @@ -1541,7 +1530,7 @@ impl ChannelMonitor { if transaction_output_index as usize >= tx.output.len() || tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 || tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() { - return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user + return (claimable_outpoints, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user } let witness_data = InputMaterial::Revoked { witness_script: expected_script, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: true, amount: tx.output[transaction_output_index as usize].value }; claimable_outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data }); @@ -1663,37 +1652,26 @@ impl ChannelMonitor { if revocation_points.0 == commitment_number + 1 { Some(point) } else { None } } else { None }; if let Some(revocation_point) = revocation_point_option { - let (revocation_pubkey, b_htlc_key, htlc_privkey) = match self.key_storage { - Storage::Local { ref keys, ref htlc_base_key, .. } => { + let (revocation_pubkey, b_htlc_key, htlc_privkey, local_payment_key) = match self.key_storage { + Storage::Local { ref keys, ref htlc_base_key, ref payment_base_key, .. } => { (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, revocation_point, &keys.pubkeys().revocation_basepoint)), ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &keys.pubkeys().htlc_basepoint)), - ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &htlc_base_key))) + ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &htlc_base_key)), + ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &payment_base_key))) }, Storage::Watchtower { .. } => { unimplemented!() } }; let a_htlc_key = match self.their_htlc_base_key { - None => return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_outputs), + None => return (claimable_outpoints, (commitment_txid, watch_outputs)), Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)), }; - // First, mark as spendable our to_remote output - for (idx, outp) in tx.output.iter().enumerate() { - if outp.script_pubkey.is_v0_p2wpkh() { - match self.key_storage { - Storage::Local { ref payment_base_key, .. } => { - if let Ok(local_key) = chan_utils::derive_private_key(&self.secp_ctx, &revocation_point, &payment_base_key) { - spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH { - outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, - key: local_key, - output: outp.clone(), - }); - } - }, - Storage::Watchtower { .. } => {} - } - break; // Only to_remote ouput is claimable - } - } + self.broadcasted_remote_payment_script = { + // Note that the Network here is ignored as we immediately drop the address for the + // script_pubkey version + let payment_hash160 = Hash160::hash(&PublicKey::from_secret_key(&self.secp_ctx, &local_payment_key).serialize()); + Some((Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script(), local_payment_key)) + }; // Then, try to find htlc outputs for (_, &(ref htlc, _)) in per_commitment_data.iter().enumerate() { @@ -1702,7 +1680,7 @@ impl ChannelMonitor { if transaction_output_index as usize >= tx.output.len() || tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 || tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() { - return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user + return (claimable_outpoints, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user } let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None }; let aggregable = if !htlc.offered { false } else { true }; @@ -1714,18 +1692,8 @@ impl ChannelMonitor { } } } - } else if let Some((ref to_remote_rescue, ref local_key)) = self.to_remote_rescue { - for (idx, outp) in tx.output.iter().enumerate() { - if to_remote_rescue == &outp.script_pubkey { - spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH { - outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, - key: local_key.clone(), - output: outp.clone(), - }); - } - } } - (claimable_outpoints, (commitment_txid, watch_outputs), spendable_outputs) + (claimable_outpoints, (commitment_txid, watch_outputs)) } /// Attempts to claim a remote HTLC-Success/HTLC-Timeout's outputs using the revocation key @@ -2035,8 +2003,7 @@ impl ChannelMonitor { }; if funding_txo.is_none() || (prevout.txid == funding_txo.as_ref().unwrap().0.txid && prevout.vout == funding_txo.as_ref().unwrap().0.index as u32) { if (tx.input[0].sequence >> 8*3) as u8 == 0x80 && (tx.lock_time >> 8*3) as u8 == 0x20 { - let (mut new_outpoints, new_outputs, mut spendable_output) = self.check_spend_remote_transaction(&tx, height); - spendable_outputs.append(&mut spendable_output); + let (mut new_outpoints, new_outputs) = self.check_spend_remote_transaction(&tx, height); if !new_outputs.1.is_empty() { watch_outputs.push(new_outputs); } @@ -2383,6 +2350,14 @@ impl ChannelMonitor { output: outp.clone(), }); } + } else if let Some(ref broadcasted_remote_payment_script) = self.broadcasted_remote_payment_script { + if broadcasted_remote_payment_script.0 == outp.script_pubkey { + return Some(SpendableOutputDescriptor::DynamicOutputP2WPKH { + outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, + key: broadcasted_remote_payment_script.1, + output: outp.clone(), + }); + } } } None @@ -2422,6 +2397,15 @@ impl ReadableArgs> for (Sha256dH 1 => { None }, _ => return Err(DecodeError::InvalidValue), }; + let broadcasted_remote_payment_script = match ::read(reader)? { + 0 => { + let payment_address = Readable::read(reader)?; + let payment_key = Readable::read(reader)?; + Some((payment_address, payment_key)) + }, + 1 => { None }, + _ => return Err(DecodeError::InvalidValue), + }; let key_storage = match ::read(reader)? { 0 => { @@ -2612,15 +2596,6 @@ impl ReadableArgs> for (Sha256dH } let last_block_hash: Sha256dHash = Readable::read(reader)?; - let to_remote_rescue = match ::read(reader)? { - 0 => None, - 1 => { - let to_remote_script = Readable::read(reader)?; - let local_key = Readable::read(reader)?; - Some((to_remote_script, local_key)) - } - _ => return Err(DecodeError::InvalidValue), - }; let waiting_threshold_conf_len: u64 = Readable::read(reader)?; let mut onchain_events_waiting_threshold_conf = HashMap::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128)); @@ -2665,6 +2640,7 @@ impl ReadableArgs> for (Sha256dH destination_script, broadcasted_local_revokable_script, + broadcasted_remote_payment_script, key_storage, their_htlc_base_key, @@ -2689,8 +2665,6 @@ impl ReadableArgs> for (Sha256dH pending_htlcs_updated, pending_events, - to_remote_rescue, - onchain_events_waiting_threshold_conf, outputs_to_watch, -- 2.30.2