Skip to content

Commit 426239b

Browse files
committed
Call all of BaseMessageHandler's methods on the send-only handler
In 9db37f2 we introduced a `send_only_message_handler` which was intended to be used to send messages to peers. However, when we did so we forgot to add calls to its `peer_connected`, `peer_disconnected`, `provided_node_features`, and `provided_init_features` methods. Here we add those methods, updating the comment somewhat to avoid implying that they won't be called.
1 parent b917107 commit 426239b

File tree

1 file changed

+58
-5
lines changed

1 file changed

+58
-5
lines changed

lightning/src/ln/peer_handler.rs

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ where
571571
/// [`IgnoringMessageHandler`].
572572
pub custom_message_handler: CustomM,
573573

574-
/// A message handler which only allows sending messages. This should generally be a
574+
/// A message handler which can be used to send messages. This should generally be a
575575
/// [`ChainMonitor`].
576576
///
577577
/// [`ChainMonitor`]: crate::chain::chainmonitor::ChainMonitor
@@ -1363,6 +1363,7 @@ where
13631363
| self.message_handler.route_handler.provided_init_features(their_node_id)
13641364
| self.message_handler.onion_message_handler.provided_init_features(their_node_id)
13651365
| self.message_handler.custom_message_handler.provided_init_features(their_node_id)
1366+
| self.message_handler.send_only_message_handler.provided_init_features(their_node_id)
13661367
}
13671368

13681369
/// Indicates a new outbound connection has been established to a node with the given `node_id`
@@ -2165,6 +2166,19 @@ where
21652166
self.message_handler.onion_message_handler.peer_disconnected(their_node_id);
21662167
return Err(PeerHandleError {}.into());
21672168
}
2169+
let sends_handler = &self.message_handler.send_only_message_handler;
2170+
if let Err(()) = sends_handler.peer_connected(their_node_id, &msg, inbound) {
2171+
log_debug!(
2172+
logger,
2173+
"Sending-Only Message Handler decided we couldn't communicate with peer {}",
2174+
log_pubkey!(their_node_id)
2175+
);
2176+
self.message_handler.route_handler.peer_disconnected(their_node_id);
2177+
self.message_handler.chan_handler.peer_disconnected(their_node_id);
2178+
self.message_handler.onion_message_handler.peer_disconnected(their_node_id);
2179+
self.message_handler.custom_message_handler.peer_disconnected(their_node_id);
2180+
return Err(PeerHandleError {}.into());
2181+
}
21682182

21692183
peer_lock.awaiting_pong_timer_tick_intervals = 0;
21702184
peer_lock.their_features = Some(msg.features);
@@ -3364,6 +3378,7 @@ where
33643378
self.message_handler.chan_handler.peer_disconnected(node_id);
33653379
self.message_handler.onion_message_handler.peer_disconnected(node_id);
33663380
self.message_handler.custom_message_handler.peer_disconnected(node_id);
3381+
self.message_handler.send_only_message_handler.peer_disconnected(node_id);
33673382
}
33683383
descriptor.disconnect_socket();
33693384
}
@@ -3396,6 +3411,7 @@ where
33963411
self.message_handler.chan_handler.peer_disconnected(node_id);
33973412
self.message_handler.onion_message_handler.peer_disconnected(node_id);
33983413
self.message_handler.custom_message_handler.peer_disconnected(node_id);
3414+
self.message_handler.send_only_message_handler.peer_disconnected(node_id);
33993415
}
34003416
},
34013417
};
@@ -3581,7 +3597,8 @@ where
35813597
let features = self.message_handler.chan_handler.provided_node_features()
35823598
| self.message_handler.route_handler.provided_node_features()
35833599
| self.message_handler.onion_message_handler.provided_node_features()
3584-
| self.message_handler.custom_message_handler.provided_node_features();
3600+
| self.message_handler.custom_message_handler.provided_node_features()
3601+
| self.message_handler.send_only_message_handler.provided_node_features();
35853602
let announcement = msgs::UnsignedNodeAnnouncement {
35863603
features,
35873604
timestamp: self.last_node_announcement_serial.fetch_add(1, Ordering::AcqRel),
@@ -3705,6 +3722,7 @@ mod tests {
37053722
chan_handler: test_utils::TestChannelMessageHandler,
37063723
routing_handler: test_utils::TestRoutingMessageHandler,
37073724
custom_handler: TestCustomMessageHandler,
3725+
send_only_handler: TestBaseMsgHandler,
37083726
logger: test_utils::TestLogger,
37093727
node_signer: test_utils::TestNodeSigner,
37103728
}
@@ -3757,6 +3775,32 @@ mod tests {
37573775
}
37583776
}
37593777

