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.

diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
index 6b0c7485610353917ef348f2ec54d1828070eea1..171caf7006e873615e11fd1f8ea0b999d5f24411 100644 (file)
--- 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<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
                        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::<u64>();
+                                               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<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
                                                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<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
                        ) {
                                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 9e61cbc4598772c1a9fa8cdd31a2f330dbd8c548..b66a2f70d3369f4aff6e012025b3e995d6918a9e 100644 (file)
--- 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<u64>,
+       pub(crate) funding_amount: Option<u64>,
        channel_type_features: ChannelTypeFeatures,
 }
 

diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs
index dbe539505d6ca733f53a44e4ae0f515523373395..ffd9b22834de567ba1833671af77949abfc19769 100644 (file)
--- 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 41ecd9da0fbef63970ba37a69291086e30159254..22b5fa7ce34136836a57dcb1530ea5dd87a41591 100644 (file)
--- 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);