Skip to content

Commit 660bab5

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 1af5ff4 commit 660bab5

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
@@ -3516,8 +3516,12 @@ macro_rules! process_events_body {
35163516

35173517
// TODO: This behavior should be documented. It's unintuitive that we query
35183518
// ChannelMonitors when clearing other events.
3519-
if $self.process_pending_monitor_events() {
3520-
result = NotifyOption::DoPersist;
3519+
match $self.process_pending_monitor_events() {
3520+
NotifyOption::DoPersist => result = NotifyOption::DoPersist,
3521+
NotifyOption::SkipPersistHandleEvents
3522+
if result == NotifyOption::SkipPersistNoEvents =>
3523+
result = NotifyOption::SkipPersistHandleEvents,
3524+
_ => {},
35213525
}
35223526
}
35233527

@@ -13655,13 +13659,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1365513659
Ok(())
1365613660
}
1365713661

13658-
/// Process pending events from the [`chain::Watch`], returning whether any events were processed.
13659-
fn process_pending_monitor_events(&self) -> bool {
13662+
/// Process pending events from the [`chain::Watch`], returning the appropriate
13663+
/// [`NotifyOption`] for persistence and event handling.
13664+
fn process_pending_monitor_events(&self) -> NotifyOption {
1366013665
debug_assert!(self.total_consistency_lock.try_write().is_err()); // Caller holds read lock
1366113666

1366213667
let mut failed_channels: Vec<(Result<Infallible, _>, _)> = Vec::new();
1366313668
let mut pending_monitor_events = self.chain_monitor.release_pending_monitor_events();
13664-
let has_pending_monitor_events = !pending_monitor_events.is_empty();
13669+
if pending_monitor_events.is_empty() {
13670+
return NotifyOption::SkipPersistNoEvents;
13671+
}
1366513672
for (funding_outpoint, channel_id, mut monitor_events, counterparty_node_id) in
1366613673
pending_monitor_events.drain(..)
1366713674
{
@@ -13785,7 +13792,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1378513792
let _ = self.handle_error(err, counterparty_node_id);
1378613793
}
1378713794

13788-
has_pending_monitor_events
13795+
NotifyOption::DoPersist
1378913796
}
1379013797

1379113798
fn handle_holding_cell_free_result(&self, result: FreeHoldingCellsResult) {
@@ -15811,8 +15818,6 @@ impl<
1581115818
fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent> {
1581215819
let events = RefCell::new(Vec::new());
1581315820
PersistenceNotifierGuard::optionally_notify(self, || {
15814-
let mut result = NotifyOption::SkipPersistNoEvents;
15815-
1581615821
// This method is quite performance-sensitive. Not only is it called very often, but it
1581715822
// *is* the critical path between generating a message for a peer and giving it to the
1581815823
// `PeerManager` to send. Thus, we should avoid adding any more logic here than we
@@ -15821,9 +15826,7 @@ impl<
1582115826

1582215827
// TODO: This behavior should be documented. It's unintuitive that we query
1582315828
// ChannelMonitors when clearing other events.
15824-
if self.process_pending_monitor_events() {
15825-
result = NotifyOption::DoPersist;
15826-
}
15829+
let mut result = self.process_pending_monitor_events();
1582715830

1582815831
if self.maybe_generate_initial_closing_signed() {
1582915832
result = NotifyOption::DoPersist;

0 commit comments

Comments
 (0)