From b8f80f8ab94ff71b0366c4747fb14a40d767dcc4 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 27 Sep 2023 10:27:44 -0700 Subject: [PATCH 1/2] Allow retrieval of SpendableOutputDescriptors from relevant transactions Currently, our API will only expose `SpendableOutputDescriptor`s once after they are no longer under reorg risk (see `ANTI_REORG_DELAY`). Users have often requested they'd like the ability to retrieve these in some other way, either for historical purposes, or to handle replaying any in the event of a failure. --- lightning/src/chain/channelmonitor.rs | 55 ++++++++++++++++++++------- lightning/src/ln/monitor_tests.rs | 39 ++++++++++++++----- 2 files changed, 72 insertions(+), 22 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 3f9c83bb5..f652bbf36 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1669,6 +1669,33 @@ impl ChannelMonitor { current_height, &broadcaster, &fee_estimator, &logger, ); } + + /// Returns the descriptor for a relevant output (i.e., one that we can spend) within the + /// transaction if one exists and the transaction has at least [`ANTI_REORG_DELAY`] + /// confirmations. + /// + /// Descriptors returned by this method are primarily exposed via [`Event::SpendableOutputs`] + /// once they are no longer under reorg risk. This method serves as a way to retrieve these + /// descriptors at a later time, either for historical purposes, or to replay any + /// missed/unhandled descriptors. For the purpose of gathering historical records, if the + /// channel close has fully resolved (i.e., [`ChannelMonitor::get_claimable_balances`] returns + /// an empty set), you can retrieve all spendable outputs by providing all descendant spending + /// transactions starting from the channel's funding or closing transaction that have at least + /// [`ANTI_REORG_DELAY`] confirmations. + /// + /// `tx` is a transaction we'll scan the outputs of. Any transaction can be provided. If an + /// output which can be spent by us is found, a descriptor is returned. + /// + /// `confirmation_height` must be the height of the block in which `tx` was included in. + pub fn get_spendable_output(&self, tx: &Transaction, confirmation_height: u32) -> Option { + let inner = self.inner.lock().unwrap(); + let current_height = inner.best_block.height; + if current_height.saturating_sub(ANTI_REORG_DELAY) + 1 >= confirmation_height { + inner.get_spendable_output(tx) + } else { + None + } + } } impl ChannelMonitorImpl { @@ -3441,7 +3468,7 @@ impl ChannelMonitorImpl { } self.is_resolving_htlc_output(&tx, height, &block_hash, &logger); - self.is_paying_spendable_output(&tx, height, &block_hash, &logger); + self.check_tx_and_push_spendable_output(&tx, height, &block_hash, &logger); } } @@ -3987,9 +4014,7 @@ impl ChannelMonitorImpl { } } - /// Check if any transaction broadcasted is paying fund back to some address we can assume to own - fn is_paying_spendable_output(&mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L) where L::Target: Logger { - let mut spendable_output = None; + fn get_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 i > ::core::u16::MAX as usize { // While it is possible that an output exists on chain which is greater than the @@ -4006,15 +4031,14 @@ impl ChannelMonitorImpl { continue; } if outp.script_pubkey == self.destination_script { - spendable_output = Some(SpendableOutputDescriptor::StaticOutput { + return Some(SpendableOutputDescriptor::StaticOutput { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), }); - break; } if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script { if broadcasted_holder_revokable_script.0 == outp.script_pubkey { - spendable_output = Some(SpendableOutputDescriptor::DelayedPaymentOutput(DelayedPaymentOutputDescriptor { + return Some(SpendableOutputDescriptor::DelayedPaymentOutput(DelayedPaymentOutputDescriptor { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, per_commitment_point: broadcasted_holder_revokable_script.1, to_self_delay: self.on_holder_tx_csv, @@ -4023,27 +4047,32 @@ impl ChannelMonitorImpl { channel_keys_id: self.channel_keys_id, channel_value_satoshis: self.channel_value_satoshis, })); - break; } } if self.counterparty_payment_script == outp.script_pubkey { - spendable_output = Some(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor { + return Some(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), channel_keys_id: self.channel_keys_id, channel_value_satoshis: self.channel_value_satoshis, })); - break; } if self.shutdown_script.as_ref() == Some(&outp.script_pubkey) { - spendable_output = Some(SpendableOutputDescriptor::StaticOutput { + return Some(SpendableOutputDescriptor::StaticOutput { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), }); - break; } } - if let Some(spendable_output) = spendable_output { + None + } + + /// Checks if the confirmed transaction is paying funds back to some address we can assume to + /// own. + fn check_tx_and_push_spendable_output( + &mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L, + ) where L::Target: Logger { + if let Some(spendable_output) = self.get_spendable_output(tx) { let entry = OnchainEventEntry { txid: tx.txid(), transaction: Some(tx.clone()), diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index cb78cda71..1ccb61cbb 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -9,7 +9,7 @@ //! Further functional tests which test blockchain reorganizations. -use crate::sign::EcdsaChannelSigner; +use crate::sign::{EcdsaChannelSigner, SpendableOutputDescriptor}; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS, Balance}; use crate::chain::transaction::OutPoint; use crate::chain::chaininterface::{LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight}; @@ -21,6 +21,7 @@ use crate::ln::msgs::ChannelMessageHandler; use crate::util::config::UserConfig; use crate::util::crypto::sign; use crate::util::ser::Writeable; +use crate::util::scid_utils::block_from_scid; use crate::util::test_utils; use bitcoin::blockdata::transaction::EcdsaSighashType; @@ -92,14 +93,15 @@ fn chanmon_fail_from_stale_commitment() { expect_payment_failed_with_update!(nodes[0], payment_hash, false, update_a.contents.short_channel_id, true); } -fn test_spendable_output<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, spendable_tx: &Transaction) { +fn test_spendable_output<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, spendable_tx: &Transaction) -> SpendableOutputDescriptor { let mut spendable = node.chain_monitor.chain_monitor.get_and_clear_pending_events(); assert_eq!(spendable.len(), 1); - if let Event::SpendableOutputs { outputs, .. } = spendable.pop().unwrap() { + if let Event::SpendableOutputs { mut outputs, .. } = spendable.pop().unwrap() { assert_eq!(outputs.len(), 1); let spend_tx = node.keys_manager.backing.spend_spendable_outputs(&[&outputs[0]], Vec::new(), Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script(), 253, None, &Secp256k1::new()).unwrap(); check_spends!(spend_tx, spendable_tx); + outputs.pop().unwrap() } else { panic!(); } } @@ -196,8 +198,8 @@ fn chanmon_claim_value_coop_close() { assert_eq!(shutdown_tx, nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0)); assert_eq!(shutdown_tx.len(), 1); - mine_transaction(&nodes[0], &shutdown_tx[0]); - mine_transaction(&nodes[1], &shutdown_tx[0]); + let shutdown_tx_conf_height_a = block_from_scid(&mine_transaction(&nodes[0], &shutdown_tx[0])); + let shutdown_tx_conf_height_b = block_from_scid(&mine_transaction(&nodes[1], &shutdown_tx[0])); assert!(nodes[0].node.list_channels().is_empty()); assert!(nodes[1].node.list_channels().is_empty()); @@ -216,16 +218,35 @@ fn chanmon_claim_value_coop_close() { }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); - connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1); - connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1); + connect_blocks(&nodes[0], ANTI_REORG_DELAY - 2); + connect_blocks(&nodes[1], ANTI_REORG_DELAY - 2); + + assert!(get_monitor!(nodes[0], chan_id) + .get_spendable_output(&shutdown_tx[0], shutdown_tx_conf_height_a).is_none()); + assert!(get_monitor!(nodes[1], chan_id) + .get_spendable_output(&shutdown_tx[0], shutdown_tx_conf_height_b).is_none()); + + connect_blocks(&nodes[0], 1); + connect_blocks(&nodes[1], 1); assert_eq!(Vec::::new(), nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); assert_eq!(Vec::::new(), nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); - test_spendable_output(&nodes[0], &shutdown_tx[0]); - test_spendable_output(&nodes[1], &shutdown_tx[0]); + let spendable_output_a = test_spendable_output(&nodes[0], &shutdown_tx[0]); + assert_eq!( + get_monitor!(nodes[0], chan_id) + .get_spendable_output(&shutdown_tx[0], shutdown_tx_conf_height_a).unwrap(), + spendable_output_a + ); + + let spendable_output_b = test_spendable_output(&nodes[1], &shutdown_tx[0]); + assert_eq!( + get_monitor!(nodes[1], chan_id) + .get_spendable_output(&shutdown_tx[0], shutdown_tx_conf_height_b).unwrap(), + spendable_output_b + ); check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 1000000); check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[0].node.get_our_node_id()], 1000000); From ffec24b3e3def1e91a94fd6c1cb590ed3d846b0b Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 28 Sep 2023 11:42:13 -0700 Subject: [PATCH 2/2] Retrieve all possible spendable outputs from transactions Assuming our keys haven't been compromised, and that random transactions aren't learning of these scripts somehow and sending funds to them, it was only possible for one spendable output to exist within a transaction. - `shutdown_script` can only exist in co-op close transactions. - `counterparty_payment_script` can only exist in counterparty commitment transactions. - `broadcasted_holder_revokable_script` can only exist in holder commitment/HTLC transactions. - `destination_script` can exist in any other type of claim we support. Now that we're exposing this API to users such that they can rescan any relevant transactions, there's no harm in allowing them to claim more funds from spendable outputs than we expected. --- lightning/src/chain/channelmonitor.rs | 49 ++++++++++----------------- lightning/src/ln/monitor_tests.rs | 24 ++++++------- 2 files changed, 29 insertions(+), 44 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index f652bbf36..7088d32d1 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1670,8 +1670,8 @@ impl ChannelMonitor { ); } - /// Returns the descriptor for a relevant output (i.e., one that we can spend) within the - /// transaction if one exists and the transaction has at least [`ANTI_REORG_DELAY`] + /// Returns the descriptors for relevant outputs (i.e., those that we can spend) within the + /// transaction if they exist and the transaction has at least [`ANTI_REORG_DELAY`] /// confirmations. /// /// Descriptors returned by this method are primarily exposed via [`Event::SpendableOutputs`] @@ -1683,17 +1683,17 @@ impl ChannelMonitor { /// transactions starting from the channel's funding or closing transaction that have at least /// [`ANTI_REORG_DELAY`] confirmations. /// - /// `tx` is a transaction we'll scan the outputs of. Any transaction can be provided. If an - /// output which can be spent by us is found, a descriptor is returned. + /// `tx` is a transaction we'll scan the outputs of. Any transaction can be provided. If any + /// outputs which can be spent by us are found, at least one descriptor is returned. /// /// `confirmation_height` must be the height of the block in which `tx` was included in. - pub fn get_spendable_output(&self, tx: &Transaction, confirmation_height: u32) -> Option { + pub fn get_spendable_outputs(&self, tx: &Transaction, confirmation_height: u32) -> Vec { let inner = self.inner.lock().unwrap(); let current_height = inner.best_block.height; if current_height.saturating_sub(ANTI_REORG_DELAY) + 1 >= confirmation_height { - inner.get_spendable_output(tx) + inner.get_spendable_outputs(tx) } else { - None + Vec::new() } } } @@ -3468,7 +3468,7 @@ impl ChannelMonitorImpl { } self.is_resolving_htlc_output(&tx, height, &block_hash, &logger); - self.check_tx_and_push_spendable_output(&tx, height, &block_hash, &logger); + self.check_tx_and_push_spendable_outputs(&tx, height, &block_hash, &logger); } } @@ -4014,31 +4014,18 @@ impl ChannelMonitorImpl { } } - fn get_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 i > ::core::u16::MAX as usize { - // While it is possible that an output exists on chain which is greater than the - // 2^16th output in a given transaction, this is only possible if the output is not - // in a lightning transaction and was instead placed there by some third party who - // wishes to give us money for no reason. - // Namely, any lightning transactions which we pre-sign will never have anywhere - // near 2^16 outputs both because such transactions must have ~2^16 outputs who's - // scripts are not longer than one byte in length and because they are inherently - // non-standard due to their size. - // Thus, it is completely safe to ignore such outputs, and while it may result in - // us ignoring non-lightning fund to us, that is only possible if someone fills - // nearly a full block with garbage just to hit this case. - continue; - } + fn get_spendable_outputs(&self, tx: &Transaction) -> Vec { + let mut spendable_outputs = Vec::new(); + for (i, outp) in tx.output.iter().enumerate() { if outp.script_pubkey == self.destination_script { - return Some(SpendableOutputDescriptor::StaticOutput { + spendable_outputs.push(SpendableOutputDescriptor::StaticOutput { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), }); } if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script { if broadcasted_holder_revokable_script.0 == outp.script_pubkey { - return Some(SpendableOutputDescriptor::DelayedPaymentOutput(DelayedPaymentOutputDescriptor { + spendable_outputs.push(SpendableOutputDescriptor::DelayedPaymentOutput(DelayedPaymentOutputDescriptor { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, per_commitment_point: broadcasted_holder_revokable_script.1, to_self_delay: self.on_holder_tx_csv, @@ -4050,7 +4037,7 @@ impl ChannelMonitorImpl { } } if self.counterparty_payment_script == outp.script_pubkey { - return Some(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor { + spendable_outputs.push(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), channel_keys_id: self.channel_keys_id, @@ -4058,21 +4045,21 @@ impl ChannelMonitorImpl { })); } if self.shutdown_script.as_ref() == Some(&outp.script_pubkey) { - return Some(SpendableOutputDescriptor::StaticOutput { + spendable_outputs.push(SpendableOutputDescriptor::StaticOutput { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), }); } } - None + spendable_outputs } /// Checks if the confirmed transaction is paying funds back to some address we can assume to /// own. - fn check_tx_and_push_spendable_output( + fn check_tx_and_push_spendable_outputs( &mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L, ) where L::Target: Logger { - if let Some(spendable_output) = self.get_spendable_output(tx) { + for spendable_output in self.get_spendable_outputs(tx) { let entry = OnchainEventEntry { txid: tx.txid(), transaction: Some(tx.clone()), diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 1ccb61cbb..892ddce2a 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -93,15 +93,15 @@ fn chanmon_fail_from_stale_commitment() { expect_payment_failed_with_update!(nodes[0], payment_hash, false, update_a.contents.short_channel_id, true); } -fn test_spendable_output<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, spendable_tx: &Transaction) -> SpendableOutputDescriptor { +fn test_spendable_output<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, spendable_tx: &Transaction) -> Vec { let mut spendable = node.chain_monitor.chain_monitor.get_and_clear_pending_events(); assert_eq!(spendable.len(), 1); - if let Event::SpendableOutputs { mut outputs, .. } = spendable.pop().unwrap() { + if let Event::SpendableOutputs { outputs, .. } = spendable.pop().unwrap() { assert_eq!(outputs.len(), 1); let spend_tx = node.keys_manager.backing.spend_spendable_outputs(&[&outputs[0]], Vec::new(), Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script(), 253, None, &Secp256k1::new()).unwrap(); check_spends!(spend_tx, spendable_tx); - outputs.pop().unwrap() + outputs } else { panic!(); } } @@ -222,9 +222,9 @@ fn chanmon_claim_value_coop_close() { connect_blocks(&nodes[1], ANTI_REORG_DELAY - 2); assert!(get_monitor!(nodes[0], chan_id) - .get_spendable_output(&shutdown_tx[0], shutdown_tx_conf_height_a).is_none()); + .get_spendable_outputs(&shutdown_tx[0], shutdown_tx_conf_height_a).is_empty()); assert!(get_monitor!(nodes[1], chan_id) - .get_spendable_output(&shutdown_tx[0], shutdown_tx_conf_height_b).is_none()); + .get_spendable_outputs(&shutdown_tx[0], shutdown_tx_conf_height_b).is_empty()); connect_blocks(&nodes[0], 1); connect_blocks(&nodes[1], 1); @@ -234,18 +234,16 @@ fn chanmon_claim_value_coop_close() { assert_eq!(Vec::::new(), nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); - let spendable_output_a = test_spendable_output(&nodes[0], &shutdown_tx[0]); + let spendable_outputs_a = test_spendable_output(&nodes[0], &shutdown_tx[0]); assert_eq!( - get_monitor!(nodes[0], chan_id) - .get_spendable_output(&shutdown_tx[0], shutdown_tx_conf_height_a).unwrap(), - spendable_output_a + get_monitor!(nodes[0], chan_id).get_spendable_outputs(&shutdown_tx[0], shutdown_tx_conf_height_a), + spendable_outputs_a ); - let spendable_output_b = test_spendable_output(&nodes[1], &shutdown_tx[0]); + let spendable_outputs_b = test_spendable_output(&nodes[1], &shutdown_tx[0]); assert_eq!( - get_monitor!(nodes[1], chan_id) - .get_spendable_output(&shutdown_tx[0], shutdown_tx_conf_height_b).unwrap(), - spendable_output_b + get_monitor!(nodes[1], chan_id).get_spendable_outputs(&shutdown_tx[0], shutdown_tx_conf_height_b), + spendable_outputs_b ); check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 1000000);