From 6da89c229340a4739e2d5f2fe2052b67fbba3d8f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 24 Mar 2026 00:53:30 +0000 Subject: [PATCH] Send BroadcastChannelAnnouncements via the broadcast queue In 47a3e5c694321dac1a1d0f53e6dcb357282a79be we started asserting that the per-peer message queue was empty when a peer connected to ensure we don't have stale messages sitting around in memory. This turned up an issue for `channel_announcement` messages generated by block connections while a peer was disconnected. Here we push those out through the broadcast message queue rather than the per-peer message queue as there's no reason to tie them to the individual peer anyway, fixing the assertions. This should fix #4437 Written by Claude --- lightning/src/ln/channelmanager.rs | 49 +++++++++++++---------- lightning/src/ln/functional_test_utils.rs | 4 -- lightning/src/ln/priv_short_conf_tests.rs | 4 +- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d042a69bf80..f9772bb120b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -13004,12 +13004,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ msg, &self.config.read().unwrap(), ); - peer_state.pending_msg_events.push(MessageSendEvent::BroadcastChannelAnnouncement { - msg: try_channel_entry!(self, peer_state, res, chan_entry), - // Note that announcement_signatures fails if the channel cannot be announced, - // so get_channel_update_for_broadcast will never fail by the time we get here. - update_msg: Some(self.get_channel_update_for_broadcast(chan).unwrap().0), - }); + let announcement_msg = try_channel_entry!(self, peer_state, res, chan_entry); + // Note that announcement_signatures fails if the channel cannot be announced, + // so get_channel_update_for_broadcast will never fail by the time we get here. + let update_msg = self.get_channel_update_for_broadcast(chan).unwrap().0; + self.pending_broadcast_messages.lock().unwrap().push( + MessageSendEvent::BroadcastChannelAnnouncement { + msg: announcement_msg, + update_msg: Some(update_msg), + }, + ); } else { return try_channel_entry!(self, peer_state, Err(ChannelError::close( "Got an announcement_signatures message for an unfunded channel!".into())), chan_entry); @@ -15485,11 +15489,14 @@ impl< &MessageSendEvent::HandleError { .. } => false, // Gossip &MessageSendEvent::SendChannelAnnouncement { .. } => false, - &MessageSendEvent::BroadcastChannelAnnouncement { .. } => true, - // [`ChannelManager::pending_broadcast_events`] holds the [`BroadcastChannelUpdate`] - // This check here is to ensure exhaustivity. + // [`ChannelManager::pending_broadcast_messages`] holds broadcast events, + // not per-peer queues. + &MessageSendEvent::BroadcastChannelAnnouncement { .. } => { + debug_assert!(false, "BroadcastChannelAnnouncement should be in pending_broadcast_messages"); + false + }, &MessageSendEvent::BroadcastChannelUpdate { .. } => { - debug_assert!(false, "This event shouldn't have been here"); + debug_assert!(false, "BroadcastChannelUpdate should be in pending_broadcast_messages"); false }, &MessageSendEvent::BroadcastNodeAnnouncement { .. } => true, @@ -15687,10 +15694,6 @@ impl< /// the chunks of `MessageSendEvent`s for different peers is random. I.e. if the array contains /// `MessageSendEvent`s for both `node_a` and `node_b`, the `MessageSendEvent`s for `node_a` /// will randomly be placed first or last in the returned array. - /// - /// Note that even though `BroadcastChannelAnnouncement` and `BroadcastChannelUpdate` - /// `MessageSendEvent`s are intended to be broadcasted to all peers, they will be placed among - /// the `MessageSendEvent`s to the specific peer they were generated under. fn get_and_clear_pending_msg_events(&self) -> Vec { let events = RefCell::new(Vec::new()); PersistenceNotifierGuard::optionally_notify(self, || { @@ -16143,14 +16146,16 @@ impl< if let Some(announcement) = funded_channel.get_signed_channel_announcement( &self.node_signer, self.chain_hash, height, &self.config.read().unwrap(), ) { - pending_msg_events.push(MessageSendEvent::BroadcastChannelAnnouncement { - msg: announcement, - // Note that get_signed_channel_announcement fails - // if the channel cannot be announced, so - // get_channel_update_for_broadcast will never fail - // by the time we get here. - update_msg: Some(self.get_channel_update_for_broadcast(funded_channel).unwrap().0), - }); + self.pending_broadcast_messages.lock().unwrap().push( + MessageSendEvent::BroadcastChannelAnnouncement { + msg: announcement, + // Note that get_signed_channel_announcement + // fails if the channel cannot be announced, so + // get_channel_update_for_broadcast will never + // fail by the time we get here. + update_msg: Some(self.get_channel_update_for_broadcast(funded_channel).unwrap().0), + }, + ); } } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 80274d180b4..e8859494071 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1121,10 +1121,6 @@ pub fn get_htlc_update_msgs(node: &Node, recipient: &PublicKey) -> msgs::Commitm /// Fetches the first `msg_event` to the passed `node_id` in the passed `msg_events` vec. /// Returns the `msg_event`. -/// -/// Note that even though `BroadcastChannelAnnouncement` and `BroadcastChannelUpdate` -/// `msg_events` are stored under specific peers, this function does not fetch such `msg_events` as -/// such messages are intended to all peers. pub fn remove_first_msg_event_to_node( msg_node_id: &PublicKey, msg_events: &mut Vec, ) -> MessageSendEvent { diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index ffe5ea6cbb1..70d58533228 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -255,7 +255,7 @@ fn do_test_1_conf_open(connect_style: ConnectStyle) { assert_eq!(bs_announce_events.len(), 2); let bs_announcement_sigs = if let MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg } = - bs_announce_events[1] + bs_announce_events[0] { assert_eq!(*node_id, node_a_id); msg.clone() @@ -264,7 +264,7 @@ fn do_test_1_conf_open(connect_style: ConnectStyle) { }; let (bs_announcement, bs_update) = if let MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } = - bs_announce_events[0] + bs_announce_events[1] { (msg.clone(), update_msg.clone().unwrap()) } else {