From: Antoine Riard Date: Wed, 18 Mar 2020 20:56:32 +0000 (-0400) Subject: Introduce ChannelMonitor::is_paying_spendable_output X-Git-Tag: v0.0.12~97^2~5 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=26ac188a3f9f7ce7bc0a1bc4abbbf20e8e3258eb;p=rust-lightning Introduce ChannelMonitor::is_paying_spendable_output Previously, we would generate SpendableOutputDescriptor::StaticOutput in OnchainTxHandler even if our claiming transaction wouldn't confirm onchain, misbehaving user wallet to think it receives more funds than in reality. Fix tests in consequence --- diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index c2dd9c352..d714f1e34 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -759,6 +759,7 @@ pub struct ChannelMonitor { latest_update_id: u64, commitment_transaction_number_obscure_factor: u64, + destination_script: Script, key_storage: Storage, their_htlc_base_key: Option, their_delayed_payment_base_key: Option, @@ -838,6 +839,7 @@ impl PartialEq for ChannelMonitor { fn eq(&self, other: &Self) -> bool { 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.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 || @@ -880,6 +882,7 @@ impl ChannelMonitor { // Set in initial Channel-object creation, so should always be set by now: U48(self.commitment_transaction_number_obscure_factor).write(writer)?; + self.destination_script.write(writer)?; 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])?; @@ -1111,6 +1114,7 @@ impl ChannelMonitor { latest_update_id: 0, commitment_transaction_number_obscure_factor, + destination_script: destination_script.clone(), key_storage: Storage::Local { keys, funding_key, @@ -2076,6 +2080,10 @@ impl ChannelMonitor { // can also be resolved in a few other ways which can have more than one output. Thus, // we call is_resolving_htlc_output here outside of the tx.input.len() == 1 check. self.is_resolving_htlc_output(&tx, height); + + if let Some(spendable_output) = self.is_paying_spendable_output(&tx) { + spendable_outputs.push(spendable_output); + } } let should_broadcast = if let Some(_) = self.current_local_signed_commitment_tx { self.would_broadcast_at_height(height) @@ -2124,8 +2132,7 @@ impl ChannelMonitor { } } } - let mut spendable_output = self.onchain_tx_handler.block_connected(txn_matched, claimable_outpoints, height, &*broadcaster, &*fee_estimator); - spendable_outputs.append(&mut spendable_output); + self.onchain_tx_handler.block_connected(txn_matched, claimable_outpoints, height, &*broadcaster, &*fee_estimator); self.last_block_hash = block_hash.clone(); for &(ref txid, ref output_scripts) in watch_outputs.iter() { @@ -2367,6 +2374,19 @@ impl ChannelMonitor { } } } + + /// Check if any transaction broadcasted is paying fund back to some address we can assume to own + fn is_paying_spendable_output(&self, tx: &Transaction) -> Option { + for (i, outp) in tx.output.iter().enumerate() { // There is max one spendable output for any channel tx, including ones generated by us + if outp.script_pubkey == self.destination_script { + return Some(SpendableOutputDescriptor::StaticOutput { + outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, + output: outp.clone(), + }); + } + } + None + } } const MAX_ALLOC_SIZE: usize = 64*1024; @@ -2391,6 +2411,8 @@ impl ReadableArgs> for (Sha256dH let latest_update_id: u64 = Readable::read(reader)?; let commitment_transaction_number_obscure_factor = ::read(reader)?.0; + let destination_script = Readable::read(reader)?; + let key_storage = match ::read(reader)? { 0 => { let keys = Readable::read(reader)?; @@ -2631,6 +2653,8 @@ impl ReadableArgs> for (Sha256dH latest_update_id, commitment_transaction_number_obscure_factor, + destination_script, + key_storage, their_htlc_base_key, their_delayed_payment_base_key, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index e00e036db..392b69e3c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4100,17 +4100,20 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() { assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, chan.3.txid()); claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000); - let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); check_closed_broadcast!(nodes[1], false); check_added_monitors!(nodes[1], 1); let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); + let header_1 = 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_1, txdata: vec![node_txn[0].clone()] }, 2); + let spend_txn = check_spendable_outputs!(nodes[1], 1); assert_eq!(spend_txn.len(), 3); - assert_eq!(spend_txn[0], spend_txn[2]); // to_remote output on revoked remote commitment_tx + assert_eq!(spend_txn[0], spend_txn[1]); // to_remote output on revoked remote commitment_tx check_spends!(spend_txn[0], revoked_local_txn[0]); - check_spends!(spend_txn[1], node_txn[0]); + check_spends!(spend_txn[2], node_txn[0]); } #[test] @@ -4150,11 +4153,13 @@ fn test_static_spendable_outputs_preimage_tx() { assert_eq!(node_txn.len(), 3); check_spends!(node_txn[0], commitment_tx[0]); assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); -eprintln!("{:?}", node_txn[1]); check_spends!(node_txn[1], chan_1.3); check_spends!(node_txn[2], node_txn[1]); - let spend_txn = check_spendable_outputs!(nodes[1], 1); // , 0, 0, 1, 1); + let header_1 = 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_1, txdata: vec![node_txn[0].clone()] }, 1); + + let spend_txn = check_spendable_outputs!(nodes[1], 1); assert_eq!(spend_txn.len(), 1); check_spends!(spend_txn[0], node_txn[0]); } @@ -4186,6 +4191,9 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() { assert_eq!(node_txn[0].input.len(), 2); check_spends!(node_txn[0], revoked_local_txn[0]); + let header_1 = 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_1, txdata: vec![node_txn[0].clone()] }, 2); + let spend_txn = check_spendable_outputs!(nodes[1], 1); assert_eq!(spend_txn.len(), 1); check_spends!(spend_txn[0], node_txn[0]); @@ -4223,7 +4231,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() { check_spends!(revoked_htlc_txn[1], chan_1.3); // B will generate justice tx from A's revoked commitment/HTLC tx - nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()] }, 1); + nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()] }, 0); check_closed_broadcast!(nodes[1], false); check_added_monitors!(nodes[1], 1); @@ -4237,6 +4245,9 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() { assert_eq!(node_txn[3].input.len(), 1); check_spends!(node_txn[3], revoked_local_txn[0]); + let header_1 = 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_1, txdata: vec![node_txn[0].clone(), node_txn[2].clone()] }, 1); + // Check B's ChannelMonitor was able to generate the right spendable output descriptor let spend_txn = check_spendable_outputs!(nodes[1], 1); assert_eq!(spend_txn.len(), 2); @@ -4284,13 +4295,17 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() { assert_eq!(node_txn[2].input.len(), 1); check_spends!(node_txn[2], revoked_htlc_txn[0]); + let header_1 = 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_1, txdata: vec![node_txn[0].clone(), node_txn[2].clone()] }, 1); + // Check A's ChannelMonitor was able to generate the right spendable output descriptor let spend_txn = check_spendable_outputs!(nodes[0], 1); assert_eq!(spend_txn.len(), 5); // Duplicated SpendableOutput due to block rescan after revoked htlc output tracking + assert_eq!(spend_txn[0], spend_txn[1]); assert_eq!(spend_txn[0], spend_txn[2]); check_spends!(spend_txn[0], revoked_local_txn[0]); // spending to_remote output from revoked local tx - check_spends!(spend_txn[1], node_txn[0]); // spending justice tx output from revoked local tx htlc received output - check_spends!(spend_txn[3], node_txn[2]); // spending justice tx output on htlc success tx + check_spends!(spend_txn[3], node_txn[0]); // spending justice tx output from revoked local tx htlc received output + check_spends!(spend_txn[4], node_txn[2]); // spending justice tx output on htlc success tx } #[test] diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 3f21d4c1f..8323a58fb 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -17,7 +17,6 @@ use ln::msgs::DecodeError; use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest}; use ln::chan_utils::HTLCType; use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT}; -use chain::keysinterface::SpendableOutputDescriptor; use util::logger::Logger; use util::ser::{ReadableArgs, Readable, Writer, Writeable}; use util::byte_utils; @@ -478,7 +477,7 @@ impl OnchainTxHandler { Some((new_timer, new_feerate, bumped_tx)) } - pub(super) fn block_connected(&mut self, txn_matched: &[&Transaction], claimable_outpoints: Vec, height: u32, broadcaster: B, fee_estimator: F) -> Vec + pub(super) fn block_connected(&mut self, txn_matched: &[&Transaction], claimable_outpoints: Vec, height: u32, broadcaster: B, fee_estimator: F) where B::Target: BroadcasterInterface, F::Target: FeeEstimator { @@ -486,7 +485,6 @@ impl OnchainTxHandler { let mut new_claims = Vec::new(); let mut aggregated_claim = HashMap::new(); let mut aggregated_soonest = ::std::u32::MAX; - let mut spendable_outputs = Vec::new(); // Try to aggregate outputs if their timelock expiration isn't imminent (absolute_timelock // <= CLTV_SHARED_CLAIM_BUFFER) and they don't require an immediate nLockTime (aggregable). @@ -522,10 +520,6 @@ impl OnchainTxHandler { self.claimable_outpoints.insert(k.clone(), (txid, height)); } log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); - spendable_outputs.push(SpendableOutputDescriptor::StaticOutput { - outpoint: BitcoinOutPoint { txid: tx.txid(), vout: 0 }, - output: tx.output[0].clone(), - }); broadcaster.broadcast_transaction(&tx); } } @@ -656,8 +650,6 @@ impl OnchainTxHandler { } else { unreachable!(); } } } - - spendable_outputs } pub(super) fn block_disconnected(&mut self, height: u32, broadcaster: B, fee_estimator: F)