Skip to content

Always forward gossip for all our channels to all our peers #4216

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+248 −108
Open

Always forward gossip for all our channels to all our peers #4216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e8d775
Include both `node_id`s in `BroadcastChannelUpdate`
TheBlueMatt Nov 9, 2025
1d516e6
Pipe channel node information through to `forward_broadcast_msg`
TheBlueMatt Nov 10, 2025
587e6fc
Always forward gossip for all our channels to all our peers
TheBlueMatt Nov 6, 2025
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: 2 additions & 2 deletions lightning-net-tokio/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,8 @@ mod tests {
}
fn handle_channel_update(
&self, _their_node_id: Option<PublicKey>, _msg: &ChannelUpdate,
) -> Result<bool, LightningError> {
Ok(false)
) -> Result<Option<(NodeId, NodeId)>, LightningError> {
Ok(None)
}
fn get_next_channel_announcement(
&self, _starting_point: u64,
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/channel_open_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2337,7 +2337,7 @@ pub fn test_funding_and_commitment_tx_confirm_same_block() {
} else {
panic!();
}
if let MessageSendEvent::BroadcastChannelUpdate { ref msg } = msg_events.remove(0) {
if let MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } = msg_events.remove(0) {
assert_eq!(msg.contents.channel_flags & 2, 2);
} else {
panic!();
Expand Down
70 changes: 41 additions & 29 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ use crate::onion_message::messenger::{
MessageRouter, MessageSendInstructions, Responder, ResponseInstruction,
};
use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
use crate::routing::gossip::NodeId;
use crate::routing::router::{
BlindedTail, FixedRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route,
RouteParameters, RouteParametersConfig, Router,
Expand Down Expand Up @@ -942,7 +943,7 @@ impl Into<LocalHTLCFailureReason> for FailureCode {
struct MsgHandleErrInternal {
err: msgs::LightningError,
closes_channel: bool,
shutdown_finish: Option<(ShutdownResult, Option<msgs::ChannelUpdate>)>,
shutdown_finish: Option<(ShutdownResult, Option<(msgs::ChannelUpdate, NodeId, NodeId)>)>,
tx_abort: Option<msgs::TxAbort>,
}
impl MsgHandleErrInternal {
Expand All @@ -966,7 +967,7 @@ impl MsgHandleErrInternal {

fn from_finish_shutdown(
err: String, channel_id: ChannelId, shutdown_res: ShutdownResult,
channel_update: Option<msgs::ChannelUpdate>,
channel_update: Option<(msgs::ChannelUpdate, NodeId, NodeId)>,
) -> Self {
let err_msg = msgs::ErrorMessage { channel_id, data: err.clone() };
let action = if shutdown_res.monitor_update.is_some() {
Expand Down Expand Up @@ -3244,10 +3245,10 @@ macro_rules! handle_error {
log_error!(logger, "Closing channel: {}", err.err);

$self.finish_close_channel(shutdown_res);
if let Some(update) = update_option {
if let Some((update, node_id_1, node_id_2)) = update_option {
let mut pending_broadcast_messages = $self.pending_broadcast_messages.lock().unwrap();
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
msg: update
msg: update, node_id_1, node_id_2
});
}
} else {
Expand Down Expand Up @@ -3574,7 +3575,7 @@ macro_rules! handle_monitor_update_completion {
// channel_update later through the announcement_signatures process for public
// channels, but there's no reason not to just inform our counterparty of our fees
// now.
if let Ok(msg) = $self.get_channel_update_for_unicast($chan) {
if let Ok((msg, _, _)) = $self.get_channel_update_for_unicast($chan) {
Some(MessageSendEvent::SendChannelUpdate {
node_id: counterparty_node_id,
msg,
Expand Down Expand Up @@ -5125,7 +5126,9 @@ where
}
}

/// Gets the current [`channel_update`] for the given channel. This first checks if the channel is
/// Gets the current [`channel_update`] for the given channel (as well as our and our
/// counterparty's [`NodeId`], which is needed for the
/// [`MessageSendEvent::BroadcastChannelUpdate`]). This first checks if the channel is
/// public, and thus should be called whenever the result is going to be passed out in a
/// [`MessageSendEvent::BroadcastChannelUpdate`] event.
///
Expand All @@ -5137,7 +5140,7 @@ where
/// [`internal_closing_signed`]: Self::internal_closing_signed
fn get_channel_update_for_broadcast(
&self, chan: &FundedChannel<SP>,
) -> Result<msgs::ChannelUpdate, LightningError> {
) -> Result<(msgs::ChannelUpdate, NodeId, NodeId), LightningError> {
if !chan.context.should_announce() {
return Err(LightningError {
err: "Cannot broadcast a channel_update for a private channel".to_owned(),
Expand All @@ -5159,10 +5162,11 @@ where
self.get_channel_update_for_unicast(chan)
}

/// Gets the current [`channel_update`] for the given channel. This does not check if the channel
/// is public (only returning an `Err` if the channel does not yet have an assigned SCID),
/// and thus MUST NOT be called unless the recipient of the resulting message has already
/// provided evidence that they know about the existence of the channel.
/// Gets the current [`channel_update`] for the given channel (as well as our and our
/// counterparty's [`NodeId`]). This does not check if the channel is public (only returning an
/// `Err` if the channel does not yet have an assigned SCID), and thus MUST NOT be called
/// unless the recipient of the resulting message has already provided evidence that they know
/// about the existence of the channel.
///
/// Note that through [`internal_closing_signed`], this function is called without the
/// `peer_state` corresponding to the channel's counterparty locked, as the channel been
Expand All @@ -5171,7 +5175,9 @@ where
/// [`channel_update`]: msgs::ChannelUpdate
/// [`internal_closing_signed`]: Self::internal_closing_signed
#[rustfmt::skip]
fn get_channel_update_for_unicast(&self, chan: &FundedChannel<SP>) -> Result<msgs::ChannelUpdate, LightningError> {
fn get_channel_update_for_unicast(
&self, chan: &FundedChannel<SP>,
) -> Result<(msgs::ChannelUpdate, NodeId, NodeId), LightningError> {
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
log_trace!(logger, "Attempting to generate channel update for channel {}", chan.context.channel_id());
let short_channel_id = match chan.funding.get_short_channel_id().or(chan.context.latest_inbound_scid_alias()) {
Expand All @@ -5181,7 +5187,9 @@ where

let logger = WithChannelContext::from(&self.logger, &chan.context, None);
log_trace!(logger, "Generating channel update for channel {}", chan.context.channel_id());
let were_node_one = self.our_network_pubkey.serialize()[..] < chan.context.get_counterparty_node_id().serialize()[..];
let our_node_id = NodeId::from_pubkey(&self.our_network_pubkey);
let their_node_id = NodeId::from_pubkey(&chan.context.get_counterparty_node_id());
let were_node_one = our_node_id < their_node_id;
let enabled = chan.context.is_enabled();

let unsigned = msgs::UnsignedChannelUpdate {
Expand All @@ -5203,10 +5211,14 @@ where
// channel.
let sig = self.node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelUpdate(&unsigned)).unwrap();

Ok(msgs::ChannelUpdate {
signature: sig,
contents: unsigned
})
Ok((
msgs::ChannelUpdate {
signature: sig,
contents: unsigned
},
if were_node_one { our_node_id } else { their_node_id },
if were_node_one { their_node_id } else { our_node_id },
))
}

#[cfg(any(test, feature = "_externalize_tests"))]
Expand Down Expand Up @@ -6649,11 +6661,11 @@ where
continue;
}
if let Some(channel) = channel.as_funded() {
if let Ok(msg) = self.get_channel_update_for_broadcast(channel) {
if let Ok((msg, node_id_1, node_id_2)) = self.get_channel_update_for_broadcast(channel) {
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate { msg });
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate { msg, node_id_1, node_id_2 });
} else if peer_state.is_connected {
if let Ok(msg) = self.get_channel_update_for_unicast(channel) {
if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(channel) {
peer_state.pending_msg_events.push(MessageSendEvent::SendChannelUpdate {
node_id: channel.context.get_counterparty_node_id(),
msg,
Expand Down Expand Up @@ -8177,10 +8189,10 @@ where
n += 1;
if n >= DISABLE_GOSSIP_TICKS {
funded_chan.set_channel_update_status(ChannelUpdateStatus::Disabled);
if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) {
if let Ok((update, node_id_1, node_id_2)) = self.get_channel_update_for_broadcast(&funded_chan) {
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
msg: update
msg: update, node_id_1, node_id_2
});
}
should_persist = NotifyOption::DoPersist;
Expand All @@ -8192,10 +8204,10 @@ where
n += 1;
if n >= ENABLE_GOSSIP_TICKS {
funded_chan.set_channel_update_status(ChannelUpdateStatus::Enabled);
if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) {
if let Ok((update, node_id_1, node_id_2)) = self.get_channel_update_for_broadcast(&funded_chan) {
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
msg: update
msg: update, node_id_1, node_id_2
});
}
should_persist = NotifyOption::DoPersist;
Expand Down Expand Up @@ -10821,7 +10833,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
// channel_update here if the channel is not public, i.e. we're not sending an
// announcement_signatures.
log_trace!(logger, "Sending private initial channel_update for our counterparty on channel {}", chan.context.channel_id());
if let Ok(msg) = self.get_channel_update_for_unicast(chan) {
if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(chan) {
peer_state.pending_msg_events.push(MessageSendEvent::SendChannelUpdate {
node_id: counterparty_node_id.clone(),
msg,
Expand Down Expand Up @@ -11620,7 +11632,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
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()),
update_msg: Some(self.get_channel_update_for_broadcast(chan).unwrap().0),
});
} else {
return try_channel_entry!(self, peer_state, Err(ChannelError::close(
Expand Down Expand Up @@ -11729,7 +11741,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
// If the channel is in a usable state (ie the channel is not being shut
// down), send a unicast channel_update to our counterparty to make sure
// they have the latest channel parameters.
if let Ok(msg) = self.get_channel_update_for_unicast(chan) {
if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(chan) {
channel_update = Some(MessageSendEvent::SendChannelUpdate {
node_id: chan.context.get_counterparty_node_id(),
msg,
Expand Down Expand Up @@ -14340,7 +14352,7 @@ where
send_channel_ready!(self, pending_msg_events, funded_channel, channel_ready);
if funded_channel.context.is_usable() && peer_state.is_connected {
log_trace!(logger, "Sending channel_ready with private initial channel_update for our counterparty on channel {}", channel_id);
if let Ok(msg) = self.get_channel_update_for_unicast(funded_channel) {
if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(funded_channel) {
pending_msg_events.push(MessageSendEvent::SendChannelUpdate {
node_id: funded_channel.context.get_counterparty_node_id(),
msg,
Expand Down Expand Up @@ -14433,7 +14445,7 @@ where
// 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()),
update_msg: Some(self.get_channel_update_for_broadcast(funded_channel).unwrap().0),
});
}
}
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/functional_test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2182,7 +2182,7 @@ macro_rules! get_closing_signed_broadcast {
assert!(events.len() == 1 || events.len() == 2);
(
match events[events.len() - 1] {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
assert_eq!(msg.contents.channel_flags & 2, 2);
msg.clone()
},
Expand Down Expand Up @@ -2253,7 +2253,7 @@ pub fn check_closed_broadcast(
.into_iter()
.filter_map(|msg_event| {
match msg_event {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
assert_eq!(msg.contents.channel_flags & 2, 2);
None
},
Expand Down Expand Up @@ -4875,7 +4875,7 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(
let events_1 = nodes[a].node.get_and_clear_pending_msg_events();
assert_eq!(events_1.len(), 2);
let as_update = match events_1[1] {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => msg.clone(),
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => msg.clone(),
_ => panic!("Unexpected event"),
};
match events_1[0] {
Expand Down Expand Up @@ -4912,7 +4912,7 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(
let events_2 = nodes[b].node.get_and_clear_pending_msg_events();
assert_eq!(events_2.len(), if needs_err_handle { 1 } else { 2 });
let bs_update = match events_2.last().unwrap() {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => msg.clone(),
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => msg.clone(),
_ => panic!("Unexpected event"),
};
if !needs_err_handle {
Expand Down
14 changes: 7 additions & 7 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ pub fn channel_monitor_network_test() {
let events = nodes[3].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 2);
let close_chan_update_1 = match events[1] {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => msg.clone(),
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => msg.clone(),
_ => panic!("Unexpected event"),
};
match events[0] {
Expand Down Expand Up @@ -752,7 +752,7 @@ pub fn channel_monitor_network_test() {
let events = nodes[4].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 2);
let close_chan_update_2 = match events[1] {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => msg.clone(),
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => msg.clone(),
_ => panic!("Unexpected event"),
};
match events[0] {
Expand Down Expand Up @@ -2167,7 +2167,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(

// Ensure that the last remaining message event is the BroadcastChannelUpdate msg for chan_2
match events[0] {
MessageSendEvent::BroadcastChannelUpdate { msg: msgs::ChannelUpdate { .. } } => {},
MessageSendEvent::BroadcastChannelUpdate { msg: msgs::ChannelUpdate { .. }, .. } => {},
_ => panic!("Unexpected event"),
}

Expand Down Expand Up @@ -6026,7 +6026,7 @@ pub fn test_announce_disable_channels() {
let mut chans_disabled = new_hash_map();
for e in msg_events {
match e {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
assert_eq!(msg.contents.channel_flags & (1 << 1), 1 << 1); // The "channel disabled" bit should be set
// Check that each channel gets updated exactly once
if chans_disabled
Expand Down Expand Up @@ -6077,7 +6077,7 @@ pub fn test_announce_disable_channels() {
assert_eq!(msg_events.len(), 3);
for e in msg_events {
match e {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
assert_eq!(msg.contents.channel_flags & (1 << 1), 0); // The "channel disabled" bit should be off
match chans_disabled.remove(&msg.contents.short_channel_id) {
// Each update should have a higher timestamp than the previous one, replacing
Expand Down Expand Up @@ -7995,13 +7995,13 @@ pub fn test_error_chans_closed() {
let events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 2);
match events[0] {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
assert_eq!(msg.contents.channel_flags & 2, 2);
},
_ => panic!("Unexpected event"),
}
match events[1] {
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
assert_eq!(msg.contents.channel_flags & 2, 2);
},
_ => panic!("Unexpected event"),
Expand Down
16 changes: 13 additions & 3 deletions lightning/src/ln/msgs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1917,6 +1917,16 @@ pub enum MessageSendEvent {
BroadcastChannelUpdate {
/// The channel_update which should be sent.
msg: ChannelUpdate,
/// The node_id of the first endpoint of the channel.
///
/// This is not used in the message broadcast, but rather is useful for deciding which
/// peer(s) to send the update to.
node_id_1: NodeId,
/// The node_id of the second endpoint of the channel.
///
/// This is not used in the message broadcast, but rather is useful for deciding which
/// peer(s) to send the update to.
node_id_2: NodeId,
Comment on lines +1920 to +1929
Copy link
Contributor

@valentinewallace valentinewallace Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these be replaced by a force_broadcast: bool field?

Copy link
Collaborator Author

@TheBlueMatt TheBlueMatt Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The network graph/P2PGossipSync (where this comes from sometimes) doesn't know our NodeId. We could give them that knowledge, but it seemed cleaner not to and to just provide the node_ids here, which is cheap.

Copy link
Contributor

@valentinewallace valentinewallace Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, I see it now in routing::utxo.

},
/// Used to indicate that a node_announcement should be broadcast to all peers.
BroadcastNodeAnnouncement {
Expand Down Expand Up @@ -2189,13 +2199,13 @@ pub trait RoutingMessageHandler: BaseMessageHandler {
fn handle_channel_announcement(
&self, their_node_id: Option<PublicKey>, msg: &ChannelAnnouncement,
) -> Result<bool, LightningError>;
/// Handle an incoming `channel_update` message, returning true if it should be forwarded on,
/// `false` or returning an `Err` otherwise.
/// Handle an incoming `channel_update` message, returning the node IDs of the channel
/// participants if the message should be forwarded on, `None` or returning an `Err` otherwise.
///
/// If `their_node_id` is `None`, the message was generated by our own local node.
fn handle_channel_update(
&self, their_node_id: Option<PublicKey>, msg: &ChannelUpdate,
) -> Result<bool, LightningError>;
) -> Result<Option<(NodeId, NodeId)>, LightningError>;
/// Gets channel announcements and updates required to dump our routing table to a remote node,
/// starting at the `short_channel_id` indicated by `starting_point` and including announcements
/// for a single channel.
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/onion_route_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1662,7 +1662,7 @@ fn do_test_onion_failure_stale_channel_update(announce_for_forwarding: bool) {
return None;
}
let new_update = match &events[0] {
MessageSendEvent::BroadcastChannelUpdate { msg } => {
MessageSendEvent::BroadcastChannelUpdate { msg, .. } => {
assert!(announce_for_forwarding);
msg.clone()
},
Expand Down
Loading