Skip to content

Commit 5605ae7

Browse files
committed
Cache and Broadcast channel update later
- Cache the force-close channel update message, and broadcast them later. - Update get_and_clear_pending_msg_events to broadcast the pending_broadcast_events along with other messages. - Also appropriately update the tests which are affected by this change
1 parent 40574d0 commit 5605ae7

File tree

2 files changed

+41
-25
lines changed

2 files changed

+41
-25
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,6 +1381,10 @@ where
13811381

13821382
pending_offers_messages: Mutex<Vec<PendingOnionMessage<OffersMessage>>>,
13831383

1384+
/// Tracks the channel_update message that were not broadcasted because
1385+
/// we were not connected to any peers.
1386+
pending_broadcast_messages: Mutex<Vec<MessageSendEvent>>,
1387+
13841388
entropy_source: ES,
13851389
node_signer: NS,
13861390
signer_provider: SP,
@@ -2455,6 +2459,7 @@ where
24552459
funding_batch_states: Mutex::new(BTreeMap::new()),
24562460

24572461
pending_offers_messages: Mutex::new(Vec::new()),
2462+
pending_broadcast_messages: Mutex::new(Vec::new()),
24582463

24592464
entropy_source,
24602465
node_signer,
@@ -2946,17 +2951,11 @@ where
29462951
}
29472952
};
29482953
if let Some(update) = update_opt {
2949-
// Try to send the `BroadcastChannelUpdate` to the peer we just force-closed on, but if
2950-
// not try to broadcast it via whatever peer we have.
2951-
let per_peer_state = self.per_peer_state.read().unwrap();
2952-
let a_peer_state_opt = per_peer_state.get(peer_node_id)
2953-
.ok_or(per_peer_state.values().next());
2954-
if let Ok(a_peer_state_mutex) = a_peer_state_opt {
2955-
let mut a_peer_state = a_peer_state_mutex.lock().unwrap();
2956-
a_peer_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
2957-
msg: update
2958-
});
2959-
}
2954+
// If we have some Channel Update to broadcast, we cache it and broadcast it later.
2955+
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
2956+
pending_broadcast_messages.push(events::MessageSendEvent::BroadcastChannelUpdate {
2957+
msg: update
2958+
});
29602959
}
29612960

29622961
Ok(counterparty_node_id)
@@ -8194,6 +8193,7 @@ where
81948193
result = NotifyOption::DoPersist;
81958194
}
81968195

8196+
let mut is_some_peer_connected = false;
81978197
let mut pending_events = Vec::new();
81988198
let per_peer_state = self.per_peer_state.read().unwrap();
81998199
for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
@@ -8202,6 +8202,15 @@ where
82028202
if peer_state.pending_msg_events.len() > 0 {
82038203
pending_events.append(&mut peer_state.pending_msg_events);
82048204
}
8205+
if peer_state.is_connected {
8206+
is_some_peer_connected = true
8207+
}
8208+
}
8209+
8210+
// Ensure that we are connected to some peers before getting broadcast messages.
8211+
if is_some_peer_connected {
8212+
let mut broadcast_msgs = self.pending_broadcast_messages.lock().unwrap();
8213+
pending_events.append(&mut broadcast_msgs);
82058214
}
82068215

