Remove handle_post_close_monitor_update macro
What the developer wrote
Authored by Joost Jager
Remove handle_post_close_monitor_update macro
Convert the handle_post_close_monitor_update! macro to a method that
returns optional completion actions, allowing callers to release locks
before processing the completion.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
What changed, and why it matters
This commit refactors a Rust macro into a regular method in the Lightning Dev Kit's channel manager. The stated goal is to let callers release internal locks before running follow-up completion actions. The change itself is a code-quality/locking refactor; there is no direct evidence in the commit message or diff that it fixes a known security vulnerability or that any exploit exists.
Treat as a normal code-quality refactor. Reviewers should verify that all call sites now drop locks before processing completion actions and that no new race conditions are introduced by moving lock release earlier. No urgent security response is warranted based on the supplied materials.
Security signals we found
Lock-holding scope reduction: completion actions are now processed after releasing peer-state locks, which can reduce risk of lock-order inversion or deadlock.
Macro-to-method refactor improves auditability and compile-time type checking of the affected code path.
No explicit security bug, CVE, advisory, or exploit evidence is present in the supplied materials.
Evidence from the diff
The patch removes the handle_post_close_monitor_update! macro and replaces it with a method handle_post_close_monitor_update() that returns Option<Vec<MonitorUpdateCompletionAction>>. Callers now explicitly drop peer_state_lock and per_peer_state before invoking handle_monitor_update_completion_actions(). This reduces lock-hold scope and removes macro-based code duplication. The diff shows three call sites updated accordingly, plus a small variable rename (peer_state -> peer_state_lock) in one background event handler. No functional behavior change is evident beyond lock scoping.
Changed components
lightning/src/ln/channelmanager.rshandle_post_close_monitor_update logicmonitor update completion action handlingInspect captured patch +60 / −45
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 655ee11..76da381 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3291,32 +3291,6 @@ macro_rules! emit_initial_channel_ready_event {
};
}
-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 (update_completed, all_updates_complete) = $self.update_channel_monitor(
- &mut $peer_state.in_flight_monitor_updates,
- $channel_id,
- $funding_txo,
- $counterparty_node_id,
- $update,
- );
- if all_updates_complete {
- 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);
- }
- update_completed
- }};
-}
macro_rules! handle_new_monitor_update {
(
$self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr,
@@ -4151,10 +4125,18 @@ where
hash_map::Entry::Vacant(_) => {},
}
- handle_post_close_monitor_update!(
- self, funding_txo, monitor_update, peer_state_lock, peer_state, per_peer_state,
- counterparty_node_id, channel_id
- );
+ if let Some(actions) = self.handle_post_close_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ funding_txo,
+ monitor_update,
+ counterparty_node_id,
+ channel_id,
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_monitor_update_completion_actions(actions);
+ }
}
/// When a channel is removed, two things need to happen:
@@ -9120,16 +9102,18 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
.push(action);
}
- handle_post_close_monitor_update!(
- self,
+ if let Some(actions) = self.handle_post_close_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
prev_hop.funding_txo,
preimage_update,
- peer_state_lock,
- peer_state,
- per_peer_state,
prev_hop.counterparty_node_id,
- chan_id
- );
+ chan_id,
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_monitor_update_completion_actions(actions);
+ }
}
fn finalize_claims(&self, sources: Vec<(HTLCSource, Option<AttributionData>)>) {
@@ -9654,6 +9638,34 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
}
+ /// Handles a monitor update for a closed channel, returning optionally the completion actions
+ /// to process after locks are released.
+ ///
+ /// Returns `Some` if all in-flight updates are complete.
+ fn handle_post_close_monitor_update(
+ &self,
+ in_flight_monitor_updates: &mut BTreeMap<ChannelId, (OutPoint, Vec<ChannelMonitorUpdate>)>,
+ monitor_update_blocked_actions: &mut BTreeMap<
+ ChannelId,
+ Vec<MonitorUpdateCompletionAction>,
+ >,
+ funding_txo: OutPoint, update: ChannelMonitorUpdate, counterparty_node_id: PublicKey,
+ channel_id: ChannelId,
+ ) -> Option<Vec<MonitorUpdateCompletionAction>> {
+ let (_update_completed, all_updates_complete) = self.update_channel_monitor(
+ in_flight_monitor_updates,
+ channel_id,
+ funding_txo,
+ counterparty_node_id,
+ update,
+ );
+ if all_updates_complete {
+ Some(monitor_update_blocked_actions.remove(&channel_id).unwrap_or(Vec::new()))
+ } else {
+ None
+ }
+ }
+
/// Returns whether the monitor update is completed, `false` if the update is in-progress.
fn handle_monitor_update_res<LG: Logger>(
&self, update_res: ChannelMonitorUpdateStatus, logger: LG,
@@ -14083,10 +14095,11 @@ where
},
) => {
let per_peer_state = self.per_peer_state.read().unwrap();
- let mut peer_state = per_peer_state
+ let mut peer_state_lock = per_peer_state
.get(&counterparty_node_id)
.map(|state| state.lock().unwrap())
.expect("Channels originating a payment resolution must have peer state");
+ let peer_state = &mut *peer_state_lock;
let update_id = peer_state
.closed_channel_monitor_update_ids
.get_mut(&channel_id)
@@ -14113,16 +14126,18 @@ where
};
self.pending_background_events.lock().unwrap().push(event);
} else {
- handle_post_close_monitor_update!(
- self,
+ if let Some(actions) = self.handle_post_close_monitor_update(
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
channel_funding_outpoint,
update,
- peer_state,
- peer_state,
- per_peer_state,
counterparty_node_id,
- channel_id
- );
+ channel_id,
+ ) {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ self.handle_monitor_update_completion_actions(actions);
+ }
}
},
}
Why this scored 24/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.