Fail held HTLC failures when force-closing
What changed, and why it matters
This commit fixes a bug in the Lightning Dev Kit where a forwarded payment failure could get permanently lost if a channel was force-closed at exactly the wrong moment. Previously, when the other side of a channel told us an HTLC (a conditional payment) had failed, that failure was temporarily held until a channel-state update finished. If the channel was force-closed while that update was still in flight, the held failure was discarded. Because the payment was also no longer in any on-chain commitment transaction the node watches, it would never be failed backwards or forwards, eventually causing another force-close. The fix hands those held failures to the ChannelMonitor as part of the force-close update, so they are properly failed once the relevant state updates are durably persisted.
Reviewers should verify that the new counterparty_failed_htlcs serialization is backward-compatible (optional_vec with default empty), that fail_counterparty_failed_htlcs correctly identifies HTLCs still in tracked commitments, and that no other pending HTLC state (forwards, finalized fulfills) has a similar force-close loss issue. Operators should upgrade to include this fix to avoid stuck forwarded payments and unnecessary force-closes.
Security signals we found
Fixes a state-loss race that could leave forwarded HTLCs unresolved
Prevents a secondary force-close caused by an un-failed HTLC
Adds ChannelMonitorUpdateStep::ChannelForceClosed field counterparty_failed_htlcs
Introduces ChannelMonitor::fail_counterparty_failed_htlcs to emit HTLC failure events from the monitor
Skips failing HTLCs still present in tracked counterparty commitment transactions to avoid premature failure when the counterparty could still claim on-chain
Uses existing pending_monitor_events mechanism so failures are released only after persistence
Adds regression tests for holder force-close, counterparty error, commitment-confirmed close, restart, stale ChannelManager, and blocked RAA update
Evidence from the diff
The patch addresses a state-loss race in LDK’s HTLC failure handling during force-close. When a counterparty fails an HTLC, LDK holds the failure in ChannelContext::monitor_pending_failures until the ChannelMonitorUpdate for the counterparty’s revoke_and_ack completes. If the channel is force-closed while that update is in-flight, the Channel is dropped and the held failure is lost. Because the ChannelMonitor had already applied the revocation, the HTLC is absent from all commitment transactions it tracks, so it is never resolved on-chain or failed backwards. The fix extends ChannelMonitorUpdateStep::ChannelForceClosed with a counterparty_failed_htlcs Vec<(HTLCSource, PaymentHash)>. On force-close, Channel drains monitor_pending_failures into this vector. ChannelMonitor::fail_counterparty_failed_htlcs then emits MonitorEvent::HTLCEvent failures for HTLCs no longer present in any tracked counterparty commitment, while skipping those still present (which remain resolvable on-chain). The events are only released after all pending monitor updates are persisted, preserving the revocation-durability invariant. Tests cover holder-initiated, counterparty-error, and commitment-confirmed force-closes, with and without node restart, and the blocked-update case where the HTLC must not be prematurely failed.
Changed components
lightning/src/chain/channelmonitor.rslightning/src/ln/channel.rslightning/src/ln/channelmanager.rslightning/src/ln/chanmon_update_fail_tests.rslightning/src/util/persist.rslightning/src/util/test_utils.rsInspect captured patch +449 / −7
### lightning/src/chain/channelmonitor.rs
@@ -696,6 +696,10 @@ pub(crate) enum ChannelMonitorUpdateStep {
/// If set to false, we shouldn't broadcast the latest holder commitment transaction as we
/// think we've fallen behind!
should_broadcast: bool,
+ /// HTLCs which the counterparty failed but whose failures had not been handled when the
+ /// channel was closed. Those which are no longer in any commitment transaction we track
+ /// are failed once this update is applied.
+ counterparty_failed_htlcs: Vec<(HTLCSource, PaymentHash)>,
},
ShutdownScript {
scriptpubkey: ScriptBuf,
@@ -770,6 +774,7 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
},
(4, ChannelForceClosed) => {
(0, should_broadcast, required),
+ (1, counterparty_failed_htlcs, optional_vec),
},
(5, ShutdownScript) => {
(0, scriptpubkey, required),
@@ -4461,9 +4466,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
ret = Err(());
}
},
- ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => {
+ ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast, counterparty_failed_htlcs } => {
log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast);
self.lockdown_from_offchain = true;
+ self.fail_counterparty_failed_htlcs(counterparty_failed_htlcs, logger, entropy_source);
if *should_broadcast {
// There's no need to broadcast our commitment transaction if we've seen one
// confirmed (even with 1 confirmation) as it'll be rejected as
@@ -4601,6 +4607,77 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
}
+ /// Fails HTLCs which the counterparty failed off-chain but whose failures were not handled
+ /// before the channel was closed.
+ ///
+ /// HTLCs still in a commitment transaction we track are skipped: we may not have been given
+ /// the revocation of the counterparty commitment transaction containing them, in which case
+ /// the counterparty can still claim them on-chain. They are instead resolved on-chain or, for
+ /// forwarded HTLCs, failed backwards as the inbound HTLCs approach expiry, like any other HTLC
+ /// we track.
+ fn fail_counterparty_failed_htlcs<L: Logger, ES: EntropySource>(
+ &mut self, htlcs: &[(HTLCSource, PaymentHash)], logger: &WithContext<L>,
+ entropy_source: &ES,
+ ) {
+ let in_counterparty_commitment = |source: &HTLCSource| {
+ [
+ self.funding.current_counterparty_commitment_txid,
+ self.funding.prev_counterparty_commitment_txid,
+ ]
+ .into_iter()
+ .flatten()
+ .filter_map(|txid| self.funding.counterparty_claimable_outpoints.get(&txid))
+ .flatten()
+ .any(|(_, source_opt)| source_opt.as_deref() == Some(source))
+ };
+ for (source, payment_hash) in htlcs {
+ let logger = WithContext::from(logger, None, None, Some(*payment_hash));
+ if in_counterparty_commitment(source) {
+ log_trace!(
+ logger,
+ "Not failing HTLC as it is still in a counterparty commitment transaction"
+ );
+ continue;
+ }
+ // An HTLC the counterparty failed is removed from our commitment transaction before
+ // it is removed from theirs, so it can't be in ours if it is in neither of their
+ // unrevoked ones.
+ debug_assert!(!holder_commitment_htlcs!(self, CURRENT_WITH_SOURCES)
+ .any(|(_, s)| s == Some(source)));
+ let duplicate_event =
+ self.pending_monitor_events.iter().chain(self.provided_monitor_events.iter()).any(
+ |(_, event)| match event {
+ MonitorEvent::HTLCEvent(upd) => upd.source == *source,
+ _ => false,
+ },
+ );
+ if duplicate_event {
+ continue;
+ }
+ // The HTLC can't have been failed on-chain, as the counterparty revoked the commitment
+ // transaction containing it and we'd have swept it ourselves had it been broadcast.
+ let newly_failed = self.failed_back_htlc_ids.insert(SentHTLCId::from_source(source));
+ debug_assert!(newly_failed);
+ log_trace!(logger, "Failing HTLC the counterparty failed before the channel closed");
+ // The value isn't used when failing HTLCs.
+ let htlc_value_msat = match source {
+ HTLCSource::OutboundRoute { first_hop_htlc_msat, .. } => Some(*first_hop_htlc_msat),
+ _ => source.inbound_htlc_amount_msat(),
+ };
+ let htlc_value_satoshis = htlc_value_msat.unwrap_or(0) / 1000;
+ push_monitor_event(
+ &mut self.pending_monitor_events,
+ MonitorEvent::HTLCEvent(HTLCUpdate {
+ source: source.clone(),
+ payment_preimage: None,
+ payment_hash: *payment_hash,
+ htlc_value_satoshis,
+ }),
+ entropy_source,
+ );
+ }
+ }
+
/// Given outbound HTLCs from a counterparty commitment update, checks if the funding output
/// has been spent on-chain. If so, creates `OnchainEvent::HTLCUpdate` entries to fail back
/// HTLCs that weren't already known to the monitor.
### lightning/src/ln/chanmon_update_fail_tests.rs
@@ -14,20 +14,26 @@
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::chainmonitor::ChainMonitor;
-use crate::chain::channelmonitor::{ChannelMonitor, MonitorEvent, ANTI_REORG_DELAY};
+use crate::chain::channelmonitor::{
+ ChannelMonitor, MonitorEvent, ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS,
+};
use crate::chain::transaction::OutPoint;
use crate::chain::{BlockLocator, ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
-use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose};
+use crate::events::{
+ ClosureReason, Event, HTLCHandlingFailureReason, HTLCHandlingFailureType, PaymentPurpose,
+};
use crate::ln::channel::AnnouncementSigsState;
use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, TrustedChannelFeatures};
use crate::ln::msgs;
use crate::ln::msgs::{
BaseMessageHandler, ChannelMessageHandler, MessageSendEvent, RoutingMessageHandler,
};
+use crate::ln::onion_utils::LocalHTLCFailureReason;
use crate::ln::outbound_payment::{RecipientOnionFields, Retry};
use crate::ln::types::ChannelId;
use crate::routing::router::{PaymentParameters, RouteParameters};
use crate::sign::NodeSigner;
+use crate::types::string::UntrustedString;
use crate::util::native_async::FutureQueue;
use crate::util::persist::{
MonitorName, MonitorUpdatingPersisterAsync, CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE,
@@ -5734,3 +5740,337 @@ fn test_stale_manager_with_since_applied_blocked_mon_update() {
expect_payment_sent(&nodes[0], preimage_2, None, true, true);
expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, true);
}
+
+#[derive(Clone, Copy, PartialEq)]
+enum ForceCloseTrigger {
+ Holder,
+ CounterpartyError,
+ CounterpartyCommitmentConfirmed,
+}
+
+fn do_test_force_close_with_monitor_pending_htlc_fail(
+ trigger: ForceCloseTrigger, reload: bool, stale_manager: bool,
+) {
+ // When our counterparty fails an HTLC we forwarded, we hold the failure until the
+ // `ChannelMonitorUpdate` for their `revoke_and_ack` completes. If the channel is force-closed
+ // while that update is in-flight, the held failure can no longer be released, and as the HTLC
+ // is no longer in any commitment transaction the `ChannelMonitor` tracks, it wouldn't be
+ // resolved on-chain either. Instead, the closing `ChannelMonitorUpdate` hands such HTLCs to the
+ // `ChannelMonitor`, which fails them backwards once both updates have been persisted. Tested
+ // with the closure triggered by us and by the counterparty, and across a restart, including
+ // with a `ChannelManager` from before the force-close, which still holds the failure and
+ // force-closes the channel on startup as the `ChannelMonitor` is ahead of it.
+ assert!(reload || !stale_manager);
+ assert!(trigger == ForceCloseTrigger::Holder || !reload);
+ let chanmon_cfgs = create_chanmon_cfgs(3);
+ let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+ let legacy_cfg = test_legacy_channel_config();
+ let persister;
+ let new_chain_monitor;
+ let nodes_1_deserialized;
+ let node_chanmgrs = create_node_chanmgrs(
+ 3,
+ &node_cfgs,
+ &[Some(legacy_cfg.clone()), Some(legacy_cfg.clone()), Some(legacy_cfg)],
+ );
+ let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+ let node_a_id = nodes[0].node.get_our_node_id();
+ let node_b_id = nodes[1].node.get_our_node_id();
+ let node_c_id = nodes[2].node.get_our_node_id();
+
+ let chan_id_ab = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+ let chan_id_bc = create_announced_chan_between_nodes(&nodes, 1, 2).2;
+
+ let (_, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+
+ nodes[2].node.fail_htlc_backwards(&payment_hash);
+ expect_and_process_pending_htlcs_and_htlc_handling_failed(
+ &nodes[2],
+ &[HTLCHandlingFailureType::Receive { payment_hash }],
+ );
+ check_added_monitors(&nodes[2], 1);
+
+ // Run the commitment dance removing the HTLC from the B <-> C channel, but leave B's
+ // `ChannelMonitorUpdate` for C's final `revoke_and_ack` in-flight.
+ let cs_updates = get_htlc_update_msgs(&nodes[2], &node_b_id);
+ nodes[1].node.handle_update_fail_htlc(node_c_id, &cs_updates.update_fail_htlcs[0]);
+ let cs_raa = commitment_signed_dance_return_raa(
+ &nodes[1],
+ &nodes[2],
+ &cs_updates.commitment_signed,
+ true,
+ );
+
+ chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+ nodes[1].node.handle_revoke_and_ack(node_c_id, &cs_raa);
+ check_added_monitors(&nodes[1], 1);
+ let (_, raa_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_bc);
+ assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+ assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+ assert!(get_monitor!(nodes[1], chan_id_bc).get_all_current_outbound_htlcs().is_empty());
+ let stale_node_b_ser = nodes[1].node.encode();
+
+ chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+ match trigger {
+ ForceCloseTrigger::Holder => {
+ let message = "Channel force-closed".to_owned();
+ nodes[1]
+ .node
+ .force_close_broadcasting_latest_txn(&chan_id_bc, &node_c_id, message.clone())
+ .unwrap();
+ check_added_monitors(&nodes[1], 1);
+ check_closed_broadcast(&nodes[1], 1, true);
+ let reason =
+ ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message };
+ check_closed_event(&nodes[1], 1, reason, &[node_c_id], 100_000);
+ },
+ ForceCloseTrigger::CounterpartyError => {
+ let data = "Bogus error".to_owned();
+ let msg = msgs::ErrorMessage { channel_id: chan_id_bc, data: data.clone() };
+ nodes[1].node.handle_error(node_c_id, &msg);
+ check_added_monitors(&nodes[1], 1);
+ check_closed_broadcast(&nodes[1], 1, false);
+ let reason = ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString(data) };
+ check_closed_event(&nodes[1], 1, reason, &[node_c_id], 100_000);
+ },
+ ForceCloseTrigger::CounterpartyCommitmentConfirmed => {
+ let cs_commitment_tx = get_local_commitment_txn!(nodes[2], chan_id_bc);
+ mine_transaction(&nodes[1], &cs_commitment_tx[0]);
+ let reason = ClosureReason::CommitmentTxConfirmed;
+ check_closed_event(&nodes[1], 1, reason, &[node_c_id], 100_000);
+ check_closed_broadcast(&nodes[1], 1, true);
+ check_added_monitors(&nodes[1], 1);
+ },
+ }
+ let (_, close_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_bc);
+
+ if reload {
+ let node_b_ser = if stale_manager { stale_node_b_ser } else { nodes[1].node.encode() };
+ let mon_ab_ser = get_monitor!(nodes[1], chan_id_ab).encode();
+ let mon_bc_ser = get_monitor!(nodes[1], chan_id_bc).encode();
+ // Reconstructing the pending HTLC set from the `Channel`s would fail the HTLC on startup
+ // already, so use the persisted set to test the failure by the `ChannelMonitor`.
+ reload_node!(
+ nodes[1],
+ &node_b_ser,
+ &[&mon_ab_ser, &mon_bc_ser],
+ persister,
+ new_chain_monitor,
+ nodes_1_deserialized,
+ Some(false)
+ );
+ if stale_manager {
+ // The stale `ChannelManager` still has the channel open, so it force-closes it on
+ // startup.
+ nodes[1].node.test_process_background_events();
+ check_added_monitors(&nodes[1], 1);
+ }
+ nodes[0].node.peer_disconnected(node_b_id);
+ nodes[2].node.peer_disconnected(node_b_id);
+ reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
+ } else {
+ // The failure isn't released while either update is in-flight.
+ let chain_monitor = &nodes[1].chain_monitor.chain_monitor;
+ chain_monitor.channel_monitor_updated(chan_id_bc, close_update_id).unwrap();
+ assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+ assert!(!nodes[1].node.needs_pending_htlc_processing());
+ chain_monitor.channel_monitor_updated(chan_id_bc, raa_update_id).unwrap();
+ }
+
+ let mut events = nodes[1].node.get_and_clear_pending_events();
+ if stale_manager {
+ match events.remove(0) {
+ Event::ChannelClosed { reason: ClosureReason::OutdatedChannelManager, .. } => {},
+ ev => panic!("Unexpected event: {ev:?}"),
+ }
+ }
+ assert_eq!(events.len(), 1, "{events:?}");
+ match &events[0] {
+ Event::HTLCHandlingFailed { failure_type, failure_reason, .. } => {
+ let expected_type = HTLCHandlingFailureType::Forward {
+ node_id: Some(node_c_id),
+ channel_id: chan_id_bc,
+ };
+ assert_eq!(*failure_type, expected_type);
+ let reason = LocalHTLCFailureReason::OnChainTimeout;
+ assert_eq!(*failure_reason, Some(HTLCHandlingFailureReason::Local { reason }));
+ },
+ ev => panic!("Unexpected event: {ev:?}"),
+ }
+ expect_and_process_pending_htlcs(&nodes[1], false);
+ check_added_monitors(&nodes[1], 1);
+ let bs_updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
+ nodes[0].node.handle_update_fail_htlc(node_b_id, &bs_updates.update_fail_htlcs[0]);
+ do_commitment_signed_dance(&nodes[0], &nodes[1], &bs_updates.commitment_signed, false, false);
+ let conditions = PaymentFailedConditions::new().blamed_chan_closed(true);
+ expect_payment_failed_conditions(&nodes[0], payment_hash, false, conditions);
+}
+
+#[test]
+fn test_force_close_with_monitor_pending_htlc_fail() {
+ do_test_force_close_with_monitor_pending_htlc_fail(ForceCloseTrigger::Holder, false, false);
+ do_test_force_close_with_monitor_pending_htlc_fail(ForceCloseTrigger::Holder, true, false);
+ do_test_force_close_with_monitor_pending_htlc_fail(ForceCloseTrigger::Holder, true, true);
+ let trigger = ForceCloseTrigger::CounterpartyError;
+ do_test_force_close_with_monitor_pending_htlc_fail(trigger, false, false);
+ let trigger = ForceCloseTrigger::CounterpartyCommitmentConfirmed;
+ do_test_force_close_with_monitor_pending_htlc_fail(trigger, false, false);
+}
+
+#[test]
+fn test_force_close_with_blocked_raa_htlc_fail() {
+ // If the `ChannelMonitorUpdate` for the counterparty's `revoke_and_ack` is blocked rather than
+ // in-flight when the channel is force-closed, the `ChannelMonitor` never learns the
+ // revocation. It then still tracks the failed HTLC in the counterparty's previous commitment
+ // transaction, which the counterparty could still broadcast to claim it. Thus, the HTLC must
+ // not be failed backwards when the channel closes, but only as the inbound HTLC approaches
+ // expiry, like any other HTLC the `ChannelMonitor` tracks.
+ let chanmon_cfgs = create_chanmon_cfgs(3);
+ let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+ let legacy_cfg = test_legacy_channel_config();
+ let node_chanmgrs = create_node_chanmgrs(
+ 3,
+ &node_cfgs,
+ &[Some(legacy_cfg.clone()), Some(legacy_cfg.clone()), Some(legacy_cfg)],
+ );
+ let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+ let node_a_id = nodes[0].node.get_our_node_id();
+ let node_b_id = nodes[1].node.get_our_node_id();
+ let node_c_id = nodes[2].node.get_our_node_id();
+
+ let chan_id_ab = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+ let chan_id_bc = create_announced_chan_between_nodes(&nodes, 1, 2).2;
+
+ let (preimage_1, payment_hash_1, ..) =
+ route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+ let (_, payment_hash_2, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+ let (_, payment_hash_3, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+
+ // C fails payment 3, and B responds with its `revoke_and_ack` and `commitment_signed`.
+ nodes[2].node.fail_htlc_backwards(&payment_hash_3);
+ expect_and_process_pending_htlcs_and_htlc_handling_failed(
+ &nodes[2],
+ &[HTLCHandlingFailureType::Receive { payment_hash: payment_hash_3 }],
+ );
+ check_added_monitors(&nodes[2], 1);
+ let cs_updates = get_htlc_update_msgs(&nodes[2], &node_b_id);
+ nodes[1].node.handle_update_fail_htlc(node_c_id, &cs_updates.update_fail_htlcs[0]);
+ nodes[1].node.handle_commitment_signed_batch_test(node_c_id, &cs_updates.commitment_signed);
+ check_added_monitors(&nodes[1], 1);
+ let (bs_raa, bs_commitment_signed) = get_revoke_commit_msgs(&nodes[1], &node_c_id);
+
+ // While C awaits B's `revoke_and_ack`, it claims payment 1 and fails payment 2, both of which
+ // go into its holding cell and are sent together once B's `revoke_and_ack` arrives.
+ nodes[2].node.claim_funds(preimage_1, Default::default());
+ check_added_monitors(&nodes[2], 1);
+ expect_payment_claimed!(nodes[2], payment_hash_1, 1_000_000);
+ nodes[2].node.fail_htlc_backwards(&payment_hash_2);
+ expect_and_process_pending_htlcs_and_htlc_handling_failed(
+ &nodes[2],
+ &[HTLCHandlingFailureType::Receive { payment_hash: payment_hash_2 }],
+ );
+ assert!(nodes[2].node.get_and_clear_pending_msg_events().is_empty());
+
+ nodes[2].node.handle_revoke_and_ack(node_b_id, &bs_raa);
+ check_added_monitors(&nodes[2], 1);
+ let mut cs_updates = get_htlc_update_msgs(&nodes[2], &node_b_id);
+ assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
+ assert_eq!(cs_updates.update_fail_htlcs.len(), 1);
+ nodes[2].node.handle_commitment_signed_batch_test(node_b_id, &bs_commitment_signed);
+ check_added_monitors(&nodes[2], 1);
+ let cs_raa = get_event_msg!(nodes[2], MessageSendEvent::SendRevokeAndACK, node_b_id);
+
+ // B claims payment 1 upstream, leaving the A <-> B `ChannelMonitorUpdate` in-flight, which
+ // blocks the `ChannelMonitorUpdate` for C's `revoke_and_ack` that removes payment 3.
+ chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+ nodes[1].node.handle_update_fulfill_htlc(node_c_id, cs_updates.update_fulfill_htlcs.remove(0));
+ check_added_monitors(&nodes[1], 1);
+ nodes[1].node.handle_update_fail_htlc(node_c_id, &cs_updates.update_fail_htlcs[0]);
+ nodes[1].node.handle_commitment_signed_batch_test(node_c_id, &cs_updates.commitment_signed);
+ check_added_monitors(&nodes[1], 1);
+ nodes[1].node.handle_revoke_and_ack(node_c_id, &cs_raa);
+ check_added_monitors(&nodes[1], 0);
+ get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, node_c_id);
+ assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+
+ let message = "Channel force-closed".to_owned();
+ nodes[1]
+ .node
+ .force_close_broadcasting_latest_txn(&chan_id_bc, &node_c_id, message.clone())
+ .unwrap();
+ check_added_monitors(&nodes[1], 1);
+ check_closed_broadcast(&nodes[1], 1, true);
+ let reason = ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message };
+ check_closed_event(&nodes[1], 1, reason, &[node_c_id], 100_000);
+
+ // Once all updates complete, B forwards the claim of payment 1 but doesn't fail payment 3.
+ let chain_monitor = &nodes[1].chain_monitor.chain_monitor;
+ for (channel_id, update_ids) in chain_monitor.list_pending_monitor_updates() {
+ for update_id in update_ids {
+ chain_monitor.channel_monitor_updated(channel_id, update_id).unwrap();
+ }
+ }
+ let events = nodes[1].node.get_and_clear_pending_events();
+ assert_eq!(events.len(), 1, "{events:?}");
+ assert!(matches!(events[0], Event::PaymentForwarded { .. }), "{events:?}");
+ assert!(!nodes[1].node.needs_pending_htlc_processing());
+ check_added_monitors(&nodes[1], 0);
+ let mut bs_updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
+ nodes[0].node.handle_update_fulfill_htlc(node_b_id, bs_updates.update_fulfill_htlcs.remove(0));
+ do_commitment_signed_dance(&nodes[0], &nodes[1], &bs_updates.commitment_signed, false, false);
+ expect_payment_sent(&nodes[0], preimage_1, None, true, true);
+
+ // Payments 2 and 3 are failed backwards once the inbound HTLCs approach expiry.
+ let htlc_expiry = nodes[1]
+ .node
+ .list_channels()
+ .iter()
+ .find(|chan| chan.channel_id == chan_id_ab)
+ .unwrap()
+ .pending_inbound_htlcs
+ .iter()
+ .map(|htlc| htlc.cltv_expiry)
+ .min()
+ .unwrap();
+ let fail_height = htlc_expiry - LATENCY_GRACE_PERIOD_BLOCKS;
+ connect_blocks(&nodes[1], fail_height - 1 - nodes[1].best_block_info().1);
+ assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+ connect_blocks(&nodes[1], 1);
+ // The already-claimed payment 1 is also still in the counterparty's commitment transaction and
+ // is failed as well, which is ignored as the inbound HTLC has already been removed.
+ let events = nodes[1].node.get_and_clear_pending_events();
+ assert_eq!(events.len(), 3, "{events:?}");
+ for event in events {
+ match event {
+ Event::HTLCHandlingFailed { failure_reason, .. } => {
+ let reason = LocalHTLCFailureReason::OnChainTimeout;
+ assert_eq!(failure_reason, Some(HTLCHandlingFailureReason::Local { reason }));
+ },
+ ev => panic!("Unexpected event: {ev:?}"),
+ }
+ }
+ expect_and_process_pending_htlcs(&nodes[1], false);
+ check_added_monitors(&nodes[1], 1);
+ let bs_updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
+ assert_eq!(bs_updates.update_fail_htlcs.len(), 2);
+ for update_fail in bs_updates.update_fail_htlcs.iter() {
+ nodes[0].node.handle_update_fail_htlc(node_b_id, update_fail);
+ }
+ do_commitment_signed_dance(&nodes[0], &nodes[1], &bs_updates.commitment_signed, false, false);
+ let mut failed_payments = Vec::new();
+ for event in nodes[0].node.get_and_clear_pending_events() {
+ match event {
+ Event::PaymentPathFailed { .. } => {},
+ Event::PaymentFailed { payment_hash, .. } => {
+ failed_payments.push(payment_hash.unwrap())
+ },
+ ev => panic!("Unexpected event: {ev:?}"),
+ }
+ }
+ failed_payments.sort();
+ let mut expected_payments = vec![payment_hash_2, payment_hash_3];
+ expected_payments.sort();
+ assert_eq!(failed_payments, expected_payments);
+}
### lightning/src/ln/channel.rs
@@ -3659,8 +3659,9 @@ pub(super) struct ChannelContext<SP: SignerProvider> {
// TODO: If a channel is drop'd, we don't know whether the `ChannelMonitor` is ultimately
// responsible for some of the HTLCs here or not - we don't know whether the update in question
- // completed or not. We currently ignore these fields entirely when force-closing a channel,
- // but need to handle this somehow or we run the risk of losing HTLCs!
+ // completed or not. Other than `monitor_pending_failures`, which are handed to the
+ // `ChannelMonitor` when force-closing, we currently ignore these fields entirely when
+ // force-closing a channel, but need to handle this somehow or we run the risk of losing HTLCs!
monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>,
monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
monitor_pending_finalized_fulfills: Vec<(HTLCSource, Option<AttributionData>)>,
@@ -6715,10 +6716,20 @@ impl<SP: SignerProvider> ChannelContext<SP> {
if self.counterparty_next_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
self.latest_monitor_update_id = self.get_latest_unblocked_monitor_update_id() + 1;
+ // HTLC failures are held until the `ChannelMonitorUpdate` for the counterparty's
+ // `revoke_and_ack` completes. As they can no longer be released, hand them to the
+ // `ChannelMonitor` to fail. It may already have been given the revocation, in
+ // which case it no longer tracks the HTLCs and wouldn't resolve them otherwise.
+ let counterparty_failed_htlcs = self
+ .monitor_pending_failures
+ .drain(..)
+ .map(|(source, payment_hash, _)| (source, payment_hash))
+ .collect();
let update = ChannelMonitorUpdate {
update_id: self.latest_monitor_update_id,
updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed {
should_broadcast: broadcast,
+ counterparty_failed_htlcs,
}],
channel_id: Some(self.channel_id()),
};
### lightning/src/ln/channelmanager.rs
@@ -1124,6 +1124,16 @@ impl HTLCSource {
}
}
+ /// Returns the total amount of the inbound HTLC(s) (i.e. the source(s) referred to by this
+ /// object), if the source was a forwarded HTLC and all inbound HTLCs were first forwarded on
+ /// LDK 0.0.117 or later.
+ pub(crate) fn inbound_htlc_amount_msat(&self) -> Option<u64> {
+ match self {
+ Self::OutboundRoute { .. } => None,
+ _ => self.previous_hop_data().iter().map(|hop| hop.amount_msat).sum(),
+ }
+ }
+
pub(crate) fn static_invoice(&self) -> Option<StaticInvoice> {
match self {
Self::OutboundRoute {
@@ -20249,6 +20259,7 @@ impl<
update_id: monitor.get_latest_update_id().saturating_add(1),
updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed {
should_broadcast: true,
+ counterparty_failed_htlcs: Vec::new(),
}],
channel_id: Some(monitor.channel_id()),
};
### lightning/src/util/persist.rs
@@ -2628,7 +2628,10 @@ mod tests {
let legacy_update = ChannelMonitorUpdate {
update_id: u64::MAX,
- updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast: true }],
+ updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed {
+ should_broadcast: true,
+ counterparty_failed_htlcs: Vec::new(),
+ }],
channel_id: Some(monitor.channel_id()),
};
### lightning/src/util/test_utils.rs
@@ -699,7 +699,7 @@ impl<'a> chain::Watch<TestChannelSigner> for TestChainMonitor<'a> {
assert_eq!(channel_id, exp.0);
assert_eq!(update.updates.len(), 1);
let update = &update.updates[0];
- if let ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } = update {
+ if let ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast, .. } = update {
assert_eq!(*should_broadcast, exp.1);
} else {
panic!();Why this scored 68/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.