82078216
if !pending_events.is_empty() {
@@ -11099,6 +11108,8 @@ where
1109911108

1110011109
pending_offers_messages: Mutex::new(Vec::new()),
1110111110

11111+
pending_broadcast_messages: Mutex::new(Vec::new()),
11112+
1110211113
entropy_source: args.entropy_source,
1110311114
node_signer: args.node_signer,
1110411115
signer_provider: args.signer_provider,
@@ -11630,10 +11641,10 @@ mod tests {
1163011641

1163111642
#[test]
1163211643
fn test_drop_disconnected_peers_when_removing_channels() {
11633-
let chanmon_cfgs = create_chanmon_cfgs(2);
11634-
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
11635-
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
11636-
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
11644+
let chanmon_cfgs = create_chanmon_cfgs(3);
11645+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
11646+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
11647+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
1163711648

1163811649
let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
1163911650

@@ -11650,15 +11661,15 @@ mod tests {
1165011661
// disconnected and the channel between has been force closed.
1165111662
let nodes_0_per_peer_state = nodes[0].node.per_peer_state.read().unwrap();
1165211663
// Assert that nodes[1] isn't removed before `timer_tick_occurred` has been executed.
11653-
assert_eq!(nodes_0_per_peer_state.len(), 1);
11664+
assert_eq!(nodes_0_per_peer_state.len(), 2);
1165411665
assert!(nodes_0_per_peer_state.get(&nodes[1].node.get_our_node_id()).is_some());
1165511666
}
1165611667

1165711668
nodes[0].node.timer_tick_occurred();
1165811669

1165911670
{
1166011671
// Assert that nodes[1] has now been removed.
11661-
assert_eq!(nodes[0].node.per_peer_state.read().unwrap().len(), 0);
11672+
assert_eq!(nodes[0].node.per_peer_state.read().unwrap().len(), 1);
1166211673
}
1166311674
}
1166411675

@@ -12362,11 +12373,11 @@ mod tests {
1236212373

1236312374
#[test]
1236412375
fn test_trigger_lnd_force_close() {
12365-
let chanmon_cfg = create_chanmon_cfgs(2);
12366-
let node_cfg = create_node_cfgs(2, &chanmon_cfg);
12376+
let chanmon_cfg = create_chanmon_cfgs(3);
12377+
let node_cfg = create_node_cfgs(3, &chanmon_cfg);
1236712378
let user_config = test_default_channel_config();
12368-
let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[Some(user_config), Some(user_config)]);
12369-
let nodes = create_network(2, &node_cfg, &node_chanmgr);
12379+
let node_chanmgr = create_node_chanmgrs(3, &node_cfg, &[Some(user_config), Some(user_config), Some(user_config)]);
12380+
let nodes = create_network(3, &node_cfg, &node_chanmgr);
1237012381

1237112382
// Open a channel, immediately disconnect each other, and broadcast Alice's latest state.
1237212383
let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 1);

lightning/src/ln/reorg_tests.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -763,21 +763,21 @@ fn test_htlc_preimage_claim_prev_counterparty_commitment_after_current_counterpa
763763
fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_counterparty_commitment: bool) {
764764
// Tests that a node will retry broadcasting its own commitment after seeing a confirmed
765765
// counterparty commitment be reorged out.
766-
let mut chanmon_cfgs = create_chanmon_cfgs(2);
766+
let mut chanmon_cfgs = create_chanmon_cfgs(3);
767767
if revoked_counterparty_commitment {
768768
chanmon_cfgs[1].keys_manager.disable_revocation_policy_check = true;
769769
}
770-
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
770+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
771771
let mut config = test_default_channel_config();
772772
if anchors {
773773
config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
774774
config.manually_accept_inbound_channels = true;
775775
}
776776
let persister;
777777
let new_chain_monitor;
778-
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), Some(config)]);
778+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(config)]);
779779
let nodes_1_deserialized;
780-
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
780+
let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
781781

782782
let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 1);
783783

@@ -801,6 +801,11 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c
801801
reload_node!(
802802
nodes[1], config, &serialized_node, &[&serialized_monitor], persister, new_chain_monitor, nodes_1_deserialized
803803
);
804+
805+
// Reconnect node[1] with node[2] to allow successful channel_update broadcast later
806+
nodes[1].node.peer_connected(&nodes[2].node.get_our_node_id(), &Init {
807+
features: nodes[2].node.init_features(), networks: None, remote_network_address: None
808+
}, true).unwrap();
804809
}
805810

806811
// Connect blocks until the HTLC expiry is met, prompting a commitment broadcast by A.

0 commit comments

Comments
 (0)