Marginally simplify `handle_new_monitor_update_internal`
What the developer wrote
Authored by Matt Corallo
Marginally simplify `handle_new_monitor_update_internal`
It turns out the arguments to track the pushed update index and map
weren't necessary, so are removed here.
What changed, and why it matters
This commit is a small internal cleanup in the code that tracks pending Lightning channel monitor updates. It removes some unnecessary macro arguments and moves the logic for removing completed updates inside a shared helper macro. There is no indication this fixes a security bug or changes security-relevant behavior; it appears to be a refactoring/simplification.
No security action required. Treat as normal code-review/refactoring commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change refactors the handle_new_monitor_update_internal! macro in lightning/src/ln/channelmanager.rs. Previously, callers declared in_flight_updates and idx identifiers and passed them into the macro, which then assigned to them. The macro callers then repeated the remove(idx) logic (and sometimes an is_empty() check) in a $completed closure. The commit makes these variables local to the macro, performs the remove and is_empty() check inside the macro, and exposes only an $all_completed callback for callers to run when the in-flight list becomes empty. The observable behavior—tracking updates, deduplicating on startup, calling chain_monitor.update_channel, removing completed updates, and triggering completion actions when the list empties—remains the same. No bounds-checking, error-handling, or concurrency semantics are visibly altered.
Changed components
lightning/src/ln/channelmanager.rshandle_new_monitor_update_internal macrohandle_post_close_monitor_update macrohandle_new_monitor_update macrohandle_new_monitor_update_locked_actions_handled_by_caller macroInspect captured patch +21 / −37
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 73d98b2..fa1df90 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3703,8 +3703,6 @@ macro_rules! handle_post_close_monitor_update {
) => {{
let logger =
WithContext::from(&$self.logger, Some($counterparty_node_id), Some($channel_id), None);
- let in_flight_updates;
- let idx;
handle_new_monitor_update_internal!(
$self,
$funding_txo,
@@ -3713,21 +3711,16 @@ macro_rules! handle_post_close_monitor_update {
logger,
$channel_id,
$counterparty_node_id,
- in_flight_updates,
- idx,
{
- 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);
+ mem::drop($peer_state_lock);
+ mem::drop($per_peer_state_lock);
- $self.handle_monitor_update_completion_actions(update_actions);
- }
+ $self.handle_monitor_update_completion_actions(update_actions);
}
)
}};
@@ -3749,8 +3742,6 @@ macro_rules! handle_new_monitor_update_locked_actions_handled_by_caller {
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_internal!(
$self,
$funding_txo,
@@ -3759,11 +3750,7 @@ macro_rules! handle_new_monitor_update_locked_actions_handled_by_caller {
logger,
chan_id,
counterparty_node_id,
- in_flight_updates,
- idx,
- {
- let _ = in_flight_updates.remove(idx);
- }
+ {}
)
}};
}
@@ -3771,10 +3758,9 @@ macro_rules! handle_new_monitor_update_locked_actions_handled_by_caller {
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,
- $completed: expr
+ $chan_id: expr, $counterparty_node_id: expr, $all_completed: expr
) => {{
- $in_flight_updates = &mut $peer_state
+ let in_flight_updates = &mut $peer_state
.in_flight_monitor_updates
.entry($chan_id)
.or_insert_with(|| ($funding_txo, Vec::new()))
@@ -3782,17 +3768,20 @@ macro_rules! handle_new_monitor_update_internal {
// 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(|| {
- $in_flight_updates.push($update);
- $in_flight_updates.len() - 1
+ let 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]);
+ $self.chain_monitor.update_channel($chan_id, &in_flight_updates[update_idx]);
let update_completed = handle_monitor_update_res($self, update_res, $chan_id, $logger);
if update_completed {
- $completed;
+ let _ = in_flight_updates.remove(update_idx);
+ if in_flight_updates.is_empty() {
+ $all_completed;
+ }
}
update_completed
} else {
@@ -3803,7 +3792,7 @@ macro_rules! handle_new_monitor_update_internal {
counterparty_node_id: $counterparty_node_id,
funding_txo: $funding_txo,
channel_id: $chan_id,
- update: $in_flight_updates[$update_idx].clone(),
+ update: in_flight_updates[update_idx].clone(),
};
// We want to track the in-flight update both in `in_flight_monitor_updates` and in
// `pending_background_events` to avoid a race condition during
@@ -3827,8 +3816,6 @@ macro_rules! handle_new_monitor_update {
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_internal!(
$self,
$funding_txo,
@@ -3837,11 +3824,8 @@ macro_rules! handle_new_monitor_update {
logger,
chan_id,
counterparty_node_id,
- in_flight_updates,
- idx,
{
- let _ = in_flight_updates.remove(idx);
- if in_flight_updates.is_empty() && $chan.blocked_monitor_updates_pending() == 0 {
+ if $chan.blocked_monitor_updates_pending() == 0 {
handle_monitor_update_completion!(
$self,
$peer_state_lock,
Why this scored 12/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.