Skip to content

Commit 7d56270

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 b1185d6 commit 7d56270

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
@@ -581,7 +581,7 @@ where
581581
/// [`IgnoringMessageHandler`].
582582
pub custom_message_handler: CustomM,
583583

584-
/// A message handler which only allows sending messages. This should generally be a
584+
/// A message handler which can be used to send messages. This should generally be a
585585
/// [`ChainMonitor`].
586586
///
587587
/// [`ChainMonitor`]: crate::chain::chainmonitor::ChainMonitor
@@ -1373,6 +1373,7 @@ where
13731373
| self.message_handler.route_handler.provided_init_features(their_node_id)
13741374
| self.message_handler.onion_message_handler.provided_init_features(their_node_id)
13751375
| self.message_handler.custom_message_handler.provided_init_features(their_node_id)
1376+
| self.message_handler.send_only_message_handler.provided_init_features(their_node_id)
13761377
}
13771378

13781379
/// Indicates a new outbound connection has been established to a node with the given `node_id`
@@ -2175,6 +2176,19 @@ where
21752176
self.message_handler.onion_message_handler.peer_disconnected(their_node_id);
21762177
return Err(PeerHandleError {}.into());
21772178
}
2179+
let sends_handler = &self.message_handler.send_only_message_handler;
2180+
if let Err(()) = sends_handler.peer_connected(their_node_id, &msg, inbound) {
2181+
log_debug!(
2182+
logger,
2183+
"Sending-Only Message Handler decided we couldn't communicate with peer {}",
2184+
log_pubkey!(their_node_id)
2185+
);
2186+
self.message_handler.route_handler.peer_disconnected(their_node_id);
2187+
self.message_handler.chan_handler.peer_disconnected(their_node_id);
2188+
self.message_handler.onion_message_handler.peer_disconnected(their_node_id);
2189+
self.message_handler.custom_message_handler.peer_disconnected(their_node_id);
2190+
return Err(PeerHandleError {}.into());
2191+
}
21782192

21792193
peer_lock.awaiting_pong_timer_tick_intervals = 0;
21802194
peer_lock.their_features = Some(msg.features);
@@ -3394,6 +3408,7 @@ where
33943408
self.message_handler.chan_handler.peer_disconnected(node_id);
33953409
self.message_handler.onion_message_handler.peer_disconnected(node_id);
33963410
self.message_handler.custom_message_handler.peer_disconnected(node_id);
3411+
self.message_handler.send_only_message_handler.peer_disconnected(node_id);
33973412
}
33983413
descriptor.disconnect_socket();
33993414
}
@@ -3426,6 +3441,7 @@ where
34263441
self.message_handler.chan_handler.peer_disconnected(node_id);
34273442
self.message_handler.onion_message_handler.peer_disconnected(node_id);
34283443
self.message_handler.custom_message_handler.peer_disconnected(node_id);
3444+
self.message_handler.send_only_message_handler.peer_disconnected(node_id);
34293445
}
34303446
},
34313447
};
@@ -3611,7 +3627,8 @@ where
36113627
let features = self.message_handler.chan_handler.provided_node_features()
36123628
| self.message_handler.route_handler.provided_node_features()
36133629
| self.message_handler.onion_message_handler.provided_node_features()
3614-
| self.message_handler.custom_message_handler.provided_node_features();
3630+
| self.message_handler.custom_message_handler.provided_node_features()
3631+
| self.message_handler.send_only_message_handler.provided_node_features();
36153632
let announcement = msgs::UnsignedNodeAnnouncement {
36163633
features,
36173634
timestamp: self.last_node_announcement_serial.fetch_add(1, Ordering::AcqRel),
@@ -3735,6 +3752,7 @@ mod tests {
37353752
chan_handler: test_utils::TestChannelMessageHandler,
37363753
routing_handler: test_utils::TestRoutingMessageHandler,
37373754
custom_handler: TestCustomMessageHandler,
3755+
send_only_handler: TestBaseMsgHandler,
37383756
logger: test_utils::TestLogger,
37393757
node_signer: test_utils::TestNodeSigner,
37403758
}
@@ -3787,6 +3805,32 @@ mod tests {
37873805
}
37883806
}
37893807

