Skip to content

Log pending in-flight updates when we are missing a monitor #2990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Log pending in-flight updates when we are missing a monitor #2990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10480f0
Log pending in-flight updates when we are missing a monitor
TheBlueMatt Apr 11, 2024
93e7763
Debug-log monitor update starts and completions in `ChainMonitor`
TheBlueMatt Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 42 additions & 9 deletions lightning/src/chain/chainmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,9 @@ where C::Target: chain::Filter,
let mut txn_outputs;
{
txn_outputs = process(monitor, txdata);
let chain_sync_update_id = self.sync_persistence_id.get_increment();
let update_id = MonitorUpdateId {
contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
contents: UpdateOrigin::ChainSync(chain_sync_update_id),
};
let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
if let Some(height) = best_height {
Expand All @@ -376,10 +377,16 @@ where C::Target: chain::Filter,
}
}

log_trace!(logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
log_trace!(logger, "Syncing Channel Monitor for channel {} for block-data update_id {}",
log_funding_info!(monitor),
chain_sync_update_id
);
match self.persister.update_persisted_channel(*funding_outpoint, None, monitor, update_id) {
ChannelMonitorUpdateStatus::Completed =>
log_trace!(logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
log_trace!(logger, "Finished syncing Channel Monitor for channel {} for block-data update_id {}",
log_funding_info!(monitor),
chain_sync_update_id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: if we want to add update_id in case of completed, you might want to add it at L:821 as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be printed a few lines up so was being lazy but went ahead and did this.

),
ChannelMonitorUpdateStatus::InProgress => {
log_debug!(logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
pending_monitor_updates.push(update_id);
Expand Down Expand Up @@ -526,7 +533,7 @@ where C::Target: chain::Filter,
pending_monitor_updates.retain(|update_id| *update_id != completed_update_id);

match completed_update_id {
MonitorUpdateId { contents: UpdateOrigin::OffChain(_) } => {
MonitorUpdateId { contents: UpdateOrigin::OffChain(completed_update_id) } => {
// Note that we only check for `UpdateOrigin::OffChain` failures here - if
// we're being told that a `UpdateOrigin::OffChain` monitor update completed,
// we only care about ensuring we don't tell the `ChannelManager` to restore
Expand All @@ -537,6 +544,14 @@ where C::Target: chain::Filter,
// `MonitorEvent`s from the monitor back to the `ChannelManager` until they
// complete.
let monitor_is_pending_updates = monitor_data.has_pending_offchain_updates(&pending_monitor_updates);
log_debug!(self.logger, "Completed off-chain monitor update {} for channel with funding outpoint {:?}, {}",
completed_update_id,
funding_txo,
if monitor_is_pending_updates {
"still have pending off-chain updates"
} else {
"all off-chain updates complete, returning a MonitorEvent"
});
if monitor_is_pending_updates {
// If there are still monitor updates pending, we cannot yet construct a
// Completed event.
Expand All @@ -547,8 +562,18 @@ where C::Target: chain::Filter,
monitor_update_id: monitor_data.monitor.get_latest_update_id(),
}], monitor_data.monitor.get_counterparty_node_id()));
},
MonitorUpdateId { contents: UpdateOrigin::ChainSync(_) } => {
if !monitor_data.has_pending_chainsync_updates(&pending_monitor_updates) {
MonitorUpdateId { contents: UpdateOrigin::ChainSync(completed_update_id) } => {
let monitor_has_pending_updates =
monitor_data.has_pending_chainsync_updates(&pending_monitor_updates);
log_debug!(self.logger, "Completed chain sync monitor update {} for channel with funding outpoint {:?}, {}",
completed_update_id,
funding_txo,
if monitor_has_pending_updates {
"still have pending chain sync updates"
} else {
"all chain sync updates complete, releasing pending MonitorEvents"
});
if !monitor_has_pending_updates {
monitor_data.last_chain_persist_height.store(self.highest_chain_height.load(Ordering::Acquire), Ordering::Release);
// The next time release_pending_monitor_events is called, any events for this
// ChannelMonitor will be returned.
Expand Down Expand Up @@ -771,7 +796,7 @@ where C::Target: chain::Filter,
Some(monitor_state) => {
let monitor = &monitor_state.monitor;
let logger = WithChannelMonitor::from(&self.logger, &monitor);
log_trace!(logger, "Updating ChannelMonitor for channel {}", log_funding_info!(monitor));
log_trace!(logger, "Updating ChannelMonitor to id {} for channel {}", update.update_id, log_funding_info!(monitor));
let update_res = monitor.update_monitor(update, &self.broadcaster, &self.fee_estimator, &self.logger);

let update_id = MonitorUpdateId::from_monitor_update(update);
Expand All @@ -790,10 +815,18 @@ where C::Target: chain::Filter,
match persist_res {
ChannelMonitorUpdateStatus::InProgress => {
pending_monitor_updates.push(update_id);
log_debug!(logger, "Persistence of ChannelMonitorUpdate for channel {} in progress", log_funding_info!(monitor));
log_debug!(logger,
"Persistence of ChannelMonitorUpdate id {:?} for channel {} in progress",
update_id,
log_funding_info!(monitor)
);
},
ChannelMonitorUpdateStatus::Completed => {
log_debug!(logger, "Persistence of ChannelMonitorUpdate for channel {} completed", log_funding_info!(monitor));
log_debug!(logger,
"Persistence of ChannelMonitorUpdate id {:?} for channel {} completed",
update_id,
log_funding_info!(monitor)
);
},
ChannelMonitorUpdateStatus::UnrecoverableError => {
// Take the monitors lock for writing so that we poison it and any future
Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10680,6 +10680,7 @@ where
log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!");
log_error!(logger, " Without the latest ChannelMonitor we cannot continue without risking funds.");
log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning");
log_error!(logger, " Pending in-flight updates are: {:?}", chan_in_flight_updates);
return Err(DecodeError::InvalidValue);
}
}
Expand Down
Loading