Add `total_consistency_lock` check in `handle_post_event_actions`
What the developer wrote
Authored by Matt Corallo
Add `total_consistency_lock` check in `handle_post_event_actions`
We expect callers of `handle_post_event_actions` to hold a read
lock on `total_consistency_lock`, and found that we forgot it in
`process_pending_events` until recently. Here we add a relevant
assertion to avoid such issues in the future.
What changed, and why it matters
This commit adds a safety check (a debug-only assertion) inside a Rust Lightning library function that handles follow-up work after events. The function expects callers to already hold a specific read lock, and one caller was recently fixed to do so. The new assertion is meant to catch future mistakes where a caller forgets the lock. It is a defensive hardening change, not a fix for an active exploit.
No immediate action required. Treat as routine defensive hardening. If running debug builds or tests, ensure the new assertion does not trigger under normal operation, which would indicate a remaining lock-ordering bug.
Security signals we found
Adds a debug assertion for a required read lock
References a recent fix where a caller forgot to acquire total_consistency_lock
Function may update channel state, motivating the locking requirement
No functional code change in release builds; purely invariant enforcement
Evidence from the diff
In lightning/src/ln/channelmanager.rs, the commit adds a debug_assert_ne! in handle_post_event_actions verifying that self.total_consistency_lock is held by the current thread. It also adds a clarifying comment in the process_events_body! macro where the lock is now taken before calling handle_post_event_actions. The assertion is debug-only and will not affect release builds. The change enforces an existing internal locking invariant.
Changed components
lightning/src/ln/channelmanager.rsprocess_events_body! macrohandle_post_event_actions methodtotal_consistency_lockInspect captured patch +8 / −0
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index af39515..dafeffe 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3438,6 +3438,10 @@ macro_rules! process_events_body {
}
if !post_event_actions.is_empty() {
+ // `handle_post_event_actions` may update channel state, so take the total
+ // consistency lock now similarly to other callers of `handle_post_event_actions`.
+ // Note that if it needs to wake the background processor for event handling or
+ // persistence it will do so directly.
let _read_guard = $self.total_consistency_lock.read().unwrap();
$self.handle_post_event_actions(post_event_actions);
// If we had some actions, go around again as we may have more events now
@@ -14315,6 +14319,10 @@ where
}
fn handle_post_event_actions<I: IntoIterator<Item = EventCompletionAction>>(&self, actions: I) {
+ debug_assert_ne!(
+ self.total_consistency_lock.held_by_thread(),
+ LockHeldState::NotHeldByThread
+ );
for action in actions.into_iter() {
match action {
EventCompletionAction::ReleaseRAAChannelMonitorUpdate {
Why this scored 23/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.