Skip to content

Commit c30d610

Browse files
joostjagerclaude
andcommitted
Return NotifyOption from process_pending_monitor_events
Refactor process_pending_monitor_events to return a NotifyOption instead of a bool, allowing callers to distinguish between DoPersist, SkipPersistHandleEvents, and SkipPersistNoEvents. Both call sites in process_events_body and get_and_clear_pending_msg_events are updated accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9ce02b3 commit c30d610

1 file changed

Lines changed: 14 additions & 11 deletions

File tree

lightning/src/ln/channelmanager.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3512,8 +3512,12 @@ macro_rules! process_events_body {
35123512

35133513
// TODO: This behavior should be documented. It's unintuitive that we query
35143514
// ChannelMonitors when clearing other events.
3515-
if $self.process_pending_monitor_events() {
3516-
result = NotifyOption::DoPersist;
3515+
match $self.process_pending_monitor_events() {
3516+
NotifyOption::DoPersist => result = NotifyOption::DoPersist,
3517+
NotifyOption::SkipPersistHandleEvents
3518+
if result == NotifyOption::SkipPersistNoEvents =>
3519+
result = NotifyOption::SkipPersistHandleEvents,
3520+
_ => {},
35173521
}
35183522
}
35193523

@@ -13732,13 +13736,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1373213736
Ok(post_update_data)
1373313737
}
1373413738

13735-
/// Process pending events from the [`chain::Watch`], returning whether any events were processed.
13736-
fn process_pending_monitor_events(&self) -> bool {
13739+
/// Process pending events from the [`chain::Watch`], returning the appropriate
13740+
/// [`NotifyOption`] for persistence and event handling.
13741+
fn process_pending_monitor_events(&self) -> NotifyOption {
1373713742
debug_assert!(self.total_consistency_lock.try_write().is_err()); // Caller holds read lock
1373813743

1373913744
let mut failed_channels: Vec<(Result<Infallible, _>, _)> = Vec::new();
1374013745
let mut pending_monitor_events = self.chain_monitor.release_pending_monitor_events();
13741-
let has_pending_monitor_events = !pending_monitor_events.is_empty();
13746+
if pending_monitor_events.is_empty() {
13747+
return NotifyOption::SkipPersistNoEvents;
13748+
}
1374213749
for (funding_outpoint, channel_id, mut monitor_events, counterparty_node_id) in
1374313750
pending_monitor_events.drain(..)
1374413751
{
@@ -13862,7 +13869,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1386213869
let _ = self.handle_error(err, counterparty_node_id);
1386313870
}
1386413871

13865-
has_pending_monitor_events
13872+
NotifyOption::DoPersist
1386613873
}
1386713874

1386813875
fn handle_holding_cell_free_result(&self, result: FreeHoldingCellsResult) {
@@ -16001,8 +16008,6 @@ impl<
1600116008
fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent> {
1600216009
let events = RefCell::new(Vec::new());
1600316010
PersistenceNotifierGuard::optionally_notify(self, || {
16004-
let mut result = NotifyOption::SkipPersistNoEvents;
16005-
1600616011
// This method is quite performance-sensitive. Not only is it called very often, but it
1600716012
// *is* the critical path between generating a message for a peer and giving it to the
1600816013
// `PeerManager` to send. Thus, we should avoid adding any more logic here than we
@@ -16011,9 +16016,7 @@ impl<
1601116016

1601216017
// TODO: This behavior should be documented. It's unintuitive that we query
1601316018
// ChannelMonitors when clearing other events.
16014-
if self.process_pending_monitor_events() {
16015-
result = NotifyOption::DoPersist;
16016-
}
16019+
let mut result = self.process_pending_monitor_events();
1601716020

1601816021
if self.maybe_generate_initial_closing_signed() {
1601916022
result = NotifyOption::DoPersist;

0 commit comments

Comments
 (0)