Always emit bump events, even when fees are sufficient
What changed, and why it matters
This change alters how Lightning anchor channel commitment transactions are broadcast. Previously, if a commitment transaction already paid enough in fees, the software would silently broadcast it without telling the user. Now it always emits a 'bump' event so the user (or a delegated service like an LSP) can see what is happening and decide whether to broadcast. The change is described as a safety improvement: broadcasting a commitment without reserves can make HTLCs unclaimable, so users should be notified and given control.
Review how downstream consumers handle BumpTransactionEvent::ChannelClose to ensure they do not accidentally broadcast a commitment when they intended to delegate or suppress it. Ensure documentation and release notes mention the new always-emitted event for anchor channels. No immediate patch or CVE appears required based on the commit alone.
Security signals we found
Change in transaction broadcast control flow for anchor channels
Removal of silent commitment broadcast path
Introduction of always-on event emission for force-close scenarios
Documentation update warning that commitment may already have sufficient feerate and only commitment should be broadcast
Test changes confirm additional bump close events and anchor spends are now expected
Evidence from the diff
The patch removes an early-return optimization in OnchainTxHandler that skipped emitting BumpTransactionEvent::ChannelClose when the pre-signed commitment transaction’s feerate already met the package target. Instead, the event is always emitted. The actual ‘is fee already sufficient’ check is moved into the bump event handler (bump_transaction/mod.rs), which now broadcasts the commitment alone when no CPFP anchor child is needed. Tests are updated to expect the new ChannelClose bump event in anchor scenarios and to assert the ordering of commitment and anchor-spend transactions.
Changed components
lightning/src/chain/onchaintx.rslightning/src/events/bump_transaction/mod.rsanchor commitment bump event handlingLightning Dev Kit rust-lightning anchor channelsInspect captured patch +99 / −42
diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
index 95df05f..134884a 100644
--- a/lightning/src/chain/onchaintx.rs
+++ b/lightning/src/chain/onchaintx.rs
@@ -23,7 +23,7 @@ use bitcoin::secp256k1::{ecdsa::Signature, Secp256k1};
use bitcoin::transaction::OutPoint as BitcoinOutPoint;
use bitcoin::transaction::Transaction;
-use crate::chain::chaininterface::{compute_feerate_sat_per_1000_weight, ConfirmationTarget};
+use crate::chain::chaininterface::ConfirmationTarget;
use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
use crate::chain::channelmonitor::ANTI_REORG_DELAY;
use crate::chain::package::{PackageSolvingData, PackageTemplate};
@@ -670,19 +670,8 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
let fee_sat = input_amount_sats - tx.output.iter()
.map(|output| output.value.to_sat()).sum::<u64>();
- let commitment_tx_feerate_sat_per_1000_weight =
- compute_feerate_sat_per_1000_weight(fee_sat, tx.weight().to_wu());
let package_target_feerate_sat_per_1000_weight = cached_request
.compute_package_feerate(fee_estimator, conf_target, feerate_strategy);
- if commitment_tx_feerate_sat_per_1000_weight >= package_target_feerate_sat_per_1000_weight {
- log_debug!(logger, "Pre-signed commitment {} already has feerate {} sat/kW above required {} sat/kW",
- tx.compute_txid(), commitment_tx_feerate_sat_per_1000_weight,
- package_target_feerate_sat_per_1000_weight);
- // The commitment transaction already meets the required feerate and doesn't
- // need a CPFP. We still want to return something other than the event to
- // register the claim.
- return Some((new_timer, 0, OnchainClaim::Tx(MaybeSignedTransaction(tx))));
- }
// We'll locate an anchor output we can spend within the commitment transaction.
let channel_parameters = output.channel_parameters.as_ref()
diff --git a/lightning/src/events/bump_transaction/mod.rs b/lightning/src/events/bump_transaction/mod.rs
index b89d2a7..6f12769 100644
--- a/lightning/src/events/bump_transaction/mod.rs
+++ b/lightning/src/events/bump_transaction/mod.rs
@@ -16,7 +16,9 @@ pub mod sync;
use alloc::collections::BTreeMap;
use core::ops::Deref;
-use crate::chain::chaininterface::{fee_for_weight, BroadcasterInterface};
+use crate::chain::chaininterface::{
+ compute_feerate_sat_per_1000_weight, fee_for_weight, BroadcasterInterface,
+};
use crate::chain::ClaimId;
use crate::io_extras::sink;
use crate::ln::chan_utils;
@@ -123,7 +125,9 @@ pub enum BumpTransactionEvent {
/// and child anchor transactions), possibly resulting in a loss of funds. Once the transaction
/// is constructed, it must be fully signed for and broadcast by the consumer of the event
/// along with the `commitment_tx` enclosed. Note that the `commitment_tx` must always be
- /// broadcast first, as the child anchor transaction depends on it.
+ /// broadcast first, as the child anchor transaction depends on it. It is also possible that the
+ /// feerate of the commitment transaction is already sufficient, in which case the child anchor
+ /// transaction is not needed and only the commitment transaction should be broadcast.
///
/// The consumer should be able to sign for any of the additional inputs included within the
/// child anchor transaction. To sign its anchor input, an [`EcdsaChannelSigner`] should be
@@ -658,6 +662,19 @@ where
commitment_tx: &Transaction, commitment_tx_fee_sat: u64,
anchor_descriptor: &AnchorDescriptor,
) -> Result<(), ()> {
+ // First, check if the commitment transaction has sufficient fees on its own.
+ let commitment_tx_feerate_sat_per_1000_weight = compute_feerate_sat_per_1000_weight(
+ commitment_tx_fee_sat,
+ commitment_tx.weight().to_wu(),
+ );
+ if commitment_tx_feerate_sat_per_1000_weight >= package_target_feerate_sat_per_1000_weight {
+ log_debug!(self.logger, "Pre-signed commitment {} already has feerate {} sat/kW above required {} sat/kW, broadcasting.",
+ commitment_tx.compute_txid(), commitment_tx_feerate_sat_per_1000_weight,
+ package_target_feerate_sat_per_1000_weight);
+ self.broadcaster.broadcast_transactions(&[&commitment_tx]);
+ return Ok(());
+ }
+
// Our commitment transaction already has fees allocated to it, so we should take them into
// account. We do so by pretending the commitment transaction's fee and weight are part of
// the anchor input.
diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs
index 511bdce..d66ba79 100644
--- a/lightning/src/ln/async_signer_tests.rs
+++ b/lightning/src/ln/async_signer_tests.rs
@@ -1051,6 +1051,9 @@ fn do_test_async_holder_signatures(anchors: bool, remote_commitment: bool) {
&nodes[0].logger,
);
}
+ if anchors {
+ handle_bump_close_event(closing_node);
+ }
let commitment_tx = {
let mut txn = closing_node.tx_broadcaster.txn_broadcast();
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index b14b228..88bdb6d 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -2211,21 +2211,37 @@ macro_rules! check_closed_event {
};
}
-pub fn handle_bump_htlc_event(node: &Node, count: usize) {
+pub fn handle_bump_events(node: &Node, expected_close: bool, expected_htlc_count: usize) {
let events = node.chain_monitor.chain_monitor.get_and_clear_pending_events();
- assert_eq!(events.len(), count);
- for event in events {
+ let mut close = false;
+ let mut htlc_count = 0;
+ for event in &events {
match event {
- Event::BumpTransaction(bump_event) => {
- if let BumpTransactionEvent::HTLCResolution { .. } = &bump_event {
- } else {
- panic!();
- }
- node.bump_tx_handler.handle_event(&bump_event);
+ Event::BumpTransaction(bump @ BumpTransactionEvent::ChannelClose { .. }) => {
+ close = true;
+ node.bump_tx_handler.handle_event(&bump);
},
- _ => panic!(),
+ Event::BumpTransaction(bump @ BumpTransactionEvent::HTLCResolution { .. }) => {
+ htlc_count += 1;
+ node.bump_tx_handler.handle_event(&bump);
+ },
+ _ => panic!("Unexpected non-bump event: {:?}.", event),
}
}
+ assert_eq!(close, expected_close, "Expected a bump close event, found {:?}.", events);
+ assert_eq!(
+ htlc_count, expected_htlc_count,
+ "Expected {} bump HTLC events, found {:?}",
+ expected_htlc_count, events
+ );
+}
+
+pub fn handle_bump_close_event(node: &Node) {
+ handle_bump_events(node, true, 0);
+}
+
+pub fn handle_bump_htlc_event(node: &Node, count: usize) {
+ handle_bump_events(node, false, count);
}
pub fn close_channel<'a, 'b, 'c>(
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index c903424..8d24fe2 100644
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -896,6 +896,9 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) {
check_closed_broadcast!(nodes[0], true);
let reason = ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message };
check_closed_event!(nodes[0], 1, reason, [nodes[1].node.get_our_node_id()], 1000000);
+ if anchors {
+ handle_bump_close_event(&nodes[0]);
+ }
let commitment_tx = {
let mut txn = nodes[0].tx_broadcaster.unique_txn_broadcast();
assert_eq!(txn.len(), 1);
@@ -905,9 +908,15 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) {
};
let commitment_tx_conf_height_a = block_from_scid(mine_transaction(&nodes[0], &commitment_tx));
if nodes[0].connect_style.borrow().updates_best_block_first() {
+ if anchors {
+ handle_bump_close_event(&nodes[0]);
+ }
let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
- assert_eq!(txn.len(), 1);
+ assert_eq!(txn.len(), if anchors { 2 } else { 1 });
assert_eq!(txn[0].compute_txid(), commitment_tx.compute_txid());
+ if anchors {
+ check_spends!(txn[1], txn[0]); // Anchor output spend.
+ }
}
let htlc_balance_known_preimage = Balance::MaybeTimeoutClaimableHTLC {
@@ -2486,6 +2495,7 @@ fn do_test_yield_anchors_events(have_htlcs: bool) {
nodes[1].node.force_close_broadcasting_latest_txn(&chan_id, &nodes[0].node.get_our_node_id(), "".to_string()).unwrap();
}
{
+ handle_bump_close_event(&nodes[1]);
let txn = nodes[1].tx_broadcaster.txn_broadcast();
assert_eq!(txn.len(), 1);
check_spends!(txn[0], funding_tx);
@@ -2553,15 +2563,18 @@ fn do_test_yield_anchors_events(have_htlcs: bool) {
}
{
+ if nodes[1].connect_style.borrow().updates_best_block_first() {
+ handle_bump_close_event(&nodes[1]);
+ }
let mut txn = nodes[1].tx_broadcaster.unique_txn_broadcast();
// Both HTLC claims are pinnable at this point,
// and will be broadcast in a single transaction.
- assert_eq!(txn.len(), if nodes[1].connect_style.borrow().updates_best_block_first() { 2 } else { 1 });
+ assert_eq!(txn.len(), if nodes[1].connect_style.borrow().updates_best_block_first() { 3 } else { 1 });
if nodes[1].connect_style.borrow().updates_best_block_first() {
- let new_commitment_tx = txn.remove(0);
- check_spends!(new_commitment_tx, funding_tx);
+ check_spends!(txn[1], funding_tx);
+ check_spends!(txn[2], txn[1]); // Anchor output spend.
}
- let htlc_claim_tx = txn.pop().unwrap();
+ let htlc_claim_tx = &txn[0];
assert_eq!(htlc_claim_tx.input.len(), 2);
assert_eq!(htlc_claim_tx.input[0].previous_output.vout, 2);
assert_eq!(htlc_claim_tx.input[1].previous_output.vout, 3);
@@ -2952,6 +2965,7 @@ fn do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(confirm_c
let reason = ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message };
check_closed_event!(&nodes[0], 1, reason, false,
[nodes[1].node.get_our_node_id()], 100000);
+ handle_bump_close_event(&nodes[0]);
let commitment_tx = {
let mut txn = nodes[0].tx_broadcaster.unique_txn_broadcast();
@@ -3036,6 +3050,9 @@ fn do_test_monitor_claims_with_random_signatures(anchors: bool, confirm_counterp
get_monitor!(closing_node, chan_id).broadcast_latest_holder_commitment_txn(
&closing_node.tx_broadcaster, &closing_node.fee_estimator, &closing_node.logger
);
+ if anchors {
+ handle_bump_close_event(&closing_node);
+ }
// The commitment transaction comes first.
let commitment_tx = {
diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs
index cbe0d82..8e26567 100644
--- a/lightning/src/ln/reorg_tests.rs
+++ b/lightning/src/ln/reorg_tests.rs
@@ -844,6 +844,9 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c
check_closed_broadcast(&nodes[0], 1, true);
check_added_monitors(&nodes[0], 1);
check_closed_event(&nodes[0], 1, ClosureReason::HTLCsTimedOut, false, &[nodes[1].node.get_our_node_id()], 100_000);
+ if anchors {
+ handle_bump_close_event(&nodes[0]);
+ }
{
let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
@@ -870,6 +873,9 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c
check_added_monitors(&nodes[1], 1);
let reason = ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message };
check_closed_event(&nodes[1], 1, reason, false, &[nodes[0].node.get_our_node_id()], 100_000);
+ if anchors {
+ handle_bump_close_event(&nodes[1]);
+ }
let commitment_b = {
let mut txn = nodes[1].tx_broadcaster.txn_broadcast();
@@ -882,13 +888,23 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c
// Confirm B's commitment, A should now broadcast an HTLC timeout for commitment B.
mine_transaction(&nodes[0], &commitment_b);
{
- let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
if nodes[0].connect_style.borrow().updates_best_block_first() {
// `commitment_a` is rebroadcast because the best block was updated prior to seeing
// `commitment_b`.
- assert_eq!(txn.len(), 2);
- check_spends!(txn.last().unwrap(), commitment_b);
+ if anchors {
+ handle_bump_close_event(&nodes[0]);
+ let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
+ assert_eq!(txn.len(), 3);
+ check_spends!(txn[0], commitment_b);
+ check_spends!(txn[1], funding_tx);
+ check_spends!(txn[2], txn[1]); // Anchor output spend transaction.
+ } else {
+ let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
+ assert_eq!(txn.len(), 2);
+ check_spends!(txn.last().unwrap(), commitment_b);
+ }
} else {
+ let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
assert_eq!(txn.len(), 1);
check_spends!(txn[0], commitment_b);
}
@@ -898,11 +914,15 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c
// blocks, one to get us back to the original height, and another to retry our pending claims.
disconnect_blocks(&nodes[0], 1);
connect_blocks(&nodes[0], 2);
+ if anchors {
+ handle_bump_close_event(&nodes[0]);
+ }
{
let mut txn = nodes[0].tx_broadcaster.unique_txn_broadcast();
if anchors {
- assert_eq!(txn.len(), 1);
+ assert_eq!(txn.len(), 2);
check_spends!(txn[0], funding_tx);
+ check_spends!(txn[1], txn[0]); // Anchor output spend.
} else {
assert_eq!(txn.len(), 2);
check_spends!(txn[0], txn[1]); // HTLC timeout A
@@ -977,6 +997,7 @@ fn do_test_split_htlc_expiry_tracking(use_third_htlc: bool, reorg_out: bool) {
let message = "Channel force-closed".to_owned();
let reason = ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message };
check_closed_event(&nodes[1], 1, reason, false, &[node_a_id], 10_000_000);
+ handle_bump_close_event(&nodes[1]);
let mut txn = nodes[1].tx_broadcaster.txn_broadcast();
assert_eq!(txn.len(), 1);
@@ -990,19 +1011,13 @@ fn do_test_split_htlc_expiry_tracking(use_third_htlc: bool, reorg_out: bool) {
check_added_monitors(&nodes[0], 1);
mine_transaction(&nodes[1], &commitment_tx);
- let mut bump_events = nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_events();
- assert_eq!(bump_events.len(), 1);
- match bump_events.pop().unwrap() {
- Event::BumpTransaction(bump_event) => {
- nodes[1].bump_tx_handler.handle_event(&bump_event);
- },
- ev => panic!("Unexpected event {ev:?}"),
- }
+ handle_bump_events(&nodes[1], nodes[1].connect_style.borrow().updates_best_block_first(), 1);
let mut txn = nodes[1].tx_broadcaster.txn_broadcast();
if nodes[1].connect_style.borrow().updates_best_block_first() {
- assert_eq!(txn.len(), 2, "{txn:?}");
+ assert_eq!(txn.len(), 3, "{txn:?}");
check_spends!(txn[0], funding_tx);
+ check_spends!(txn[1], txn[0]); // Anchor output spend.
} else {
assert_eq!(txn.len(), 1, "{txn:?}");
}
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.