From c00571c5e875b245fe2978a2d313986e4f7dd2a3 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 1 Oct 2025 11:43:25 +0200 Subject: [PATCH 01/16] Rustfmt handle_new_monitor_update We'll be breaking up this macro in upcoming commits. --- lightning/src/ln/channelmanager.rs | 95 ++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 26 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 1d87eccfe66..046664c2339 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3665,19 +3665,23 @@ macro_rules! handle_new_monitor_update { $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $logger: expr, $chan_id: expr, $counterparty_node_id: expr, $in_flight_updates: ident, $update_idx: ident, _internal_outer, $completed: expr - ) => { { - $in_flight_updates = &mut $peer_state.in_flight_monitor_updates.entry($chan_id) - .or_insert_with(|| ($funding_txo, Vec::new())).1; + ) => {{ + $in_flight_updates = &mut $peer_state + .in_flight_monitor_updates + .entry($chan_id) + .or_insert_with(|| ($funding_txo, Vec::new())) + .1; // During startup, we push monitor updates as background events through to here in // order to replay updates that were in-flight when we shut down. Thus, we have to // filter for uniqueness here. - $update_idx = $in_flight_updates.iter().position(|upd| upd == &$update) - .unwrap_or_else(|| { + $update_idx = + $in_flight_updates.iter().position(|upd| upd == &$update).unwrap_or_else(|| { $in_flight_updates.push($update); $in_flight_updates.len() - 1 }); if $self.background_events_processed_since_startup.load(Ordering::Acquire) { - let update_res = $self.chain_monitor.update_channel($chan_id, &$in_flight_updates[$update_idx]); + let update_res = + $self.chain_monitor.update_channel($chan_id, &$in_flight_updates[$update_idx]); handle_new_monitor_update!($self, update_res, $logger, $chan_id, _internal, $completed) } else { // We blindly assume that the ChannelMonitorUpdate will be regenerated on startup if we @@ -3700,62 +3704,101 @@ macro_rules! handle_new_monitor_update { $self.pending_background_events.lock().unwrap().push(event); false } - } }; + }}; ( $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $chan_context: expr, REMAIN_LOCKED_UPDATE_ACTIONS_PROCESSED_LATER - ) => { { + ) => {{ let logger = WithChannelContext::from(&$self.logger, &$chan_context, None); let chan_id = $chan_context.channel_id(); let counterparty_node_id = $chan_context.get_counterparty_node_id(); let in_flight_updates; let idx; - handle_new_monitor_update!($self, $funding_txo, $update, $peer_state, logger, chan_id, - counterparty_node_id, in_flight_updates, idx, _internal_outer, + handle_new_monitor_update!( + $self, + $funding_txo, + $update, + $peer_state, + logger, + chan_id, + counterparty_node_id, + in_flight_updates, + idx, + _internal_outer, { let _ = in_flight_updates.remove(idx); - }) - } }; + } + ) + }}; ( $self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $counterparty_node_id: expr, $channel_id: expr, POST_CHANNEL_CLOSE - ) => { { - let logger = WithContext::from(&$self.logger, Some($counterparty_node_id), Some($channel_id), None); + ) => {{ + let logger = + WithContext::from(&$self.logger, Some($counterparty_node_id), Some($channel_id), None); let in_flight_updates; let idx; - handle_new_monitor_update!($self, $funding_txo, $update, $peer_state, logger, - $channel_id, $counterparty_node_id, in_flight_updates, idx, _internal_outer, + handle_new_monitor_update!( + $self, + $funding_txo, + $update, + $peer_state, + logger, + $channel_id, + $counterparty_node_id, + in_flight_updates, + idx, + _internal_outer, { let _ = in_flight_updates.remove(idx); if in_flight_updates.is_empty() { - let update_actions = $peer_state.monitor_update_blocked_actions - .remove(&$channel_id).unwrap_or(Vec::new()); + let update_actions = $peer_state + .monitor_update_blocked_actions + .remove(&$channel_id) + .unwrap_or(Vec::new()); mem::drop($peer_state_lock); mem::drop($per_peer_state_lock); $self.handle_monitor_update_completion_actions(update_actions); } - }) - } }; + } + ) + }}; ( $self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr - ) => { { + ) => {{ let logger = WithChannelContext::from(&$self.logger, &$chan.context, None); let chan_id = $chan.context.channel_id(); let counterparty_node_id = $chan.context.get_counterparty_node_id(); let in_flight_updates; let idx; - handle_new_monitor_update!($self, $funding_txo, $update, $peer_state, logger, chan_id, - counterparty_node_id, in_flight_updates, idx, _internal_outer, + handle_new_monitor_update!( + $self, + $funding_txo, + $update, + $peer_state, + logger, + chan_id, + counterparty_node_id, + in_flight_updates, + idx, + _internal_outer, { let _ = in_flight_updates.remove(idx); if in_flight_updates.is_empty() && $chan.blocked_monitor_updates_pending() == 0 { - handle_monitor_update_completion!($self, $peer_state_lock, $peer_state, $per_peer_state_lock, $chan); + handle_monitor_update_completion!( + $self, + $peer_state_lock, + $peer_state, + $per_peer_state_lock, + $chan + ); } - }) - } }; + } + ) + }}; } #[rustfmt::skip] From 8a46e8e31d132451ff04b3071f4b714703acfe4b Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 1 Oct 2025 11:47:42 +0200 Subject: [PATCH 02/16] Extract monitor update res handling to fn --- lightning/src/ln/channelmanager.rs | 76 ++++++++++++++++++------------ 1 file changed, 46 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 046664c2339..87e5489dd67 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3629,37 +3629,19 @@ macro_rules! handle_monitor_update_completion { } macro_rules! handle_new_monitor_update { - ($self: ident, $update_res: expr, $logger: expr, $channel_id: expr, _internal, $completed: expr) => { { - debug_assert!($self.background_events_processed_since_startup.load(Ordering::Acquire)); - match $update_res { - ChannelMonitorUpdateStatus::UnrecoverableError => { - let err_str = "ChannelMonitor[Update] persistence failed unrecoverably. This indicates we cannot continue normal operation and must shut down."; - log_error!($logger, "{}", err_str); - panic!("{}", err_str); - }, - ChannelMonitorUpdateStatus::InProgress => { - #[cfg(not(any(test, feature = "_externalize_tests")))] - if $self.monitor_update_type.swap(1, Ordering::Relaxed) == 2 { - panic!("Cannot use both ChannelMonitorUpdateStatus modes InProgress and Completed without restart"); - } - log_debug!($logger, "ChannelMonitor update for {} in flight, holding messages until the update completes.", - $channel_id); - false - }, - ChannelMonitorUpdateStatus::Completed => { - #[cfg(not(any(test, feature = "_externalize_tests")))] - if $self.monitor_update_type.swap(2, Ordering::Relaxed) == 1 { - panic!("Cannot use both ChannelMonitorUpdateStatus modes InProgress and Completed without restart"); - } - $completed; - true - }, - } - } }; ($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, INITIAL_MONITOR) => { let logger = WithChannelContext::from(&$self.logger, &$chan.context, None); - handle_new_monitor_update!($self, $update_res, logger, $chan.context.channel_id(), _internal, - handle_monitor_update_completion!($self, $peer_state_lock, $peer_state, $per_peer_state_lock, $chan)) + let update_completed = + $self.handle_monitor_update_res($update_res, $chan.context.channel_id(), logger); + if update_completed { + handle_monitor_update_completion!( + $self, + $peer_state_lock, + $peer_state, + $per_peer_state_lock, + $chan + ); + } }; ( $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $logger: expr, @@ -3682,7 +3664,11 @@ macro_rules! handle_new_monitor_update { if $self.background_events_processed_since_startup.load(Ordering::Acquire) { let update_res = $self.chain_monitor.update_channel($chan_id, &$in_flight_updates[$update_idx]); - handle_new_monitor_update!($self, update_res, $logger, $chan_id, _internal, $completed) + let update_completed = $self.handle_monitor_update_res(update_res, $chan_id, $logger); + if update_completed { + $completed; + } + update_completed } else { // We blindly assume that the ChannelMonitorUpdate will be regenerated on startup if we // fail to persist it. This is a fairly safe assumption, however, since anything we do @@ -9549,6 +9535,36 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } + /// Returns whether the monitor update is completed, `false` if the update is in-progress. + fn handle_monitor_update_res( + &self, update_res: ChannelMonitorUpdateStatus, channel_id: ChannelId, logger: LG, + ) -> bool { + debug_assert!(self.background_events_processed_since_startup.load(Ordering::Acquire)); + match update_res { + ChannelMonitorUpdateStatus::UnrecoverableError => { + let err_str = "ChannelMonitor[Update] persistence failed unrecoverably. This indicates we cannot continue normal operation and must shut down."; + log_error!(logger, "{}", err_str); + panic!("{}", err_str); + }, + ChannelMonitorUpdateStatus::InProgress => { + #[cfg(not(any(test, feature = "_externalize_tests")))] + if self.monitor_update_type.swap(1, Ordering::Relaxed) == 2 { + panic!("Cannot use both ChannelMonitorUpdateStatus modes InProgress and Completed without restart"); + } + log_debug!(logger, "ChannelMonitor update for {} in flight, holding messages until the update completes.", + channel_id); + false + }, + ChannelMonitorUpdateStatus::Completed => { + #[cfg(not(any(test, feature = "_externalize_tests")))] + if self.monitor_update_type.swap(2, Ordering::Relaxed) == 1 { + panic!("Cannot use both ChannelMonitorUpdateStatus modes InProgress and Completed without restart"); + } + true + }, + } + } + /// Accepts a request to open a channel after a [`Event::OpenChannelRequest`]. /// /// The `temporary_channel_id` parameter indicates which inbound channel should be accepted, From d3db9c4c619dc597fcf202d04b134951afc3ca83 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 1 Oct 2025 12:09:52 +0200 Subject: [PATCH 03/16] Extract macro for initial monitor persist Makes handle_new_monitor_update more readable. --- lightning/src/ln/channelmanager.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 87e5489dd67..96dec3310c3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3628,8 +3628,8 @@ macro_rules! handle_monitor_update_completion { } } } -macro_rules! handle_new_monitor_update { - ($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, INITIAL_MONITOR) => { +macro_rules! handle_initial_monitor { + ($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr) => { let logger = WithChannelContext::from(&$self.logger, &$chan.context, None); let update_completed = $self.handle_monitor_update_res($update_res, $chan.context.channel_id(), logger); @@ -3643,6 +3643,9 @@ macro_rules! handle_new_monitor_update { ); } }; +} + +macro_rules! handle_new_monitor_update { ( $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $logger: expr, $chan_id: expr, $counterparty_node_id: expr, $in_flight_updates: ident, $update_idx: ident, @@ -10115,8 +10118,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } if let Some(funded_chan) = e.insert(Channel::from(chan)).as_funded_mut() { - handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state, - per_peer_state, funded_chan, INITIAL_MONITOR); + handle_initial_monitor!(self, persist_state, peer_state_lock, peer_state, + per_peer_state, funded_chan); } else { unreachable!("This must be a funded channel as we just inserted it."); } @@ -10279,7 +10282,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }) { Ok((funded_chan, persist_status)) => { - handle_new_monitor_update!(self, persist_status, peer_state_lock, peer_state, per_peer_state, funded_chan, INITIAL_MONITOR); + handle_initial_monitor!(self, persist_status, peer_state_lock, peer_state, per_peer_state, funded_chan); Ok(()) }, Err(e) => try_channel_entry!(self, peer_state, Err(e), chan_entry), @@ -10904,8 +10907,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(monitor) = monitor_opt { let monitor_res = self.chain_monitor.watch_channel(monitor.channel_id(), monitor); if let Ok(persist_state) = monitor_res { - handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state, - per_peer_state, chan, INITIAL_MONITOR); + handle_initial_monitor!(self, persist_state, peer_state_lock, peer_state, + per_peer_state, chan); } else { let logger = WithChannelContext::from(&self.logger, &chan.context, None); log_error!(logger, "Persisting initial ChannelMonitor failed, implying the channel ID was duplicated"); From 1c82b171da1a7718cfd8b801a33e7446e47e9b07 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 1 Oct 2025 12:27:01 +0200 Subject: [PATCH 04/16] Extract post-close monitor update macro --- lightning/src/ln/channelmanager.rs | 87 +++++++++++++++--------------- 1 file changed, 44 insertions(+), 43 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 96dec3310c3..2bcadd2a600 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3645,6 +3645,44 @@ macro_rules! handle_initial_monitor { }; } +macro_rules! handle_post_close_monitor_update { + ( + $self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, + $per_peer_state_lock: expr, $counterparty_node_id: expr, $channel_id: expr + ) => {{ + let logger = + WithContext::from(&$self.logger, Some($counterparty_node_id), Some($channel_id), None); + let in_flight_updates; + let idx; + handle_new_monitor_update!( + $self, + $funding_txo, + $update, + $peer_state, + logger, + $channel_id, + $counterparty_node_id, + in_flight_updates, + idx, + _internal_outer, + { + let _ = in_flight_updates.remove(idx); + if in_flight_updates.is_empty() { + let update_actions = $peer_state + .monitor_update_blocked_actions + .remove(&$channel_id) + .unwrap_or(Vec::new()); + + mem::drop($peer_state_lock); + mem::drop($per_peer_state_lock); + + $self.handle_monitor_update_completion_actions(update_actions); + } + } + ) + }}; +} + macro_rules! handle_new_monitor_update { ( $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $logger: expr, @@ -3719,41 +3757,6 @@ macro_rules! handle_new_monitor_update { } ) }}; - ( - $self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, - $per_peer_state_lock: expr, $counterparty_node_id: expr, $channel_id: expr, POST_CHANNEL_CLOSE - ) => {{ - let logger = - WithContext::from(&$self.logger, Some($counterparty_node_id), Some($channel_id), None); - let in_flight_updates; - let idx; - handle_new_monitor_update!( - $self, - $funding_txo, - $update, - $peer_state, - logger, - $channel_id, - $counterparty_node_id, - in_flight_updates, - idx, - _internal_outer, - { - let _ = in_flight_updates.remove(idx); - if in_flight_updates.is_empty() { - let update_actions = $peer_state - .monitor_update_blocked_actions - .remove(&$channel_id) - .unwrap_or(Vec::new()); - - mem::drop($peer_state_lock); - mem::drop($per_peer_state_lock); - - $self.handle_monitor_update_completion_actions(update_actions); - } - } - ) - }}; ( $self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr @@ -4464,9 +4467,9 @@ where hash_map::Entry::Vacant(_) => {}, } - handle_new_monitor_update!( + handle_post_close_monitor_update!( self, funding_txo, monitor_update, peer_state_lock, peer_state, per_peer_state, - counterparty_node_id, channel_id, POST_CHANNEL_CLOSE + counterparty_node_id, channel_id ); } @@ -8925,7 +8928,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .push(action); } - handle_new_monitor_update!( + handle_post_close_monitor_update!( self, prev_hop.funding_txo, preimage_update, @@ -8933,8 +8936,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ peer_state, per_peer_state, prev_hop.counterparty_node_id, - chan_id, - POST_CHANNEL_CLOSE + chan_id ); } @@ -13345,7 +13347,7 @@ where }; self.pending_background_events.lock().unwrap().push(event); } else { - handle_new_monitor_update!( + handle_post_close_monitor_update!( self, channel_funding_outpoint, update, @@ -13353,8 +13355,7 @@ where peer_state, per_peer_state, counterparty_node_id, - channel_id, - POST_CHANNEL_CLOSE + channel_id ); } }, From c58cccf5249933aa3ab99bf4a6178bed75dd4814 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 1 Oct 2025 17:42:43 +0200 Subject: [PATCH 05/16] Extract util for XXX --- lightning/src/ln/channelmanager.rs | 62 +++++++++++++++--------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2bcadd2a600..039cf6cb785 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3271,8 +3271,8 @@ macro_rules! locked_close_channel { }}; ($self: ident, $peer_state: expr, $funded_chan: expr, $shutdown_res_mut: expr, FUNDED) => {{ if let Some((_, funding_txo, _, update)) = $shutdown_res_mut.monitor_update.take() { - handle_new_monitor_update!($self, funding_txo, update, $peer_state, - $funded_chan.context, REMAIN_LOCKED_UPDATE_ACTIONS_PROCESSED_LATER); + handle_new_monitor_update_todo_name!($self, funding_txo, update, $peer_state, + $funded_chan.context); } // If there's a possibility that we need to generate further monitor updates for this // channel, we need to store the last update_id of it. However, we don't want to insert @@ -3666,6 +3666,7 @@ macro_rules! handle_post_close_monitor_update { idx, _internal_outer, { + // If we get a monitor update for a closed channel let _ = in_flight_updates.remove(idx); if in_flight_updates.is_empty() { let update_actions = $peer_state @@ -3683,6 +3684,33 @@ macro_rules! handle_post_close_monitor_update { }}; } +macro_rules! handle_new_monitor_update_todo_name { + ( + $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $chan_context: expr + ) => {{ + let logger = WithChannelContext::from(&$self.logger, &$chan_context, None); + let chan_id = $chan_context.channel_id(); + let counterparty_node_id = $chan_context.get_counterparty_node_id(); + let in_flight_updates; + let idx; + handle_new_monitor_update!( + $self, + $funding_txo, + $update, + $peer_state, + logger, + chan_id, + counterparty_node_id, + in_flight_updates, + idx, + _internal_outer, + { + let _ = in_flight_updates.remove(idx); + } + ) + }}; +} + macro_rules! handle_new_monitor_update { ( $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $logger: expr, @@ -3732,31 +3760,6 @@ macro_rules! handle_new_monitor_update { false } }}; - ( - $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $chan_context: expr, - REMAIN_LOCKED_UPDATE_ACTIONS_PROCESSED_LATER - ) => {{ - let logger = WithChannelContext::from(&$self.logger, &$chan_context, None); - let chan_id = $chan_context.channel_id(); - let counterparty_node_id = $chan_context.get_counterparty_node_id(); - let in_flight_updates; - let idx; - handle_new_monitor_update!( - $self, - $funding_txo, - $update, - $peer_state, - logger, - chan_id, - counterparty_node_id, - in_flight_updates, - idx, - _internal_outer, - { - let _ = in_flight_updates.remove(idx); - } - ) - }}; ( $self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr @@ -14039,13 +14042,12 @@ where insert_short_channel_id!(short_to_chan_info, funded_channel); if let Some(monitor_update) = monitor_update_opt { - handle_new_monitor_update!( + handle_new_monitor_update_todo_name!( self, funding_txo, monitor_update, peer_state, - funded_channel.context, - REMAIN_LOCKED_UPDATE_ACTIONS_PROCESSED_LATER + funded_channel.context ); to_process_monitor_update_actions.push(( counterparty_node_id, channel_id From c7e5c70a7d25225b13a076c02adda63b78d40cad Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 1 Oct 2025 17:46:08 +0200 Subject: [PATCH 06/16] Extract _internal macro for new mon update handling Finishes work over the past few commits of breaking up the handle_new_monitor_update macro. --- lightning/src/ln/channelmanager.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 039cf6cb785..a569e1bb3f9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3654,7 +3654,7 @@ macro_rules! handle_post_close_monitor_update { WithContext::from(&$self.logger, Some($counterparty_node_id), Some($channel_id), None); let in_flight_updates; let idx; - handle_new_monitor_update!( + handle_new_monitor_update_internal!( $self, $funding_txo, $update, @@ -3664,7 +3664,6 @@ macro_rules! handle_post_close_monitor_update { $counterparty_node_id, in_flight_updates, idx, - _internal_outer, { // If we get a monitor update for a closed channel let _ = in_flight_updates.remove(idx); @@ -3693,7 +3692,7 @@ macro_rules! handle_new_monitor_update_todo_name { let counterparty_node_id = $chan_context.get_counterparty_node_id(); let in_flight_updates; let idx; - handle_new_monitor_update!( + handle_new_monitor_update_internal!( $self, $funding_txo, $update, @@ -3703,7 +3702,6 @@ macro_rules! handle_new_monitor_update_todo_name { counterparty_node_id, in_flight_updates, idx, - _internal_outer, { let _ = in_flight_updates.remove(idx); } @@ -3711,11 +3709,11 @@ macro_rules! handle_new_monitor_update_todo_name { }}; } -macro_rules! handle_new_monitor_update { +macro_rules! handle_new_monitor_update_internal { ( $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $logger: expr, $chan_id: expr, $counterparty_node_id: expr, $in_flight_updates: ident, $update_idx: ident, - _internal_outer, $completed: expr + $completed: expr ) => {{ $in_flight_updates = &mut $peer_state .in_flight_monitor_updates @@ -3760,6 +3758,9 @@ macro_rules! handle_new_monitor_update { false } }}; +} + +macro_rules! handle_new_monitor_update { ( $self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr @@ -3769,7 +3770,7 @@ macro_rules! handle_new_monitor_update { let counterparty_node_id = $chan.context.get_counterparty_node_id(); let in_flight_updates; let idx; - handle_new_monitor_update!( + handle_new_monitor_update_internal!( $self, $funding_txo, $update, @@ -3779,7 +3780,6 @@ macro_rules! handle_new_monitor_update { counterparty_node_id, in_flight_updates, idx, - _internal_outer, { let _ = in_flight_updates.remove(idx); if in_flight_updates.is_empty() && $chan.blocked_monitor_updates_pending() == 0 { From 39212b567f8c630c4d3f7aa958883db55cac4d4f Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 1 Oct 2025 17:34:19 +0200 Subject: [PATCH 07/16] add encoded channel to update methods --- lightning/src/chain/chainmonitor.rs | 16 ++++++++++++---- lightning/src/chain/mod.rs | 2 +- lightning/src/ln/chanmon_update_fail_tests.rs | 12 ++++++------ lightning/src/ln/channelmanager.rs | 7 +++++-- lightning/src/ln/functional_tests.rs | 10 +++++----- lightning/src/util/persist.rs | 5 +++-- lightning/src/util/test_utils.rs | 10 +++++----- 7 files changed, 37 insertions(+), 25 deletions(-) diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 4abd0cd88c0..443f87a40c2 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -180,7 +180,7 @@ pub trait Persist { /// [`Writeable::write`]: crate::util::ser::Writeable::write fn update_persisted_channel( &self, monitor_name: MonitorName, monitor_update: Option<&ChannelMonitorUpdate>, - monitor: &ChannelMonitor, + encoded_channel: Option<&[u8]>, monitor: &ChannelMonitor, ) -> ChannelMonitorUpdateStatus; /// Prevents the channel monitor from being loaded on startup. /// @@ -320,6 +320,7 @@ where fn update_persisted_channel( &self, monitor_name: MonitorName, monitor_update: Option<&ChannelMonitorUpdate>, + encoded_channel: Option<&[u8]>, monitor: &ChannelMonitor<::EcdsaSigner>, ) -> ChannelMonitorUpdateStatus { self.persister.spawn_async_update_persisted_channel(monitor_name, monitor_update, monitor); @@ -579,8 +580,12 @@ where // `ChannelMonitorUpdate` after a channel persist for a channel with the same // `latest_update_id`. let _pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap(); - match self.persister.update_persisted_channel(monitor.persistence_key(), None, monitor) - { + match self.persister.update_persisted_channel( + monitor.persistence_key(), + None, + None, + monitor, + ) { ChannelMonitorUpdateStatus::Completed => log_trace!( logger, "Finished syncing Channel Monitor for channel {} for block-data", @@ -944,6 +949,7 @@ where self.persister.update_persisted_channel( monitor_holder.monitor.persistence_key(), None, + None, &monitor_holder.monitor, ); } @@ -1392,7 +1398,7 @@ where } fn update_channel( - &self, channel_id: ChannelId, update: &ChannelMonitorUpdate, + &self, channel_id: ChannelId, update: &ChannelMonitorUpdate, encoded_channel: Option<&[u8]>, ) -> ChannelMonitorUpdateStatus { // `ChannelMonitorUpdate`'s `channel_id` is `None` prior to 0.0.121 and all channels in those // versions are V1-established. For 0.0.121+ the `channel_id` fields is always `Some`. @@ -1445,12 +1451,14 @@ where self.persister.update_persisted_channel( monitor.persistence_key(), None, + None, monitor, ) } else { self.persister.update_persisted_channel( monitor.persistence_key(), Some(update), + None, monitor, ) }; diff --git a/lightning/src/chain/mod.rs b/lightning/src/chain/mod.rs index 2a6d3d23e80..3a513d27ed9 100644 --- a/lightning/src/chain/mod.rs +++ b/lightning/src/chain/mod.rs @@ -326,7 +326,7 @@ pub trait Watch { /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager fn update_channel( - &self, channel_id: ChannelId, update: &ChannelMonitorUpdate, + &self, channel_id: ChannelId, update: &ChannelMonitorUpdate, encoded_channel: Option<&[u8]>, ) -> ChannelMonitorUpdateStatus; /// Returns any monitor events since the last call. Subsequent calls must only return new diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 1bc1bfbc2ff..1239ebe1716 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -143,7 +143,7 @@ fn test_monitor_and_persister_update_fail() { // Check that the persister returns InProgress (and will never actually complete) // as the monitor update errors. if let ChannelMonitorUpdateStatus::InProgress = - chain_mon.chain_monitor.update_channel(chan.2, &update) + chain_mon.chain_monitor.update_channel(chan.2, &update, None) { } else { panic!("Expected monitor paused"); @@ -158,7 +158,7 @@ fn test_monitor_and_persister_update_fail() { // Apply the monitor update to the original ChainMonitor, ensuring the // ChannelManager and ChannelMonitor aren't out of sync. assert_eq!( - nodes[0].chain_monitor.update_channel(chan.2, &update), + nodes[0].chain_monitor.update_channel(chan.2, &update, None), ChannelMonitorUpdateStatus::Completed ); } else { @@ -4961,10 +4961,10 @@ fn native_async_persist() { // Now test two async `ChannelMonitorUpdate`s in flight at once, completing them in-order but // separately. - let update_status = async_chain_monitor.update_channel(chan_id, &updates[0]); + let update_status = async_chain_monitor.update_channel(chan_id, &updates[0], None); assert_eq!(update_status, ChannelMonitorUpdateStatus::InProgress); - let update_status = async_chain_monitor.update_channel(chan_id, &updates[1]); + let update_status = async_chain_monitor.update_channel(chan_id, &updates[1], None); assert_eq!(update_status, ChannelMonitorUpdateStatus::InProgress); persist_futures.poll_futures(); @@ -5010,10 +5010,10 @@ fn native_async_persist() { // Finally, test two async `ChanelMonitorUpdate`s in flight at once, completing them // out-of-order and ensuring that no `MonitorEvent::Completed` is generated until they are both // completed (and that it marks both as completed when it is generated). - let update_status = async_chain_monitor.update_channel(chan_id, &updates[2]); + let update_status = async_chain_monitor.update_channel(chan_id, &updates[2], None); assert_eq!(update_status, ChannelMonitorUpdateStatus::InProgress); - let update_status = async_chain_monitor.update_channel(chan_id, &updates[3]); + let update_status = async_chain_monitor.update_channel(chan_id, &updates[3], None); assert_eq!(update_status, ChannelMonitorUpdateStatus::InProgress); persist_futures.poll_futures(); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a569e1bb3f9..e2ac198346d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3729,8 +3729,11 @@ macro_rules! handle_new_monitor_update_internal { $in_flight_updates.len() - 1 }); if $self.background_events_processed_since_startup.load(Ordering::Acquire) { - let update_res = - $self.chain_monitor.update_channel($chan_id, &$in_flight_updates[$update_idx]); + let update_res = $self.chain_monitor.update_channel( + $chan_id, + &$in_flight_updates[$update_idx], + None, + ); let update_completed = $self.handle_monitor_update_res(update_res, $chan_id, $logger); if update_completed { $completed; diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index d1c0ac8f12b..46f96c5baa8 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -7259,11 +7259,11 @@ pub fn test_update_err_monitor_lockdown() { &node_cfgs[0].logger, ) { assert_eq!( - watchtower.chain_monitor.update_channel(chan_1.2, &update), + watchtower.chain_monitor.update_channel(chan_1.2, &update, None), ChannelMonitorUpdateStatus::InProgress ); assert_eq!( - nodes[0].chain_monitor.update_channel(chan_1.2, &update), + nodes[0].chain_monitor.update_channel(chan_1.2, &update, None), ChannelMonitorUpdateStatus::Completed ); } else { @@ -7416,15 +7416,15 @@ pub fn test_concurrent_monitor_claim() { ) { // Watchtower Alice should already have seen the block and reject the update assert_eq!( - watchtower_alice.chain_monitor.update_channel(chan_1.2, &update), + watchtower_alice.chain_monitor.update_channel(chan_1.2, &update, None), ChannelMonitorUpdateStatus::InProgress ); assert_eq!( - watchtower_bob.chain_monitor.update_channel(chan_1.2, &update), + watchtower_bob.chain_monitor.update_channel(chan_1.2, &update, None), ChannelMonitorUpdateStatus::Completed ); assert_eq!( - nodes[0].chain_monitor.update_channel(chan_1.2, &update), + nodes[0].chain_monitor.update_channel(chan_1.2, &update, None), ChannelMonitorUpdateStatus::Completed ); } else { diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index ce7c8311813..42a44363d34 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -317,7 +317,7 @@ impl Persist, - monitor: &ChannelMonitor, + encoded_channel: Option<&[u8]>, monitor: &ChannelMonitor, ) -> chain::ChannelMonitorUpdateStatus { match self.write( CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE, @@ -660,7 +660,7 @@ where /// - The update is at [`u64::MAX`], indicating an update generated by pre-0.1 LDK. fn update_persisted_channel( &self, monitor_name: MonitorName, update: Option<&ChannelMonitorUpdate>, - monitor: &ChannelMonitor, + encoded_channel: Option<&[u8]>, monitor: &ChannelMonitor, ) -> chain::ChannelMonitorUpdateStatus { let inner = Arc::clone(&self.0 .0); let res = poll_sync_future(inner.update_persisted_channel(monitor_name, update, monitor)); @@ -1677,6 +1677,7 @@ mod tests { match ro_persister.update_persisted_channel( monitor_name, Some(cmu), + None, &added_monitors[0].1, ) { ChannelMonitorUpdateStatus::UnrecoverableError => { diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index aacc38e366a..5641b454c28 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -580,7 +580,7 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { } fn update_channel( - &self, channel_id: ChannelId, update: &ChannelMonitorUpdate, + &self, channel_id: ChannelId, update: &ChannelMonitorUpdate, encoded_channel: Option<&[u8]>, ) -> chain::ChannelMonitorUpdateStatus { #[cfg(feature = "std")] if let Some(blocker) = &*self.write_blocker.lock().unwrap() { @@ -614,7 +614,7 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { .lock() .unwrap() .insert(channel_id, (update.update_id, update.update_id)); - let update_res = self.chain_monitor.update_channel(channel_id, update); + let update_res = self.chain_monitor.update_channel(channel_id, update, encoded_channel); // At every point where we get a monitor update, we should be able to send a useful monitor // to a watchtower and disk... let monitor = self.chain_monitor.get_monitor(channel_id).unwrap(); @@ -744,9 +744,9 @@ impl Persist for WatchtowerPers fn update_persisted_channel( &self, monitor_name: MonitorName, update: Option<&ChannelMonitorUpdate>, - data: &ChannelMonitor, + encoded_channel: Option<&[u8]>, data: &ChannelMonitor, ) -> chain::ChannelMonitorUpdateStatus { - let res = self.persister.update_persisted_channel(monitor_name, update, data); + let res = self.persister.update_persisted_channel(monitor_name, update, None, data); if let Some(update) = update { let commitment_txs = data.counterparty_commitment_txs_from_update(update); @@ -830,7 +830,7 @@ impl Persist for TestPersister fn update_persisted_channel( &self, monitor_name: MonitorName, update: Option<&ChannelMonitorUpdate>, - _data: &ChannelMonitor, + encoded_channel: Option<&[u8]>, _data: &ChannelMonitor, ) -> chain::ChannelMonitorUpdateStatus { let mut ret = chain::ChannelMonitorUpdateStatus::Completed; if let Some(update_ret) = self.update_rets.lock().unwrap().pop_front() { From 8f527cfdd8ed9869fc47cc3d0a3c4a27e60a445b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 6 Oct 2025 08:09:34 +0200 Subject: [PATCH 08/16] wip1 --- lightning/src/ln/channelmanager.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e2ac198346d..ef03cf41630 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3658,6 +3658,7 @@ macro_rules! handle_post_close_monitor_update { $self, $funding_txo, $update, + None, $peer_state, logger, $channel_id, @@ -3696,6 +3697,7 @@ macro_rules! handle_new_monitor_update_todo_name { $self, $funding_txo, $update, + None, $peer_state, logger, chan_id, @@ -3711,7 +3713,7 @@ macro_rules! handle_new_monitor_update_todo_name { macro_rules! handle_new_monitor_update_internal { ( - $self: ident, $funding_txo: expr, $update: expr, $peer_state: expr, $logger: expr, + $self: ident, $funding_txo: expr, $update: expr, $channel: expr, $peer_state: expr, $logger: expr, $chan_id: expr, $counterparty_node_id: expr, $in_flight_updates: ident, $update_idx: ident, $completed: expr ) => {{ @@ -3777,6 +3779,7 @@ macro_rules! handle_new_monitor_update { $self, $funding_txo, $update, + None, $peer_state, logger, chan_id, From 3a537a58164419f357796aa8551632fc0c54ca04 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 6 Oct 2025 08:15:05 +0200 Subject: [PATCH 09/16] wip2 --- lightning/src/ln/channelmanager.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ef03cf41630..8186537d2ee 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3734,7 +3734,7 @@ macro_rules! handle_new_monitor_update_internal { let update_res = $self.chain_monitor.update_channel( $chan_id, &$in_flight_updates[$update_idx], - None, + $channel, ); let update_completed = $self.handle_monitor_update_res(update_res, $chan_id, $logger); if update_completed { @@ -3767,7 +3767,7 @@ macro_rules! handle_new_monitor_update_internal { macro_rules! handle_new_monitor_update { ( - $self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, + $self: ident, $funding_txo: expr, $update: expr, $channel: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr ) => {{ let logger = WithChannelContext::from(&$self.logger, &$chan.context, None); @@ -3779,7 +3779,7 @@ macro_rules! handle_new_monitor_update { $self, $funding_txo, $update, - None, + $channel, $peer_state, logger, chan_id, @@ -4343,7 +4343,7 @@ where // Update the monitor with the shutdown script if necessary. if let Some(monitor_update) = monitor_update_opt.take() { - handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update, + handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update, None, peer_state_lock, peer_state, per_peer_state, chan); } } else { @@ -4467,7 +4467,7 @@ where hash_map::Entry::Occupied(mut chan_entry) => { if let Some(chan) = chan_entry.get_mut().as_funded_mut() { handle_new_monitor_update!(self, funding_txo, - monitor_update, peer_state_lock, peer_state, per_peer_state, chan); + monitor_update, None, peer_state_lock, peer_state, per_peer_state, chan); return; } else { debug_assert!(false, "We shouldn't have an update for a non-funded channel"); @@ -5193,6 +5193,7 @@ where self, funding_txo, monitor_update, + None, peer_state_lock, peer_state, per_peer_state, @@ -8771,6 +8772,7 @@ where self, prev_hop.funding_txo, monitor_update, + None, peer_state_lock, peer_state, per_peer_state, @@ -10622,6 +10624,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ self, funding_txo_opt.unwrap(), monitor_update, + None, peer_state_lock, peer_state, per_peer_state, @@ -10929,7 +10932,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ try_channel_entry!(self, peer_state, Err(err), chan_entry) } } else if let Some(monitor_update) = monitor_update_opt { - handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock, + handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, None, peer_state_lock, peer_state, per_peer_state, chan); } } @@ -10961,7 +10964,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(monitor_update) = monitor_update_opt { handle_new_monitor_update!( - self, funding_txo.unwrap(), monitor_update, peer_state_lock, peer_state, + self, funding_txo.unwrap(), monitor_update, None, peer_state_lock, peer_state, per_peer_state, chan ); } @@ -11207,7 +11210,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(monitor_update) = monitor_update_opt { let funding_txo = funding_txo_opt .expect("Funding outpoint must have been set for RAA handling to succeed"); - handle_new_monitor_update!(self, funding_txo, monitor_update, + handle_new_monitor_update!(self, funding_txo, monitor_update, None, peer_state_lock, peer_state, per_peer_state, chan); } (htlcs_to_fail, static_invoices) @@ -11688,6 +11691,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ self, splice_promotion.funding_txo, monitor_update, + None, peer_state_lock, peer_state, per_peer_state, @@ -11878,6 +11882,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ self, funding_txo.unwrap(), monitor_update, + None, peer_state_lock, peer_state, per_peer_state, @@ -13283,7 +13288,7 @@ where if let Some((monitor_update, further_update_exists)) = chan.unblock_next_blocked_monitor_update() { log_debug!(logger, "Unlocking monitor updating for channel {} and updating monitor", channel_id); - handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update, + handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update,None, peer_state_lck, peer_state, per_peer_state, chan); if further_update_exists { // If there are more `ChannelMonitorUpdate`s to process, restart at the From c363923bb450f13e0f9fbc061895b556647b88c4 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 6 Oct 2025 08:34:05 +0200 Subject: [PATCH 10/16] wip3 --- lightning/src/ln/channelmanager.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8186537d2ee..622dddd357f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5189,11 +5189,12 @@ where ); match break_channel_entry!(self, peer_state, send_res, chan_entry) { Some(monitor_update) => { + let encoded_channel = chan.encode(); let ok = handle_new_monitor_update!( self, funding_txo, monitor_update, - None, + Some(&encoded_channel), peer_state_lock, peer_state, per_peer_state, @@ -8768,11 +8769,12 @@ where .or_insert_with(Vec::new) .push(raa_blocker); } + let encoded_chan = chan.encode(); handle_new_monitor_update!( self, prev_hop.funding_txo, monitor_update, - None, + Some(&encoded_chan), peer_state_lock, peer_state, per_peer_state, @@ -10932,7 +10934,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ try_channel_entry!(self, peer_state, Err(err), chan_entry) } } else if let Some(monitor_update) = monitor_update_opt { - handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, None, peer_state_lock, + let encoded_chan = chan.encode(); + handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, Some(&encoded_chan), peer_state_lock, peer_state, per_peer_state, chan); } } @@ -10963,8 +10966,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ); if let Some(monitor_update) = monitor_update_opt { + let encoded_chan = chan.encode(); handle_new_monitor_update!( - self, funding_txo.unwrap(), monitor_update, None, peer_state_lock, peer_state, + self, funding_txo.unwrap(), monitor_update, Some(&encoded_chan), peer_state_lock, peer_state, per_peer_state, chan ); } @@ -11210,7 +11214,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(monitor_update) = monitor_update_opt { let funding_txo = funding_txo_opt .expect("Funding outpoint must have been set for RAA handling to succeed"); - handle_new_monitor_update!(self, funding_txo, monitor_update, None, + let encoded_chan = chan.encode(); + handle_new_monitor_update!(self, funding_txo, monitor_update, Some(&encoded_chan), peer_state_lock, peer_state, per_peer_state, chan); } (htlcs_to_fail, static_invoices) @@ -11877,12 +11882,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } if let Some(monitor_update) = monitor_opt { has_monitor_update = true; - + let encoded_channel = chan.encode(); handle_new_monitor_update!( self, funding_txo.unwrap(), monitor_update, - None, + Some(&encoded_channel), peer_state_lock, peer_state, per_peer_state, @@ -13288,7 +13293,8 @@ where if let Some((monitor_update, further_update_exists)) = chan.unblock_next_blocked_monitor_update() { log_debug!(logger, "Unlocking monitor updating for channel {} and updating monitor", channel_id); - handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update,None, + let encoded_chan = chan.encode(); + handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update, Some(&encoded_chan), peer_state_lck, peer_state, per_peer_state, chan); if further_update_exists { // If there are more `ChannelMonitorUpdate`s to process, restart at the From b6cce3f0aed4ad7d4210f0eb838038602d778052 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 6 Oct 2025 08:39:16 +0200 Subject: [PATCH 11/16] wip4 --- lightning/src/chain/chainmonitor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 443f87a40c2..413782132f2 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -1451,14 +1451,14 @@ where self.persister.update_persisted_channel( monitor.persistence_key(), None, - None, + encoded_channel, monitor, ) } else { self.persister.update_persisted_channel( monitor.persistence_key(), Some(update), - None, + encoded_channel, monitor, ) }; From 941b0b409c23602b90022f37bc28acfd7d27f4fb Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 24 Sep 2025 13:00:47 +0200 Subject: [PATCH 12/16] Strip chan mgr serialization --- lightning/src/ln/channelmanager.rs | 1860 +++++++++++----------------- lightning/src/ln/reload_tests.rs | 7 +- 2 files changed, 740 insertions(+), 1127 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 622dddd357f..ea5e11abf4b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -16381,7 +16381,15 @@ where is_connected: false, }; - let mut failed_htlcs = Vec::new(); + // let mut failed_htlcs: Vec<( + // HTLCSource, + // PaymentHash, + // PublicKey, + // ChannelId, + // LocalHTLCFailureReason, + // Option, + // )> = Vec::new(); + let channel_count: u64 = Readable::read(reader)?; let mut channel_id_set = hash_set_with_capacity(cmp::min(channel_count as usize, 128)); let mut per_peer_state = hash_map_with_capacity(cmp::min( @@ -16389,7 +16397,8 @@ where MAX_ALLOC_SIZE / mem::size_of::<(PublicKey, Mutex>)>(), )); let mut short_to_chan_info = hash_map_with_capacity(cmp::min(channel_count as usize, 128)); - let mut channel_closures = VecDeque::new(); + let mut channel_closures: VecDeque<(Event, Option)> = + VecDeque::new(); let mut close_background_events = Vec::new(); for _ in 0..channel_count { let mut channel: FundedChannel = FundedChannel::read( @@ -16475,10 +16484,10 @@ where }, ); } - for (source, hash, cp_id, chan_id) in shutdown_result.dropped_outbound_htlcs { - let reason = LocalHTLCFailureReason::ChannelClosed; - failed_htlcs.push((source, hash, cp_id, chan_id, reason, None)); - } + // for (source, hash, cp_id, chan_id) in shutdown_result.dropped_outbound_htlcs { + // let reason = LocalHTLCFailureReason::ChannelClosed; + // failed_htlcs.push((source, hash, cp_id, chan_id, reason, None)); + // } channel_closures.push_back(( events::Event::ChannelClosed { channel_id: channel.context.channel_id(), @@ -16491,40 +16500,40 @@ where }, None, )); - for (channel_htlc_source, payment_hash) in channel.inflight_htlc_sources() { - let mut found_htlc = false; - for (monitor_htlc_source, _) in monitor.get_all_current_outbound_htlcs() { - if *channel_htlc_source == monitor_htlc_source { - found_htlc = true; - break; - } - } - if !found_htlc { - // If we have some HTLCs in the channel which are not present in the newer - // ChannelMonitor, they have been removed and should be failed back to - // ensure we don't forget them entirely. Note that if the missing HTLC(s) - // were actually claimed we'd have generated and ensured the previous-hop - // claim update ChannelMonitor updates were persisted prior to persising - // the ChannelMonitor update for the forward leg, so attempting to fail the - // backwards leg of the HTLC will simply be rejected. - let logger = WithChannelContext::from( - &args.logger, - &channel.context, - Some(*payment_hash), - ); - log_info!(logger, - "Failing HTLC with hash {} as it is missing in the ChannelMonitor for channel {} but was present in the (stale) ChannelManager", - &channel.context.channel_id(), &payment_hash); - failed_htlcs.push(( - channel_htlc_source.clone(), - *payment_hash, - channel.context.get_counterparty_node_id(), - channel.context.channel_id(), - LocalHTLCFailureReason::ChannelClosed, - None, - )); - } - } + // for (channel_htlc_source, payment_hash) in channel.inflight_htlc_sources() { + // let mut found_htlc = false; + // for (monitor_htlc_source, _) in monitor.get_all_current_outbound_htlcs() { + // if *channel_htlc_source == monitor_htlc_source { + // found_htlc = true; + // break; + // } + // } + // if !found_htlc { + // // If we have some HTLCs in the channel which are not present in the newer + // // ChannelMonitor, they have been removed and should be failed back to + // // ensure we don't forget them entirely. Note that if the missing HTLC(s) + // // were actually claimed we'd have generated and ensured the previous-hop + // // claim update ChannelMonitor updates were persisted prior to persising + // // the ChannelMonitor update for the forward leg, so attempting to fail the + // // backwards leg of the HTLC will simply be rejected. + // let logger = WithChannelContext::from( + // &args.logger, + // &channel.context, + // Some(*payment_hash), + // ); + // log_info!(logger, + // "Failing HTLC with hash {} as it is missing in the ChannelMonitor for channel {} but was present in the (stale) ChannelManager", + // &channel.context.channel_id(), &payment_hash); + // failed_htlcs.push(( + // channel_htlc_source.clone(), + // *payment_hash, + // channel.context.get_counterparty_node_id(), + // channel.context.channel_id(), + // LocalHTLCFailureReason::ChannelClosed, + // None, + // )); + // } + // } } else { channel.on_startup_drop_completed_blocked_mon_updates_through( &logger, @@ -16653,18 +16662,18 @@ where for _ in 0..forward_htlcs_count { let short_channel_id = Readable::read(reader)?; let pending_forwards_count: u64 = Readable::read(reader)?; - let mut pending_forwards = Vec::with_capacity(cmp::min( - pending_forwards_count as usize, - MAX_ALLOC_SIZE / mem::size_of::(), - )); - for _ in 0..pending_forwards_count { - pending_forwards.push(Readable::read(reader)?); - } - forward_htlcs.insert(short_channel_id, pending_forwards); + // let mut pending_forwards = Vec::with_capacity(cmp::min( + // pending_forwards_count as usize, + // MAX_ALLOC_SIZE / mem::size_of::(), + // )); + // for _ in 0..pending_forwards_count { + // pending_forwards.push(Readable::read(reader)?); + // } + // forward_htlcs.insert(short_channel_id, pending_forwards); } let claimable_htlcs_count: u64 = Readable::read(reader)?; - let mut claimable_htlcs_list = + let mut claimable_htlcs_list: Vec<(PaymentHash, Vec)> = Vec::with_capacity(cmp::min(claimable_htlcs_count as usize, 128)); for _ in 0..claimable_htlcs_count { let payment_hash = Readable::read(reader)?; @@ -16676,7 +16685,7 @@ where for _ in 0..previous_hops_len { previous_hops.push(::read(reader)?); } - claimable_htlcs_list.push((payment_hash, previous_hops)); + // claimable_htlcs_list.push((payment_hash, previous_hops)); } let peer_count: u64 = Readable::read(reader)?; @@ -16719,50 +16728,54 @@ where let highest_seen_timestamp: u32 = Readable::read(reader)?; // The last version where a pending inbound payment may have been added was 0.0.116. - let pending_inbound_payment_count: u64 = Readable::read(reader)?; - for _ in 0..pending_inbound_payment_count { - let payment_hash: PaymentHash = Readable::read(reader)?; - let logger = WithContext::from(&args.logger, None, None, Some(payment_hash)); - let inbound: PendingInboundPayment = Readable::read(reader)?; - log_warn!( - logger, - "Ignoring deprecated pending inbound payment with payment hash {}: {:?}", - payment_hash, - inbound - ); - } - - let pending_outbound_payments_count_compat: u64 = Readable::read(reader)?; - let mut pending_outbound_payments_compat: HashMap = - hash_map_with_capacity(cmp::min( - pending_outbound_payments_count_compat as usize, - MAX_ALLOC_SIZE / 32, - )); - for _ in 0..pending_outbound_payments_count_compat { - let session_priv = Readable::read(reader)?; - let payment = PendingOutboundPayment::Legacy { - session_privs: hash_set_from_iter([session_priv]), - }; - if pending_outbound_payments_compat.insert(PaymentId(session_priv), payment).is_some() { - return Err(DecodeError::InvalidValue); - }; - } + // let pending_inbound_payment_count: u64 = Readable::read(reader)?; + // for _ in 0..pending_inbound_payment_count { + // let payment_hash: PaymentHash = Readable::read(reader)?; + // let logger = WithContext::from(&args.logger, None, None, Some(payment_hash)); + // let inbound: PendingInboundPayment = Readable::read(reader)?; + // log_warn!( + // logger, + // "Ignoring deprecated pending inbound payment with payment hash {}: {:?}", + // payment_hash, + // inbound + // ); + // } + + // let pending_outbound_payments_count_compat: u64 = Readable::read(reader)?; + // let mut pending_outbound_payments_compat: HashMap = + // hash_map_with_capacity(cmp::min( + // pending_outbound_payments_count_compat as usize, + // MAX_ALLOC_SIZE / 32, + // )); + // for _ in 0..pending_outbound_payments_count_compat { + // let session_priv = Readable::read(reader)?; + // let payment = PendingOutboundPayment::Legacy { + // session_privs: hash_set_from_iter([session_priv]), + // }; + // if pending_outbound_payments_compat.insert(PaymentId(session_priv), payment).is_some() { + // return Err(DecodeError::InvalidValue); + // }; + // } // pending_outbound_payments_no_retry is for compatibility with 0.0.101 clients. let mut pending_outbound_payments_no_retry: Option>> = None; - let mut pending_outbound_payments = None; + let mut pending_outbound_payments: Option< + hash_map::HashMap, + > = None; let mut pending_intercepted_htlcs: Option> = Some(new_hash_map()); let mut received_network_pubkey: Option = None; let mut fake_scid_rand_bytes: Option<[u8; 32]> = None; let mut probing_cookie_secret: Option<[u8; 32]> = None; - let mut claimable_htlc_purposes = None; - let mut claimable_htlc_onion_fields = None; + let mut claimable_htlc_purposes: Option> = None; + let mut claimable_htlc_onion_fields: Option>> = None; let mut pending_claiming_payments = Some(new_hash_map()); - let mut monitor_update_blocked_actions_per_peer: Option>)>> = - Some(Vec::new()); - let mut events_override = None; + let mut monitor_update_blocked_actions_per_peer: Option< + Vec<(PublicKey, BTreeMap>)>, + > = Some(Vec::new()); + + let mut events_override: Option)>> = None; let mut legacy_in_flight_monitor_updates: Option< HashMap<(PublicKey, OutPoint), Vec>, > = None; @@ -16775,27 +16788,36 @@ where let mut inbound_payment_id_secret = None; let mut peer_storage_dir: Option)>> = None; let mut async_receive_offer_cache: AsyncReceiveOfferCache = AsyncReceiveOfferCache::new(); - read_tlv_fields!(reader, { - (1, pending_outbound_payments_no_retry, option), - (2, pending_intercepted_htlcs, option), - (3, pending_outbound_payments, option), - (4, pending_claiming_payments, option), - (5, received_network_pubkey, option), - (6, monitor_update_blocked_actions_per_peer, option), - (7, fake_scid_rand_bytes, option), - (8, events_override, option), - (9, claimable_htlc_purposes, optional_vec), - (10, legacy_in_flight_monitor_updates, option), - (11, probing_cookie_secret, option), - (13, claimable_htlc_onion_fields, optional_vec), - (14, decode_update_add_htlcs, option), - (15, inbound_payment_id_secret, option), - (17, in_flight_monitor_updates, option), - (19, peer_storage_dir, optional_vec), - (21, async_receive_offer_cache, (default_value, async_receive_offer_cache)), - }); + // read_tlv_fields!(reader, { + // (1, pending_outbound_payments_no_retry, option), + // (2, pending_intercepted_htlcs, option), + // (3, pending_outbound_payments, option), + // (4, pending_claiming_payments, option), + // (5, received_network_pubkey, option), + // (6, monitor_update_blocked_actions_per_peer, option), + // (7, fake_scid_rand_bytes, option), + // (8, events_override, option), + // (9, claimable_htlc_purposes, optional_vec), + // (10, legacy_in_flight_monitor_updates, option), + // (11, probing_cookie_secret, option), + // (13, claimable_htlc_onion_fields, optional_vec), + // (14, decode_update_add_htlcs, option), + // (15, inbound_payment_id_secret, option), + // (17, in_flight_monitor_updates, option), + // (19, peer_storage_dir, optional_vec), + // (21, async_receive_offer_cache, (default_value, async_receive_offer_cache)), + // }); + + // Discard all the rest. + let mut buf = [0u8; 8192]; + loop { + if reader.read(&mut buf)? == 0 { + break; + } + } + let mut decode_update_add_htlcs = decode_update_add_htlcs.unwrap_or_else(|| new_hash_map()); - let peer_storage_dir: Vec<(PublicKey, Vec)> = peer_storage_dir.unwrap_or_else(Vec::new); + // let peer_storage_dir: Vec<(PublicKey, Vec)> = peer_storage_dir.unwrap_or_else(Vec::new); if fake_scid_rand_bytes.is_none() { fake_scid_rand_bytes = Some(args.entropy_source.get_secure_random_bytes()); } @@ -16808,53 +16830,52 @@ where inbound_payment_id_secret = Some(args.entropy_source.get_secure_random_bytes()); } - if let Some(events) = events_override { - pending_events_read = events; - } + // if let Some(events) = events_override { + // pending_events_read = events; + // } - if !channel_closures.is_empty() { - pending_events_read.append(&mut channel_closures); - } + // if !channel_closures.is_empty() { + // pending_events_read.append(&mut channel_closures); + // } - if pending_outbound_payments.is_none() && pending_outbound_payments_no_retry.is_none() { - pending_outbound_payments = Some(pending_outbound_payments_compat); - } else if pending_outbound_payments.is_none() { - let mut outbounds = new_hash_map(); - for (id, session_privs) in pending_outbound_payments_no_retry.unwrap().drain() { - outbounds.insert(id, PendingOutboundPayment::Legacy { session_privs }); - } - pending_outbound_payments = Some(outbounds); - } - let pending_outbounds = - OutboundPayments::new(pending_outbound_payments.unwrap(), args.logger.clone()); + // if pending_outbound_payments.is_none() && pending_outbound_payments_no_retry.is_none() { + // pending_outbound_payments = Some(pending_outbound_payments_compat); + // } else if pending_outbound_payments.is_none() { + // let mut outbounds = new_hash_map(); + // for (id, session_privs) in pending_outbound_payments_no_retry.unwrap().drain() { + // outbounds.insert(id, PendingOutboundPayment::Legacy { session_privs }); + // } + // pending_outbound_payments = Some(outbounds); + // } + let pending_outbounds = OutboundPayments::new(new_hash_map(), args.logger.clone()); - for (peer_pubkey, peer_storage) in peer_storage_dir { - if let Some(peer_state) = per_peer_state.get_mut(&peer_pubkey) { - peer_state.get_mut().unwrap().peer_storage = peer_storage; - } - } + // for (peer_pubkey, peer_storage) in peer_storage_dir { + // if let Some(peer_state) = per_peer_state.get_mut(&peer_pubkey) { + // peer_state.get_mut().unwrap().peer_storage = peer_storage; + // } + // } // Handle transitioning from the legacy TLV to the new one on upgrades. - if let Some(legacy_in_flight_upds) = legacy_in_flight_monitor_updates { - // We should never serialize an empty map. - if legacy_in_flight_upds.is_empty() { - return Err(DecodeError::InvalidValue); - } - if in_flight_monitor_updates.is_none() { - let in_flight_upds = - in_flight_monitor_updates.get_or_insert_with(|| new_hash_map()); - for ((counterparty_node_id, funding_txo), updates) in legacy_in_flight_upds { - // All channels with legacy in flight monitor updates are v1 channels. - let channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); - in_flight_upds.insert((counterparty_node_id, channel_id), updates); - } - } else { - // We should never serialize an empty map. - if in_flight_monitor_updates.as_ref().unwrap().is_empty() { - return Err(DecodeError::InvalidValue); - } - } - } + // if let Some(legacy_in_flight_upds) = legacy_in_flight_monitor_updates { + // // We should never serialize an empty map. + // if legacy_in_flight_upds.is_empty() { + // return Err(DecodeError::InvalidValue); + // } + // if in_flight_monitor_updates.is_none() { + // let in_flight_upds = + // in_flight_monitor_updates.get_or_insert_with(|| new_hash_map()); + // for ((counterparty_node_id, funding_txo), updates) in legacy_in_flight_upds { + // // All channels with legacy in flight monitor updates are v1 channels. + // let channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); + // in_flight_upds.insert((counterparty_node_id, channel_id), updates); + // } + // } else { + // // We should never serialize an empty map. + // if in_flight_monitor_updates.as_ref().unwrap().is_empty() { + // return Err(DecodeError::InvalidValue); + // } + // } + // } // We have to replay (or skip, if they were completed after we wrote the `ChannelManager`) // each `ChannelMonitorUpdate` in `in_flight_monitor_updates`. After doing so, we have to @@ -16868,153 +16889,153 @@ where // // Because the actual handling of the in-flight updates is the same, it's macro'ized here: let mut pending_background_events = Vec::new(); - macro_rules! handle_in_flight_updates { - ($counterparty_node_id: expr, $chan_in_flight_upds: expr, $monitor: expr, - $peer_state: expr, $logger: expr, $channel_info_log: expr - ) => { { - let mut max_in_flight_update_id = 0; - let starting_len = $chan_in_flight_upds.len(); - $chan_in_flight_upds.retain(|upd| upd.update_id > $monitor.get_latest_update_id()); - if $chan_in_flight_upds.len() < starting_len { - log_debug!( - $logger, - "{} ChannelMonitorUpdates completed after ChannelManager was last serialized", - starting_len - $chan_in_flight_upds.len() - ); - } - let funding_txo = $monitor.get_funding_txo(); - for update in $chan_in_flight_upds.iter() { - log_debug!($logger, "Replaying ChannelMonitorUpdate {} for {}channel {}", - update.update_id, $channel_info_log, &$monitor.channel_id()); - max_in_flight_update_id = cmp::max(max_in_flight_update_id, update.update_id); - pending_background_events.push( - BackgroundEvent::MonitorUpdateRegeneratedOnStartup { - counterparty_node_id: $counterparty_node_id, - funding_txo: funding_txo, - channel_id: $monitor.channel_id(), - update: update.clone(), - }); - } - if $chan_in_flight_upds.is_empty() { - // We had some updates to apply, but it turns out they had completed before we - // were serialized, we just weren't notified of that. Thus, we may have to run - // the completion actions for any monitor updates, but otherwise are done. - pending_background_events.push( - BackgroundEvent::MonitorUpdatesComplete { - counterparty_node_id: $counterparty_node_id, - channel_id: $monitor.channel_id(), - }); - } else { - $peer_state.closed_channel_monitor_update_ids.entry($monitor.channel_id()) - .and_modify(|v| *v = cmp::max(max_in_flight_update_id, *v)) - .or_insert(max_in_flight_update_id); - } - if $peer_state.in_flight_monitor_updates.insert($monitor.channel_id(), (funding_txo, $chan_in_flight_upds)).is_some() { - log_error!($logger, "Duplicate in-flight monitor update set for the same channel!"); - return Err(DecodeError::InvalidValue); - } - max_in_flight_update_id - } } - } - - for (counterparty_id, peer_state_mtx) in per_peer_state.iter_mut() { - let mut peer_state_lock = peer_state_mtx.lock().unwrap(); - let peer_state = &mut *peer_state_lock; - for (chan_id, chan) in peer_state.channel_by_id.iter() { - if let Some(funded_chan) = chan.as_funded() { - let logger = WithChannelContext::from(&args.logger, &funded_chan.context, None); - - // Channels that were persisted have to be funded, otherwise they should have been - // discarded. - let monitor = args - .channel_monitors - .get(chan_id) - .expect("We already checked for monitor presence when loading channels"); - let mut max_in_flight_update_id = monitor.get_latest_update_id(); - if let Some(in_flight_upds) = &mut in_flight_monitor_updates { - if let Some(mut chan_in_flight_upds) = - in_flight_upds.remove(&(*counterparty_id, *chan_id)) - { - max_in_flight_update_id = cmp::max( - max_in_flight_update_id, - handle_in_flight_updates!( - *counterparty_id, - chan_in_flight_upds, - monitor, - peer_state, - logger, - "" - ), - ); - } - } - if funded_chan.get_latest_unblocked_monitor_update_id() - > max_in_flight_update_id - { - // If the channel is ahead of the monitor, return DangerousValue: - log_error!(logger, "A ChannelMonitor is stale compared to the current ChannelManager! This indicates a potentially-critical violation of the chain::Watch API!"); - log_error!(logger, " The ChannelMonitor for channel {} is at update_id {} with update_id through {} in-flight", - chan_id, monitor.get_latest_update_id(), max_in_flight_update_id); - log_error!( - logger, - " but the ChannelManager is at update_id {}.", - funded_chan.get_latest_unblocked_monitor_update_id() - ); - log_error!(logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); - log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!"); - log_error!(logger, " Without the latest ChannelMonitor we cannot continue without risking funds."); - log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning"); - return Err(DecodeError::DangerousValue); - } - } else { - // We shouldn't have persisted (or read) any unfunded channel types so none should have been - // created in this `channel_by_id` map. - debug_assert!(false); - return Err(DecodeError::InvalidValue); - } - } - } - - if let Some(in_flight_upds) = in_flight_monitor_updates { - for ((counterparty_id, channel_id), mut chan_in_flight_updates) in in_flight_upds { - let logger = - WithContext::from(&args.logger, Some(counterparty_id), Some(channel_id), None); - if let Some(monitor) = args.channel_monitors.get(&channel_id) { - // Now that we've removed all the in-flight monitor updates for channels that are - // still open, we need to replay any monitor updates that are for closed channels, - // creating the neccessary peer_state entries as we go. - let peer_state_mutex = per_peer_state - .entry(counterparty_id) - .or_insert_with(|| Mutex::new(empty_peer_state())); - let mut peer_state = peer_state_mutex.lock().unwrap(); - handle_in_flight_updates!( - counterparty_id, - chan_in_flight_updates, - monitor, - peer_state, - logger, - "closed " - ); - } else { - log_error!(logger, "A ChannelMonitor is missing even though we have in-flight updates for it! This indicates a potentially-critical violation of the chain::Watch API!"); - log_error!( - logger, - " The ChannelMonitor for channel {} is missing.", - channel_id - ); - log_error!(logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); - log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!"); - log_error!(logger, " Without the latest ChannelMonitor we cannot continue without risking funds."); - log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning"); - log_error!( - logger, - " Pending in-flight updates are: {:?}", - chan_in_flight_updates - ); - return Err(DecodeError::InvalidValue); - } - } - } + // macro_rules! handle_in_flight_updates { + // ($counterparty_node_id: expr, $chan_in_flight_upds: expr, $monitor: expr, + // $peer_state: expr, $logger: expr, $channel_info_log: expr + // ) => { { + // let mut max_in_flight_update_id = 0; + // let starting_len = $chan_in_flight_upds.len(); + // $chan_in_flight_upds.retain(|upd| upd.update_id > $monitor.get_latest_update_id()); + // if $chan_in_flight_upds.len() < starting_len { + // log_debug!( + // $logger, + // "{} ChannelMonitorUpdates completed after ChannelManager was last serialized", + // starting_len - $chan_in_flight_upds.len() + // ); + // } + // let funding_txo = $monitor.get_funding_txo(); + // for update in $chan_in_flight_upds.iter() { + // log_debug!($logger, "Replaying ChannelMonitorUpdate {} for {}channel {}", + // update.update_id, $channel_info_log, &$monitor.channel_id()); + // max_in_flight_update_id = cmp::max(max_in_flight_update_id, update.update_id); + // pending_background_events.push( + // BackgroundEvent::MonitorUpdateRegeneratedOnStartup { + // counterparty_node_id: $counterparty_node_id, + // funding_txo: funding_txo, + // channel_id: $monitor.channel_id(), + // update: update.clone(), + // }); + // } + // if $chan_in_flight_upds.is_empty() { + // // We had some updates to apply, but it turns out they had completed before we + // // were serialized, we just weren't notified of that. Thus, we may have to run + // // the completion actions for any monitor updates, but otherwise are done. + // pending_background_events.push( + // BackgroundEvent::MonitorUpdatesComplete { + // counterparty_node_id: $counterparty_node_id, + // channel_id: $monitor.channel_id(), + // }); + // } else { + // $peer_state.closed_channel_monitor_update_ids.entry($monitor.channel_id()) + // .and_modify(|v| *v = cmp::max(max_in_flight_update_id, *v)) + // .or_insert(max_in_flight_update_id); + // } + // if $peer_state.in_flight_monitor_updates.insert($monitor.channel_id(), (funding_txo, $chan_in_flight_upds)).is_some() { + // log_error!($logger, "Duplicate in-flight monitor update set for the same channel!"); + // return Err(DecodeError::InvalidValue); + // } + // max_in_flight_update_id + // } } + // } + + // for (counterparty_id, peer_state_mtx) in per_peer_state.iter_mut() { + // let mut peer_state_lock = peer_state_mtx.lock().unwrap(); + // let peer_state = &mut *peer_state_lock; + // for (chan_id, chan) in peer_state.channel_by_id.iter() { + // if let Some(funded_chan) = chan.as_funded() { + // let logger = WithChannelContext::from(&args.logger, &funded_chan.context, None); + + // // Channels that were persisted have to be funded, otherwise they should have been + // // discarded. + // let monitor = args + // .channel_monitors + // .get(chan_id) + // .expect("We already checked for monitor presence when loading channels"); + // let mut max_in_flight_update_id = monitor.get_latest_update_id(); + // if let Some(in_flight_upds) = &mut in_flight_monitor_updates { + // if let Some(mut chan_in_flight_upds) = + // in_flight_upds.remove(&(*counterparty_id, *chan_id)) + // { + // max_in_flight_update_id = cmp::max( + // max_in_flight_update_id, + // handle_in_flight_updates!( + // *counterparty_id, + // chan_in_flight_upds, + // monitor, + // peer_state, + // logger, + // "" + // ), + // ); + // } + // } + // if funded_chan.get_latest_unblocked_monitor_update_id() + // > max_in_flight_update_id + // { + // // If the channel is ahead of the monitor, return DangerousValue: + // log_error!(logger, "A ChannelMonitor is stale compared to the current ChannelManager! This indicates a potentially-critical violation of the chain::Watch API!"); + // log_error!(logger, " The ChannelMonitor for channel {} is at update_id {} with update_id through {} in-flight", + // chan_id, monitor.get_latest_update_id(), max_in_flight_update_id); + // log_error!( + // logger, + // " but the ChannelManager is at update_id {}.", + // funded_chan.get_latest_unblocked_monitor_update_id() + // ); + // log_error!(logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); + // log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!"); + // log_error!(logger, " Without the latest ChannelMonitor we cannot continue without risking funds."); + // log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning"); + // return Err(DecodeError::DangerousValue); + // } + // } else { + // // We shouldn't have persisted (or read) any unfunded channel types so none should have been + // // created in this `channel_by_id` map. + // debug_assert!(false); + // return Err(DecodeError::InvalidValue); + // } + // } + // } + + // if let Some(in_flight_upds) = in_flight_monitor_updates { + // for ((counterparty_id, channel_id), mut chan_in_flight_updates) in in_flight_upds { + // let logger = + // WithContext::from(&args.logger, Some(counterparty_id), Some(channel_id), None); + // if let Some(monitor) = args.channel_monitors.get(&channel_id) { + // // Now that we've removed all the in-flight monitor updates for channels that are + // // still open, we need to replay any monitor updates that are for closed channels, + // // creating the neccessary peer_state entries as we go. + // let peer_state_mutex = per_peer_state + // .entry(counterparty_id) + // .or_insert_with(|| Mutex::new(empty_peer_state())); + // let mut peer_state = peer_state_mutex.lock().unwrap(); + // handle_in_flight_updates!( + // counterparty_id, + // chan_in_flight_updates, + // monitor, + // peer_state, + // logger, + // "closed " + // ); + // } else { + // log_error!(logger, "A ChannelMonitor is missing even though we have in-flight updates for it! This indicates a potentially-critical violation of the chain::Watch API!"); + // log_error!( + // logger, + // " The ChannelMonitor for channel {} is missing.", + // channel_id + // ); + // log_error!(logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); + // log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!"); + // log_error!(logger, " Without the latest ChannelMonitor we cannot continue without risking funds."); + // log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning"); + // log_error!( + // logger, + // " Pending in-flight updates are: {:?}", + // chan_in_flight_updates + // ); + // return Err(DecodeError::InvalidValue); + // } + // } + // } // The newly generated `close_background_events` have to be added after any updates that // were already in-flight on shutdown, so we append them here. @@ -17082,432 +17103,432 @@ where pending_background_events.push(new_event); } - // If there's any preimages for forwarded HTLCs hanging around in ChannelMonitors we - // should ensure we try them again on the inbound edge. We put them here and do so after we - // have a fully-constructed `ChannelManager` at the end. - let mut pending_claims_to_replay = Vec::new(); - - { - // If we're tracking pending payments, ensure we haven't lost any by looking at the - // ChannelMonitor data for any channels for which we do not have authorative state - // (i.e. those for which we just force-closed above or we otherwise don't have a - // corresponding `Channel` at all). - // This avoids several edge-cases where we would otherwise "forget" about pending - // payments which are still in-flight via their on-chain state. - // We only rebuild the pending payments map if we were most recently serialized by - // 0.0.102+ - // - // First we rebuild all pending payments, then separately re-claim and re-fail pending - // payments. This avoids edge-cases around MPP payments resulting in redundant actions. - for (channel_id, monitor) in args.channel_monitors.iter() { - let mut is_channel_closed = true; - let counterparty_node_id = monitor.get_counterparty_node_id(); - if let Some(peer_state_mtx) = per_peer_state.get(&counterparty_node_id) { - let mut peer_state_lock = peer_state_mtx.lock().unwrap(); - let peer_state = &mut *peer_state_lock; - is_channel_closed = !peer_state.channel_by_id.contains_key(channel_id); - } - - if is_channel_closed { - for (htlc_source, (htlc, _)) in monitor.get_all_current_outbound_htlcs() { - let logger = WithChannelMonitor::from( - &args.logger, - monitor, - Some(htlc.payment_hash), - ); - if let HTLCSource::OutboundRoute { - payment_id, session_priv, path, .. - } = htlc_source - { - if path.hops.is_empty() { - log_error!(logger, "Got an empty path for a pending payment"); - return Err(DecodeError::InvalidValue); - } - - let mut session_priv_bytes = [0; 32]; - session_priv_bytes[..].copy_from_slice(&session_priv[..]); - pending_outbounds.insert_from_monitor_on_startup( - payment_id, - htlc.payment_hash, - session_priv_bytes, - &path, - best_block_height, - ); - } - } - } - } - for (channel_id, monitor) in args.channel_monitors.iter() { - let mut is_channel_closed = true; - let counterparty_node_id = monitor.get_counterparty_node_id(); - if let Some(peer_state_mtx) = per_peer_state.get(&counterparty_node_id) { - let mut peer_state_lock = peer_state_mtx.lock().unwrap(); - let peer_state = &mut *peer_state_lock; - is_channel_closed = !peer_state.channel_by_id.contains_key(channel_id); - } - - if is_channel_closed { - for (htlc_source, (htlc, preimage_opt)) in - monitor.get_all_current_outbound_htlcs() - { - let logger = WithChannelMonitor::from( - &args.logger, - monitor, - Some(htlc.payment_hash), - ); - let htlc_id = SentHTLCId::from_source(&htlc_source); - match htlc_source { - HTLCSource::PreviousHopData(prev_hop_data) => { - let pending_forward_matches_htlc = |info: &PendingAddHTLCInfo| { - info.prev_funding_outpoint == prev_hop_data.outpoint - && info.prev_htlc_id == prev_hop_data.htlc_id - }; - // The ChannelMonitor is now responsible for this HTLC's - // failure/success and will let us know what its outcome is. If we - // still have an entry for this HTLC in `forward_htlcs` or - // `pending_intercepted_htlcs`, we were apparently not persisted after - // the monitor was when forwarding the payment. - decode_update_add_htlcs.retain(|src_outb_alias, update_add_htlcs| { - update_add_htlcs.retain(|update_add_htlc| { - let matches = *src_outb_alias == prev_hop_data.prev_outbound_scid_alias && - update_add_htlc.htlc_id == prev_hop_data.htlc_id; - if matches { - log_info!(logger, "Removing pending to-decode HTLC with hash {} as it was forwarded to the closed channel {}", - &htlc.payment_hash, &monitor.channel_id()); - } - !matches - }); - !update_add_htlcs.is_empty() - }); - forward_htlcs.retain(|_, forwards| { - forwards.retain(|forward| { - if let HTLCForwardInfo::AddHTLC(htlc_info) = forward { - if pending_forward_matches_htlc(&htlc_info) { - log_info!(logger, "Removing pending to-forward HTLC with hash {} as it was forwarded to the closed channel {}", - &htlc.payment_hash, &monitor.channel_id()); - false - } else { true } - } else { true } - }); - !forwards.is_empty() - }); - pending_intercepted_htlcs.as_mut().unwrap().retain(|intercepted_id, htlc_info| { - if pending_forward_matches_htlc(&htlc_info) { - log_info!(logger, "Removing pending intercepted HTLC with hash {} as it was forwarded to the closed channel {}", - &htlc.payment_hash, &monitor.channel_id()); - pending_events_read.retain(|(event, _)| { - if let Event::HTLCIntercepted { intercept_id: ev_id, .. } = event { - intercepted_id != ev_id - } else { true } - }); - false - } else { true } - }); - }, - HTLCSource::OutboundRoute { - payment_id, - session_priv, - path, - bolt12_invoice, - .. - } => { - if let Some(preimage) = preimage_opt { - let pending_events = Mutex::new(pending_events_read); - let update = PaymentCompleteUpdate { - counterparty_node_id: monitor.get_counterparty_node_id(), - channel_funding_outpoint: monitor.get_funding_txo(), - channel_id: monitor.channel_id(), - htlc_id, - }; - let mut compl_action = Some( - EventCompletionAction::ReleasePaymentCompleteChannelMonitorUpdate(update) - ); - pending_outbounds.claim_htlc( - payment_id, - preimage, - bolt12_invoice, - session_priv, - path, - true, - &mut compl_action, - &pending_events, - ); - // If the completion action was not consumed, then there was no - // payment to claim, and we need to tell the `ChannelMonitor` - // we don't need to hear about the HTLC again, at least as long - // as the PaymentSent event isn't still sitting around in our - // event queue. - let have_action = if compl_action.is_some() { - let pending_events = pending_events.lock().unwrap(); - pending_events.iter().any(|(_, act)| *act == compl_action) - } else { - false - }; - if !have_action && compl_action.is_some() { - let mut peer_state = per_peer_state - .get(&counterparty_node_id) - .map(|state| state.lock().unwrap()) - .expect("Channels originating a preimage must have peer state"); - let update_id = peer_state - .closed_channel_monitor_update_ids - .get_mut(channel_id) - .expect("Channels originating a preimage must have a monitor"); - *update_id += 1; - - pending_background_events.push(BackgroundEvent::MonitorUpdateRegeneratedOnStartup { - counterparty_node_id: monitor.get_counterparty_node_id(), - funding_txo: monitor.get_funding_txo(), - channel_id: monitor.channel_id(), - update: ChannelMonitorUpdate { - update_id: *update_id, - channel_id: Some(monitor.channel_id()), - updates: vec![ChannelMonitorUpdateStep::ReleasePaymentComplete { - htlc: htlc_id, - }], - }, - }); - } - pending_events_read = pending_events.into_inner().unwrap(); - } - }, - } - } - for (htlc_source, payment_hash) in monitor.get_onchain_failed_outbound_htlcs() { - log_info!( - args.logger, - "Failing HTLC with payment hash {} as it was resolved on-chain.", - payment_hash - ); - let completion_action = Some(PaymentCompleteUpdate { - counterparty_node_id: monitor.get_counterparty_node_id(), - channel_funding_outpoint: monitor.get_funding_txo(), - channel_id: monitor.channel_id(), - htlc_id: SentHTLCId::from_source(&htlc_source), - }); - - failed_htlcs.push(( - htlc_source, - payment_hash, - monitor.get_counterparty_node_id(), - monitor.channel_id(), - LocalHTLCFailureReason::OnChainTimeout, - completion_action, - )); - } - } - - // Whether the downstream channel was closed or not, try to re-apply any payment - // preimages from it which may be needed in upstream channels for forwarded - // payments. - let mut fail_read = false; - let outbound_claimed_htlcs_iter = monitor.get_all_current_outbound_htlcs() - .into_iter() - .filter_map(|(htlc_source, (htlc, preimage_opt))| { - if let HTLCSource::PreviousHopData(prev_hop) = &htlc_source { - if let Some(payment_preimage) = preimage_opt { - let inbound_edge_monitor = args.channel_monitors.get(&prev_hop.channel_id); - // Note that for channels which have gone to chain, - // `get_all_current_outbound_htlcs` is never pruned and always returns - // a constant set until the monitor is removed/archived. Thus, we - // want to skip replaying claims that have definitely been resolved - // on-chain. - - // If the inbound monitor is not present, we assume it was fully - // resolved and properly archived, implying this payment had plenty - // of time to get claimed and we can safely skip any further - // attempts to claim it (they wouldn't succeed anyway as we don't - // have a monitor against which to do so). - let inbound_edge_monitor = if let Some(monitor) = inbound_edge_monitor { - monitor - } else { - return None; - }; - // Second, if the inbound edge of the payment's monitor has been - // fully claimed we've had at least `ANTI_REORG_DELAY` blocks to - // get any PaymentForwarded event(s) to the user and assume that - // there's no need to try to replay the claim just for that. - let inbound_edge_balances = inbound_edge_monitor.get_claimable_balances(); - if inbound_edge_balances.is_empty() { - return None; - } - - if prev_hop.counterparty_node_id.is_none() { - // We no longer support claiming an HTLC where we don't have - // the counterparty_node_id available if the claim has to go to - // a closed channel. Its possible we can get away with it if - // the channel is not yet closed, but its by no means a - // guarantee. - - // Thus, in this case we are a bit more aggressive with our - // pruning - if we have no use for the claim (because the - // inbound edge of the payment's monitor has already claimed - // the HTLC) we skip trying to replay the claim. - let htlc_payment_hash: PaymentHash = payment_preimage.into(); - let balance_could_incl_htlc = |bal| match bal { - &Balance::ClaimableOnChannelClose { .. } => { - // The channel is still open, assume we can still - // claim against it - true - }, - &Balance::MaybePreimageClaimableHTLC { payment_hash, .. } => { - payment_hash == htlc_payment_hash - }, - _ => false, - }; - let htlc_may_be_in_balances = - inbound_edge_balances.iter().any(balance_could_incl_htlc); - if !htlc_may_be_in_balances { - return None; - } - - // First check if we're absolutely going to fail - if we need - // to replay this claim to get the preimage into the inbound - // edge monitor but the channel is closed (and thus we'll - // immediately panic if we call claim_funds_from_hop). - if short_to_chan_info.get(&prev_hop.prev_outbound_scid_alias).is_none() { - log_error!(args.logger, - "We need to replay the HTLC claim for payment_hash {} (preimage {}) but cannot do so as the HTLC was forwarded prior to LDK 0.0.124.\ - All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1", - htlc_payment_hash, - payment_preimage, - ); - fail_read = true; - } - - // At this point we're confident we need the claim, but the - // inbound edge channel is still live. As long as this remains - // the case, we can conceivably proceed, but we run some risk - // of panicking at runtime. The user ideally should have read - // the release notes and we wouldn't be here, but we go ahead - // and let things run in the hope that it'll all just work out. - log_error!(args.logger, - "We need to replay the HTLC claim for payment_hash {} (preimage {}) but don't have all the required information to do so reliably.\ - As long as the channel for the inbound edge of the forward remains open, this may work okay, but we may panic at runtime!\ - All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1\ - Continuing anyway, though panics may occur!", - htlc_payment_hash, - payment_preimage, - ); - } - - Some((htlc_source, payment_preimage, htlc.amount_msat, - is_channel_closed, monitor.get_counterparty_node_id(), - monitor.get_funding_txo(), monitor.channel_id())) - } else { None } - } else { - // If it was an outbound payment, we've handled it above - if a preimage - // came in and we persisted the `ChannelManager` we either handled it and - // are good to go or the channel force-closed - we don't have to handle the - // channel still live case here. - None - } - }); - for tuple in outbound_claimed_htlcs_iter { - pending_claims_to_replay.push(tuple); - } - if fail_read { - return Err(DecodeError::InvalidValue); - } - } - } + // // If there's any preimages for forwarded HTLCs hanging around in ChannelMonitors we + // // should ensure we try them again on the inbound edge. We put them here and do so after we + // // have a fully-constructed `ChannelManager` at the end. + // let mut pending_claims_to_replay = Vec::new(); + + // { + // // If we're tracking pending payments, ensure we haven't lost any by looking at the + // // ChannelMonitor data for any channels for which we do not have authorative state + // // (i.e. those for which we just force-closed above or we otherwise don't have a + // // corresponding `Channel` at all). + // // This avoids several edge-cases where we would otherwise "forget" about pending + // // payments which are still in-flight via their on-chain state. + // // We only rebuild the pending payments map if we were most recently serialized by + // // 0.0.102+ + // // + // // First we rebuild all pending payments, then separately re-claim and re-fail pending + // // payments. This avoids edge-cases around MPP payments resulting in redundant actions. + // for (channel_id, monitor) in args.channel_monitors.iter() { + // let mut is_channel_closed = true; + // let counterparty_node_id = monitor.get_counterparty_node_id(); + // if let Some(peer_state_mtx) = per_peer_state.get(&counterparty_node_id) { + // let mut peer_state_lock = peer_state_mtx.lock().unwrap(); + // let peer_state = &mut *peer_state_lock; + // is_channel_closed = !peer_state.channel_by_id.contains_key(channel_id); + // } + + // if is_channel_closed { + // for (htlc_source, (htlc, _)) in monitor.get_all_current_outbound_htlcs() { + // let logger = WithChannelMonitor::from( + // &args.logger, + // monitor, + // Some(htlc.payment_hash), + // ); + // if let HTLCSource::OutboundRoute { + // payment_id, session_priv, path, .. + // } = htlc_source + // { + // if path.hops.is_empty() { + // log_error!(logger, "Got an empty path for a pending payment"); + // return Err(DecodeError::InvalidValue); + // } + + // let mut session_priv_bytes = [0; 32]; + // session_priv_bytes[..].copy_from_slice(&session_priv[..]); + // pending_outbounds.insert_from_monitor_on_startup( + // payment_id, + // htlc.payment_hash, + // session_priv_bytes, + // &path, + // best_block_height, + // ); + // } + // } + // } + // } + // for (channel_id, monitor) in args.channel_monitors.iter() { + // let mut is_channel_closed = true; + // let counterparty_node_id = monitor.get_counterparty_node_id(); + // if let Some(peer_state_mtx) = per_peer_state.get(&counterparty_node_id) { + // let mut peer_state_lock = peer_state_mtx.lock().unwrap(); + // let peer_state = &mut *peer_state_lock; + // is_channel_closed = !peer_state.channel_by_id.contains_key(channel_id); + // } + + // if is_channel_closed { + // for (htlc_source, (htlc, preimage_opt)) in + // monitor.get_all_current_outbound_htlcs() + // { + // let logger = WithChannelMonitor::from( + // &args.logger, + // monitor, + // Some(htlc.payment_hash), + // ); + // let htlc_id = SentHTLCId::from_source(&htlc_source); + // match htlc_source { + // HTLCSource::PreviousHopData(prev_hop_data) => { + // let pending_forward_matches_htlc = |info: &PendingAddHTLCInfo| { + // info.prev_funding_outpoint == prev_hop_data.outpoint + // && info.prev_htlc_id == prev_hop_data.htlc_id + // }; + // // The ChannelMonitor is now responsible for this HTLC's + // // failure/success and will let us know what its outcome is. If we + // // still have an entry for this HTLC in `forward_htlcs` or + // // `pending_intercepted_htlcs`, we were apparently not persisted after + // // the monitor was when forwarding the payment. + // decode_update_add_htlcs.retain(|src_outb_alias, update_add_htlcs| { + // update_add_htlcs.retain(|update_add_htlc| { + // let matches = *src_outb_alias == prev_hop_data.prev_outbound_scid_alias && + // update_add_htlc.htlc_id == prev_hop_data.htlc_id; + // if matches { + // log_info!(logger, "Removing pending to-decode HTLC with hash {} as it was forwarded to the closed channel {}", + // &htlc.payment_hash, &monitor.channel_id()); + // } + // !matches + // }); + // !update_add_htlcs.is_empty() + // }); + // forward_htlcs.retain(|_, forwards| { + // forwards.retain(|forward| { + // if let HTLCForwardInfo::AddHTLC(htlc_info) = forward { + // if pending_forward_matches_htlc(&htlc_info) { + // log_info!(logger, "Removing pending to-forward HTLC with hash {} as it was forwarded to the closed channel {}", + // &htlc.payment_hash, &monitor.channel_id()); + // false + // } else { true } + // } else { true } + // }); + // !forwards.is_empty() + // }); + // pending_intercepted_htlcs.as_mut().unwrap().retain(|intercepted_id, htlc_info| { + // if pending_forward_matches_htlc(&htlc_info) { + // log_info!(logger, "Removing pending intercepted HTLC with hash {} as it was forwarded to the closed channel {}", + // &htlc.payment_hash, &monitor.channel_id()); + // pending_events_read.retain(|(event, _)| { + // if let Event::HTLCIntercepted { intercept_id: ev_id, .. } = event { + // intercepted_id != ev_id + // } else { true } + // }); + // false + // } else { true } + // }); + // }, + // HTLCSource::OutboundRoute { + // payment_id, + // session_priv, + // path, + // bolt12_invoice, + // .. + // } => { + // if let Some(preimage) = preimage_opt { + // let pending_events = Mutex::new(pending_events_read); + // let update = PaymentCompleteUpdate { + // counterparty_node_id: monitor.get_counterparty_node_id(), + // channel_funding_outpoint: monitor.get_funding_txo(), + // channel_id: monitor.channel_id(), + // htlc_id, + // }; + // let mut compl_action = Some( + // EventCompletionAction::ReleasePaymentCompleteChannelMonitorUpdate(update) + // ); + // pending_outbounds.claim_htlc( + // payment_id, + // preimage, + // bolt12_invoice, + // session_priv, + // path, + // true, + // &mut compl_action, + // &pending_events, + // ); + // // If the completion action was not consumed, then there was no + // // payment to claim, and we need to tell the `ChannelMonitor` + // // we don't need to hear about the HTLC again, at least as long + // // as the PaymentSent event isn't still sitting around in our + // // event queue. + // let have_action = if compl_action.is_some() { + // let pending_events = pending_events.lock().unwrap(); + // pending_events.iter().any(|(_, act)| *act == compl_action) + // } else { + // false + // }; + // if !have_action && compl_action.is_some() { + // let mut peer_state = per_peer_state + // .get(&counterparty_node_id) + // .map(|state| state.lock().unwrap()) + // .expect("Channels originating a preimage must have peer state"); + // let update_id = peer_state + // .closed_channel_monitor_update_ids + // .get_mut(channel_id) + // .expect("Channels originating a preimage must have a monitor"); + // *update_id += 1; + + // pending_background_events.push(BackgroundEvent::MonitorUpdateRegeneratedOnStartup { + // counterparty_node_id: monitor.get_counterparty_node_id(), + // funding_txo: monitor.get_funding_txo(), + // channel_id: monitor.channel_id(), + // update: ChannelMonitorUpdate { + // update_id: *update_id, + // channel_id: Some(monitor.channel_id()), + // updates: vec![ChannelMonitorUpdateStep::ReleasePaymentComplete { + // htlc: htlc_id, + // }], + // }, + // }); + // } + // pending_events_read = pending_events.into_inner().unwrap(); + // } + // }, + // } + // } + // for (htlc_source, payment_hash) in monitor.get_onchain_failed_outbound_htlcs() { + // log_info!( + // args.logger, + // "Failing HTLC with payment hash {} as it was resolved on-chain.", + // payment_hash + // ); + // let completion_action = Some(PaymentCompleteUpdate { + // counterparty_node_id: monitor.get_counterparty_node_id(), + // channel_funding_outpoint: monitor.get_funding_txo(), + // channel_id: monitor.channel_id(), + // htlc_id: SentHTLCId::from_source(&htlc_source), + // }); + + // failed_htlcs.push(( + // htlc_source, + // payment_hash, + // monitor.get_counterparty_node_id(), + // monitor.channel_id(), + // LocalHTLCFailureReason::OnChainTimeout, + // completion_action, + // )); + // } + // } + + // // Whether the downstream channel was closed or not, try to re-apply any payment + // // preimages from it which may be needed in upstream channels for forwarded + // // payments. + // let mut fail_read = false; + // let outbound_claimed_htlcs_iter = monitor.get_all_current_outbound_htlcs() + // .into_iter() + // .filter_map(|(htlc_source, (htlc, preimage_opt))| { + // if let HTLCSource::PreviousHopData(prev_hop) = &htlc_source { + // if let Some(payment_preimage) = preimage_opt { + // let inbound_edge_monitor = args.channel_monitors.get(&prev_hop.channel_id); + // // Note that for channels which have gone to chain, + // // `get_all_current_outbound_htlcs` is never pruned and always returns + // // a constant set until the monitor is removed/archived. Thus, we + // // want to skip replaying claims that have definitely been resolved + // // on-chain. + + // // If the inbound monitor is not present, we assume it was fully + // // resolved and properly archived, implying this payment had plenty + // // of time to get claimed and we can safely skip any further + // // attempts to claim it (they wouldn't succeed anyway as we don't + // // have a monitor against which to do so). + // let inbound_edge_monitor = if let Some(monitor) = inbound_edge_monitor { + // monitor + // } else { + // return None; + // }; + // // Second, if the inbound edge of the payment's monitor has been + // // fully claimed we've had at least `ANTI_REORG_DELAY` blocks to + // // get any PaymentForwarded event(s) to the user and assume that + // // there's no need to try to replay the claim just for that. + // let inbound_edge_balances = inbound_edge_monitor.get_claimable_balances(); + // if inbound_edge_balances.is_empty() { + // return None; + // } + + // if prev_hop.counterparty_node_id.is_none() { + // // We no longer support claiming an HTLC where we don't have + // // the counterparty_node_id available if the claim has to go to + // // a closed channel. Its possible we can get away with it if + // // the channel is not yet closed, but its by no means a + // // guarantee. + + // // Thus, in this case we are a bit more aggressive with our + // // pruning - if we have no use for the claim (because the + // // inbound edge of the payment's monitor has already claimed + // // the HTLC) we skip trying to replay the claim. + // let htlc_payment_hash: PaymentHash = payment_preimage.into(); + // let balance_could_incl_htlc = |bal| match bal { + // &Balance::ClaimableOnChannelClose { .. } => { + // // The channel is still open, assume we can still + // // claim against it + // true + // }, + // &Balance::MaybePreimageClaimableHTLC { payment_hash, .. } => { + // payment_hash == htlc_payment_hash + // }, + // _ => false, + // }; + // let htlc_may_be_in_balances = + // inbound_edge_balances.iter().any(balance_could_incl_htlc); + // if !htlc_may_be_in_balances { + // return None; + // } + + // // First check if we're absolutely going to fail - if we need + // // to replay this claim to get the preimage into the inbound + // // edge monitor but the channel is closed (and thus we'll + // // immediately panic if we call claim_funds_from_hop). + // if short_to_chan_info.get(&prev_hop.prev_outbound_scid_alias).is_none() { + // log_error!(args.logger, + // "We need to replay the HTLC claim for payment_hash {} (preimage {}) but cannot do so as the HTLC was forwarded prior to LDK 0.0.124.\ + // All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1", + // htlc_payment_hash, + // payment_preimage, + // ); + // fail_read = true; + // } + + // // At this point we're confident we need the claim, but the + // // inbound edge channel is still live. As long as this remains + // // the case, we can conceivably proceed, but we run some risk + // // of panicking at runtime. The user ideally should have read + // // the release notes and we wouldn't be here, but we go ahead + // // and let things run in the hope that it'll all just work out. + // log_error!(args.logger, + // "We need to replay the HTLC claim for payment_hash {} (preimage {}) but don't have all the required information to do so reliably.\ + // As long as the channel for the inbound edge of the forward remains open, this may work okay, but we may panic at runtime!\ + // All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1\ + // Continuing anyway, though panics may occur!", + // htlc_payment_hash, + // payment_preimage, + // ); + // } + + // Some((htlc_source, payment_preimage, htlc.amount_msat, + // is_channel_closed, monitor.get_counterparty_node_id(), + // monitor.get_funding_txo(), monitor.channel_id())) + // } else { None } + // } else { + // // If it was an outbound payment, we've handled it above - if a preimage + // // came in and we persisted the `ChannelManager` we either handled it and + // // are good to go or the channel force-closed - we don't have to handle the + // // channel still live case here. + // None + // } + // }); + // for tuple in outbound_claimed_htlcs_iter { + // pending_claims_to_replay.push(tuple); + // } + // if fail_read { + // return Err(DecodeError::InvalidValue); + // } + // } + // } let expanded_inbound_key = args.node_signer.get_expanded_key(); let mut claimable_payments = hash_map_with_capacity(claimable_htlcs_list.len()); - if let Some(purposes) = claimable_htlc_purposes { - if purposes.len() != claimable_htlcs_list.len() { - return Err(DecodeError::InvalidValue); - } - if let Some(onion_fields) = claimable_htlc_onion_fields { - if onion_fields.len() != claimable_htlcs_list.len() { - return Err(DecodeError::InvalidValue); - } - for (purpose, (onion, (payment_hash, htlcs))) in purposes - .into_iter() - .zip(onion_fields.into_iter().zip(claimable_htlcs_list.into_iter())) - { - let claimable = ClaimablePayment { purpose, htlcs, onion_fields: onion }; - let existing_payment = claimable_payments.insert(payment_hash, claimable); - if existing_payment.is_some() { - return Err(DecodeError::InvalidValue); - } - } - } else { - for (purpose, (payment_hash, htlcs)) in - purposes.into_iter().zip(claimable_htlcs_list.into_iter()) - { - let claimable = ClaimablePayment { purpose, htlcs, onion_fields: None }; - let existing_payment = claimable_payments.insert(payment_hash, claimable); - if existing_payment.is_some() { - return Err(DecodeError::InvalidValue); - } - } - } - } else { - // LDK versions prior to 0.0.107 did not write a `pending_htlc_purposes`, but do - // include a `_legacy_hop_data` in the `OnionPayload`. - for (payment_hash, htlcs) in claimable_htlcs_list.drain(..) { - if htlcs.is_empty() { - return Err(DecodeError::InvalidValue); - } - let purpose = match &htlcs[0].onion_payload { - OnionPayload::Invoice { _legacy_hop_data } => { - if let Some(hop_data) = _legacy_hop_data { - events::PaymentPurpose::Bolt11InvoicePayment { - payment_preimage: match inbound_payment::verify( - payment_hash, - &hop_data, - 0, - &expanded_inbound_key, - &args.logger, - ) { - Ok((payment_preimage, _)) => payment_preimage, - Err(()) => { - log_error!(args.logger, "Failed to read claimable payment data for HTLC with payment hash {} - was not a pending inbound payment and didn't match our payment key", &payment_hash); - return Err(DecodeError::InvalidValue); - }, - }, - payment_secret: hop_data.payment_secret, - } - } else { - return Err(DecodeError::InvalidValue); - } - }, - OnionPayload::Spontaneous(payment_preimage) => { - events::PaymentPurpose::SpontaneousPayment(*payment_preimage) - }, - }; - claimable_payments - .insert(payment_hash, ClaimablePayment { purpose, htlcs, onion_fields: None }); - } - } - - // Similar to the above cases for forwarded payments, if we have any pending inbound HTLCs - // which haven't yet been claimed, we may be missing counterparty_node_id info and would - // panic if we attempted to claim them at this point. - for (payment_hash, payment) in claimable_payments.iter() { - for htlc in payment.htlcs.iter() { - if htlc.prev_hop.counterparty_node_id.is_some() { - continue; - } - if short_to_chan_info.get(&htlc.prev_hop.prev_outbound_scid_alias).is_some() { - log_error!(args.logger, - "We do not have the required information to claim a pending payment with payment hash {} reliably.\ - As long as the channel for the inbound edge of the forward remains open, this may work okay, but we may panic at runtime!\ - All HTLCs that were received by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1\ - Continuing anyway, though panics may occur!", - payment_hash, - ); - } else { - log_error!(args.logger, - "We do not have the required information to claim a pending payment with payment hash {}.\ - All HTLCs that were received by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1", - payment_hash, - ); - return Err(DecodeError::InvalidValue); - } - } - } + // if let Some(purposes) = claimable_htlc_purposes { + // if purposes.len() != claimable_htlcs_list.len() { + // return Err(DecodeError::InvalidValue); + // } + // if let Some(onion_fields) = claimable_htlc_onion_fields { + // if onion_fields.len() != claimable_htlcs_list.len() { + // return Err(DecodeError::InvalidValue); + // } + // for (purpose, (onion, (payment_hash, htlcs))) in purposes + // .into_iter() + // .zip(onion_fields.into_iter().zip(claimable_htlcs_list.into_iter())) + // { + // let claimable = ClaimablePayment { purpose, htlcs, onion_fields: onion }; + // let existing_payment = claimable_payments.insert(payment_hash, claimable); + // if existing_payment.is_some() { + // return Err(DecodeError::InvalidValue); + // } + // } + // } else { + // for (purpose, (payment_hash, htlcs)) in + // purposes.into_iter().zip(claimable_htlcs_list.into_iter()) + // { + // let claimable = ClaimablePayment { purpose, htlcs, onion_fields: None }; + // let existing_payment = claimable_payments.insert(payment_hash, claimable); + // if existing_payment.is_some() { + // return Err(DecodeError::InvalidValue); + // } + // } + // } + // } else { + // // LDK versions prior to 0.0.107 did not write a `pending_htlc_purposes`, but do + // // include a `_legacy_hop_data` in the `OnionPayload`. + // for (payment_hash, htlcs) in claimable_htlcs_list.drain(..) { + // if htlcs.is_empty() { + // return Err(DecodeError::InvalidValue); + // } + // let purpose = match &htlcs[0].onion_payload { + // OnionPayload::Invoice { _legacy_hop_data } => { + // if let Some(hop_data) = _legacy_hop_data { + // events::PaymentPurpose::Bolt11InvoicePayment { + // payment_preimage: match inbound_payment::verify( + // payment_hash, + // &hop_data, + // 0, + // &expanded_inbound_key, + // &args.logger, + // ) { + // Ok((payment_preimage, _)) => payment_preimage, + // Err(()) => { + // log_error!(args.logger, "Failed to read claimable payment data for HTLC with payment hash {} - was not a pending inbound payment and didn't match our payment key", &payment_hash); + // return Err(DecodeError::InvalidValue); + // }, + // }, + // payment_secret: hop_data.payment_secret, + // } + // } else { + // return Err(DecodeError::InvalidValue); + // } + // }, + // OnionPayload::Spontaneous(payment_preimage) => { + // events::PaymentPurpose::SpontaneousPayment(*payment_preimage) + // }, + // }; + // claimable_payments + // .insert(payment_hash, ClaimablePayment { purpose, htlcs, onion_fields: None }); + // } + // } + + // // Similar to the above cases for forwarded payments, if we have any pending inbound HTLCs + // // which haven't yet been claimed, we may be missing counterparty_node_id info and would + // // panic if we attempted to claim them at this point. + // for (payment_hash, payment) in claimable_payments.iter() { + // for htlc in payment.htlcs.iter() { + // if htlc.prev_hop.counterparty_node_id.is_some() { + // continue; + // } + // if short_to_chan_info.get(&htlc.prev_hop.prev_outbound_scid_alias).is_some() { + // log_error!(args.logger, + // "We do not have the required information to claim a pending payment with payment hash {} reliably.\ + // As long as the channel for the inbound edge of the forward remains open, this may work okay, but we may panic at runtime!\ + // All HTLCs that were received by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1\ + // Continuing anyway, though panics may occur!", + // payment_hash, + // ); + // } else { + // log_error!(args.logger, + // "We do not have the required information to claim a pending payment with payment hash {}.\ + // All HTLCs that were received by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1", + // payment_hash, + // ); + // return Err(DecodeError::InvalidValue); + // } + // } + // } let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&args.entropy_source.get_secure_random_bytes()); @@ -17524,143 +17545,9 @@ where } let mut outbound_scid_aliases = new_hash_set(); - for (_peer_node_id, peer_state_mutex) in per_peer_state.iter_mut() { - let mut peer_state_lock = peer_state_mutex.lock().unwrap(); - let peer_state = &mut *peer_state_lock; - for (chan_id, chan) in peer_state.channel_by_id.iter_mut() { - if let Some(funded_chan) = chan.as_funded_mut() { - let logger = WithChannelContext::from(&args.logger, &funded_chan.context, None); - if funded_chan.context.outbound_scid_alias() == 0 { - let mut outbound_scid_alias; - loop { - outbound_scid_alias = fake_scid::Namespace::OutboundAlias - .get_fake_scid( - best_block_height, - &chain_hash, - fake_scid_rand_bytes.as_ref().unwrap(), - &args.entropy_source, - ); - if outbound_scid_aliases.insert(outbound_scid_alias) { - break; - } - } - funded_chan.context.set_outbound_scid_alias(outbound_scid_alias); - } else if !outbound_scid_aliases - .insert(funded_chan.context.outbound_scid_alias()) - { - // Note that in rare cases its possible to hit this while reading an older - // channel if we just happened to pick a colliding outbound alias above. - log_error!( - logger, - "Got duplicate outbound SCID alias; {}", - funded_chan.context.outbound_scid_alias() - ); - return Err(DecodeError::InvalidValue); - } - if funded_chan.context.is_usable() { - let alias = funded_chan.context.outbound_scid_alias(); - let cp_id = funded_chan.context.get_counterparty_node_id(); - if short_to_chan_info.insert(alias, (cp_id, *chan_id)).is_some() { - // Note that in rare cases its possible to hit this while reading an older - // channel if we just happened to pick a colliding outbound alias above. - log_error!( - logger, - "Got duplicate outbound SCID alias; {}", - funded_chan.context.outbound_scid_alias() - ); - return Err(DecodeError::InvalidValue); - } - } - } else { - // We shouldn't have persisted (or read) any unfunded channel types so none should have been - // created in this `channel_by_id` map. - debug_assert!(false); - return Err(DecodeError::InvalidValue); - } - } - } let bounded_fee_estimator = LowerBoundedFeeEstimator::new(args.fee_estimator); - for (node_id, monitor_update_blocked_actions) in - monitor_update_blocked_actions_per_peer.unwrap() - { - if let Some(peer_state) = per_peer_state.get(&node_id) { - for (channel_id, actions) in monitor_update_blocked_actions.iter() { - let logger = - WithContext::from(&args.logger, Some(node_id), Some(*channel_id), None); - for action in actions.iter() { - if let MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel { - downstream_counterparty_and_funding_outpoint: - Some(EventUnblockedChannel { - counterparty_node_id: blocked_node_id, - funding_txo: _, - channel_id: blocked_channel_id, - blocking_action, - }), - .. - } = action - { - if let Some(blocked_peer_state) = per_peer_state.get(blocked_node_id) { - log_trace!(logger, - "Holding the next revoke_and_ack from {} until the preimage is durably persisted in the inbound edge's ChannelMonitor", - blocked_channel_id); - blocked_peer_state - .lock() - .unwrap() - .actions_blocking_raa_monitor_updates - .entry(*blocked_channel_id) - .or_insert_with(Vec::new) - .push(blocking_action.clone()); - } else { - // If the channel we were blocking has closed, we don't need to - // worry about it - the blocked monitor update should never have - // been released from the `Channel` object so it can't have - // completed, and if the channel closed there's no reason to bother - // anymore. - } - } - if let MonitorUpdateCompletionAction::FreeOtherChannelImmediately { - .. - } = action - { - debug_assert!(false, "Non-event-generating channel freeing should not appear in our queue"); - } - } - // Note that we may have a post-update action for a channel that has no pending - // `ChannelMonitorUpdate`s, but unlike the no-peer-state case, it may simply be - // because we had a `ChannelMonitorUpdate` complete after the last time this - // `ChannelManager` was serialized. In that case, we'll run the post-update - // actions as soon as we get going. - } - peer_state.lock().unwrap().monitor_update_blocked_actions = - monitor_update_blocked_actions; - } else { - for actions in monitor_update_blocked_actions.values() { - for action in actions.iter() { - if matches!(action, MonitorUpdateCompletionAction::PaymentClaimed { .. }) { - // If there are no state for this channel but we have pending - // post-update actions, its possible that one was left over from pre-0.1 - // payment claims where MPP claims led to a channel blocked on itself - // and later `ChannelMonitorUpdate`s didn't get their post-update - // actions run. - // This should only have happened for `PaymentClaimed` post-update actions, - // which we ignore here. - } else { - let logger = WithContext::from(&args.logger, Some(node_id), None, None); - log_error!( - logger, - "Got blocked actions {:?} without a per-peer-state for {}", - monitor_update_blocked_actions, - node_id - ); - return Err(DecodeError::InvalidValue); - } - } - } - } - } - let best_block = BestBlock::new(best_block_hash, best_block_height); let flow = OffersMessageFlow::new( chain_hash, @@ -17739,281 +17626,6 @@ where testing_dnssec_proof_offer_resolution_override: Mutex::new(new_hash_map()), }; - let mut processed_claims: HashSet> = new_hash_set(); - for (_, monitor) in args.channel_monitors.iter() { - for (payment_hash, (payment_preimage, payment_claims)) in monitor.get_stored_preimages() - { - if !payment_claims.is_empty() { - for payment_claim in payment_claims { - if processed_claims.contains(&payment_claim.mpp_parts) { - // We might get the same payment a few times from different channels - // that the MPP payment was received using. There's no point in trying - // to claim the same payment again and again, so we check if the HTLCs - // are the same and skip the payment here. - continue; - } - if payment_claim.mpp_parts.is_empty() { - return Err(DecodeError::InvalidValue); - } - { - let payments = channel_manager.claimable_payments.lock().unwrap(); - if !payments.claimable_payments.contains_key(&payment_hash) { - if let Some(payment) = - payments.pending_claiming_payments.get(&payment_hash) - { - if payment.payment_id - == payment_claim.claiming_payment.payment_id - { - // If this payment already exists and was marked as - // being-claimed then the serialized state must contain all - // of the pending `ChannelMonitorUpdate`s required to get - // the preimage on disk in all MPP parts. Thus we can skip - // the replay below. - continue; - } - } - } - } - - let mut channels_without_preimage = payment_claim - .mpp_parts - .iter() - .map(|htlc_info| (htlc_info.counterparty_node_id, htlc_info.channel_id)) - .collect::>(); - // If we have multiple MPP parts which were received over the same channel, - // we only track it once as once we get a preimage durably in the - // `ChannelMonitor` it will be used for all HTLCs with a matching hash. - channels_without_preimage.sort_unstable(); - channels_without_preimage.dedup(); - let pending_claims = PendingMPPClaim { - channels_without_preimage, - channels_with_preimage: Vec::new(), - }; - let pending_claim_ptr_opt = Some(Arc::new(Mutex::new(pending_claims))); - - // While it may be duplicative to generate a PaymentClaimed here, trying to - // figure out if the user definitely saw it before shutdown would require some - // nontrivial logic and may break as we move away from regularly persisting - // ChannelManager. Instead, we rely on the users' event handler being - // idempotent and just blindly generate one no matter what, letting the - // preimages eventually timing out from ChannelMonitors to prevent us from - // doing so forever. - - let claim_found = channel_manager - .claimable_payments - .lock() - .unwrap() - .begin_claiming_payment( - payment_hash, - &channel_manager.node_signer, - &channel_manager.logger, - &channel_manager.inbound_payment_id_secret, - true, - ); - if claim_found.is_err() { - let mut claimable_payments = - channel_manager.claimable_payments.lock().unwrap(); - match claimable_payments.pending_claiming_payments.entry(payment_hash) { - hash_map::Entry::Occupied(_) => { - debug_assert!( - false, - "Entry was added in begin_claiming_payment" - ); - return Err(DecodeError::InvalidValue); - }, - hash_map::Entry::Vacant(entry) => { - entry.insert(payment_claim.claiming_payment); - }, - } - } - - for part in payment_claim.mpp_parts.iter() { - let pending_mpp_claim = pending_claim_ptr_opt.as_ref().map(|ptr| { - ( - part.counterparty_node_id, - part.channel_id, - PendingMPPClaimPointer(Arc::clone(&ptr)), - ) - }); - let pending_claim_ptr = pending_claim_ptr_opt.as_ref().map(|ptr| { - RAAMonitorUpdateBlockingAction::ClaimedMPPPayment { - pending_claim: PendingMPPClaimPointer(Arc::clone(&ptr)), - } - }); - // Note that we don't need to pass the `payment_info` here - its - // already (clearly) durably on disk in the `ChannelMonitor` so there's - // no need to worry about getting it into others. - // - // We don't encode any attribution data, because the required onion shared secret isn't - // available here. - channel_manager.claim_mpp_part( - part.into(), - payment_preimage, - None, - None, - |_, _| { - ( - Some(MonitorUpdateCompletionAction::PaymentClaimed { - payment_hash, - pending_mpp_claim, - }), - pending_claim_ptr, - ) - }, - ); - } - processed_claims.insert(payment_claim.mpp_parts); - } - } else { - let per_peer_state = channel_manager.per_peer_state.read().unwrap(); - let mut claimable_payments = channel_manager.claimable_payments.lock().unwrap(); - let payment = claimable_payments.claimable_payments.remove(&payment_hash); - mem::drop(claimable_payments); - if let Some(payment) = payment { - log_info!(channel_manager.logger, "Re-claiming HTLCs with payment hash {} as we've released the preimage to a ChannelMonitor!", &payment_hash); - let mut claimable_amt_msat = 0; - let mut receiver_node_id = Some(our_network_pubkey); - let phantom_shared_secret = payment.htlcs[0].prev_hop.phantom_shared_secret; - if phantom_shared_secret.is_some() { - let phantom_pubkey = channel_manager - .node_signer - .get_node_id(Recipient::PhantomNode) - .expect("Failed to get node_id for phantom node recipient"); - receiver_node_id = Some(phantom_pubkey) - } - for claimable_htlc in &payment.htlcs { - claimable_amt_msat += claimable_htlc.value; - - // Add a holding-cell claim of the payment to the Channel, which should be - // applied ~immediately on peer reconnection. Because it won't generate a - // new commitment transaction we can just provide the payment preimage to - // the corresponding ChannelMonitor and nothing else. - // - // We do so directly instead of via the normal ChannelMonitor update - // procedure as the ChainMonitor hasn't yet been initialized, implying - // we're not allowed to call it directly yet. Further, we do the update - // without incrementing the ChannelMonitor update ID as there isn't any - // reason to. - // If we were to generate a new ChannelMonitor update ID here and then - // crash before the user finishes block connect we'd end up force-closing - // this channel as well. On the flip side, there's no harm in restarting - // without the new monitor persisted - we'll end up right back here on - // restart. - let previous_channel_id = claimable_htlc.prev_hop.channel_id; - let peer_node_id = monitor.get_counterparty_node_id(); - { - let peer_state_mutex = per_peer_state.get(&peer_node_id).unwrap(); - let mut peer_state_lock = peer_state_mutex.lock().unwrap(); - let peer_state = &mut *peer_state_lock; - if let Some(channel) = peer_state - .channel_by_id - .get_mut(&previous_channel_id) - .and_then(Channel::as_funded_mut) - { - let logger = WithChannelContext::from( - &channel_manager.logger, - &channel.context, - Some(payment_hash), - ); - channel - .claim_htlc_while_disconnected_dropping_mon_update_legacy( - claimable_htlc.prev_hop.htlc_id, - payment_preimage, - &&logger, - ); - } - } - if let Some(previous_hop_monitor) = - args.channel_monitors.get(&claimable_htlc.prev_hop.channel_id) - { - // Note that this is unsafe as we no longer require the - // `ChannelMonitor`s to be re-persisted prior to this - // `ChannelManager` being persisted after we get started running. - // If this `ChannelManager` gets persisted first then we crash, we - // won't have the `claimable_payments` entry we need to re-enter - // this code block, causing us to not re-apply the preimage to this - // `ChannelMonitor`. - // - // We should never be here with modern payment claims, however, as - // they should always include the HTLC list. Instead, this is only - // for nodes during upgrade, and we explicitly require the old - // persistence semantics on upgrade in the release notes. - previous_hop_monitor.provide_payment_preimage_unsafe_legacy( - &payment_hash, - &payment_preimage, - &channel_manager.tx_broadcaster, - &channel_manager.fee_estimator, - &channel_manager.logger, - ); - } - } - let mut pending_events = channel_manager.pending_events.lock().unwrap(); - let payment_id = - payment.inbound_payment_id(&inbound_payment_id_secret.unwrap()); - let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(); - let sender_intended_total_msat = - payment.htlcs.first().map(|htlc| htlc.total_msat); - pending_events.push_back(( - events::Event::PaymentClaimed { - receiver_node_id, - payment_hash, - purpose: payment.purpose, - amount_msat: claimable_amt_msat, - htlcs, - sender_intended_total_msat, - onion_fields: payment.onion_fields, - payment_id: Some(payment_id), - }, - // Note that we don't bother adding a EventCompletionAction here to - // ensure the `PaymentClaimed` event is durable processed as this - // should only be hit for particularly old channels and we don't have - // enough information to generate such an action. - None, - )); - } - } - } - } - - for htlc_source in failed_htlcs { - let (source, hash, counterparty_id, channel_id, failure_reason, ev_action) = - htlc_source; - let receiver = - HTLCHandlingFailureType::Forward { node_id: Some(counterparty_id), channel_id }; - let reason = HTLCFailReason::from_failure_code(failure_reason); - channel_manager - .fail_htlc_backwards_internal(&source, &hash, &reason, receiver, ev_action); - } - - for ( - source, - preimage, - downstream_value, - downstream_closed, - downstream_node_id, - downstream_funding, - downstream_channel_id, - ) in pending_claims_to_replay - { - // We use `downstream_closed` in place of `from_onchain` here just as a guess - we - // don't remember in the `ChannelMonitor` where we got a preimage from, but if the - // channel is closed we just assume that it probably came from an on-chain claim. - // The same holds for attribution data. We don't have any, so we pass an empty one. - channel_manager.claim_funds_internal( - source, - preimage, - Some(downstream_value), - None, - downstream_closed, - downstream_node_id, - downstream_funding, - downstream_channel_id, - None, - None, - None, - ); - } - //TODO: Broadcast channel update for closed channels, but only after we've made a //connection or two. diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs index 8c9e552fa04..fefffe65528 100644 --- a/lightning/src/ln/reload_tests.rs +++ b/lightning/src/ln/reload_tests.rs @@ -345,6 +345,8 @@ fn test_simple_manager_serialize_deserialize() { let (our_payment_preimage, ..) = route_payment(&nodes[0], &[&nodes[1]], 1000000); let (_, our_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 1000000); + fail_payment(&nodes[0], &[&nodes[1]], our_payment_hash); + claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage); nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id()); @@ -353,8 +355,8 @@ fn test_simple_manager_serialize_deserialize() { reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1])); - fail_payment(&nodes[0], &[&nodes[1]], our_payment_hash); - claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage); + assert_eq!(nodes[0].node.list_channels().len(), 1); + } #[test] @@ -1419,4 +1421,3 @@ fn test_peer_storage() { let res = std::panic::catch_unwind(|| drop(nodes)); assert!(res.is_err()); } - From ac95748e3854acb6b6777d358274ada330cddbf7 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 6 Oct 2025 15:50:02 +0200 Subject: [PATCH 13/16] Simplify node reload logic in tests Store the latest persisted monitor in TestChainMonitor so it no longer needs to be passed into reload. This change also makes the test more realistic. If the monitor wasn't actually persisted, it won't reload properly. --- lightning/src/ln/async_payments_tests.rs | 7 ++--- lightning/src/ln/channelmanager.rs | 5 ++-- lightning/src/ln/functional_test_utils.rs | 32 +++++++++++++++++++++++ lightning/src/ln/monitor_tests.rs | 13 +++------ lightning/src/ln/payment_tests.rs | 5 ++-- lightning/src/ln/reload_tests.rs | 32 +++++++---------------- lightning/src/util/test_utils.rs | 9 +++++++ 7 files changed, 60 insertions(+), 43 deletions(-) diff --git a/lightning/src/ln/async_payments_tests.rs b/lightning/src/ln/async_payments_tests.rs index d56670f4d67..3212f748f6c 100644 --- a/lightning/src/ln/async_payments_tests.rs +++ b/lightning/src/ln/async_payments_tests.rs @@ -2167,8 +2167,7 @@ fn offer_cache_round_trip_ser() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let payee_node_deserialized; let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let chan_id = - create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0).0.channel_id; + create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); let server = &nodes[0]; let recipient = &nodes[1]; @@ -2188,12 +2187,10 @@ fn offer_cache_round_trip_ser() { // offers. let cached_offers_pre_ser = recipient.node.flow.test_get_async_receive_offers(); let config = test_default_channel_config(); - let serialized_monitor = get_monitor!(recipient, chan_id).encode(); - reload_node!( + reload_node_and_monitors!( nodes[1], config, recipient.node.encode(), - &[&serialized_monitor], persister, chain_monitor, payee_node_deserialized diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ea5e11abf4b..c39a1c2f615 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -18760,10 +18760,9 @@ mod tests { *chanmgr_fwd_htlcs = forward_htlcs.clone(); core::mem::drop(chanmgr_fwd_htlcs); - reload_node!(nodes[0], nodes[0].node.encode(), &[], persister, chain_monitor, deserialized_chanmgr); + reload_node_and_monitors!(nodes[0], nodes[0].node.encode(), persister, chain_monitor, deserialized_chanmgr); - let mut deserialized_fwd_htlcs = nodes[0].node.forward_htlcs.lock().unwrap(); - for scid in [scid_1, scid_2].iter() { + let mut deserialized_fwd_htlcs = nodes[0].node.forward_htlcs.lock().unwrap(); for scid in [scid_1, scid_2].iter() { let deserialized_htlcs = deserialized_fwd_htlcs.remove(scid).unwrap(); assert_eq!(forward_htlcs.remove(scid).unwrap(), deserialized_htlcs); } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 6bde99bb59b..767f155472f 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1382,6 +1382,38 @@ macro_rules! reload_node { }; } +#[cfg(test)] +macro_rules! reload_node_and_monitors { + ($node: expr, $new_config: expr, $chanman_encoded: expr, $persister: ident, $new_chain_monitor: ident, $new_channelmanager: ident) => { + let monitors_serialized = { + let monitor_map = $node.chain_monitor.persisted_monitors.lock().unwrap(); + monitor_map.values().cloned().collect::>() + }; + let monitors_serialized_ref: Vec<&[u8]> = + monitors_serialized.iter().map(|v| v.as_slice()).collect(); + + reload_node!( + $node, + $new_config, + $chanman_encoded, + &monitors_serialized_ref, + $persister, + $new_chain_monitor, + $new_channelmanager + ); + }; + ($node: expr, $chanman_encoded: expr, $persister: ident, $new_chain_monitor: ident, $new_channelmanager: ident) => { + reload_node_and_monitors!( + $node, + $crate::util::config::UserConfig::default(), + $chanman_encoded, + $persister, + $new_chain_monitor, + $new_channelmanager + ); + }; +} + pub fn create_funding_transaction<'a, 'b, 'c>( node: &Node<'a, 'b, 'c>, expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128, diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index b316381398e..eec4491709a 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -3244,7 +3244,7 @@ fn test_event_replay_causing_monitor_replay() { let node_deserialized; let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000); let payment_preimage = route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0; @@ -3262,9 +3262,7 @@ fn test_event_replay_causing_monitor_replay() { expect_payment_sent(&nodes[0], payment_preimage, None, true, true /* expected post-event monitor update*/); assert!(nodes[0].node.get_and_clear_needs_persistence()); - let serialized_monitor = get_monitor!(nodes[0], chan.2).encode(); - reload_node!(nodes[0], &serialized_channel_manager, &[&serialized_monitor], persister, new_chain_monitor, node_deserialized); - + reload_node_and_monitors!(nodes[0], &serialized_channel_manager, persister, new_chain_monitor, node_deserialized); // Expect the `PaymentSent` to get replayed, this time without the duplicate monitor update expect_payment_sent(&nodes[0], payment_preimage, None, false, false /* expected post-event monitor update*/); } @@ -3363,7 +3361,7 @@ fn test_claim_event_never_handled() { let init_node_ser = nodes[1].node.encode(); - let chan = create_announced_chan_between_nodes(&nodes, 0, 1); + create_announced_chan_between_nodes(&nodes, 0, 1); // Send the payment we'll ultimately test the PaymentClaimed event for. let (preimage_a, payment_hash_a, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000); @@ -3392,10 +3390,7 @@ fn test_claim_event_never_handled() { // Finally, reload node B with an empty `ChannelManager` and check that we get the // `PaymentClaimed` event. - let chan_0_monitor_serialized = get_monitor!(nodes[1], chan.2).encode(); - let mons = &[&chan_0_monitor_serialized[..]]; - reload_node!(nodes[1], &init_node_ser, mons, persister, new_chain_mon, nodes_1_reload); - + reload_node_and_monitors!(nodes[1], &init_node_ser, persister, new_chain_mon, nodes_1_reload); expect_payment_claimed!(nodes[1], payment_hash_a, 1_000_000); // The reload logic spuriously generates a redundant payment preimage-containing // `ChannelMonitorUpdate`. diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index d2479bbb0e5..25cac0aa789 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -2613,7 +2613,7 @@ fn do_automatic_retries(test: AutoRetry) { let node_b_id = nodes[1].node.get_our_node_id(); let node_c_id = nodes[2].node.get_our_node_id(); - let channel_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1).2; + create_announced_chan_between_nodes(&nodes, 0, 1); let channel_id_2 = create_announced_chan_between_nodes(&nodes, 2, 1).2; // Marshall data to send the payment @@ -2801,8 +2801,7 @@ fn do_automatic_retries(test: AutoRetry) { // Restart the node and ensure that ChannelManager does not use its remaining retry attempt let node_encoded = nodes[0].node.encode(); - let mon_ser = get_monitor!(nodes[0], channel_id_1).encode(); - reload_node!(nodes[0], node_encoded, &[&mon_ser], persister, chain_monitor, node_a_reload); + reload_node_and_monitors!(nodes[0], node_encoded, persister, chain_monitor, node_a_reload); nodes[0].node.process_pending_htlc_forwards(); // Make sure we don't retry again. diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs index fefffe65528..db2b297ac2a 100644 --- a/lightning/src/ln/reload_tests.rs +++ b/lightning/src/ln/reload_tests.rs @@ -93,11 +93,9 @@ fn test_funding_peer_disconnect() { nodes[1].node.handle_channel_reestablish(nodes[0].node.get_our_node_id(), &as_reestablish); let events_4 = nodes[1].node.get_and_clear_pending_msg_events(); assert_eq!(events_4.len(), 3); - let chan_id; let bs_channel_ready = match events_4[0] { MessageSendEvent::SendChannelReady { ref node_id, ref msg } => { assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - chan_id = msg.channel_id; msg.clone() }, _ => panic!("Unexpected event {:?}", events_4[0]), @@ -183,9 +181,7 @@ fn test_funding_peer_disconnect() { // channel_announcement from the cached signatures. nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id()); - let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode(); - - reload_node!(nodes[0], &nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized); + reload_node_and_monitors!(nodes[0], &nodes[0].node.encode(), persister, new_chain_monitor, nodes_0_deserialized); reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1])); } @@ -205,10 +201,7 @@ fn test_no_txn_manager_serialize_deserialize() { nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id()); - let chan_0_monitor_serialized = - get_monitor!(nodes[0], ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.compute_txid(), index: 0 })).encode(); - reload_node!(nodes[0], nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized); - + reload_node_and_monitors!(nodes[0], nodes[0].node.encode(), persister, new_chain_monitor, nodes_0_deserialized); nodes[0].node.peer_connected(nodes[1].node.get_our_node_id(), &msgs::Init { features: nodes[1].node.init_features(), networks: None, remote_network_address: None }, true).unwrap(); @@ -291,11 +284,9 @@ fn test_manager_serialize_deserialize_events() { nodes.push(node_b); // Start the de/seriailization process mid-channel creation to check that the channel manager will hold onto events that are serialized - let chan_0_monitor_serialized = get_monitor!(nodes[0], bs_funding_signed.channel_id).encode(); - reload_node!(nodes[0], nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized); + reload_node_and_monitors!(nodes[0], nodes[0].node.encode(), persister, new_chain_monitor, nodes_0_deserialized); nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id()); - // After deserializing, make sure the funding_transaction is still held by the channel manager let events_4 = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events_4.len(), 0); @@ -341,7 +332,7 @@ fn test_simple_manager_serialize_deserialize() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes_0_deserialized; let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2; + create_announced_chan_between_nodes(&nodes, 0, 1); let (our_payment_preimage, ..) = route_payment(&nodes[0], &[&nodes[1]], 1000000); let (_, our_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 1000000); @@ -350,8 +341,7 @@ fn test_simple_manager_serialize_deserialize() { nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id()); - let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode(); - reload_node!(nodes[0], nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized); + reload_node_and_monitors!(nodes[0], nodes[0].node.encode(), persister, new_chain_monitor, nodes_0_deserialized); reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1])); @@ -1114,7 +1104,7 @@ fn removed_payment_no_manager_persistence() { let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); - let chan_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1).2; + create_announced_chan_between_nodes(&nodes, 0, 1); let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2).2; let (_, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000); @@ -1137,9 +1127,7 @@ fn removed_payment_no_manager_persistence() { _ => panic!("Unexpected event"), } - let chan_0_monitor_serialized = get_monitor!(nodes[1], chan_id_1).encode(); - let chan_1_monitor_serialized = get_monitor!(nodes[1], chan_id_2).encode(); - reload_node!(nodes[1], node_encoded, &[&chan_0_monitor_serialized, &chan_1_monitor_serialized], persister, new_chain_monitor, nodes_1_deserialized); + reload_node_and_monitors!(nodes[1], node_encoded, persister, new_chain_monitor, nodes_1_deserialized); match nodes[1].node.pop_pending_event().unwrap() { Event::ChannelClosed { ref reason, .. } => { @@ -1208,8 +1196,7 @@ fn test_reload_partial_funding_batch() { // Reload the node while a subset of the channels in the funding batch have persisted monitors. let channel_id_1 = ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.compute_txid(), index: 0 }); let node_encoded = nodes[0].node.encode(); - let channel_monitor_1_serialized = get_monitor!(nodes[0], channel_id_1).encode(); - reload_node!(nodes[0], node_encoded, &[&channel_monitor_1_serialized], new_persister, new_chain_monitor, new_channel_manager); + reload_node_and_monitors!(nodes[0], node_encoded, new_persister, new_chain_monitor, new_channel_manager); // Process monitor events. assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); @@ -1285,8 +1272,7 @@ fn test_htlc_localremoved_persistence() { nodes[0].node.peer_disconnected(nodes[1].node.get_our_node_id()); nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id()); - let monitor_encoded = get_monitor!(nodes[1], _chan.3).encode(); - reload_node!(nodes[1], nodes[1].node.encode(), &[&monitor_encoded], persister, chain_monitor, deserialized_chanmgr); + reload_node_and_monitors!(nodes[1], nodes[1].node.encode(), persister, chain_monitor, deserialized_chanmgr); nodes[0].node.peer_connected(nodes[1].node.get_our_node_id(), &msgs::Init { features: nodes[1].node.init_features(), networks: None, remote_network_address: None diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 5641b454c28..eb98b558d80 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -486,6 +486,8 @@ pub struct TestChainMonitor<'a> { pub expect_monitor_round_trip_fail: Mutex>, #[cfg(feature = "std")] pub write_blocker: Mutex>>, + /// The latest persisted monitor for each channel. + pub persisted_monitors: Mutex>>, } impl<'a> TestChainMonitor<'a> { pub fn new( @@ -511,6 +513,7 @@ impl<'a> TestChainMonitor<'a> { expect_monitor_round_trip_fail: Mutex::new(None), #[cfg(feature = "std")] write_blocker: Mutex::new(None), + persisted_monitors: Mutex::new(new_hash_map()), } } @@ -564,6 +567,9 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { // monitor to a serialized copy and get he same one back. let mut w = TestVecWriter(Vec::new()); monitor.write(&mut w).unwrap(); + + self.persisted_monitors.lock().unwrap().insert(channel_id, w.0.clone()); + let new_monitor = <(BlockHash, ChannelMonitor)>::read( &mut io::Cursor::new(&w.0), (self.keys_manager, self.keys_manager), @@ -620,6 +626,9 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { let monitor = self.chain_monitor.get_monitor(channel_id).unwrap(); w.0.clear(); monitor.write(&mut w).unwrap(); + + self.persisted_monitors.lock().unwrap().insert(channel_id, w.0.clone()); + let new_monitor = <(BlockHash, ChannelMonitor)>::read( &mut io::Cursor::new(&w.0), (self.keys_manager, self.keys_manager), From bf1c3b038ccfabf2ab9cdb71d33e47cf9a0b1ba0 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 7 Oct 2025 10:58:45 +0200 Subject: [PATCH 14/16] don't reload chans in chanmgr --- lightning/src/ln/channelmanager.rs | 379 +++++++++++++++-------------- 1 file changed, 190 insertions(+), 189 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c39a1c2f615..be0f6b43e12 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -16391,7 +16391,8 @@ where // )> = Vec::new(); let channel_count: u64 = Readable::read(reader)?; - let mut channel_id_set = hash_set_with_capacity(cmp::min(channel_count as usize, 128)); + let mut channel_id_set: hashbrown::HashSet = + hash_set_with_capacity(cmp::min(channel_count as usize, 128)); let mut per_peer_state = hash_map_with_capacity(cmp::min( channel_count as usize, MAX_ALLOC_SIZE / mem::size_of::<(PublicKey, Mutex>)>(), @@ -16409,194 +16410,194 @@ where &provided_channel_type_features(&args.config), ), )?; - let logger = WithChannelContext::from(&args.logger, &channel.context, None); - let channel_id = channel.context.channel_id(); - channel_id_set.insert(channel_id); - if let Some(ref mut monitor) = args.channel_monitors.get_mut(&channel_id) { - if channel.get_cur_holder_commitment_transaction_number() - > monitor.get_cur_holder_commitment_number() - || channel.get_revoked_counterparty_commitment_transaction_number() - > monitor.get_min_seen_secret() - || channel.get_cur_counterparty_commitment_transaction_number() - > monitor.get_cur_counterparty_commitment_number() - || channel.context.get_latest_monitor_update_id() - < monitor.get_latest_update_id() - { - // But if the channel is behind of the monitor, close the channel: - log_error!( - logger, - "A ChannelManager is stale compared to the current ChannelMonitor!" - ); - log_error!(logger, " The channel will be force-closed and the latest commitment transaction from the ChannelMonitor broadcast."); - if channel.context.get_latest_monitor_update_id() - < monitor.get_latest_update_id() - { - log_error!(logger, " The ChannelMonitor for channel {} is at update_id {} but the ChannelManager is at update_id {}.", - &channel.context.channel_id(), monitor.get_latest_update_id(), channel.context.get_latest_monitor_update_id()); - } - if channel.get_cur_holder_commitment_transaction_number() - > monitor.get_cur_holder_commitment_number() - { - log_error!(logger, " The ChannelMonitor for channel {} is at holder commitment number {} but the ChannelManager is at holder commitment number {}.", - &channel.context.channel_id(), monitor.get_cur_holder_commitment_number(), channel.get_cur_holder_commitment_transaction_number()); - } - if channel.get_revoked_counterparty_commitment_transaction_number() - > monitor.get_min_seen_secret() - { - log_error!(logger, " The ChannelMonitor for channel {} is at revoked counterparty transaction number {} but the ChannelManager is at revoked counterparty transaction number {}.", - &channel.context.channel_id(), monitor.get_min_seen_secret(), channel.get_revoked_counterparty_commitment_transaction_number()); - } - if channel.get_cur_counterparty_commitment_transaction_number() - > monitor.get_cur_counterparty_commitment_number() - { - log_error!(logger, " The ChannelMonitor for channel {} is at counterparty commitment transaction number {} but the ChannelManager is at counterparty commitment transaction number {}.", - &channel.context.channel_id(), monitor.get_cur_counterparty_commitment_number(), channel.get_cur_counterparty_commitment_transaction_number()); - } - let shutdown_result = - channel.force_shutdown(ClosureReason::OutdatedChannelManager); - if shutdown_result.unbroadcasted_batch_funding_txid.is_some() { - return Err(DecodeError::InvalidValue); - } - if let Some((counterparty_node_id, funding_txo, channel_id, mut update)) = - shutdown_result.monitor_update - { - // Our channel information is out of sync with the `ChannelMonitor`, so - // force the update to use the `ChannelMonitor`'s update_id for the close - // update. - let latest_update_id = monitor.get_latest_update_id().saturating_add(1); - update.update_id = latest_update_id; - per_peer_state - .entry(counterparty_node_id) - .or_insert_with(|| Mutex::new(empty_peer_state())) - .lock() - .unwrap() - .closed_channel_monitor_update_ids - .entry(channel_id) - .and_modify(|v| *v = cmp::max(latest_update_id, *v)) - .or_insert(latest_update_id); - - close_background_events.push( - BackgroundEvent::MonitorUpdateRegeneratedOnStartup { - counterparty_node_id, - funding_txo, - channel_id, - update, - }, - ); - } - // for (source, hash, cp_id, chan_id) in shutdown_result.dropped_outbound_htlcs { - // let reason = LocalHTLCFailureReason::ChannelClosed; - // failed_htlcs.push((source, hash, cp_id, chan_id, reason, None)); - // } - channel_closures.push_back(( - events::Event::ChannelClosed { - channel_id: channel.context.channel_id(), - user_channel_id: channel.context.get_user_id(), - reason: ClosureReason::OutdatedChannelManager, - counterparty_node_id: Some(channel.context.get_counterparty_node_id()), - channel_capacity_sats: Some(channel.funding.get_value_satoshis()), - channel_funding_txo: channel.funding.get_funding_txo(), - last_local_balance_msat: Some(channel.funding.get_value_to_self_msat()), - }, - None, - )); - // for (channel_htlc_source, payment_hash) in channel.inflight_htlc_sources() { - // let mut found_htlc = false; - // for (monitor_htlc_source, _) in monitor.get_all_current_outbound_htlcs() { - // if *channel_htlc_source == monitor_htlc_source { - // found_htlc = true; - // break; - // } - // } - // if !found_htlc { - // // If we have some HTLCs in the channel which are not present in the newer - // // ChannelMonitor, they have been removed and should be failed back to - // // ensure we don't forget them entirely. Note that if the missing HTLC(s) - // // were actually claimed we'd have generated and ensured the previous-hop - // // claim update ChannelMonitor updates were persisted prior to persising - // // the ChannelMonitor update for the forward leg, so attempting to fail the - // // backwards leg of the HTLC will simply be rejected. - // let logger = WithChannelContext::from( - // &args.logger, - // &channel.context, - // Some(*payment_hash), - // ); - // log_info!(logger, - // "Failing HTLC with hash {} as it is missing in the ChannelMonitor for channel {} but was present in the (stale) ChannelManager", - // &channel.context.channel_id(), &payment_hash); - // failed_htlcs.push(( - // channel_htlc_source.clone(), - // *payment_hash, - // channel.context.get_counterparty_node_id(), - // channel.context.channel_id(), - // LocalHTLCFailureReason::ChannelClosed, - // None, - // )); - // } - // } - } else { - channel.on_startup_drop_completed_blocked_mon_updates_through( - &logger, - monitor.get_latest_update_id(), - ); - log_info!(logger, "Successfully loaded channel {} at update_id {} against monitor at update id {} with {} blocked updates", - &channel.context.channel_id(), channel.context.get_latest_monitor_update_id(), - monitor.get_latest_update_id(), channel.blocked_monitor_updates_pending()); - if let Some(short_channel_id) = channel.funding.get_short_channel_id() { - short_to_chan_info.insert( - short_channel_id, - ( - channel.context.get_counterparty_node_id(), - channel.context.channel_id(), - ), - ); - } - - for short_channel_id in channel.context.historical_scids() { - let cp_id = channel.context.get_counterparty_node_id(); - let chan_id = channel.context.channel_id(); - short_to_chan_info.insert(*short_channel_id, (cp_id, chan_id)); - } - - per_peer_state - .entry(channel.context.get_counterparty_node_id()) - .or_insert_with(|| Mutex::new(empty_peer_state())) - .get_mut() - .unwrap() - .channel_by_id - .insert(channel.context.channel_id(), Channel::from(channel)); - } - } else if channel.is_awaiting_initial_mon_persist() { - // If we were persisted and shut down while the initial ChannelMonitor persistence - // was in-progress, we never broadcasted the funding transaction and can still - // safely discard the channel. - channel_closures.push_back(( - events::Event::ChannelClosed { - channel_id: channel.context.channel_id(), - user_channel_id: channel.context.get_user_id(), - reason: ClosureReason::DisconnectedPeer, - counterparty_node_id: Some(channel.context.get_counterparty_node_id()), - channel_capacity_sats: Some(channel.funding.get_value_satoshis()), - channel_funding_txo: channel.funding.get_funding_txo(), - last_local_balance_msat: Some(channel.funding.get_value_to_self_msat()), - }, - None, - )); - } else { - log_error!( - logger, - "Missing ChannelMonitor for channel {} needed by ChannelManager.", - &channel.context.channel_id() - ); - log_error!(logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); - log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!"); - log_error!( - logger, - " Without the ChannelMonitor we cannot continue without risking funds." - ); - log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning"); - return Err(DecodeError::InvalidValue); - } + // let logger = WithChannelContext::from(&args.logger, &channel.context, None); + // let channel_id = channel.context.channel_id(); + // channel_id_set.insert(channel_id); + // if let Some(ref mut monitor) = args.channel_monitors.get_mut(&channel_id) { + // if channel.get_cur_holder_commitment_transaction_number() + // > monitor.get_cur_holder_commitment_number() + // || channel.get_revoked_counterparty_commitment_transaction_number() + // > monitor.get_min_seen_secret() + // || channel.get_cur_counterparty_commitment_transaction_number() + // > monitor.get_cur_counterparty_commitment_number() + // || channel.context.get_latest_monitor_update_id() + // < monitor.get_latest_update_id() + // { + // // But if the channel is behind of the monitor, close the channel: + // log_error!( + // logger, + // "A ChannelManager is stale compared to the current ChannelMonitor!" + // ); + // log_error!(logger, " The channel will be force-closed and the latest commitment transaction from the ChannelMonitor broadcast."); + // if channel.context.get_latest_monitor_update_id() + // < monitor.get_latest_update_id() + // { + // log_error!(logger, " The ChannelMonitor for channel {} is at update_id {} but the ChannelManager is at update_id {}.", + // &channel.context.channel_id(), monitor.get_latest_update_id(), channel.context.get_latest_monitor_update_id()); + // } + // if channel.get_cur_holder_commitment_transaction_number() + // > monitor.get_cur_holder_commitment_number() + // { + // log_error!(logger, " The ChannelMonitor for channel {} is at holder commitment number {} but the ChannelManager is at holder commitment number {}.", + // &channel.context.channel_id(), monitor.get_cur_holder_commitment_number(), channel.get_cur_holder_commitment_transaction_number()); + // } + // if channel.get_revoked_counterparty_commitment_transaction_number() + // > monitor.get_min_seen_secret() + // { + // log_error!(logger, " The ChannelMonitor for channel {} is at revoked counterparty transaction number {} but the ChannelManager is at revoked counterparty transaction number {}.", + // &channel.context.channel_id(), monitor.get_min_seen_secret(), channel.get_revoked_counterparty_commitment_transaction_number()); + // } + // if channel.get_cur_counterparty_commitment_transaction_number() + // > monitor.get_cur_counterparty_commitment_number() + // { + // log_error!(logger, " The ChannelMonitor for channel {} is at counterparty commitment transaction number {} but the ChannelManager is at counterparty commitment transaction number {}.", + // &channel.context.channel_id(), monitor.get_cur_counterparty_commitment_number(), channel.get_cur_counterparty_commitment_transaction_number()); + // } + // let shutdown_result = + // channel.force_shutdown(ClosureReason::OutdatedChannelManager); + // if shutdown_result.unbroadcasted_batch_funding_txid.is_some() { + // return Err(DecodeError::InvalidValue); + // } + // if let Some((counterparty_node_id, funding_txo, channel_id, mut update)) = + // shutdown_result.monitor_update + // { + // // Our channel information is out of sync with the `ChannelMonitor`, so + // // force the update to use the `ChannelMonitor`'s update_id for the close + // // update. + // let latest_update_id = monitor.get_latest_update_id().saturating_add(1); + // update.update_id = latest_update_id; + // per_peer_state + // .entry(counterparty_node_id) + // .or_insert_with(|| Mutex::new(empty_peer_state())) + // .lock() + // .unwrap() + // .closed_channel_monitor_update_ids + // .entry(channel_id) + // .and_modify(|v| *v = cmp::max(latest_update_id, *v)) + // .or_insert(latest_update_id); + + // close_background_events.push( + // BackgroundEvent::MonitorUpdateRegeneratedOnStartup { + // counterparty_node_id, + // funding_txo, + // channel_id, + // update, + // }, + // ); + // } + // for (source, hash, cp_id, chan_id) in shutdown_result.dropped_outbound_htlcs { + // let reason = LocalHTLCFailureReason::ChannelClosed; + // failed_htlcs.push((source, hash, cp_id, chan_id, reason, None)); + // } + // channel_closures.push_back(( + // events::Event::ChannelClosed { + // channel_id: channel.context.channel_id(), + // user_channel_id: channel.context.get_user_id(), + // reason: ClosureReason::OutdatedChannelManager, + // counterparty_node_id: Some(channel.context.get_counterparty_node_id()), + // channel_capacity_sats: Some(channel.funding.get_value_satoshis()), + // channel_funding_txo: channel.funding.get_funding_txo(), + // last_local_balance_msat: Some(channel.funding.get_value_to_self_msat()), + // }, + // None, + // )); + // for (channel_htlc_source, payment_hash) in channel.inflight_htlc_sources() { + // let mut found_htlc = false; + // for (monitor_htlc_source, _) in monitor.get_all_current_outbound_htlcs() { + // if *channel_htlc_source == monitor_htlc_source { + // found_htlc = true; + // break; + // } + // } + // if !found_htlc { + // // If we have some HTLCs in the channel which are not present in the newer + // // ChannelMonitor, they have been removed and should be failed back to + // // ensure we don't forget them entirely. Note that if the missing HTLC(s) + // // were actually claimed we'd have generated and ensured the previous-hop + // // claim update ChannelMonitor updates were persisted prior to persising + // // the ChannelMonitor update for the forward leg, so attempting to fail the + // // backwards leg of the HTLC will simply be rejected. + // let logger = WithChannelContext::from( + // &args.logger, + // &channel.context, + // Some(*payment_hash), + // ); + // log_info!(logger, + // "Failing HTLC with hash {} as it is missing in the ChannelMonitor for channel {} but was present in the (stale) ChannelManager", + // &channel.context.channel_id(), &payment_hash); + // failed_htlcs.push(( + // channel_htlc_source.clone(), + // *payment_hash, + // channel.context.get_counterparty_node_id(), + // channel.context.channel_id(), + // LocalHTLCFailureReason::ChannelClosed, + // None, + // )); + // } + // } + // } else { + // channel.on_startup_drop_completed_blocked_mon_updates_through( + // &logger, + // monitor.get_latest_update_id(), + // ); + // log_info!(logger, "Successfully loaded channel {} at update_id {} against monitor at update id {} with {} blocked updates", + // &channel.context.channel_id(), channel.context.get_latest_monitor_update_id(), + // monitor.get_latest_update_id(), channel.blocked_monitor_updates_pending()); + // if let Some(short_channel_id) = channel.funding.get_short_channel_id() { + // short_to_chan_info.insert( + // short_channel_id, + // ( + // channel.context.get_counterparty_node_id(), + // channel.context.channel_id(), + // ), + // ); + // } + + // for short_channel_id in channel.context.historical_scids() { + // let cp_id = channel.context.get_counterparty_node_id(); + // let chan_id = channel.context.channel_id(); + // short_to_chan_info.insert(*short_channel_id, (cp_id, chan_id)); + // } + + // per_peer_state + // .entry(channel.context.get_counterparty_node_id()) + // .or_insert_with(|| Mutex::new(empty_peer_state())) + // .get_mut() + // .unwrap() + // .channel_by_id + // .insert(channel.context.channel_id(), Channel::from(channel)); + // } + // } else if channel.is_awaiting_initial_mon_persist() { + // // If we were persisted and shut down while the initial ChannelMonitor persistence + // // was in-progress, we never broadcasted the funding transaction and can still + // // safely discard the channel. + // channel_closures.push_back(( + // events::Event::ChannelClosed { + // channel_id: channel.context.channel_id(), + // user_channel_id: channel.context.get_user_id(), + // reason: ClosureReason::DisconnectedPeer, + // counterparty_node_id: Some(channel.context.get_counterparty_node_id()), + // channel_capacity_sats: Some(channel.funding.get_value_satoshis()), + // channel_funding_txo: channel.funding.get_funding_txo(), + // last_local_balance_msat: Some(channel.funding.get_value_to_self_msat()), + // }, + // None, + // )); + // } else { + // log_error!( + // logger, + // "Missing ChannelMonitor for channel {} needed by ChannelManager.", + // &channel.context.channel_id() + // ); + // log_error!(logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); + // log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!"); + // log_error!( + // logger, + // " Without the ChannelMonitor we cannot continue without risking funds." + // ); + // log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning"); + // return Err(DecodeError::InvalidValue); + // } } for (channel_id, monitor) in args.channel_monitors.iter() { From 4d11303d2028a3e012268a52e72f9b9ecbda4bba Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 8 Oct 2025 13:36:24 +0200 Subject: [PATCH 15/16] pass in encoded chans --- lightning/src/ln/channelmanager.rs | 6 + lightning/src/ln/functional_test_utils.rs | 207 +++++++++++++--------- lightning/src/ln/reload_tests.rs | 2 + lightning/src/util/test_utils.rs | 11 +- 4 files changed, 137 insertions(+), 89 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index be0f6b43e12..396c26714eb 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -16248,6 +16248,8 @@ pub struct ChannelManagerReadArgs< /// This is not exported to bindings users because we have no HashMap bindings pub channel_monitors: HashMap::EcdsaSigner>>, + + pub funded_channels: HashMap>, } impl< @@ -16281,6 +16283,7 @@ where chain_monitor: M, tx_broadcaster: T, router: R, message_router: MR, logger: L, config: UserConfig, mut channel_monitors: Vec<&'a ChannelMonitor<::EcdsaSigner>>, + mut funded_channels: Vec>, ) -> Self { Self { entropy_source, @@ -16296,6 +16299,9 @@ where channel_monitors: hash_map_from_iter( channel_monitors.drain(..).map(|monitor| (monitor.channel_id(), monitor)), ), + funded_channels: hash_map_from_iter( + funded_channels.drain(..).map(|chan| (chan.context.channel_id(), chan)), + ), } } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 767f155472f..d1aa3ec3686 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -22,9 +22,11 @@ use crate::events::{ PaymentFailureReason, PaymentPurpose, }; use crate::ln::chan_utils::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC}; +use crate::ln::channel::FundedChannel; use crate::ln::channelmanager::{ - AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, PaymentId, - RAACommitmentOrder, RecipientOnionFields, MIN_CLTV_EXPIRY_DELTA, + provided_channel_type_features, AChannelManager, ChainParameters, ChannelManager, + ChannelManagerReadArgs, PaymentId, RAACommitmentOrder, RecipientOnionFields, + MIN_CLTV_EXPIRY_DELTA, }; use crate::ln::funding::FundingTxInput; use crate::ln::msgs; @@ -856,80 +858,80 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> { // Before using all the new monitors to check the watch outpoints, use the full set of // them to ensure we can write and reload our ChannelManager. - { - let mut channel_monitors = new_hash_map(); - for monitor in deserialized_monitors.iter() { - channel_monitors.insert(monitor.channel_id(), monitor); - } - - let scorer = RwLock::new(test_utils::TestScorer::new()); - let mut w = test_utils::TestVecWriter(Vec::new()); - self.node.write(&mut w).unwrap(); - <( - BlockHash, - ChannelManager< - &test_utils::TestChainMonitor, - &test_utils::TestBroadcaster, - &test_utils::TestKeysInterface, - &test_utils::TestKeysInterface, - &test_utils::TestKeysInterface, - &test_utils::TestFeeEstimator, - &test_utils::TestRouter, - &test_utils::TestMessageRouter, - &test_utils::TestLogger, - >, - )>::read( - &mut io::Cursor::new(w.0), - ChannelManagerReadArgs { - config: self.node.get_current_config(), - entropy_source: self.keys_manager, - node_signer: self.keys_manager, - signer_provider: self.keys_manager, - fee_estimator: &test_utils::TestFeeEstimator::new(253), - router: &test_utils::TestRouter::new( - Arc::clone(&network_graph), - &self.logger, - &scorer, - ), - message_router: &test_utils::TestMessageRouter::new_default( - network_graph, - self.keys_manager, - ), - chain_monitor: self.chain_monitor, - tx_broadcaster: &broadcaster, - logger: &self.logger, - channel_monitors, - }, - ) - .unwrap(); - } - - let persister = test_utils::TestPersister::new(); - let chain_source = test_utils::TestChainSource::new(Network::Testnet); - let chain_monitor = test_utils::TestChainMonitor::new( - Some(&chain_source), - &broadcaster, - &self.logger, - &feeest, - &persister, - &self.keys_manager, - ); - for deserialized_monitor in deserialized_monitors.drain(..) { - let channel_id = deserialized_monitor.channel_id(); - if chain_monitor.watch_channel(channel_id, deserialized_monitor) - != Ok(ChannelMonitorUpdateStatus::Completed) - { - panic!(); - } - } - assert_eq!( - *chain_source.watched_txn.unsafe_well_ordered_double_lock_self(), - *self.chain_source.watched_txn.unsafe_well_ordered_double_lock_self() - ); - assert_eq!( - *chain_source.watched_outputs.unsafe_well_ordered_double_lock_self(), - *self.chain_source.watched_outputs.unsafe_well_ordered_double_lock_self() - ); + // { + // let mut channel_monitors = new_hash_map(); + // for monitor in deserialized_monitors.iter() { + // channel_monitors.insert(monitor.channel_id(), monitor); + // } + + // let scorer = RwLock::new(test_utils::TestScorer::new()); + // let mut w = test_utils::TestVecWriter(Vec::new()); + // self.node.write(&mut w).unwrap(); + // <( + // BlockHash, + // ChannelManager< + // &test_utils::TestChainMonitor, + // &test_utils::TestBroadcaster, + // &test_utils::TestKeysInterface, + // &test_utils::TestKeysInterface, + // &test_utils::TestKeysInterface, + // &test_utils::TestFeeEstimator, + // &test_utils::TestRouter, + // &test_utils::TestMessageRouter, + // &test_utils::TestLogger, + // >, + // )>::read( + // &mut io::Cursor::new(w.0), + // ChannelManagerReadArgs { + // config: self.node.get_current_config(), + // entropy_source: self.keys_manager, + // node_signer: self.keys_manager, + // signer_provider: self.keys_manager, + // fee_estimator: &test_utils::TestFeeEstimator::new(253), + // router: &test_utils::TestRouter::new( + // Arc::clone(&network_graph), + // &self.logger, + // &scorer, + // ), + // message_router: &test_utils::TestMessageRouter::new_default( + // network_graph, + // self.keys_manager, + // ), + // chain_monitor: self.chain_monitor, + // tx_broadcaster: &broadcaster, + // logger: &self.logger, + // channel_monitors, + // }, + // ) + // .unwrap(); + // } + + // let persister = test_utils::TestPersister::new(); + // let chain_source = test_utils::TestChainSource::new(Network::Testnet); + // let chain_monitor = test_utils::TestChainMonitor::new( + // Some(&chain_source), + // &broadcaster, + // &self.logger, + // &feeest, + // &persister, + // &self.keys_manager, + // ); + // for deserialized_monitor in deserialized_monitors.drain(..) { + // let channel_id = deserialized_monitor.channel_id(); + // if chain_monitor.watch_channel(channel_id, deserialized_monitor) + // != Ok(ChannelMonitorUpdateStatus::Completed) + // { + // panic!(); + // } + // } + // assert_eq!( + // *chain_source.watched_txn.unsafe_well_ordered_double_lock_self(), + // *self.chain_source.watched_txn.unsafe_well_ordered_double_lock_self() + // ); + // assert_eq!( + // *chain_source.watched_outputs.unsafe_well_ordered_double_lock_self(), + // *self.chain_source.watched_outputs.unsafe_well_ordered_double_lock_self() + // ); } } } @@ -1295,7 +1297,7 @@ fn check_claimed_htlcs_match_route<'a, 'b, 'c>( pub fn _reload_node<'a, 'b, 'c>( node: &'a Node<'a, 'b, 'c>, config: UserConfig, chanman_encoded: &[u8], - monitors_encoded: &[&[u8]], + monitors_encoded: &[&[u8]], channels_encoded: &[&[u8]], ) -> TestChannelManager<'b, 'c> { let mut monitors_read = Vec::with_capacity(monitors_encoded.len()); for encoded in monitors_encoded { @@ -1309,6 +1311,23 @@ pub fn _reload_node<'a, 'b, 'c>( monitors_read.push(monitor); } + let mut channels_read = new_hash_map(); + for encoded in channels_encoded { + let mut channel_read = &encoded[..]; + let mut channel: FundedChannel<&test_utils::TestKeysInterface> = FundedChannel::read( + &mut channel_read, + ( + &node.keys_manager, + &node.keys_manager, + &ChannelTypeFeatures::from_init(&node.node.init_features()), + ), + ) + .unwrap(); + + assert!(channel_read.is_empty()); + channels_read.insert(channel.context.channel_id(), channel); + } + let mut node_read = &chanman_encoded[..]; let (_, node_deserialized) = { let mut channel_monitors = new_hash_map(); @@ -1329,6 +1348,7 @@ pub fn _reload_node<'a, 'b, 'c>( tx_broadcaster: node.tx_broadcaster, logger: node.logger, channel_monitors, + funded_channels: channels_read, }, ) .unwrap() @@ -1364,7 +1384,7 @@ macro_rules! reload_node { $node.chain_monitor = &$new_chain_monitor; $new_channelmanager = - _reload_node(&$node, $new_config, &chanman_encoded, $monitors_encoded); + _reload_node(&$node, $new_config, &chanman_encoded, $monitors_encoded, &[]); $node.node = &$new_channelmanager; $node.onion_messenger.set_offers_handler(&$new_channelmanager); $node.onion_messenger.set_async_payments_handler(&$new_channelmanager); @@ -1390,17 +1410,34 @@ macro_rules! reload_node_and_monitors { monitor_map.values().cloned().collect::>() }; let monitors_serialized_ref: Vec<&[u8]> = - monitors_serialized.iter().map(|v| v.as_slice()).collect(); + monitors_serialized.iter().map(|v| v.0.as_slice()).collect(); - reload_node!( - $node, + let channels_serialized_ref = + monitors_serialized.iter().filter_map(|v| v.1.as_deref()).collect::>(); + + let chanman_encoded = $chanman_encoded; + + $persister = $crate::util::test_utils::TestPersister::new(); + $new_chain_monitor = $crate::util::test_utils::TestChainMonitor::new( + Some($node.chain_source), + $node.tx_broadcaster.clone(), + $node.logger, + $node.fee_estimator, + &$persister, + &$node.keys_manager, + ); + $node.chain_monitor = &$new_chain_monitor; + + $new_channelmanager = _reload_node( + &$node, $new_config, - $chanman_encoded, + &chanman_encoded, &monitors_serialized_ref, - $persister, - $new_chain_monitor, - $new_channelmanager + &channels_serialized_ref, ); + $node.node = &$new_channelmanager; + $node.onion_messenger.set_offers_handler(&$new_channelmanager); + $node.onion_messenger.set_async_payments_handler(&$new_channelmanager); }; ($node: expr, $chanman_encoded: expr, $persister: ident, $new_chain_monitor: ident, $new_channelmanager: ident) => { reload_node_and_monitors!( diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs index db2b297ac2a..0607f24d593 100644 --- a/lightning/src/ln/reload_tests.rs +++ b/lightning/src/ln/reload_tests.rs @@ -429,6 +429,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { tx_broadcaster: nodes[0].tx_broadcaster, logger: &logger, channel_monitors: node_0_stale_monitors.iter().map(|monitor| { (monitor.channel_id(), monitor) }).collect(), + funded_channels: new_hash_map(), }) { } else { panic!("If the monitor(s) are stale, this indicates a bug and we should get an Err return"); }; @@ -447,6 +448,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { tx_broadcaster: nodes[0].tx_broadcaster, logger: &logger, channel_monitors: node_0_monitors.iter().map(|monitor| { (monitor.channel_id(), monitor) }).collect(), + funded_channels: new_hash_map(), }).unwrap(); nodes_0_deserialized = nodes_0_deserialized_tmp; assert!(nodes_0_read.is_empty()); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index eb98b558d80..1a5cad3d36a 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -486,8 +486,8 @@ pub struct TestChainMonitor<'a> { pub expect_monitor_round_trip_fail: Mutex>, #[cfg(feature = "std")] pub write_blocker: Mutex>>, - /// The latest persisted monitor for each channel. - pub persisted_monitors: Mutex>>, + /// The latest persisted monitor and channel for each channel id. + pub persisted_monitors: Mutex, Option>)>>, } impl<'a> TestChainMonitor<'a> { pub fn new( @@ -568,7 +568,7 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { let mut w = TestVecWriter(Vec::new()); monitor.write(&mut w).unwrap(); - self.persisted_monitors.lock().unwrap().insert(channel_id, w.0.clone()); + self.persisted_monitors.lock().unwrap().insert(channel_id, (w.0.clone(), None)); let new_monitor = <(BlockHash, ChannelMonitor)>::read( &mut io::Cursor::new(&w.0), @@ -627,7 +627,10 @@ impl<'a> chain::Watch for TestChainMonitor<'a> { w.0.clear(); monitor.write(&mut w).unwrap(); - self.persisted_monitors.lock().unwrap().insert(channel_id, w.0.clone()); + self.persisted_monitors + .lock() + .unwrap() + .insert(channel_id, (w.0.clone(), encoded_channel.map(|e| e.to_vec()))); let new_monitor = <(BlockHash, ChannelMonitor)>::read( &mut io::Cursor::new(&w.0), From 8cb3c60a4aca651cfb2bed2c4b2630fe6b64383d Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 8 Oct 2025 14:17:57 +0200 Subject: [PATCH 16/16] works --- lightning/src/ln/channelmanager.rs | 410 +++++++++++----------- lightning/src/ln/functional_test_utils.rs | 2 +- 2 files changed, 210 insertions(+), 202 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 396c26714eb..fbaa6d042d6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -16387,16 +16387,23 @@ where is_connected: false, }; - // let mut failed_htlcs: Vec<( - // HTLCSource, - // PaymentHash, - // PublicKey, - // ChannelId, - // LocalHTLCFailureReason, - // Option, - // )> = Vec::new(); - - let channel_count: u64 = Readable::read(reader)?; + let mut failed_htlcs: Vec<( + HTLCSource, + PaymentHash, + PublicKey, + ChannelId, + LocalHTLCFailureReason, + Option, + )> = Vec::new(); + + let legacy_channel_count = Readable::read(reader)?; + + let channel_count = args.funded_channels.len() as u64; + if channel_count != legacy_channel_count { + panic!("The number of funded channels provided ({}) does not match the number of channels in the serialized ChannelManager ({}). You must provide all channels to resume operation.", + channel_count, legacy_channel_count); + } + let mut channel_id_set: hashbrown::HashSet = hash_set_with_capacity(cmp::min(channel_count as usize, 128)); let mut per_peer_state = hash_map_with_capacity(cmp::min( @@ -16407,203 +16414,204 @@ where let mut channel_closures: VecDeque<(Event, Option)> = VecDeque::new(); let mut close_background_events = Vec::new(); - for _ in 0..channel_count { - let mut channel: FundedChannel = FundedChannel::read( + for (_, mut channel) in args.funded_channels.drain() { + _ = FundedChannel::read( reader, ( &args.entropy_source, &args.signer_provider, &provided_channel_type_features(&args.config), ), - )?; - // let logger = WithChannelContext::from(&args.logger, &channel.context, None); - // let channel_id = channel.context.channel_id(); - // channel_id_set.insert(channel_id); - // if let Some(ref mut monitor) = args.channel_monitors.get_mut(&channel_id) { - // if channel.get_cur_holder_commitment_transaction_number() - // > monitor.get_cur_holder_commitment_number() - // || channel.get_revoked_counterparty_commitment_transaction_number() - // > monitor.get_min_seen_secret() - // || channel.get_cur_counterparty_commitment_transaction_number() - // > monitor.get_cur_counterparty_commitment_number() - // || channel.context.get_latest_monitor_update_id() - // < monitor.get_latest_update_id() - // { - // // But if the channel is behind of the monitor, close the channel: - // log_error!( - // logger, - // "A ChannelManager is stale compared to the current ChannelMonitor!" - // ); - // log_error!(logger, " The channel will be force-closed and the latest commitment transaction from the ChannelMonitor broadcast."); - // if channel.context.get_latest_monitor_update_id() - // < monitor.get_latest_update_id() - // { - // log_error!(logger, " The ChannelMonitor for channel {} is at update_id {} but the ChannelManager is at update_id {}.", - // &channel.context.channel_id(), monitor.get_latest_update_id(), channel.context.get_latest_monitor_update_id()); - // } - // if channel.get_cur_holder_commitment_transaction_number() - // > monitor.get_cur_holder_commitment_number() - // { - // log_error!(logger, " The ChannelMonitor for channel {} is at holder commitment number {} but the ChannelManager is at holder commitment number {}.", - // &channel.context.channel_id(), monitor.get_cur_holder_commitment_number(), channel.get_cur_holder_commitment_transaction_number()); - // } - // if channel.get_revoked_counterparty_commitment_transaction_number() - // > monitor.get_min_seen_secret() - // { - // log_error!(logger, " The ChannelMonitor for channel {} is at revoked counterparty transaction number {} but the ChannelManager is at revoked counterparty transaction number {}.", - // &channel.context.channel_id(), monitor.get_min_seen_secret(), channel.get_revoked_counterparty_commitment_transaction_number()); - // } - // if channel.get_cur_counterparty_commitment_transaction_number() - // > monitor.get_cur_counterparty_commitment_number() - // { - // log_error!(logger, " The ChannelMonitor for channel {} is at counterparty commitment transaction number {} but the ChannelManager is at counterparty commitment transaction number {}.", - // &channel.context.channel_id(), monitor.get_cur_counterparty_commitment_number(), channel.get_cur_counterparty_commitment_transaction_number()); - // } - // let shutdown_result = - // channel.force_shutdown(ClosureReason::OutdatedChannelManager); - // if shutdown_result.unbroadcasted_batch_funding_txid.is_some() { - // return Err(DecodeError::InvalidValue); - // } - // if let Some((counterparty_node_id, funding_txo, channel_id, mut update)) = - // shutdown_result.monitor_update - // { - // // Our channel information is out of sync with the `ChannelMonitor`, so - // // force the update to use the `ChannelMonitor`'s update_id for the close - // // update. - // let latest_update_id = monitor.get_latest_update_id().saturating_add(1); - // update.update_id = latest_update_id; - // per_peer_state - // .entry(counterparty_node_id) - // .or_insert_with(|| Mutex::new(empty_peer_state())) - // .lock() - // .unwrap() - // .closed_channel_monitor_update_ids - // .entry(channel_id) - // .and_modify(|v| *v = cmp::max(latest_update_id, *v)) - // .or_insert(latest_update_id); - - // close_background_events.push( - // BackgroundEvent::MonitorUpdateRegeneratedOnStartup { - // counterparty_node_id, - // funding_txo, - // channel_id, - // update, - // }, - // ); - // } - // for (source, hash, cp_id, chan_id) in shutdown_result.dropped_outbound_htlcs { - // let reason = LocalHTLCFailureReason::ChannelClosed; - // failed_htlcs.push((source, hash, cp_id, chan_id, reason, None)); - // } - // channel_closures.push_back(( - // events::Event::ChannelClosed { - // channel_id: channel.context.channel_id(), - // user_channel_id: channel.context.get_user_id(), - // reason: ClosureReason::OutdatedChannelManager, - // counterparty_node_id: Some(channel.context.get_counterparty_node_id()), - // channel_capacity_sats: Some(channel.funding.get_value_satoshis()), - // channel_funding_txo: channel.funding.get_funding_txo(), - // last_local_balance_msat: Some(channel.funding.get_value_to_self_msat()), - // }, - // None, - // )); - // for (channel_htlc_source, payment_hash) in channel.inflight_htlc_sources() { - // let mut found_htlc = false; - // for (monitor_htlc_source, _) in monitor.get_all_current_outbound_htlcs() { - // if *channel_htlc_source == monitor_htlc_source { - // found_htlc = true; - // break; - // } - // } - // if !found_htlc { - // // If we have some HTLCs in the channel which are not present in the newer - // // ChannelMonitor, they have been removed and should be failed back to - // // ensure we don't forget them entirely. Note that if the missing HTLC(s) - // // were actually claimed we'd have generated and ensured the previous-hop - // // claim update ChannelMonitor updates were persisted prior to persising - // // the ChannelMonitor update for the forward leg, so attempting to fail the - // // backwards leg of the HTLC will simply be rejected. - // let logger = WithChannelContext::from( - // &args.logger, - // &channel.context, - // Some(*payment_hash), - // ); - // log_info!(logger, - // "Failing HTLC with hash {} as it is missing in the ChannelMonitor for channel {} but was present in the (stale) ChannelManager", - // &channel.context.channel_id(), &payment_hash); - // failed_htlcs.push(( - // channel_htlc_source.clone(), - // *payment_hash, - // channel.context.get_counterparty_node_id(), - // channel.context.channel_id(), - // LocalHTLCFailureReason::ChannelClosed, - // None, - // )); - // } - // } - // } else { - // channel.on_startup_drop_completed_blocked_mon_updates_through( - // &logger, - // monitor.get_latest_update_id(), - // ); - // log_info!(logger, "Successfully loaded channel {} at update_id {} against monitor at update id {} with {} blocked updates", - // &channel.context.channel_id(), channel.context.get_latest_monitor_update_id(), - // monitor.get_latest_update_id(), channel.blocked_monitor_updates_pending()); - // if let Some(short_channel_id) = channel.funding.get_short_channel_id() { - // short_to_chan_info.insert( - // short_channel_id, - // ( - // channel.context.get_counterparty_node_id(), - // channel.context.channel_id(), - // ), - // ); - // } - - // for short_channel_id in channel.context.historical_scids() { - // let cp_id = channel.context.get_counterparty_node_id(); - // let chan_id = channel.context.channel_id(); - // short_to_chan_info.insert(*short_channel_id, (cp_id, chan_id)); - // } - - // per_peer_state - // .entry(channel.context.get_counterparty_node_id()) - // .or_insert_with(|| Mutex::new(empty_peer_state())) - // .get_mut() - // .unwrap() - // .channel_by_id - // .insert(channel.context.channel_id(), Channel::from(channel)); - // } - // } else if channel.is_awaiting_initial_mon_persist() { - // // If we were persisted and shut down while the initial ChannelMonitor persistence - // // was in-progress, we never broadcasted the funding transaction and can still - // // safely discard the channel. - // channel_closures.push_back(( - // events::Event::ChannelClosed { - // channel_id: channel.context.channel_id(), - // user_channel_id: channel.context.get_user_id(), - // reason: ClosureReason::DisconnectedPeer, - // counterparty_node_id: Some(channel.context.get_counterparty_node_id()), - // channel_capacity_sats: Some(channel.funding.get_value_satoshis()), - // channel_funding_txo: channel.funding.get_funding_txo(), - // last_local_balance_msat: Some(channel.funding.get_value_to_self_msat()), - // }, - // None, - // )); - // } else { - // log_error!( - // logger, - // "Missing ChannelMonitor for channel {} needed by ChannelManager.", - // &channel.context.channel_id() - // ); - // log_error!(logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); - // log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!"); - // log_error!( - // logger, - // " Without the ChannelMonitor we cannot continue without risking funds." - // ); - // log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning"); - // return Err(DecodeError::InvalidValue); - // } + )?; // Legacy channel + + let logger = WithChannelContext::from(&args.logger, &channel.context, None); + let channel_id = channel.context.channel_id(); + channel_id_set.insert(channel_id); + if let Some(ref mut monitor) = args.channel_monitors.get_mut(&channel_id) { + if channel.get_cur_holder_commitment_transaction_number() + > monitor.get_cur_holder_commitment_number() + || channel.get_revoked_counterparty_commitment_transaction_number() + > monitor.get_min_seen_secret() + || channel.get_cur_counterparty_commitment_transaction_number() + > monitor.get_cur_counterparty_commitment_number() + || channel.context.get_latest_monitor_update_id() + < monitor.get_latest_update_id() + { + // But if the channel is behind of the monitor, close the channel: + log_error!( + logger, + "A ChannelManager is stale compared to the current ChannelMonitor!" + ); + log_error!(logger, " The channel will be force-closed and the latest commitment transaction from the ChannelMonitor broadcast."); + if channel.context.get_latest_monitor_update_id() + < monitor.get_latest_update_id() + { + log_error!(logger, " The ChannelMonitor for channel {} is at update_id {} but the ChannelManager is at update_id {}.", + &channel.context.channel_id(), monitor.get_latest_update_id(), channel.context.get_latest_monitor_update_id()); + } + if channel.get_cur_holder_commitment_transaction_number() + > monitor.get_cur_holder_commitment_number() + { + log_error!(logger, " The ChannelMonitor for channel {} is at holder commitment number {} but the ChannelManager is at holder commitment number {}.", + &channel.context.channel_id(), monitor.get_cur_holder_commitment_number(), channel.get_cur_holder_commitment_transaction_number()); + } + if channel.get_revoked_counterparty_commitment_transaction_number() + > monitor.get_min_seen_secret() + { + log_error!(logger, " The ChannelMonitor for channel {} is at revoked counterparty transaction number {} but the ChannelManager is at revoked counterparty transaction number {}.", + &channel.context.channel_id(), monitor.get_min_seen_secret(), channel.get_revoked_counterparty_commitment_transaction_number()); + } + if channel.get_cur_counterparty_commitment_transaction_number() + > monitor.get_cur_counterparty_commitment_number() + { + log_error!(logger, " The ChannelMonitor for channel {} is at counterparty commitment transaction number {} but the ChannelManager is at counterparty commitment transaction number {}.", + &channel.context.channel_id(), monitor.get_cur_counterparty_commitment_number(), channel.get_cur_counterparty_commitment_transaction_number()); + } + let shutdown_result = + channel.force_shutdown(ClosureReason::OutdatedChannelManager); + if shutdown_result.unbroadcasted_batch_funding_txid.is_some() { + return Err(DecodeError::InvalidValue); + } + if let Some((counterparty_node_id, funding_txo, channel_id, mut update)) = + shutdown_result.monitor_update + { + // Our channel information is out of sync with the `ChannelMonitor`, so + // force the update to use the `ChannelMonitor`'s update_id for the close + // update. + let latest_update_id = monitor.get_latest_update_id().saturating_add(1); + update.update_id = latest_update_id; + per_peer_state + .entry(counterparty_node_id) + .or_insert_with(|| Mutex::new(empty_peer_state())) + .lock() + .unwrap() + .closed_channel_monitor_update_ids + .entry(channel_id) + .and_modify(|v| *v = cmp::max(latest_update_id, *v)) + .or_insert(latest_update_id); + + close_background_events.push( + BackgroundEvent::MonitorUpdateRegeneratedOnStartup { + counterparty_node_id, + funding_txo, + channel_id, + update, + }, + ); + } + for (source, hash, cp_id, chan_id) in shutdown_result.dropped_outbound_htlcs { + let reason = LocalHTLCFailureReason::ChannelClosed; + failed_htlcs.push((source, hash, cp_id, chan_id, reason, None)); + } + channel_closures.push_back(( + events::Event::ChannelClosed { + channel_id: channel.context.channel_id(), + user_channel_id: channel.context.get_user_id(), + reason: ClosureReason::OutdatedChannelManager, + counterparty_node_id: Some(channel.context.get_counterparty_node_id()), + channel_capacity_sats: Some(channel.funding.get_value_satoshis()), + channel_funding_txo: channel.funding.get_funding_txo(), + last_local_balance_msat: Some(channel.funding.get_value_to_self_msat()), + }, + None, + )); + for (channel_htlc_source, payment_hash) in channel.inflight_htlc_sources() { + let mut found_htlc = false; + for (monitor_htlc_source, _) in monitor.get_all_current_outbound_htlcs() { + if *channel_htlc_source == monitor_htlc_source { + found_htlc = true; + break; + } + } + if !found_htlc { + // If we have some HTLCs in the channel which are not present in the newer + // ChannelMonitor, they have been removed and should be failed back to + // ensure we don't forget them entirely. Note that if the missing HTLC(s) + // were actually claimed we'd have generated and ensured the previous-hop + // claim update ChannelMonitor updates were persisted prior to persising + // the ChannelMonitor update for the forward leg, so attempting to fail the + // backwards leg of the HTLC will simply be rejected. + let logger = WithChannelContext::from( + &args.logger, + &channel.context, + Some(*payment_hash), + ); + log_info!(logger, + "Failing HTLC with hash {} as it is missing in the ChannelMonitor for channel {} but was present in the (stale) ChannelManager", + &channel.context.channel_id(), &payment_hash); + failed_htlcs.push(( + channel_htlc_source.clone(), + *payment_hash, + channel.context.get_counterparty_node_id(), + channel.context.channel_id(), + LocalHTLCFailureReason::ChannelClosed, + None, + )); + } + } + } else { + channel.on_startup_drop_completed_blocked_mon_updates_through( + &logger, + monitor.get_latest_update_id(), + ); + log_info!(logger, "Successfully loaded channel {} at update_id {} against monitor at update id {} with {} blocked updates", + &channel.context.channel_id(), channel.context.get_latest_monitor_update_id(), + monitor.get_latest_update_id(), channel.blocked_monitor_updates_pending()); + if let Some(short_channel_id) = channel.funding.get_short_channel_id() { + short_to_chan_info.insert( + short_channel_id, + ( + channel.context.get_counterparty_node_id(), + channel.context.channel_id(), + ), + ); + } + + for short_channel_id in channel.context.historical_scids() { + let cp_id = channel.context.get_counterparty_node_id(); + let chan_id = channel.context.channel_id(); + short_to_chan_info.insert(*short_channel_id, (cp_id, chan_id)); + } + + per_peer_state + .entry(channel.context.get_counterparty_node_id()) + .or_insert_with(|| Mutex::new(empty_peer_state())) + .get_mut() + .unwrap() + .channel_by_id + .insert(channel.context.channel_id(), Channel::from(channel)); + } + } else if channel.is_awaiting_initial_mon_persist() { + // If we were persisted and shut down while the initial ChannelMonitor persistence + // was in-progress, we never broadcasted the funding transaction and can still + // safely discard the channel. + channel_closures.push_back(( + events::Event::ChannelClosed { + channel_id: channel.context.channel_id(), + user_channel_id: channel.context.get_user_id(), + reason: ClosureReason::DisconnectedPeer, + counterparty_node_id: Some(channel.context.get_counterparty_node_id()), + channel_capacity_sats: Some(channel.funding.get_value_satoshis()), + channel_funding_txo: channel.funding.get_funding_txo(), + last_local_balance_msat: Some(channel.funding.get_value_to_self_msat()), + }, + None, + )); + } else { + log_error!( + logger, + "Missing ChannelMonitor for channel {} needed by ChannelManager.", + &channel.context.channel_id() + ); + log_error!(logger, " The chain::Watch API *requires* that monitors are persisted durably before returning,"); + log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!"); + log_error!( + logger, + " Without the ChannelMonitor we cannot continue without risking funds." + ); + log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning"); + return Err(DecodeError::InvalidValue); + } } for (channel_id, monitor) in args.channel_monitors.iter() { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index d1aa3ec3686..d12dfcdf707 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1314,7 +1314,7 @@ pub fn _reload_node<'a, 'b, 'c>( let mut channels_read = new_hash_map(); for encoded in channels_encoded { let mut channel_read = &encoded[..]; - let mut channel: FundedChannel<&test_utils::TestKeysInterface> = FundedChannel::read( + let channel: FundedChannel<&test_utils::TestKeysInterface> = FundedChannel::read( &mut channel_read, ( &node.keys_manager,