From a2bdadaeedf93df5ffa5e211335ae457b1cfcc70 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 18 Mar 2020 17:57:29 -0400 Subject: [PATCH] Move SpendableOutputDescirptor::DynamicOutputP2WSH in is_paying_spendable_output Add ChannelMonitor::broadcasted_local_revokable_script to detect onchain local txn paying back to us. Fix tests in consequence --- lightning/src/ln/channelmonitor.rs | 81 ++++++++++++++++------------ lightning/src/ln/functional_tests.rs | 41 ++++++++------ 2 files changed, 70 insertions(+), 52 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index d714f1e3..70e41ea5 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -760,6 +760,8 @@ pub struct ChannelMonitor { commitment_transaction_number_obscure_factor: u64, destination_script: Script, + broadcasted_local_revokable_script: Option<(Script, SecretKey, Script)>, + key_storage: Storage, their_htlc_base_key: Option, their_delayed_payment_base_key: Option, @@ -840,6 +842,7 @@ impl PartialEq for ChannelMonitor { if self.latest_update_id != other.latest_update_id || 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.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 || @@ -883,6 +886,15 @@ impl ChannelMonitor { U48(self.commitment_transaction_number_obscure_factor).write(writer)?; self.destination_script.write(writer)?; + if let Some(ref broadcasted_local_revokable_script) = self.broadcasted_local_revokable_script { + writer.write_all(&[0; 1])?; + broadcasted_local_revokable_script.0.write(writer)?; + broadcasted_local_revokable_script.1.write(writer)?; + broadcasted_local_revokable_script.2.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])?; @@ -1115,6 +1127,8 @@ impl ChannelMonitor { commitment_transaction_number_obscure_factor, destination_script: destination_script.clone(), + broadcasted_local_revokable_script: None, + key_storage: Storage::Local { keys, funding_key, @@ -1752,33 +1766,14 @@ impl ChannelMonitor { (claimable_outpoints, Some((htlc_txid, tx.output.clone()))) } - fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx, delayed_payment_base_key: &SecretKey) -> (Vec, Vec, Vec) { + fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx, delayed_payment_base_key: &SecretKey) -> (Vec, Vec, Option<(Script, SecretKey, Script)>) { let mut res = Vec::with_capacity(local_tx.htlc_outputs.len()); - let mut spendable_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut watch_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); - macro_rules! add_dynamic_output { - ($father_tx: expr, $vout: expr) => { - if let Ok(local_delayedkey) = chan_utils::derive_private_key(&self.secp_ctx, &local_tx.per_commitment_point, delayed_payment_base_key) { - spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WSH { - outpoint: BitcoinOutPoint { txid: $father_tx.txid(), vout: $vout }, - key: local_delayedkey, - witness_script: chan_utils::get_revokeable_redeemscript(&local_tx.revocation_key, self.our_to_self_delay, &local_tx.delayed_payment_key), - to_self_delay: self.our_to_self_delay, - output: $father_tx.output[$vout as usize].clone(), - }); - } - } - } - let redeemscript = chan_utils::get_revokeable_redeemscript(&local_tx.revocation_key, self.their_to_self_delay.unwrap(), &local_tx.delayed_payment_key); - let revokeable_p2wsh = redeemscript.to_v0_p2wsh(); - for (idx, output) in local_tx.tx.without_valid_witness().output.iter().enumerate() { - if output.script_pubkey == revokeable_p2wsh { - add_dynamic_output!(local_tx.tx.without_valid_witness(), idx as u32); - break; - } - } + let broadcasted_local_revokable_script = if let Ok(local_delayedkey) = chan_utils::derive_private_key(&self.secp_ctx, &local_tx.per_commitment_point, delayed_payment_base_key) { + Some((redeemscript.to_v0_p2wsh(), local_delayedkey, redeemscript)) + } else { None }; if let &Storage::Local { ref htlc_base_key, .. } = &self.key_storage { for &(ref htlc, ref sigs, _) in local_tx.htlc_outputs.iter() { @@ -1793,7 +1788,6 @@ impl ChannelMonitor { Err(_) => continue, }; - add_dynamic_output!(htlc_timeout_tx, 0); let mut per_input_material = HashMap::with_capacity(1); per_input_material.insert(htlc_timeout_tx.input[0].previous_output, InputMaterial::LocalHTLC { witness_script: htlc_script, sigs: (*their_sig, our_sig), preimage: None, amount: htlc.amount_msat / 1000}); //TODO: with option_simplified_commitment track outpoint too @@ -1809,7 +1803,6 @@ impl ChannelMonitor { Err(_) => continue, }; - add_dynamic_output!(htlc_success_tx, 0); let mut per_input_material = HashMap::with_capacity(1); per_input_material.insert(htlc_success_tx.input[0].previous_output, InputMaterial::LocalHTLC { witness_script: htlc_script, sigs: (*their_sig, our_sig), preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000}); //TODO: with option_simplified_commitment track outpoint too @@ -1823,16 +1816,15 @@ impl ChannelMonitor { } } - (res, spendable_outputs, watch_outputs) + (res, watch_outputs, broadcasted_local_revokable_script) } /// Attempts to claim any claimable HTLCs in a commitment transaction which was not (yet) /// revoked using data in local_claimable_outpoints. /// Should not be used if check_spend_revoked_transaction succeeds. - fn check_spend_local_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, Vec, (Sha256dHash, Vec)) { + fn check_spend_local_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, (Sha256dHash, Vec)) { let commitment_txid = tx.txid(); let mut local_txn = Vec::new(); - let mut spendable_outputs = Vec::new(); let mut watch_outputs = Vec::new(); macro_rules! wait_threshold_conf { @@ -1860,8 +1852,8 @@ impl ChannelMonitor { macro_rules! append_onchain_update { ($updates: expr) => { local_txn.append(&mut $updates.0); - spendable_outputs.append(&mut $updates.1); - watch_outputs.append(&mut $updates.2); + watch_outputs.append(&mut $updates.1); + self.broadcasted_local_revokable_script = $updates.2; } } @@ -1938,7 +1930,7 @@ impl ChannelMonitor { } } - (local_txn, spendable_outputs, (commitment_txid, watch_outputs)) + (local_txn, (commitment_txid, watch_outputs)) } /// Generate a spendable output event when closing_transaction get registered onchain. @@ -2049,8 +2041,7 @@ impl ChannelMonitor { watch_outputs.push(new_outputs); } if new_outpoints.is_empty() { - let (local_txn, mut spendable_output, new_outputs) = self.check_spend_local_transaction(&tx, height); - spendable_outputs.append(&mut spendable_output); + let (local_txn, new_outputs) = self.check_spend_local_transaction(&tx, height); for tx in local_txn.iter() { log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); broadcaster.broadcast_transaction(tx); @@ -2104,8 +2095,7 @@ impl ChannelMonitor { broadcaster.broadcast_transaction(&cur_local_tx.tx.with_valid_witness()); match self.key_storage { Storage::Local { ref delayed_payment_base_key, .. } => { - let (txs, mut spendable_output, new_outputs) = self.broadcast_by_local_state(&cur_local_tx, delayed_payment_base_key); - spendable_outputs.append(&mut spendable_output); + let (txs, new_outputs, _) = self.broadcast_by_local_state(&cur_local_tx, delayed_payment_base_key); if !new_outputs.is_empty() { watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); } @@ -2383,6 +2373,16 @@ impl ChannelMonitor { outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, output: outp.clone(), }); + } else if let Some(ref broadcasted_local_revokable_script) = self.broadcasted_local_revokable_script { + if broadcasted_local_revokable_script.0 == outp.script_pubkey { + return Some(SpendableOutputDescriptor::DynamicOutputP2WSH { + outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, + key: broadcasted_local_revokable_script.1, + witness_script: broadcasted_local_revokable_script.2.clone(), + to_self_delay: self.their_to_self_delay.unwrap(), + output: outp.clone(), + }); + } } } None @@ -2412,6 +2412,16 @@ impl ReadableArgs> for (Sha256dH let commitment_transaction_number_obscure_factor = ::read(reader)?.0; let destination_script = Readable::read(reader)?; + let broadcasted_local_revokable_script = match ::read(reader)? { + 0 => { + let revokable_address = Readable::read(reader)?; + let local_delayedkey = Readable::read(reader)?; + let revokable_script = Readable::read(reader)?; + Some((revokable_address, local_delayedkey, revokable_script)) + }, + 1 => { None }, + _ => return Err(DecodeError::InvalidValue), + }; let key_storage = match ::read(reader)? { 0 => { @@ -2654,6 +2664,7 @@ impl ReadableArgs> for (Sha256dH commitment_transaction_number_obscure_factor, destination_script, + broadcasted_local_revokable_script, key_storage, their_htlc_base_key, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 392b69e3..275ecb08 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4563,16 +4563,22 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() { MessageSendEvent::BroadcastChannelUpdate { .. } => {}, _ => panic!("Unexepected event"), } - let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn[0].input.len(), 1); - assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); - check_spends!(node_txn[0], local_txn[0]); + let node_txn = { + let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); + assert_eq!(node_txn[0].input.len(), 1); + assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); + check_spends!(node_txn[0], local_txn[0]); + vec![node_txn[0].clone(), node_txn[2].clone()] + }; + + let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + nodes[1].block_notifier.block_connected(&Block { header: header_201, txdata: node_txn.clone() }, 201); // Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor let spend_txn = check_spendable_outputs!(nodes[1], 1); assert_eq!(spend_txn.len(), 2); check_spends!(spend_txn[0], node_txn[0]); - check_spends!(spend_txn[1], node_txn[2]); + check_spends!(spend_txn[1], node_txn[1]); } fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, announce_latest: bool) { @@ -4849,22 +4855,23 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() { check_closed_broadcast!(nodes[0], false); check_added_monitors!(nodes[0], 1); - let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn[0].input.len(), 1); - assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); - check_spends!(node_txn[0], local_txn[0]); + let htlc_timeout = { + let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); + assert_eq!(node_txn[0].input.len(), 1); + assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); + check_spends!(node_txn[0], local_txn[0]); + node_txn[0].clone() + }; + + let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + nodes[0].block_notifier.block_connected(&Block { header: header_201, txdata: vec![htlc_timeout.clone()] }, 201); // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor let spend_txn = check_spendable_outputs!(nodes[0], 1); - assert_eq!(spend_txn.len(), 8); - assert_eq!(spend_txn[0], spend_txn[2]); - assert_eq!(spend_txn[0], spend_txn[4]); - assert_eq!(spend_txn[0], spend_txn[6]); - assert_eq!(spend_txn[1], spend_txn[3]); - assert_eq!(spend_txn[1], spend_txn[5]); - assert_eq!(spend_txn[1], spend_txn[7]); + assert_eq!(spend_txn.len(), 3); + assert_eq!(spend_txn[0], spend_txn[1]); check_spends!(spend_txn[0], local_txn[0]); - check_spends!(spend_txn[1], node_txn[0]); + check_spends!(spend_txn[2], htlc_timeout); } #[test] -- 2.30.2