Skip to content

Clarify and consolidate event handling requirements #1697

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5671,10 +5671,6 @@ where
///
/// An [`EventHandler`] may safely call back to the provider in order to handle an event.
/// However, it must not call [`Writeable::write`] as doing so would result in a deadlock.
///
/// Pending events are persisted as part of [`ChannelManager`]. While these events are cleared
/// when processed, an [`EventHandler`] must be able to handle previously seen events when
/// restarting from an old state.
fn process_pending_events<H: Deref>(&self, handler: H) where H::Target: EventHandler {
PersistenceNotifierGuard::optionally_notify(&self.total_consistency_lock, &self.persistence_notifier, || {
let mut result = NotifyOption::SkipPersist;
Expand Down
18 changes: 11 additions & 7 deletions lightning/src/util/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1207,11 +1207,17 @@ pub trait OnionMessageProvider {
///
/// # Requirements
///
/// See [`process_pending_events`] for requirements around event processing.
///
/// When using this trait, [`process_pending_events`] will call [`handle_event`] for each pending
/// event since the last invocation. The handler must either act upon the event immediately
/// or preserve it for later handling.
/// event since the last invocation.
///
/// In order to ensure no [`Event`]s are lost, implementors of this trait will persist [`Event`]s
/// and replay any unhandled events on startup. An [`Event`] is considered handled when
/// [`process_pending_events`] returns, thus handlers MUST fully handle [`Event`]s and persist any
/// relevant changes to disk *before* returning.
///
/// Further, because an application may crash between an [`Event`] being handled and the
/// implementor of this trait being re-serialized, [`Event`] handling must be idempotent - in
/// effect, [`Event`]s may be replayed.
///
/// Note, handlers may call back into the provider and thus deadlocking must be avoided. Be sure to
/// consult the provider's documentation on the implication of processing events and how a handler
Expand All @@ -1228,9 +1234,7 @@ pub trait OnionMessageProvider {
pub trait EventsProvider {
/// Processes any events generated since the last call using the given event handler.
///
/// Subsequent calls must only process new events. However, handlers must be capable of handling
/// duplicate events across process restarts. This may occur if the provider was recovered from
/// an old state (i.e., it hadn't been successfully persisted after processing pending events).
/// See the trait-level documentation for requirements.
fn process_pending_events<H: Deref>(&self, handler: H) where H::Target: EventHandler;
}

Expand Down