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.

Author: wpaulino

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,19 +624,31 @@ 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) {
 						// An anchor output was found, so we should yield a funding event externally.
 						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));

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,
 }
 

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,
 		)?;

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);