From 135f4149ed41edcef47c4eaa0dc48947555c47e5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 9 May 2023 21:02:01 +0000 Subject: [PATCH] Stop persisting background shutdown monitor updates In d4810087c1 we added logic to apply `ChannelMonitorUpdate`s which were a part of a channel closure async via a background queue to address some startup issues. When we did that we persisted those updates to ensure we replayed them when starting next time. However, there was no reason to - if we persisted and then restarted even without those monitor updates we'd find a monitor without a channel, which we'd tell to broadcast the latest commitment transaction to force-close. Since adding that logic, we've used the same background queue for several purposes. --- lightning/src/ln/channelmanager.rs | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d4a3af3bb..951ce9787 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7448,17 +7448,12 @@ where } } - let background_events = self.pending_background_events.lock().unwrap(); - (background_events.len() as u64).write(writer)?; - for event in background_events.iter() { - match event { - BackgroundEvent::ClosingMonitorUpdate((funding_txo, monitor_update)) => { - 0u8.write(writer)?; - funding_txo.write(writer)?; - monitor_update.write(writer)?; - }, - } - } + // LDK versions prior to 0.0.116 wrote the `pending_background_events` + // `ClosingMonitorUpdate`s here, however there was never a reason to do so - the closing + // monitor updates were always effectively replayed on startup (either directly by calling + // `broadcast_latest_holder_commitment_txn` on a `ChannelMonitor` during deserialization + // or, in 0.0.115, by regenerating the monitor update itself). + 0u64.write(writer)?; // Prior to 0.0.111 we tracked node_announcement serials here, however that now happens in // `PeerManager`, and thus we simply write the `highest_seen_timestamp` twice, which is @@ -7905,13 +7900,11 @@ where for _ in 0..background_event_count { match ::read(reader)? { 0 => { - let (funding_txo, monitor_update): (OutPoint, ChannelMonitorUpdate) = (Readable::read(reader)?, Readable::read(reader)?); - if pending_background_events.iter().find(|e| { - let BackgroundEvent::ClosingMonitorUpdate((pending_funding_txo, pending_monitor_update)) = e; - *pending_funding_txo == funding_txo && *pending_monitor_update == monitor_update - }).is_none() { - pending_background_events.push(BackgroundEvent::ClosingMonitorUpdate((funding_txo, monitor_update))); - } + // LDK versions prior to 0.0.116 wrote pending `ClosingMonitorUpdate`s here, + // however we really don't (and never did) need them - we regenerate all + // on-startup monitor updates. + let _: OutPoint = Readable::read(reader)?; + let _: ChannelMonitorUpdate = Readable::read(reader)?; } _ => return Err(DecodeError::InvalidValue), } -- 2.39.5