3808+
struct TestBaseMsgHandler(test_utils::ConnectionTracker);
3809+
3810+
impl BaseMessageHandler for TestBaseMsgHandler {
3811+
fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent> {
3812+
Vec::new()
3813+
}
3814+
3815+
fn peer_disconnected(&self, their_node_id: PublicKey) {
3816+
self.0.peer_disconnected(their_node_id);
3817+
}
3818+
3819+
fn peer_connected(
3820+
&self, their_node_id: PublicKey, _msg: &Init, _inbound: bool,
3821+
) -> Result<(), ()> {
3822+
self.0.peer_connected(their_node_id)
3823+
}
3824+
3825+
fn provided_node_features(&self) -> NodeFeatures {
3826+
NodeFeatures::empty()
3827+
}
3828+
3829+
fn provided_init_features(&self, _: PublicKey) -> InitFeatures {
3830+
InitFeatures::empty()
3831+
}
3832+
}
3833+
37903834
fn create_peermgr_cfgs(peer_count: usize) -> Vec<PeerManagerCfg> {
37913835
let mut cfgs = Vec::new();
37923836
for i in 0..peer_count {
@@ -3803,6 +3847,7 @@ mod tests {
38033847
logger: test_utils::TestLogger::with_id(i.to_string()),
38043848
routing_handler: test_utils::TestRoutingMessageHandler::new(),
38053849
custom_handler: TestCustomMessageHandler::new(features),
3850+
send_only_handler: TestBaseMsgHandler(test_utils::ConnectionTracker::new()),
38063851
node_signer: test_utils::TestNodeSigner::new(node_secret),
38073852
});
38083853
}
@@ -3826,6 +3871,7 @@ mod tests {
38263871
logger: test_utils::TestLogger::new(),
38273872
routing_handler: test_utils::TestRoutingMessageHandler::new(),
38283873
custom_handler: TestCustomMessageHandler::new(features),
3874+
send_only_handler: TestBaseMsgHandler(test_utils::ConnectionTracker::new()),
38293875
node_signer: test_utils::TestNodeSigner::new(node_secret),
38303876
});
38313877
}
@@ -3844,6 +3890,7 @@ mod tests {
38443890
logger: test_utils::TestLogger::new(),
38453891
routing_handler: test_utils::TestRoutingMessageHandler::new(),
38463892
custom_handler: TestCustomMessageHandler::new(features),
3893+
send_only_handler: TestBaseMsgHandler(test_utils::ConnectionTracker::new()),
38473894
node_signer: test_utils::TestNodeSigner::new(node_secret),
38483895
});
38493896
}
@@ -3860,7 +3907,7 @@ mod tests {
38603907
route_handler: &cfgs[i].routing_handler,
38613908
onion_message_handler: IgnoringMessageHandler {},
38623909
custom_message_handler: &cfgs[i].custom_handler,
3863-
send_only_message_handler: IgnoringMessageHandler {},
3910+
send_only_message_handler: &cfgs[i].send_only_handler,
38643911
};
38653912
let peer = PeerManager::new(
38663913
msg_handler,
@@ -3883,7 +3930,7 @@ mod tests {
38833930
&'a test_utils::TestLogger,
38843931
&'a TestCustomMessageHandler,
38853932
&'a test_utils::TestNodeSigner,
3886-
IgnoringMessageHandler,
3933+
&'a TestBaseMsgHandler,
38873934
>;
38883935

38893936
fn try_establish_connection<'a>(
@@ -4259,6 +4306,7 @@ mod tests {
42594306
let chan_handler = peers[handler & 1].message_handler.chan_handler;
42604307
let route_handler = peers[handler & 1].message_handler.route_handler;
42614308
let custom_message_handler = peers[handler & 1].message_handler.custom_message_handler;
4309+
let send_only_msg_handler = peers[handler & 1].message_handler.send_only_message_handler;
42624310

42634311
match handler & !1 {
42644312
0 => {
@@ -4270,6 +4318,9 @@ mod tests {
42704318
4 => {
42714319
custom_message_handler.conn_tracker.fail_connections.store(true, Ordering::Release);
42724320
},
4321+
6 => {
4322+
send_only_msg_handler.0.fail_connections.store(true, Ordering::Release);
4323+
},
42734324
_ => panic!(),
42744325
}
42754326
let (_sd1, _sd2, a_refused, b_refused) = try_establish_connection(&peers[0], &peers[1]);
@@ -4285,16 +4336,18 @@ mod tests {
42854336
chan_handler.conn_tracker.had_peers.load(Ordering::Acquire)
42864337
|| route_handler.conn_tracker.had_peers.load(Ordering::Acquire)
42874338
|| custom_message_handler.conn_tracker.had_peers.load(Ordering::Acquire)
4339+
|| send_only_msg_handler.0.had_peers.load(Ordering::Acquire)
42884340
);
42894341
// And both message handlers doing tracking should see the disconnection
42904342
assert!(chan_handler.conn_tracker.connected_peers.lock().unwrap().is_empty());
42914343
assert!(route_handler.conn_tracker.connected_peers.lock().unwrap().is_empty());
42924344
assert!(custom_message_handler.conn_tracker.connected_peers.lock().unwrap().is_empty());
4345+
assert!(send_only_msg_handler.0.connected_peers.lock().unwrap().is_empty());
42934346
}
42944347

42954348
#[test]
42964349
fn test_peer_connected_error_disconnects() {
4297-
for i in 0..6 {
4350+
for i in 0..8 {
42984351
do_test_peer_connected_error_disconnects(i);
42994352
}
43004353
}

0 commit comments

Comments
 (0)