From cb5183666ca98df0edfa269fb5ea5f56927f6d41 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Sat, 10 Feb 2024 18:18:01 +0000 Subject: [PATCH] Add HTLCsTimedOut closing reason --- lightning/src/chain/channelmonitor.rs | 74 +++++++++++++++++++++++++-- lightning/src/events/mod.rs | 4 ++ lightning/src/ln/channelmanager.rs | 9 +++- lightning/src/ln/functional_tests.rs | 12 ++--- lightning/src/ln/monitor_tests.rs | 4 +- lightning/src/ln/reorg_tests.rs | 4 +- 6 files changed, 90 insertions(+), 17 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index cdbec81abf5..9f215ef9373 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -50,7 +50,7 @@ use crate::chain::Filter; use crate::util::logger::{Logger, Record}; use crate::util::ser::{Readable, ReadableArgs, RequiredWrapper, MaybeReadable, UpgradableRequired, Writer, Writeable, U48}; use crate::util::byte_utils; -use crate::events::{Event, EventHandler}; +use crate::events::{ClosureReason, Event, EventHandler}; use crate::events::bump_transaction::{AnchorDescriptor, BumpTransactionEvent}; use crate::prelude::*; @@ -155,6 +155,17 @@ pub enum MonitorEvent { /// A monitor event containing an HTLCUpdate. HTLCEvent(HTLCUpdate), + /// Indicates we broadcasted the channel's latest commitment transaction and thus closed the + /// channel. Holds information the channel and why it was closed. + HolderForceClosedWithInfo { + /// The reason the channel was closed. + reason: ClosureReason, + /// The funding outpoint of the channel. + outpoint: OutPoint, + /// The channel ID of the channel. + channel_id: ChannelId, + }, + /// Indicates we broadcasted the channel's latest commitment transaction and thus closed the /// channel. HolderForceClosed(OutPoint), @@ -184,6 +195,11 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorEvent, (2, monitor_update_id, required), (4, channel_id, required), }, + (5, HolderForceClosedWithInfo) => { + (0, reason, upgradable_required), + (2, outpoint, required), + (4, channel_id, required), + }, ; (2, HTLCEvent), (4, HolderForceClosed), @@ -1059,6 +1075,7 @@ impl Writeable for ChannelMonitorImpl true, MonitorEvent::HolderForceClosed(_) => true, + MonitorEvent::HolderForceClosedWithInfo { .. } => true, _ => false, }).count() as u64).to_be_bytes())?; for event in self.pending_monitor_events.iter() { @@ -1068,6 +1085,14 @@ impl Writeable for ChannelMonitorImpl 1u8.write(writer)?, + MonitorEvent::HolderForceClosedWithInfo { + reason, outpoint, channel_id + } => { + 2u8.write(writer)?; + reason.write(writer)?; + outpoint.write(writer)?; + channel_id.write(writer)?; + }, _ => {}, // Covered in the TLV writes below } } @@ -2700,7 +2725,7 @@ impl ChannelMonitorImpl { } } - fn generate_claimable_outpoints_and_watch_outputs(&mut self) -> (Vec, Vec) { + fn generate_claimable_outpoints_and_watch_outputs(&mut self, is_htlc_timeout: bool) -> (Vec, Vec) { let funding_outp = HolderFundingOutput::build( self.funding_redeemscript.clone(), self.channel_value_satoshis, @@ -2712,7 +2737,20 @@ impl ChannelMonitorImpl { self.best_block.height(), self.best_block.height() ); let mut claimable_outpoints = vec![commitment_package]; + let reason = if is_htlc_timeout { + ClosureReason::HTLCsTimedOut + } else { + ClosureReason::HolderForceClosed + }; + let event = MonitorEvent::HolderForceClosedWithInfo { + reason, + outpoint: self.funding_info.0, + channel_id: self.channel_id, + }; + self.pending_monitor_events.push(event); + // add old version as well for backwards compatability self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0)); + // Although we aren't signing the transaction directly here, the transaction will be signed // in the claim that is queued to OnchainTxHandler. We set holder_tx_signed here to reject // new channel updates. @@ -2748,7 +2786,7 @@ impl ChannelMonitorImpl { F::Target: FeeEstimator, L::Target: Logger, { - let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs(); + let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs(false); self.onchain_tx_handler.update_claims_view_from_requests( claimable_outpoints, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger @@ -3787,7 +3825,7 @@ impl ChannelMonitorImpl { let should_broadcast = self.should_broadcast_holder_commitment_txn(logger); if should_broadcast { - let (mut new_outpoints, mut new_outputs) = self.generate_claimable_outpoints_and_watch_outputs(); + let (mut new_outpoints, mut new_outputs) = self.generate_claimable_outpoints_and_watch_outputs(true); claimable_outpoints.append(&mut new_outpoints); watch_outputs.append(&mut new_outputs); } @@ -4532,9 +4570,35 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP let ev = match ::read(reader)? { 0 => MonitorEvent::HTLCEvent(Readable::read(reader)?), 1 => MonitorEvent::HolderForceClosed(funding_info.0), + 2 => { + let reason: Option = MaybeReadable::read(reader)?; + let outpoint: OutPoint = Readable::read(reader)?; + let channel_id: ChannelId = Readable::read(reader)?; + MonitorEvent::HolderForceClosedWithInfo { + reason: reason.unwrap_or(ClosureReason::HolderForceClosed), + outpoint, + channel_id, + } + }, _ => return Err(DecodeError::InvalidValue) }; - pending_monitor_events.as_mut().unwrap().push(ev); + + match &ev { + MonitorEvent::HolderForceClosed(_) => { + let events = pending_monitor_events.as_mut().unwrap(); + // only insert if there is no HolderForceClosedWithInfo event + if !events.iter().any(|e| matches!(e, MonitorEvent::HolderForceClosedWithInfo { .. })) { + events.push(ev); + } + } + MonitorEvent::HolderForceClosedWithInfo {..} => { + // if there is a HolderForceClosed event, override it with this one + let events = pending_monitor_events.as_mut().unwrap(); + events.retain(|e| !matches!(e, MonitorEvent::HolderForceClosed(_))); + events.push(ev); + } + _ => pending_monitor_events.as_mut().unwrap().push(ev), + }; } let pending_events_len: u64 = Readable::read(reader)?; diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index d08d3c5ac6b..cde90be8e6c 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -232,6 +232,8 @@ pub enum ClosureReason { /// Another channel in the same funding batch closed before the funding transaction /// was ready to be broadcast. FundingBatchClosure, + /// One of our HTLCs timed out in a channel, causing us to force close the channel. + HTLCsTimedOut, } impl core::fmt::Display for ClosureReason { @@ -255,6 +257,7 @@ impl core::fmt::Display for ClosureReason { ClosureReason::OutdatedChannelManager => f.write_str("the ChannelManager read from disk was stale compared to ChannelMonitor(s)"), ClosureReason::CounterpartyCoopClosedUnfundedChannel => f.write_str("the peer requested the unfunded channel be closed"), ClosureReason::FundingBatchClosure => f.write_str("another channel in the same funding batch closed"), + ClosureReason::HTLCsTimedOut => f.write_str("htlcs on the channel timed out"), } } } @@ -272,6 +275,7 @@ impl_writeable_tlv_based_enum_upgradable!(ClosureReason, (15, FundingBatchClosure) => {}, (17, CounterpartyInitiatedCooperativeClosure) => {}, (19, LocallyInitiatedCooperativeClosure) => {}, + (21, HTLCsTimedOut) => {}, ); /// Intended destination of a failed HTLC as indicated in [`Event::HTLCHandlingFailed`]. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 87ac05ccb2b..30913d9197c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7315,7 +7315,7 @@ where self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver); } }, - MonitorEvent::HolderForceClosed(_funding_outpoint) => { + MonitorEvent::HolderForceClosed(_) | MonitorEvent::HolderForceClosedWithInfo { .. } => { let counterparty_node_id_opt = match counterparty_node_id { Some(cp_id) => Some(cp_id), None => { @@ -7333,7 +7333,12 @@ where let pending_msg_events = &mut peer_state.pending_msg_events; if let hash_map::Entry::Occupied(chan_phase_entry) = peer_state.channel_by_id.entry(channel_id) { if let ChannelPhase::Funded(mut chan) = remove_channel_phase!(self, chan_phase_entry) { - failed_channels.push(chan.context.force_shutdown(false, ClosureReason::HolderForceClosed)); + let reason = if let MonitorEvent::HolderForceClosedWithInfo { reason, ..} = monitor_event { + reason + } else { + ClosureReason::HolderForceClosed + }; + failed_channels.push(chan.context.force_shutdown(false, reason)); if let Ok(update) = self.get_channel_update_for_broadcast(&chan) { pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index aeb60940bb1..bcd2d0881c2 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2417,7 +2417,7 @@ fn channel_monitor_network_test() { } check_added_monitors!(nodes[4], 1); test_txn_broadcast(&nodes[4], &chan_4, None, HTLCType::SUCCESS); - check_closed_event!(nodes[4], 1, ClosureReason::HolderForceClosed, [nodes[3].node.get_our_node_id()], 100000); + check_closed_event!(nodes[4], 1, ClosureReason::HTLCsTimedOut, [nodes[3].node.get_our_node_id()], 100000); mine_transaction(&nodes[4], &node_txn[0]); check_preimage_claim(&nodes[4], &node_txn); @@ -2430,7 +2430,7 @@ fn channel_monitor_network_test() { assert_eq!(nodes[3].chain_monitor.chain_monitor.watch_channel(OutPoint { txid: chan_3.3.txid(), index: 0 }, chan_3_mon), Ok(ChannelMonitorUpdateStatus::Completed)); - check_closed_event!(nodes[3], 1, ClosureReason::HolderForceClosed, [nodes[4].node.get_our_node_id()], 100000); + check_closed_event!(nodes[3], 1, ClosureReason::HTLCsTimedOut, [nodes[4].node.get_our_node_id()], 100000); } #[test] @@ -5682,7 +5682,7 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) { test_txn_broadcast(&nodes[1], &chan, None, if use_dust { HTLCType::NONE } else { HTLCType::SUCCESS }); check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); - check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [nodes[0].node.get_our_node_id()], 100000); } fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) { @@ -5713,7 +5713,7 @@ fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) { test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE); check_closed_broadcast!(nodes[0], true); check_added_monitors!(nodes[0], 1); - check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[0], 1, ClosureReason::HTLCsTimedOut, [nodes[1].node.get_our_node_id()], 100000); } fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no_close: bool) { @@ -5759,7 +5759,7 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE); check_closed_broadcast!(nodes[0], true); check_added_monitors!(nodes[0], 1); - check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[0], 1, ClosureReason::HTLCsTimedOut, [nodes[1].node.get_our_node_id()], 100000); } else { expect_payment_failed!(nodes[0], our_payment_hash, true); } @@ -8654,7 +8654,7 @@ fn test_concurrent_monitor_claim() { let height = HTLC_TIMEOUT_BROADCAST + 1; connect_blocks(&nodes[0], height - nodes[0].best_block_info().1); check_closed_broadcast(&nodes[0], 1, true); - check_closed_event!(&nodes[0], 1, ClosureReason::HolderForceClosed, false, + check_closed_event!(&nodes[0], 1, ClosureReason::HTLCsTimedOut, false, [nodes[1].node.get_our_node_id()], 100000); watchtower_alice.chain_monitor.block_connected(&create_dummy_block(BlockHash::all_zeros(), 42, vec![bob_state_y.clone()]), height); check_added_monitors(&nodes[0], 1); diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 260b4d7c0c6..2e73bd83c2c 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -1188,14 +1188,14 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ assert!(failed_payments.is_empty()); if let Event::PendingHTLCsForwardable { .. } = events[0] {} else { panic!(); } match &events[1] { - Event::ChannelClosed { reason: ClosureReason::HolderForceClosed, .. } => {}, + Event::ChannelClosed { reason: ClosureReason::HTLCsTimedOut, .. } => {}, _ => panic!(), } connect_blocks(&nodes[1], htlc_cltv_timeout + 1 - 10); check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); - check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 1000000); + check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [nodes[0].node.get_our_node_id()], 1000000); // Prior to channel closure, B considers the preimage HTLC as its own, and otherwise only // lists the two on-chain timeout-able HTLCs as claimable balances. diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index a31d9520dad..62c82b01f59 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -465,7 +465,7 @@ fn test_set_outpoints_partial_claiming() { // Connect blocks on node B connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1); check_closed_broadcast!(nodes[1], true); - check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 1000000); + check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [nodes[0].node.get_our_node_id()], 1000000); check_added_monitors!(nodes[1], 1); // Verify node B broadcast 2 HTLC-timeout txn let partial_claim_tx = { @@ -807,7 +807,7 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1); check_closed_broadcast(&nodes[0], 1, true); check_added_monitors(&nodes[0], 1); - check_closed_event(&nodes[0], 1, ClosureReason::HolderForceClosed, false, &[nodes[1].node.get_our_node_id()], 100_000); + check_closed_event(&nodes[0], 1, ClosureReason::HTLCsTimedOut, false, &[nodes[1].node.get_our_node_id()], 100_000); { let mut txn = nodes[0].tx_broadcaster.txn_broadcast();