From aa64a2007b1dbaabe5c09895deccc1dc70a4f9df Mon Sep 17 00:00:00 2001 From: Arik Sosman Date: Mon, 27 Jan 2025 08:11:03 -0800 Subject: [PATCH 1/2] Add target delta to UrgentOnChainSweep --- fuzz/src/chanmon_consistency.rs | 2 +- lightning/src/chain/chaininterface.rs | 4 +++- lightning/src/chain/channelmonitor.rs | 25 +++++++++++++++++++------ lightning/src/chain/onchaintx.rs | 4 ++-- lightning/src/chain/package.rs | 2 +- lightning/src/ln/monitor_tests.rs | 2 +- 6 files changed, 27 insertions(+), 12 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 26c9fb2177d..7222f1066cf 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -98,7 +98,7 @@ impl FeeEstimator for FuzzEstimator { // always return a HighPriority feerate here which is >= the maximum Normal feerate and a // Background feerate which is <= the minimum Normal feerate. match conf_target { - ConfirmationTarget::MaximumFeeEstimate | ConfirmationTarget::UrgentOnChainSweep => { + ConfirmationTarget::MaximumFeeEstimate | ConfirmationTarget::UrgentOnChainSweep(_) => { MAX_FEE }, ConfirmationTarget::ChannelCloseMinimum diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index ebef21657b7..98356bec169 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -61,7 +61,9 @@ pub enum ConfirmationTarget { /// Generally we have in the high tens to low hundreds of blocks to get our transaction /// on-chain (it doesn't have to happen in the next few blocks!), but we shouldn't risk too low /// a fee - this should be a relatively high priority feerate. - UrgentOnChainSweep, + /// + /// If a target confirmation delta is known, it can be set as the parameter. + UrgentOnChainSweep(Option), /// This is the lowest feerate we will allow our channel counterparty to have in an anchor /// channel in order to close the channel if a channel party goes away. /// diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 3f6bdc3f256..0bcca3b313c 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2744,19 +2744,32 @@ impl ChannelMonitorImpl { // Treat the sweep as urgent as long as there is at least one HTLC which is pending on a // valid commitment transaction. if !self.current_holder_commitment_tx.htlc_outputs.is_empty() { - return ConfirmationTarget::UrgentOnChainSweep; + let minimum_expiry = self.current_holder_commitment_tx.htlc_outputs + .iter() + .map(|o| o.0.cltv_expiry) + .min(); + return ConfirmationTarget::UrgentOnChainSweep(minimum_expiry); } if self.prev_holder_signed_commitment_tx.as_ref().map(|t| !t.htlc_outputs.is_empty()).unwrap_or(false) { - return ConfirmationTarget::UrgentOnChainSweep; + let minimum_expiry = self.prev_holder_signed_commitment_tx.as_ref().map(|t| t.htlc_outputs + .iter() + .map(|o| o.0.cltv_expiry) + .min() + ).flatten(); + return ConfirmationTarget::UrgentOnChainSweep(minimum_expiry); } if let Some(txid) = self.current_counterparty_commitment_txid { - if !self.counterparty_claimable_outpoints.get(&txid).unwrap().is_empty() { - return ConfirmationTarget::UrgentOnChainSweep; + let claimable_outpoints = self.counterparty_claimable_outpoints.get(&txid).unwrap(); + if !claimable_outpoints.is_empty() { + let minimum_expiry = claimable_outpoints.iter().map(|o|o.0.cltv_expiry).min(); + return ConfirmationTarget::UrgentOnChainSweep(minimum_expiry); } } if let Some(txid) = self.prev_counterparty_commitment_txid { - if !self.counterparty_claimable_outpoints.get(&txid).unwrap().is_empty() { - return ConfirmationTarget::UrgentOnChainSweep; + let claimable_outpoints = self.counterparty_claimable_outpoints.get(&txid).unwrap(); + if !claimable_outpoints.is_empty() { + let minimum_expiry = claimable_outpoints.iter().map(|o|o.0.cltv_expiry).min(); + return ConfirmationTarget::UrgentOnChainSweep(minimum_expiry); } } ConfirmationTarget::OutputSpendingFee diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 2a43b006920..2b42fd95907 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1417,7 +1417,7 @@ mod tests { 1, 1, &&broadcaster, - ConfirmationTarget::UrgentOnChainSweep, + ConfirmationTarget::UrgentOnChainSweep(Some(2)), &fee_estimator, &logger, ); @@ -1440,7 +1440,7 @@ mod tests { 2, 2, &&broadcaster, - ConfirmationTarget::UrgentOnChainSweep, + ConfirmationTarget::UrgentOnChainSweep(Some(3)), &fee_estimator, &logger, ); diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 52ccf03f168..83048c59be1 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -1689,7 +1689,7 @@ mod tests { let test_fee_estimator = &TestFeeEstimator { sat_per_kw }; let fee_estimator = LowerBoundedFeeEstimator::new(test_fee_estimator); let fee_rate_strategy = FeerateStrategy::ForceBump; - let confirmation_target = ConfirmationTarget::UrgentOnChainSweep; + let confirmation_target = ConfirmationTarget::UrgentOnChainSweep(None); { // Check underflow doesn't occur diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 9556e988b4e..f25742ed33e 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -2556,7 +2556,7 @@ fn do_test_yield_anchors_events(have_htlcs: bool) { // Note that if we use the wrong target, we will immediately broadcast the commitment // transaction as no bump is required. if have_htlcs { - nodes[0].fee_estimator.target_override.lock().unwrap().insert(ConfirmationTarget::UrgentOnChainSweep, 500); + nodes[0].fee_estimator.target_override.lock().unwrap().insert(ConfirmationTarget::UrgentOnChainSweep(Some(81)), 500); } else { nodes[0].fee_estimator.target_override.lock().unwrap().insert(ConfirmationTarget::OutputSpendingFee, 500); } From 56b73e77291d2fc8124f7092d03aa081eb86c54d Mon Sep 17 00:00:00 2001 From: Arik Sosman Date: Mon, 10 Feb 2025 22:27:36 -0800 Subject: [PATCH 2/2] f: return global minimum --- lightning/src/chain/channelmonitor.rs | 28 +++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 0bcca3b313c..f43687ad735 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2743,35 +2743,47 @@ impl ChannelMonitorImpl { fn closure_conf_target(&self) -> ConfirmationTarget { // Treat the sweep as urgent as long as there is at least one HTLC which is pending on a // valid commitment transaction. + let mut minimum_expiry = None; + let update_minimum_expiry = |local_minimum: Option, global_minimum: &mut Option| { + if let Some(expiry) = local_minimum { + *global_minimum = Some(global_minimum.map_or(expiry, |m| cmp::min(expiry, m))); + } + }; + if !self.current_holder_commitment_tx.htlc_outputs.is_empty() { - let minimum_expiry = self.current_holder_commitment_tx.htlc_outputs + let local_minimum_expiry = self.current_holder_commitment_tx.htlc_outputs .iter() .map(|o| o.0.cltv_expiry) .min(); - return ConfirmationTarget::UrgentOnChainSweep(minimum_expiry); + update_minimum_expiry(local_minimum_expiry, &mut minimum_expiry); } if self.prev_holder_signed_commitment_tx.as_ref().map(|t| !t.htlc_outputs.is_empty()).unwrap_or(false) { - let minimum_expiry = self.prev_holder_signed_commitment_tx.as_ref().map(|t| t.htlc_outputs + let local_minimum_expiry = self.prev_holder_signed_commitment_tx.as_ref().map(|t| t.htlc_outputs .iter() .map(|o| o.0.cltv_expiry) .min() ).flatten(); - return ConfirmationTarget::UrgentOnChainSweep(minimum_expiry); + update_minimum_expiry(local_minimum_expiry, &mut minimum_expiry); } if let Some(txid) = self.current_counterparty_commitment_txid { let claimable_outpoints = self.counterparty_claimable_outpoints.get(&txid).unwrap(); if !claimable_outpoints.is_empty() { - let minimum_expiry = claimable_outpoints.iter().map(|o|o.0.cltv_expiry).min(); - return ConfirmationTarget::UrgentOnChainSweep(minimum_expiry); + let local_minimum_expiry = claimable_outpoints.iter().map(|o|o.0.cltv_expiry).min(); + update_minimum_expiry(local_minimum_expiry, &mut minimum_expiry); } } if let Some(txid) = self.prev_counterparty_commitment_txid { let claimable_outpoints = self.counterparty_claimable_outpoints.get(&txid).unwrap(); if !claimable_outpoints.is_empty() { - let minimum_expiry = claimable_outpoints.iter().map(|o|o.0.cltv_expiry).min(); - return ConfirmationTarget::UrgentOnChainSweep(minimum_expiry); + let local_minimum_expiry = claimable_outpoints.iter().map(|o|o.0.cltv_expiry).min(); + update_minimum_expiry(local_minimum_expiry, &mut minimum_expiry); } } + + if let Some(global_minimum) = minimum_expiry { + return ConfirmationTarget::UrgentOnChainSweep(Some(global_minimum)) + } + ConfirmationTarget::OutputSpendingFee }