Stop persisting background shutdown monitor updates
authorMatt Corallo <git@bluematt.me>
Tue, 9 May 2023 21:02:01 +0000 (21:02 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 9 May 2023 21:03:07 +0000 (21:03 +0000)
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

index d4a3af3bbd8cab93c15a6f51a55b688bd3902eba..951ce9787c7ba44602d99db276f03d29054e7ec8 100644 (file)
@@ -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 <u8 as Readable>::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),
                        }