3778+
struct TestBaseMsgHandler(test_utils::ConnectionTracker);
3779+
3780+
impl BaseMessageHandler for TestBaseMsgHandler {
3781+
fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent> {
3782+
Vec::new()
3783+
}
3784+
3785+
fn peer_disconnected(&self, their_node_id: PublicKey) {
3786+
self.0.peer_disconnected(their_node_id);
3787+
}
3788+
3789+
fn peer_connected(
3790+
&self, their_node_id: PublicKey, _msg: &Init, _inbound: bool,
3791+
) -> Result<(), ()> {
3792+
self.0.peer_connected(their_node_id)
3793+
}
3794+
3795+
fn provided_node_features(&self) -> NodeFeatures {
3796+
NodeFeatures::empty()
3797+
}
3798+
3799+
fn provided_init_features(&self, _: PublicKey) -> InitFeatures {
3800+
InitFeatures::empty()
3801+
}
3802+
}
3803+
37603804
fn create_peermgr_cfgs(peer_count: usize) -> Vec<PeerManagerCfg> {
37613805
let mut cfgs = Vec::new();
37623806
for i in 0..peer_count {
@@ -3773,6 +3817,7 @@ mod tests {
37733817
logger: test_utils::TestLogger::with_id(i.to_string()),
37743818
routing_handler: test_utils::TestRoutingMessageHandler::new(),
37753819
custom_handler: TestCustomMessageHandler::new(features),
3820+
send_only_handler: TestBaseMsgHandler(test_utils::ConnectionTracker::new()),
37763821
node_signer: test_utils::TestNodeSigner::new(node_secret),
37773822
});
37783823
}
@@ -3796,6 +3841,7 @@ mod tests {
37963841
logger: test_utils::TestLogger::new(),
37973842
routing_handler: test_utils::TestRoutingMessageHandler::new(),
37983843
custom_handler: TestCustomMessageHandler::new(features),
3844+
send_only_handler: TestBaseMsgHandler(test_utils::ConnectionTracker::new()),
37993845
node_signer: test_utils::TestNodeSigner::new(node_secret),
38003846
});
38013847
}
@@ -3814,6 +3860,7 @@ mod tests {
38143860
logger: test_utils::TestLogger::new(),
38153861
routing_handler: test_utils::TestRoutingMessageHandler::new(),
38163862
custom_handler: TestCustomMessageHandler::new(features),
3863+
send_only_handler: TestBaseMsgHandler(test_utils::ConnectionTracker::new()),
38173864
node_signer: test_utils::TestNodeSigner::new(node_secret),
38183865
});
38193866
}
@@ -3830,7 +3877,7 @@ mod tests {
38303877
route_handler: &cfgs[i].routing_handler,
38313878
onion_message_handler: IgnoringMessageHandler {},
38323879
custom_message_handler: &cfgs[i].custom_handler,
3833-
send_only_message_handler: IgnoringMessageHandler {},
3880+
send_only_message_handler: &cfgs[i].send_only_handler,
38343881
};
38353882
let peer = PeerManager::new(
38363883
msg_handler,
@@ -3853,7 +3900,7 @@ mod tests {
38533900
&'a test_utils::TestLogger,
38543901
&'a TestCustomMessageHandler,
38553902
&'a test_utils::TestNodeSigner,
3856-
IgnoringMessageHandler,
3903+
&'a TestBaseMsgHandler,
38573904
>;
38583905

38593906
fn try_establish_connection<'a>(
@@ -4229,6 +4276,7 @@ mod tests {
42294276
let chan_handler = peers[handler & 1].message_handler.chan_handler;
42304277
let route_handler = peers[handler & 1].message_handler.route_handler;
42314278
let custom_message_handler = peers[handler & 1].message_handler.custom_message_handler;
4279+
let send_only_msg_handler = peers[handler & 1].message_handler.send_only_message_handler;
42324280

42334281
match handler & !1 {
42344282
0 => {
@@ -4240,6 +4288,9 @@ mod tests {
42404288
4 => {
42414289
custom_message_handler.conn_tracker.fail_connections.store(true, Ordering::Release);
42424290
},
4291+
6 => {
4292+
send_only_msg_handler.0.fail_connections.store(true, Ordering::Release);
4293+
},
42434294
_ => panic!(),
42444295
}
42454296
let (_sd1, _sd2, a_refused, b_refused) = try_establish_connection(&peers[0], &peers[1]);
@@ -4255,16 +4306,18 @@ mod tests {
42554306
chan_handler.conn_tracker.had_peers.load(Ordering::Acquire)
42564307
|| route_handler.conn_tracker.had_peers.load(Ordering::Acquire)
42574308
|| custom_message_handler.conn_tracker.had_peers.load(Ordering::Acquire)
4309+
|| send_only_msg_handler.0.had_peers.load(Ordering::Acquire)
42584310
);
42594311
// And both message handlers doing tracking should see the disconnection
42604312
assert!(chan_handler.conn_tracker.connected_peers.lock().unwrap().is_empty());
42614313
assert!(route_handler.conn_tracker.connected_peers.lock().unwrap().is_empty());
42624314
assert!(custom_message_handler.conn_tracker.connected_peers.lock().unwrap().is_empty());
4315+
assert!(send_only_msg_handler.0.connected_peers.lock().unwrap().is_empty());
42634316
}
42644317

42654318
#[test]
42664319
fn test_peer_connected_error_disconnects() {
4267-
for i in 0..6 {
4320+
for i in 0..8 {
42684321
do_test_peer_connected_error_disconnects(i);
42694322
}
42704323
}

0 commit comments

Comments
 (0)