Skip to content

Allow retrieval of SpendableOutputDescriptors from relevant transactions #2609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 29, 2023
Merged

Allow retrieval of SpendableOutputDescriptors from relevant transactions #2609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8f80f8
Allow retrieval of SpendableOutputDescriptors from relevant transactions
wpaulino Sep 27, 2023
ffec24b
Retrieve all possible spendable outputs from transactions
wpaulino Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 44 additions & 28 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1669,6 +1669,33 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
current_height, &broadcaster, &fee_estimator, &logger,
);
}

/// 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`]
/// 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to imply you only need to pass the one immediate descendant - the commitment tx, but actually we need to go two layers deep.

/// 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 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_outputs(&self, tx: &Transaction, confirmation_height: u32) -> Vec<SpendableOutputDescriptor> {
let inner = self.inner.lock().unwrap();
let current_height = inner.best_block.height;
if current_height.saturating_sub(ANTI_REORG_DELAY) + 1 >= confirmation_height {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One API advance could be to pass a user-selected value rather than to rely on the hardcoded ANTI_REORG_DELAY. If the spend is low-value or you wish to have the liquidity locked up in a let’s say StaticPaymentOutputDescriptor available to spend now 1-conf or 2-conf might be enough.

Here ANTI_REORG_DELAY doesn’t necessarily have the same level of severity than in other locations of channel monitor, where we wanna to be sure a HTLC-timeout cannot be reorged out by a HTLC-preimage on an outbound link before passing backward the timeout. I think a descriptor_confirmation_delay user setting could be introduced in the future.

inner.get_spendable_outputs(tx)
} else {
Vec::new()
}
}
}

impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
Expand Down Expand Up @@ -3441,7 +3468,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
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_outputs(&tx, height, &block_hash, &logger);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a counterparty is paying to one of the spendable descriptor (e.g our payment_point as a P2WSH or P2WPKH) from let’s say a second-stage malleable HTLC-transaction on our local / holder commitment transaction, we might receive a descriptor of a “tag” value that can be latter used for e.g deanonymization attack (at the image of sending dust value to on-chain wallet to fingerprint them).

I don’t think this is a new concern and this sounds already permissible by current is_paying_spendable_output. l still think our transaction pattern matching in transactions_confirmed could be stronger to check that non-commitment-or-closing-transaction-paying-to-us-is-spent-by-us.

}
}

Expand Down Expand Up @@ -3987,34 +4014,18 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
}

/// Check if any transaction broadcasted is paying fund back to some address we can assume to own
fn is_paying_spendable_output<L: Deref>(&mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L) where L::Target: Logger {
let mut spendable_output = None;
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<SpendableOutputDescriptor> {
let mut spendable_outputs = Vec::new();
for (i, outp) in tx.output.iter().enumerate() {
if outp.script_pubkey == self.destination_script {
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
spendable_outputs.push(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 {
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,
Expand All @@ -4023,27 +4034,32 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
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 {
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,
channel_value_satoshis: self.channel_value_satoshis,
}));
break;
}
if self.shutdown_script.as_ref() == Some(&outp.script_pubkey) {
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
output: outp.clone(),
});
break;
}
}
if let Some(spendable_output) = spendable_output {
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_outputs<L: Deref>(
&mut self, tx: &Transaction, height: u32, block_hash: &BlockHash, logger: &L,
) where L::Target: Logger {
for spendable_output in self.get_spendable_outputs(tx) {
let entry = OnchainEventEntry {
txid: tx.txid(),
transaction: Some(tx.clone()),
Expand Down
35 changes: 27 additions & 8 deletions lightning/src/ln/monitor_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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) -> Vec<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() {
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
} else { panic!(); }
}

Expand Down Expand Up @@ -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());
Expand All @@ -216,16 +218,33 @@ 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_outputs(&shutdown_tx[0], shutdown_tx_conf_height_a).is_empty());
assert!(get_monitor!(nodes[1], chan_id)
.get_spendable_outputs(&shutdown_tx[0], shutdown_tx_conf_height_b).is_empty());

connect_blocks(&nodes[0], 1);
connect_blocks(&nodes[1], 1);

assert_eq!(Vec::<Balance>::new(),
nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances());
assert_eq!(Vec::<Balance>::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_outputs_a = test_spendable_output(&nodes[0], &shutdown_tx[0]);
assert_eq!(
get_monitor!(nodes[0], chan_id).get_spendable_outputs(&shutdown_tx[0], shutdown_tx_conf_height_a),
spendable_outputs_a
);

let spendable_outputs_b = test_spendable_output(&nodes[1], &shutdown_tx[0]);
assert_eq!(
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);
check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[0].node.get_our_node_id()], 1000000);
Expand Down