← Watch feed
Remove handle_new_monitor_update macro
Public commit record
66/100 · Adequate
What the developer wrote
Authored by Joost Jager
Remove handle_new_monitor_update macro
Convert the handle_new_monitor_update macro to methods and update all
call sites. Adds handle_new_monitor_update and
handle_new_monitor_update_with_status methods that return completion
data for processing after locks are released.
Also removes handle_monitor_update_completion macro as it's no longer
needed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
✓ Descriptive subject✓ Provides detailed explanatory context✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit is a straightforward internal code cleanup in the Lightning Dev Kit's channel manager. It replaces a Rust macro used to handle channel monitor updates with two regular methods, and updates all places that called the macro. There is no change to user-facing behavior, protocol logic, or security-sensitive operations; it is purely a refactoring to make the code easier to maintain and to satisfy Rust's borrow checker.
Recommended action
No security action required. Treat as a normal refactoring review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit removes the handle_new_monitor_update! and handle_monitor_update_completion! macros from lightning/src/ln/channelmanager.rs and introduces handle_new_monitor_update and handle_new_monitor_update_with_status methods. The new methods take individual PeerState fields as arguments to avoid borrow-checker conflicts when the channel is borrowed from peer_state.channel_by_id. All call sites are updated to explicitly drop peer_state_lock and per_peer_state before calling handle_post_monitor_update_chan_resume. The functional behavior of tracking in-flight monitor updates, resuming channels, and processing completion actions is preserved.
Changed components
lightning/src/ln/channelmanager.rsInspect captured patch +203 / −87
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 76da381..ce5e88e 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3291,36 +3291,6 @@ macro_rules! emit_initial_channel_ready_event {
};
}
-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
- ) => {{
- let (update_completed, all_updates_complete) = $self.update_channel_monitor(
- &mut $peer_state.in_flight_monitor_updates,
- $chan.context.channel_id(),
- $funding_txo,
- $chan.context.get_counterparty_node_id(),
- $update,
- );
- if all_updates_complete {
- let completion_data = $self.try_resume_channel_post_monitor_update(
- &mut $peer_state.in_flight_monitor_updates,
- &mut $peer_state.monitor_update_blocked_actions,
- &mut $peer_state.pending_msg_events,
- $peer_state.is_connected,
- $chan,
- );
-
- mem::drop($peer_state_lock);
- mem::drop($per_peer_state_lock);
-
- $self.handle_post_monitor_update_chan_resume(completion_data);
- }
- update_completed
- }};
-}
-
fn convert_channel_err_internal<
Close: FnOnce(ClosureReason, &str) -> (ShutdownResult, Option<(msgs::ChannelUpdate, NodeId, NodeId)>),
>(
@@ -3982,15 +3952,19 @@ where
// Update the monitor with the shutdown script if necessary.
if let Some(monitor_update) = monitor_update_opt.take() {
- handle_new_monitor_update!(
- self,
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
funding_txo_opt.unwrap(),
monitor_update,
- peer_state_lock,
- peer_state,
- per_peer_state,
- chan
- );
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
}
} else {
let reason = ClosureReason::LocallyCoopClosedUnfundedChannel;
@@ -4115,8 +4089,19 @@ where
match peer_state.channel_by_id.entry(channel_id) {
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);
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
+ funding_txo,
+ monitor_update,
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
return;
} else {
debug_assert!(false, "We shouldn't have an update for a non-funded channel");
@@ -5260,16 +5245,22 @@ where
);
match break_channel_entry!(self, peer_state, send_res, chan_entry) {
Some(monitor_update) => {
- let ok = handle_new_monitor_update!(
- self,
- funding_txo,
- monitor_update,
- peer_state_lock,
- peer_state,
- per_peer_state,
- chan
- );
- if !ok {
+ let (update_completed, completion_data) = self
+ .handle_new_monitor_update_with_status(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
+ funding_txo,
+ monitor_update,
+ );
+ if let Some(data) = completion_data {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
+ if !update_completed {
// Note that MonitorUpdateInProgress here indicates (per function
// docs) that we will resend the commitment update once monitor
// updating completes. Therefore, we must return an error
@@ -8933,15 +8924,19 @@ where
.or_insert_with(Vec::new)
.push(raa_blocker);
}
- handle_new_monitor_update!(
- self,
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
prev_hop.funding_txo,
monitor_update,
- peer_state_lock,
- peer_state,
- per_peer_state,
- chan
- );
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
},
UpdateFulfillCommitFetch::DuplicateClaim {} => {
let (action_opt, raa_blocker_opt) = completion_action(None, true);
@@ -9728,6 +9723,73 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
}
+ /// Applies a new monitor update and attempts to resume the channel if all updates are complete.
+ ///
+ /// Returns [`PostMonitorUpdateChanResume`] if all in-flight updates are complete, which should
+ /// be passed to [`Self::handle_post_monitor_update_chan_resume`] after releasing locks.
+ ///
+ /// Note: This method takes individual fields from [`PeerState`] rather than the whole struct
+ /// to avoid borrow checker issues when the channel is borrowed from `peer_state.channel_by_id`.
+ fn handle_new_monitor_update(
+ &self,
+ in_flight_monitor_updates: &mut BTreeMap<ChannelId, (OutPoint, Vec<ChannelMonitorUpdate>)>,
+ monitor_update_blocked_actions: &mut BTreeMap<
+ ChannelId,
+ Vec<MonitorUpdateCompletionAction>,
+ >,
+ pending_msg_events: &mut Vec<MessageSendEvent>, is_connected: bool,
+ chan: &mut FundedChannel<SP>, funding_txo: OutPoint, update: ChannelMonitorUpdate,
+ ) -> Option<PostMonitorUpdateChanResume> {
+ self.handle_new_monitor_update_with_status(
+ in_flight_monitor_updates,
+ monitor_update_blocked_actions,
+ pending_msg_events,
+ is_connected,
+ chan,
+ funding_txo,
+ update,
+ )
+ .1
+ }
+
+ /// Like [`Self::handle_new_monitor_update`], but also returns whether this specific update
+ /// completed (as opposed to being in-progress).
+ fn handle_new_monitor_update_with_status(
+ &self,
+ in_flight_monitor_updates: &mut BTreeMap<ChannelId, (OutPoint, Vec<ChannelMonitorUpdate>)>,
+ monitor_update_blocked_actions: &mut BTreeMap<
+ ChannelId,
+ Vec<MonitorUpdateCompletionAction>,
+ >,
+ pending_msg_events: &mut Vec<MessageSendEvent>, is_connected: bool,
+ chan: &mut FundedChannel<SP>, funding_txo: OutPoint, update: ChannelMonitorUpdate,
+ ) -> (bool, Option<PostMonitorUpdateChanResume>) {
+ let chan_id = chan.context.channel_id();
+ let counterparty_node_id = chan.context.get_counterparty_node_id();
+
+ let (update_completed, all_updates_complete) = self.update_channel_monitor(
+ in_flight_monitor_updates,
+ chan_id,
+ funding_txo,
+ counterparty_node_id,
+ update,
+ );
+
+ let completion_data = if all_updates_complete {
+ Some(self.try_resume_channel_post_monitor_update(
+ in_flight_monitor_updates,
+ monitor_update_blocked_actions,
+ pending_msg_events,
+ is_connected,
+ chan,
+ ))
+ } else {
+ None
+ };
+
+ (update_completed, completion_data)
+ }
+
/// Attempts to resume a channel after a monitor update completes, while locks are still held.
///
/// If the channel has no more blocked monitor updates, this resumes normal operation by
@@ -11347,15 +11409,19 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
// Update the monitor with the shutdown script if necessary.
if let Some(monitor_update) = monitor_update_opt {
- handle_new_monitor_update!(
- self,
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
funding_txo_opt.unwrap(),
monitor_update,
- peer_state_lock,
- peer_state,
- per_peer_state,
- chan
- );
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
}
},
None => {
@@ -11668,8 +11734,19 @@ 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,
- peer_state, per_peer_state, chan);
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
+ funding_txo.unwrap(),
+ monitor_update,
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
}
}
Ok(())
@@ -11699,10 +11776,19 @@ 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,
- per_peer_state, chan
- );
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
+ funding_txo.unwrap(),
+ monitor_update,
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
}
}
Ok(())
@@ -11939,8 +12025,19 @@ 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,
- peer_state_lock, peer_state, per_peer_state, chan);
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
+ funding_txo,
+ monitor_update,
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
}
(htlcs_to_fail, static_invoices)
} else {
@@ -12418,15 +12515,19 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
if let Some(monitor_update) = splice_promotion.monitor_update {
- handle_new_monitor_update!(
- self,
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
splice_promotion.funding_txo,
monitor_update,
- peer_state_lock,
- peer_state,
- per_peer_state,
- chan
- );
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
}
}
} else {
@@ -12614,15 +12715,19 @@ 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;
- handle_new_monitor_update!(
- self,
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
funding_txo.unwrap(),
monitor_update,
- peer_state_lock,
- peer_state,
- per_peer_state,
- chan
- );
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
continue 'peer_loop;
}
}
@@ -14051,8 +14156,19 @@ where
if let Some((monitor_update, further_update_exists)) = chan.unblock_next_blocked_monitor_update() {
log_debug!(logger, "Unlocking monitor updating and updating monitor",
);
- handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update,
- peer_state_lck, peer_state, per_peer_state, chan);
+ if let Some(data) = self.handle_new_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ chan,
+ channel_funding_outpoint,
+ monitor_update,
+ ) {
+ mem::drop(peer_state_lck);
+ mem::drop(per_peer_state);
+ self.handle_post_monitor_update_chan_resume(data);
+ }
if further_update_exists {
// If there are more `ChannelMonitorUpdate`s to process, restart at the
// top of the loop.
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.