Skip to content

Commit d4828c0

Browse files
authored
Merge pull request #4216 from TheBlueMatt/2025-11-forward-peer-gossip
Always forward gossip for all our channels to all our peers
2 parents 61aca4d + 587e6fc commit d4828c0

File tree

13 files changed

+248
-108
lines changed

13 files changed

+248
-108
lines changed

lightning-net-tokio/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -660,8 +660,8 @@ mod tests {
660660
}
661661
fn handle_channel_update(
662662
&self, _their_node_id: Option<PublicKey>, _msg: &ChannelUpdate,
663-
) -> Result<bool, LightningError> {
664-
Ok(false)
663+
) -> Result<Option<(NodeId, NodeId)>, LightningError> {
664+
Ok(None)
665665
}
666666
fn get_next_channel_announcement(
667667
&self, _starting_point: u64,

lightning/src/ln/channel_open_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2337,7 +2337,7 @@ pub fn test_funding_and_commitment_tx_confirm_same_block() {
23372337
} else {
23382338
panic!();
23392339
}
2340-
if let MessageSendEvent::BroadcastChannelUpdate { ref msg } = msg_events.remove(0) {
2340+
if let MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } = msg_events.remove(0) {
23412341
assert_eq!(msg.contents.channel_flags & 2, 2);
23422342
} else {
23432343
panic!();

lightning/src/ln/channelmanager.rs

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ use crate::onion_message::messenger::{
111111
MessageRouter, MessageSendInstructions, Responder, ResponseInstruction,
112112
};
113113
use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
114+
use crate::routing::gossip::NodeId;
114115
use crate::routing::router::{
115116
BlindedTail, FixedRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route,
116117
RouteParameters, RouteParametersConfig, Router,
@@ -942,7 +943,7 @@ impl Into<LocalHTLCFailureReason> for FailureCode {
942943
struct MsgHandleErrInternal {
943944
err: msgs::LightningError,
944945
closes_channel: bool,
945-
shutdown_finish: Option<(ShutdownResult, Option<msgs::ChannelUpdate>)>,
946+
shutdown_finish: Option<(ShutdownResult, Option<(msgs::ChannelUpdate, NodeId, NodeId)>)>,
946947
tx_abort: Option<msgs::TxAbort>,
947948
}
948949
impl MsgHandleErrInternal {
@@ -966,7 +967,7 @@ impl MsgHandleErrInternal {
966967

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

32463247
$self.finish_close_channel(shutdown_res);
3247-
if let Some(update) = update_option {
3248+
if let Some((update, node_id_1, node_id_2)) = update_option {
32483249
let mut pending_broadcast_messages = $self.pending_broadcast_messages.lock().unwrap();
32493250
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
3250-
msg: update
3251+
msg: update, node_id_1, node_id_2
32513252
});
32523253
}
32533254
} else {
@@ -3574,7 +3575,7 @@ macro_rules! handle_monitor_update_completion {
35743575
// channel_update later through the announcement_signatures process for public
35753576
// channels, but there's no reason not to just inform our counterparty of our fees
35763577
// now.
3577-
if let Ok(msg) = $self.get_channel_update_for_unicast($chan) {
3578+
if let Ok((msg, _, _)) = $self.get_channel_update_for_unicast($chan) {
35783579
Some(MessageSendEvent::SendChannelUpdate {
35793580
node_id: counterparty_node_id,
35803581
msg,
@@ -5125,7 +5126,9 @@ where
51255126
}
51265127
}
51275128

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

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

51825188
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
51835189
log_trace!(logger, "Generating channel update for channel {}", chan.context.channel_id());
5184-
let were_node_one = self.our_network_pubkey.serialize()[..] < chan.context.get_counterparty_node_id().serialize()[..];
5190+
let our_node_id = NodeId::from_pubkey(&self.our_network_pubkey);
5191+
let their_node_id = NodeId::from_pubkey(&chan.context.get_counterparty_node_id());
5192+
let were_node_one = our_node_id < their_node_id;
51855193
let enabled = chan.context.is_enabled();
51865194

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

5206-
Ok(msgs::ChannelUpdate {
5207-
signature: sig,
5208-
contents: unsigned
5209-
})
5214+
Ok((
5215+
msgs::ChannelUpdate {
5216+
signature: sig,
5217+
contents: unsigned
5218+
},
5219+
if were_node_one { our_node_id } else { their_node_id },
5220+
if were_node_one { their_node_id } else { our_node_id },
5221+
))
52105222
}
52115223

52125224
#[cfg(any(test, feature = "_externalize_tests"))]
@@ -6649,11 +6661,11 @@ where
66496661
continue;
66506662
}
66516663
if let Some(channel) = channel.as_funded() {
6652-
if let Ok(msg) = self.get_channel_update_for_broadcast(channel) {
6664+
if let Ok((msg, node_id_1, node_id_2)) = self.get_channel_update_for_broadcast(channel) {
66536665
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
6654-
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate { msg });
6666+
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate { msg, node_id_1, node_id_2 });
66556667
} else if peer_state.is_connected {
6656-
if let Ok(msg) = self.get_channel_update_for_unicast(channel) {
6668+
if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(channel) {
66576669
peer_state.pending_msg_events.push(MessageSendEvent::SendChannelUpdate {
66586670
node_id: channel.context.get_counterparty_node_id(),
66596671
msg,
@@ -8177,10 +8189,10 @@ where
81778189
n += 1;
81788190
if n >= DISABLE_GOSSIP_TICKS {
81798191
funded_chan.set_channel_update_status(ChannelUpdateStatus::Disabled);
8180-
if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) {
8192+
if let Ok((update, node_id_1, node_id_2)) = self.get_channel_update_for_broadcast(&funded_chan) {
81818193
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
81828194
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
8183-
msg: update
8195+
msg: update, node_id_1, node_id_2
81848196
});
81858197
}
81868198
should_persist = NotifyOption::DoPersist;
@@ -8192,10 +8204,10 @@ where
81928204
n += 1;
81938205
if n >= ENABLE_GOSSIP_TICKS {
81948206
funded_chan.set_channel_update_status(ChannelUpdateStatus::Enabled);
8195-
if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) {
8207+
if let Ok((update, node_id_1, node_id_2)) = self.get_channel_update_for_broadcast(&funded_chan) {
81968208
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
81978209
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
8198-
msg: update
8210+
msg: update, node_id_1, node_id_2
81998211
});
82008212
}
82018213
should_persist = NotifyOption::DoPersist;
@@ -10821,7 +10833,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1082110833
// channel_update here if the channel is not public, i.e. we're not sending an
1082210834
// announcement_signatures.
1082310835
log_trace!(logger, "Sending private initial channel_update for our counterparty on channel {}", chan.context.channel_id());
10824-
if let Ok(msg) = self.get_channel_update_for_unicast(chan) {
10836+
if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(chan) {
1082510837
peer_state.pending_msg_events.push(MessageSendEvent::SendChannelUpdate {
1082610838
node_id: counterparty_node_id.clone(),
1082710839
msg,
@@ -11620,7 +11632,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1162011632
msg: try_channel_entry!(self, peer_state, res, chan_entry),
1162111633
// Note that announcement_signatures fails if the channel cannot be announced,
1162211634
// so get_channel_update_for_broadcast will never fail by the time we get here.
11623-
update_msg: Some(self.get_channel_update_for_broadcast(chan).unwrap()),
11635+
update_msg: Some(self.get_channel_update_for_broadcast(chan).unwrap().0),
1162411636
});
1162511637
} else {
1162611638
return try_channel_entry!(self, peer_state, Err(ChannelError::close(
@@ -11729,7 +11741,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1172911741
// If the channel is in a usable state (ie the channel is not being shut
1173011742
// down), send a unicast channel_update to our counterparty to make sure
1173111743
// they have the latest channel parameters.
11732-
if let Ok(msg) = self.get_channel_update_for_unicast(chan) {
11744+
if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(chan) {
1173311745
channel_update = Some(MessageSendEvent::SendChannelUpdate {
1173411746
node_id: chan.context.get_counterparty_node_id(),
1173511747
msg,
@@ -14340,7 +14352,7 @@ where
1434014352
send_channel_ready!(self, pending_msg_events, funded_channel, channel_ready);
1434114353
if funded_channel.context.is_usable() && peer_state.is_connected {
1434214354
log_trace!(logger, "Sending channel_ready with private initial channel_update for our counterparty on channel {}", channel_id);
14343-
if let Ok(msg) = self.get_channel_update_for_unicast(funded_channel) {
14355+
if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(funded_channel) {
1434414356
pending_msg_events.push(MessageSendEvent::SendChannelUpdate {
1434514357
node_id: funded_channel.context.get_counterparty_node_id(),
1434614358
msg,
@@ -14433,7 +14445,7 @@ where
1443314445
// if the channel cannot be announced, so
1443414446
// get_channel_update_for_broadcast will never fail
1443514447
// by the time we get here.
14436-
update_msg: Some(self.get_channel_update_for_broadcast(funded_channel).unwrap()),
14448+
update_msg: Some(self.get_channel_update_for_broadcast(funded_channel).unwrap().0),
1443714449
});
1443814450
}
1443914451
}

lightning/src/ln/functional_test_utils.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,7 +2172,7 @@ macro_rules! get_closing_signed_broadcast {
21722172
assert!(events.len() == 1 || events.len() == 2);
21732173
(
21742174
match events[events.len() - 1] {
2175-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
2175+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
21762176
assert_eq!(msg.contents.channel_flags & 2, 2);
21772177
msg.clone()
21782178
},
@@ -2243,7 +2243,7 @@ pub fn check_closed_broadcast(
22432243
.into_iter()
22442244
.filter_map(|msg_event| {
22452245
match msg_event {
2246-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
2246+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
22472247
assert_eq!(msg.contents.channel_flags & 2, 2);
22482248
None
22492249
},
@@ -4827,7 +4827,7 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(
48274827
let events_1 = nodes[a].node.get_and_clear_pending_msg_events();
48284828
assert_eq!(events_1.len(), 2);
48294829
let as_update = match events_1[1] {
4830-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => msg.clone(),
4830+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => msg.clone(),
48314831
_ => panic!("Unexpected event"),
48324832
};
48334833
match events_1[0] {
@@ -4864,7 +4864,7 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(
48644864
let events_2 = nodes[b].node.get_and_clear_pending_msg_events();
48654865
assert_eq!(events_2.len(), if needs_err_handle { 1 } else { 2 });
48664866
let bs_update = match events_2.last().unwrap() {
4867-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => msg.clone(),
4867+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => msg.clone(),
48684868
_ => panic!("Unexpected event"),
48694869
};
48704870
if !needs_err_handle {

lightning/src/ln/functional_tests.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ pub fn channel_monitor_network_test() {
717717
let events = nodes[3].node.get_and_clear_pending_msg_events();
718718
assert_eq!(events.len(), 2);
719719
let close_chan_update_1 = match events[1] {
720-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => msg.clone(),
720+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => msg.clone(),
721721
_ => panic!("Unexpected event"),
722722
};
723723
match events[0] {
@@ -752,7 +752,7 @@ pub fn channel_monitor_network_test() {
752752
let events = nodes[4].node.get_and_clear_pending_msg_events();
753753
assert_eq!(events.len(), 2);
754754
let close_chan_update_2 = match events[1] {
755-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => msg.clone(),
755+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => msg.clone(),
756756
_ => panic!("Unexpected event"),
757757
};
758758
match events[0] {
@@ -2167,7 +2167,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(
21672167

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

@@ -6032,7 +6032,7 @@ pub fn test_announce_disable_channels() {
60326032
let mut chans_disabled = new_hash_map();
60336033
for e in msg_events {
60346034
match e {
6035-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
6035+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
60366036
assert_eq!(msg.contents.channel_flags & (1 << 1), 1 << 1); // The "channel disabled" bit should be set
60376037
// Check that each channel gets updated exactly once
60386038
if chans_disabled
@@ -6083,7 +6083,7 @@ pub fn test_announce_disable_channels() {
60836083
assert_eq!(msg_events.len(), 3);
60846084
for e in msg_events {
60856085
match e {
6086-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
6086+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
60876087
assert_eq!(msg.contents.channel_flags & (1 << 1), 0); // The "channel disabled" bit should be off
60886088
match chans_disabled.remove(&msg.contents.short_channel_id) {
60896089
// Each update should have a higher timestamp than the previous one, replacing
@@ -8009,13 +8009,13 @@ pub fn test_error_chans_closed() {
80098009
let events = nodes[0].node.get_and_clear_pending_msg_events();
80108010
assert_eq!(events.len(), 2);
80118011
match events[0] {
8012-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
8012+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
80138013
assert_eq!(msg.contents.channel_flags & 2, 2);
80148014
},
80158015
_ => panic!("Unexpected event"),
80168016
}
80178017
match events[1] {
8018-
MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
8018+
MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => {
80198019
assert_eq!(msg.contents.channel_flags & 2, 2);
80208020
},
80218021
_ => panic!("Unexpected event"),

lightning/src/ln/msgs.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1917,6 +1917,16 @@ pub enum MessageSendEvent {
19171917
BroadcastChannelUpdate {
19181918
/// The channel_update which should be sent.
19191919
msg: ChannelUpdate,
1920+
/// The node_id of the first endpoint of the channel.
1921+
///
1922+
/// This is not used in the message broadcast, but rather is useful for deciding which
1923+
/// peer(s) to send the update to.
1924+
node_id_1: NodeId,
1925+
/// The node_id of the second endpoint of the channel.
1926+
///
1927+
/// This is not used in the message broadcast, but rather is useful for deciding which
1928+
/// peer(s) to send the update to.
1929+
node_id_2: NodeId,
19201930
},
19211931
/// Used to indicate that a node_announcement should be broadcast to all peers.
19221932
BroadcastNodeAnnouncement {
@@ -2189,13 +2199,13 @@ pub trait RoutingMessageHandler: BaseMessageHandler {
21892199
fn handle_channel_announcement(
21902200
&self, their_node_id: Option<PublicKey>, msg: &ChannelAnnouncement,
21912201
) -> Result<bool, LightningError>;
2192-
/// Handle an incoming `channel_update` message, returning true if it should be forwarded on,
2193-
/// `false` or returning an `Err` otherwise.
2202+
/// Handle an incoming `channel_update` message, returning the node IDs of the channel
2203+
/// participants if the message should be forwarded on, `None` or returning an `Err` otherwise.
21942204
///
21952205
/// If `their_node_id` is `None`, the message was generated by our own local node.
21962206
fn handle_channel_update(
21972207
&self, their_node_id: Option<PublicKey>, msg: &ChannelUpdate,
2198-
) -> Result<bool, LightningError>;
2208+
) -> Result<Option<(NodeId, NodeId)>, LightningError>;
21992209
/// Gets channel announcements and updates required to dump our routing table to a remote node,
22002210
/// starting at the `short_channel_id` indicated by `starting_point` and including announcements
22012211
/// for a single channel.

lightning/src/ln/onion_route_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1664,7 +1664,7 @@ fn do_test_onion_failure_stale_channel_update(announce_for_forwarding: bool) {
16641664
return None;
16651665
}
16661666
let new_update = match &events[0] {
1667-
MessageSendEvent::BroadcastChannelUpdate { msg } => {
1667+
MessageSendEvent::BroadcastChannelUpdate { msg, .. } => {
16681668
assert!(announce_for_forwarding);
16691669
msg.clone()
16701670
},

0 commit comments

Comments
 (0)