From 990c5000999d3497f13b802f7fd4dce014704a08 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 11 Jul 2023 15:16:16 -0700 Subject: [PATCH] Avoid yielding ChannelClose bump events with sufficient feerate There's no need to yield such an event when the commitment transaction already meets the target feerate on its own, so we can simply broadcast it without an anchor child transaction. This may be a common occurrence until we are less aggressive about feerate updates. --- lightning/src/chain/onchaintx.rs | 24 ++++++++++++++++++++---- lightning/src/chain/package.rs | 2 +- lightning/src/events/bump_transaction.rs | 12 ------------ lightning/src/ln/monitor_tests.rs | 2 ++ 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 6b0c74856..171caf700 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -22,6 +22,7 @@ use bitcoin::hash_types::{Txid, BlockHash}; use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; use bitcoin::secp256k1; +use crate::chain::chaininterface::compute_feerate_sat_per_1000_weight; use crate::sign::{ChannelSigner, EntropySource, SignerProvider}; use crate::ln::msgs::DecodeError; use crate::ln::PaymentPreimage; @@ -623,9 +624,24 @@ impl OnchainTxHandler return inputs.find_map(|input| match input { // Commitment inputs with anchors support are the only untractable inputs supported // thus far that require external funding. - PackageSolvingData::HolderFundingOutput(..) => { + PackageSolvingData::HolderFundingOutput(output) => { debug_assert_eq!(tx.txid(), self.holder_commitment.trust().txid(), "Holder commitment transaction mismatch"); + + let conf_target = ConfirmationTarget::HighPriority; + let package_target_feerate_sat_per_1000_weight = cached_request + .compute_package_feerate(fee_estimator, conf_target, force_feerate_bump); + if let Some(input_amount_sat) = output.funding_amount { + let fee_sat = input_amount_sat - tx.output.iter().map(|output| output.value).sum::(); + if compute_feerate_sat_per_1000_weight(fee_sat, tx.weight() as u64) >= + package_target_feerate_sat_per_1000_weight + { + log_debug!(logger, "Commitment transaction {} already meets required feerate {} sat/kW", + tx.txid(), package_target_feerate_sat_per_1000_weight); + return Some((new_timer, 0, OnchainClaim::Tx(tx.clone()))); + } + } + // We'll locate an anchor output we can spend within the commitment transaction. let funding_pubkey = &self.channel_transaction_parameters.holder_pubkeys.funding_pubkey; match chan_utils::get_anchor_output(&tx, funding_pubkey) { @@ -633,9 +649,6 @@ impl OnchainTxHandler Some((idx, _)) => { // TODO: Use a lower confirmation target when both our and the // counterparty's latest commitment don't have any HTLCs present. - let conf_target = ConfirmationTarget::HighPriority; - let package_target_feerate_sat_per_1000_weight = cached_request - .compute_package_feerate(fee_estimator, conf_target, force_feerate_bump); Some(( new_timer, package_target_feerate_sat_per_1000_weight as u64, @@ -739,6 +752,9 @@ impl OnchainTxHandler ) { req.set_timer(new_timer); req.set_feerate(new_feerate); + // Once a pending claim has an id assigned, it remains fixed until the claim is + // satisfied, regardless of whether the claim switches between different variants of + // `OnchainClaim`. let claim_id = match claim { OnchainClaim::Tx(tx) => { log_info!(logger, "Broadcasting onchain {}", log_tx!(tx)); diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 9e61cbc45..b66a2f70d 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -429,7 +429,7 @@ impl Readable for HolderHTLCOutput { #[derive(Clone, PartialEq, Eq)] pub(crate) struct HolderFundingOutput { funding_redeemscript: Script, - funding_amount: Option, + pub(crate) funding_amount: Option, channel_type_features: ChannelTypeFeatures, } diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index dbe539505..ffd9b2283 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -700,18 +700,6 @@ where &self, claim_id: ClaimId, package_target_feerate_sat_per_1000_weight: u32, commitment_tx: &Transaction, commitment_tx_fee_sat: u64, anchor_descriptor: &AnchorDescriptor, ) -> Result<(), ()> { - // Compute the feerate the anchor transaction must meet to meet the overall feerate for the - // package (commitment + anchor transactions). - let commitment_tx_sat_per_1000_weight: u32 = compute_feerate_sat_per_1000_weight( - commitment_tx_fee_sat, commitment_tx.weight() as u64, - ); - if commitment_tx_sat_per_1000_weight >= package_target_feerate_sat_per_1000_weight { - // If the commitment transaction already has a feerate high enough on its own, broadcast - // it as is without a child. - self.broadcaster.broadcast_transactions(&[&commitment_tx]); - return Ok(()); - } - let mut anchor_tx = self.build_anchor_tx( claim_id, package_target_feerate_sat_per_1000_weight, commitment_tx, anchor_descriptor, )?; diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 41ecd9da0..22b5fa7ce 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -1878,6 +1878,7 @@ fn test_yield_anchors_events() { assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); + *nodes[0].fee_estimator.sat_per_kw.lock().unwrap() *= 2; connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1); check_closed_broadcast!(&nodes[0], true); assert!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty()); @@ -2054,6 +2055,7 @@ fn test_anchors_aggregated_revoked_htlc_tx() { // Bob force closes by restarting with the outdated state, prompting the ChannelMonitors to // broadcast the latest commitment transaction known to them, which in our case is the one with // the HTLCs still pending. + *nodes[1].fee_estimator.sat_per_kw.lock().unwrap() *= 2; nodes[1].node.timer_tick_occurred(); check_added_monitors(&nodes[1], 2); check_closed_event!(&nodes[1], 2, ClosureReason::OutdatedChannelManager); -- 2.39.5