From c8ff32197aa78c3e66f4156b44bc3d6580202321 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 13 Nov 2023 11:00:41 +0100 Subject: [PATCH] Return confirmation height via `Confirm::get_relevant_txids` We previously included the block hash, but it's also useful to include the height under which we expect the respective transaction to be confirmed. --- lightning-transaction-sync/src/esplora.rs | 4 ++-- .../tests/integration_tests.rs | 4 ++-- lightning/src/chain/chainmonitor.rs | 6 +++--- lightning/src/chain/channelmonitor.rs | 12 ++++++------ lightning/src/chain/mod.rs | 8 ++++---- lightning/src/chain/onchaintx.rs | 10 +++++----- lightning/src/ln/channel.rs | 10 ++++++++++ lightning/src/ln/channelmanager.rs | 9 ++++++--- lightning/src/ln/reorg_tests.rs | 6 ++++-- pending_changelog/electrum.txt | 3 +++ 10 files changed, 45 insertions(+), 27 deletions(-) create mode 100644 pending_changelog/electrum.txt diff --git a/lightning-transaction-sync/src/esplora.rs b/lightning-transaction-sync/src/esplora.rs index f68be08a5..91787c629 100644 --- a/lightning-transaction-sync/src/esplora.rs +++ b/lightning-transaction-sync/src/esplora.rs @@ -316,11 +316,11 @@ where let relevant_txids = confirmables .iter() .flat_map(|c| c.get_relevant_txids()) - .collect::)>>(); + .collect::)>>(); let mut unconfirmed_txs = Vec::new(); - for (txid, block_hash_opt) in relevant_txids { + for (txid, _conf_height, block_hash_opt) in relevant_txids { if let Some(block_hash) = block_hash_opt { let block_status = maybe_await!(self.client.get_block_status(&block_hash))?; if block_status.in_best_chain { diff --git a/lightning-transaction-sync/tests/integration_tests.rs b/lightning-transaction-sync/tests/integration_tests.rs index 8a69dce3f..218d187a4 100644 --- a/lightning-transaction-sync/tests/integration_tests.rs +++ b/lightning-transaction-sync/tests/integration_tests.rs @@ -143,8 +143,8 @@ impl Confirm for TestConfirmable { self.events.lock().unwrap().push(TestConfirmableEvent::BestBlockUpdated(block_hash, height)); } - fn get_relevant_txids(&self) -> Vec<(Txid, Option)> { - self.confirmed_txs.lock().unwrap().iter().map(|(&txid, (hash, _))| (txid, Some(*hash))).collect::>() + fn get_relevant_txids(&self) -> Vec<(Txid, u32, Option)> { + self.confirmed_txs.lock().unwrap().iter().map(|(&txid, (hash, height))| (txid, *height, Some(*hash))).collect::>() } } diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 0186ac374..800bee8e3 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -689,15 +689,15 @@ where }); } - fn get_relevant_txids(&self) -> Vec<(Txid, Option)> { + fn get_relevant_txids(&self) -> Vec<(Txid, u32, Option)> { let mut txids = Vec::new(); let monitor_states = self.monitors.read().unwrap(); for monitor_state in monitor_states.values() { txids.append(&mut monitor_state.monitor.get_relevant_txids()); } - txids.sort_unstable(); - txids.dedup(); + txids.sort_unstable_by(|a, b| a.0.cmp(&b.0).then(b.1.cmp(&a.1))); + txids.dedup_by_key(|(txid, _, _)| *txid); txids } } diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index d78f40f3f..65b587227 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1634,15 +1634,15 @@ impl ChannelMonitor { } /// Returns the set of txids that should be monitored for re-organization out of the chain. - pub fn get_relevant_txids(&self) -> Vec<(Txid, Option)> { + pub fn get_relevant_txids(&self) -> Vec<(Txid, u32, Option)> { let inner = self.inner.lock().unwrap(); - let mut txids: Vec<(Txid, Option)> = inner.onchain_events_awaiting_threshold_conf + let mut txids: Vec<(Txid, u32, Option)> = inner.onchain_events_awaiting_threshold_conf .iter() - .map(|entry| (entry.txid, entry.block_hash)) + .map(|entry| (entry.txid, entry.height, entry.block_hash)) .chain(inner.onchain_tx_handler.get_relevant_txids().into_iter()) .collect(); - txids.sort_unstable(); - txids.dedup(); + txids.sort_unstable_by(|a, b| a.0.cmp(&b.0).then(b.1.cmp(&a.1))); + txids.dedup_by_key(|(txid, _, _)| *txid); txids } @@ -4171,7 +4171,7 @@ where self.0.best_block_updated(header, height, &*self.1, &*self.2, &*self.3); } - fn get_relevant_txids(&self) -> Vec<(Txid, Option)> { + fn get_relevant_txids(&self) -> Vec<(Txid, u32, Option)> { self.0.get_relevant_txids() } } diff --git a/lightning/src/chain/mod.rs b/lightning/src/chain/mod.rs index 239f7c91d..a7c3a6d88 100644 --- a/lightning/src/chain/mod.rs +++ b/lightning/src/chain/mod.rs @@ -152,7 +152,7 @@ pub trait Confirm { /// blocks. fn best_block_updated(&self, header: &Header, height: u32); /// Returns transactions that must be monitored for reorganization out of the chain along - /// with the hash of the block as part of which it had been previously confirmed. + /// with the height and the hash of the block as part of which it had been previously confirmed. /// /// Note that the returned `Option` might be `None` for channels created with LDK /// 0.0.112 and prior, in which case you need to manually track previous confirmations. @@ -167,12 +167,12 @@ pub trait Confirm { /// given to [`transaction_unconfirmed`]. /// /// If any of the returned transactions are confirmed in a block other than the one with the - /// given hash, they need to be unconfirmed and reconfirmed via [`transaction_unconfirmed`] and - /// [`transactions_confirmed`], respectively. + /// given hash at the given height, they need to be unconfirmed and reconfirmed via + /// [`transaction_unconfirmed`] and [`transactions_confirmed`], respectively. /// /// [`transactions_confirmed`]: Self::transactions_confirmed /// [`transaction_unconfirmed`]: Self::transaction_unconfirmed - fn get_relevant_txids(&self) -> Vec<(Txid, Option)>; + fn get_relevant_txids(&self) -> Vec<(Txid, u32, Option)>; } /// An enum representing the status of a channel monitor update persistence. diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 257385489..c28c572e6 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1076,13 +1076,13 @@ impl OnchainTxHandler self.claimable_outpoints.get(outpoint).is_some() } - pub(crate) fn get_relevant_txids(&self) -> Vec<(Txid, Option)> { - let mut txids: Vec<(Txid, Option)> = self.onchain_events_awaiting_threshold_conf + pub(crate) fn get_relevant_txids(&self) -> Vec<(Txid, u32, Option)> { + let mut txids: Vec<(Txid, u32, Option)> = self.onchain_events_awaiting_threshold_conf .iter() - .map(|entry| (entry.txid, entry.block_hash)) + .map(|entry| (entry.txid, entry.height, entry.block_hash)) .collect(); - txids.sort_unstable_by_key(|(txid, _)| *txid); - txids.dedup(); + txids.sort_unstable_by(|a, b| a.0.cmp(&b.0).then(b.1.cmp(&a.1))); + txids.dedup_by_key(|(txid, _, _)| *txid); txids } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 6803b1128..740d26448 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1111,6 +1111,16 @@ impl ChannelContext where SP::Target: SignerProvider { self.channel_transaction_parameters.funding_outpoint } + /// Returns the height in which our funding transaction was confirmed. + pub fn get_funding_tx_confirmation_height(&self) -> Option { + let conf_height = self.funding_tx_confirmation_height; + if conf_height > 0 { + Some(conf_height) + } else { + None + } + } + /// Returns the block hash in which our funding transaction was confirmed. pub fn get_funding_tx_confirmed_in(&self) -> Option { self.funding_tx_confirmed_in diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c40ab4858..3f6277b44 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8336,14 +8336,17 @@ where }); } - fn get_relevant_txids(&self) -> Vec<(Txid, Option)> { + fn get_relevant_txids(&self) -> Vec<(Txid, u32, Option)> { let mut res = Vec::with_capacity(self.short_to_chan_info.read().unwrap().len()); for (_cp_id, peer_state_mutex) in self.per_peer_state.read().unwrap().iter() { let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; for chan in peer_state.channel_by_id.values().filter_map(|phase| if let ChannelPhase::Funded(chan) = phase { Some(chan) } else { None }) { - if let (Some(funding_txo), Some(block_hash)) = (chan.context.get_funding_txo(), chan.context.get_funding_tx_confirmed_in()) { - res.push((funding_txo.txid, Some(block_hash))); + let txid_opt = chan.context.get_funding_txo(); + let height_opt = chan.context.get_funding_tx_confirmation_height(); + let hash_opt = chan.context.get_funding_tx_confirmed_in(); + if let (Some(funding_txo), Some(conf_height), Some(block_hash)) = (txid_opt, height_opt, hash_opt) { + res.push((funding_txo.txid, conf_height, Some(block_hash))); } } } diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index b745453a3..badb78f24 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -270,8 +270,9 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_ if use_funding_unconfirmed { let relevant_txids = nodes[0].node.get_relevant_txids(); assert_eq!(relevant_txids.len(), 1); - let block_hash_opt = relevant_txids[0].1; + let block_hash_opt = relevant_txids[0].2; let expected_hash = nodes[0].get_block_header(chan_conf_height).block_hash(); + assert_eq!(relevant_txids[0].1, chan_conf_height); assert_eq!(block_hash_opt, Some(expected_hash)); let txid = relevant_txids[0].0; assert_eq!(txid, chan.3.txid()); @@ -315,8 +316,9 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_ if use_funding_unconfirmed { let relevant_txids = nodes[0].node.get_relevant_txids(); assert_eq!(relevant_txids.len(), 1); - let block_hash_opt = relevant_txids[0].1; + let block_hash_opt = relevant_txids[0].2; let expected_hash = nodes[0].get_block_header(chan_conf_height).block_hash(); + assert_eq!(chan_conf_height, relevant_txids[0].1); assert_eq!(block_hash_opt, Some(expected_hash)); let txid = relevant_txids[0].0; assert_eq!(txid, chan.3.txid()); diff --git a/pending_changelog/electrum.txt b/pending_changelog/electrum.txt new file mode 100644 index 000000000..5171f5ea0 --- /dev/null +++ b/pending_changelog/electrum.txt @@ -0,0 +1,3 @@ +## API Updates + +- The `Confirm::get_relevant_txids()` call now also returns the height under which LDK expects the respective transaction to be confirmed. -- 2.39.5