Skip to content

Commit d2a9ae0

Browse files
valentinewallacevalentinewallace
authored
Merge pull request #1717 from TheBlueMatt/2022-09-req-features-in-handlers
Move checking of specific require peer feature bits to handlers
2 parents a922830 + f725c5a commit d2a9ae0

17 files changed

+134
-91
lines changed

‎lightning-background-processor/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -754,8 +754,8 @@ mod tests {
754754

755755
for i in 0..num_nodes {
756756
for j in (i+1)..num_nodes {
757-
nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None });
758-
nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None });
757+
nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
758+
nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
759759
}
760760
}
761761

‎lightning-net-tokio/src/lib.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ mod tests {
577577
fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result<bool, LightningError> { Ok(false) }
578578
fn get_next_channel_announcement(&self, _starting_point: u64) -> Option<(ChannelAnnouncement, Option<ChannelUpdate>, Option<ChannelUpdate>)> { None }
579579
fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option<NodeAnnouncement> { None }
580-
fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init) { }
580+
fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init) -> Result<(), ()> { Ok(()) }
581581
fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: ReplyChannelRange) -> Result<(), LightningError> { Ok(()) }
582582
fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) }
583583
fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: QueryChannelRange) -> Result<(), LightningError> { Ok(()) }
@@ -608,10 +608,11 @@ mod tests {
608608
self.pubkey_disconnected.clone().try_send(()).unwrap();
609609
}
610610
}
611-
fn peer_connected(&self, their_node_id: &PublicKey, _msg: &Init) {
611+
fn peer_connected(&self, their_node_id: &PublicKey, _init_msg: &Init) -> Result<(), ()> {
612612
if *their_node_id == self.expected_pubkey {
613613
self.pubkey_connected.clone().try_send(()).unwrap();
614614
}
615+
Ok(())
615616
}
616617
fn handle_channel_reestablish(&self, _their_node_id: &PublicKey, _msg: &ChannelReestablish) {}
617618
fn handle_error(&self, _their_node_id: &PublicKey, _msg: &ErrorMessage) {}

‎lightning/src/ln/chanmon_update_fail_tests.rs

+18-18
Original file line numberDiff line numberDiff line change
@@ -347,10 +347,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
347347
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
348348
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
349349

350-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
350+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
351351
let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
352352
assert_eq!(reestablish_1.len(), 1);
353-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
353+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
354354
let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
355355
assert_eq!(reestablish_2.len(), 1);
356356

@@ -369,10 +369,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
369369
assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
370370
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
371371

372-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
372+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
373373
let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
374374
assert_eq!(reestablish_1.len(), 1);
375-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
375+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
376376
let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
377377
assert_eq!(reestablish_2.len(), 1);
378378

@@ -1123,8 +1123,8 @@ fn test_monitor_update_fail_reestablish() {
11231123
commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, false);
11241124

11251125
chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
1126-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
1127-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
1126+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
1127+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
11281128

11291129
let as_reestablish = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
11301130
let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
@@ -1142,8 +1142,8 @@ fn test_monitor_update_fail_reestablish() {
11421142
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
11431143
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
11441144

1145-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
1146-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
1145+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
1146+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
11471147

11481148
assert_eq!(get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap(), as_reestablish);
11491149
assert_eq!(get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap(), bs_reestablish);
@@ -1316,8 +1316,8 @@ fn claim_while_disconnected_monitor_update_fail() {
13161316
check_added_monitors!(nodes[1], 1);
13171317
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
13181318

1319-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
1320-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
1319+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
1320+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
13211321

13221322
let as_reconnect = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
13231323
let bs_reconnect = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
@@ -1448,8 +1448,8 @@ fn monitor_failed_no_reestablish_response() {
14481448
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
14491449
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
14501450

1451-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
1452-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
1451+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
1452+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
14531453

14541454
let as_reconnect = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
14551455
let bs_reconnect = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
@@ -2031,9 +2031,9 @@ fn test_pending_update_fee_ack_on_reconnect() {
20312031
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
20322032
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
20332033

2034-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
2034+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
20352035
let as_connect_msg = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
2036-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
2036+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
20372037
let bs_connect_msg = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
20382038

20392039
nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_connect_msg);
@@ -2159,9 +2159,9 @@ fn do_update_fee_resend_test(deliver_update: bool, parallel_updates: bool) {
21592159
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
21602160
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
21612161

2162-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
2162+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
21632163
let as_connect_msg = get_chan_reestablish_msgs!(nodes[0], nodes[1]).pop().unwrap();
2164-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
2164+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
21652165
let bs_connect_msg = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap();
21662166

21672167
nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_connect_msg);
@@ -2324,10 +2324,10 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
23242324
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
23252325

23262326
// Now reconnect the two
2327-
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
2327+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
23282328
let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
23292329
assert_eq!(reestablish_1.len(), 1);
2330-
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
2330+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
23312331
let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
23322332
assert_eq!(reestablish_2.len(), 1);
23332333

‎lightning/src/ln/channel.rs

+6
Original file line numberDiff line numberDiff line change
@@ -3550,6 +3550,12 @@ impl<Signer: Sign> Channel<Signer> {
35503550
return;
35513551
}
35523552

3553+
if self.channel_state & (ChannelState::PeerDisconnected as u32) == (ChannelState::PeerDisconnected as u32) {
3554+
// While the below code should be idempotent, it's simpler to just return early, as
3555+
// redundant disconnect events can fire, though they should be rare.
3556+
return;
3557+
}
3558+
35533559
if self.announcement_sigs_state == AnnouncementSigsState::MessageSent || self.announcement_sigs_state == AnnouncementSigsState::Committed {
35543560
self.announcement_sigs_state = AnnouncementSigsState::NotSent;
35553561
}

‎lightning/src/ln/channelmanager.rs

+13-7
Original file line numberDiff line numberDiff line change
@@ -6034,7 +6034,12 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
60346034
}
60356035
}
60366036

6037-
fn peer_connected(&self, counterparty_node_id: &PublicKey, init_msg: &msgs::Init) {
6037+
fn peer_connected(&self, counterparty_node_id: &PublicKey, init_msg: &msgs::Init) -> Result<(), ()> {
6038+
if !init_msg.features.supports_static_remote_key() {
6039+
log_debug!(self.logger, "Peer {} does not support static remote key, disconnecting with no_connection_possible", log_pubkey!(counterparty_node_id));
6040+
return Err(());
6041+
}
6042+
60386043
log_debug!(self.logger, "Generating channel_reestablish events for {}", log_pubkey!(counterparty_node_id));
60396044

60406045
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
@@ -6085,6 +6090,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
60856090
retain
60866091
});
60876092
//TODO: Also re-broadcast announcement_signatures
6093+
Ok(())
60886094
}
60896095

60906096
fn handle_error(&self, counterparty_node_id: &PublicKey, msg: &msgs::ErrorMessage) {
@@ -7480,8 +7486,8 @@ mod tests {
74807486

74817487
let payer_pubkey = nodes[0].node.get_our_node_id();
74827488
let payee_pubkey = nodes[1].node.get_our_node_id();
7483-
nodes[0].node.peer_connected(&payee_pubkey, &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
7484-
nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
7489+
nodes[0].node.peer_connected(&payee_pubkey, &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
7490+
nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
74857491

74867492
let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known());
74877493
let route_params = RouteParameters {
@@ -7524,8 +7530,8 @@ mod tests {
75247530

75257531
let payer_pubkey = nodes[0].node.get_our_node_id();
75267532
let payee_pubkey = nodes[1].node.get_our_node_id();
7527-
nodes[0].node.peer_connected(&payee_pubkey, &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
7528-
nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known(), remote_network_address: None });
7533+
nodes[0].node.peer_connected(&payee_pubkey, &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
7534+
nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
75297535

75307536
let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known());
75317537
let route_params = RouteParameters {
@@ -7803,8 +7809,8 @@ pub mod bench {
78037809
});
78047810
let node_b_holder = NodeHolder { node: &node_b };
78057811

7806-
node_a.peer_connected(&node_b.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None });
7807-
node_b.peer_connected(&node_a.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None });
7812+
node_a.peer_connected(&node_b.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
7813+
node_b.peer_connected(&node_a.get_our_node_id(), &Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
78087814
node_a.create_channel(node_b.get_our_node_id(), 8_000_000, 100_000_000, 42, None).unwrap();
78097815
node_b.handle_open_channel(&node_a.get_our_node_id(), InitFeatures::known(), &get_event_msg!(node_a_holder, MessageSendEvent::SendOpenChannel, node_b.get_our_node_id()));
78107816
node_a.handle_accept_channel(&node_b.get_our_node_id(), InitFeatures::known(), &get_event_msg!(node_b_holder, MessageSendEvent::SendAcceptChannel, node_a.get_our_node_id()));

‎lightning/src/ln/functional_test_utils.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -2108,8 +2108,8 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
21082108

21092109
for i in 0..node_count {
21102110
for j in (i+1)..node_count {
2111-
nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &msgs::Init { features: cfgs[j].features.clone(), remote_network_address: None });
2112-
nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &msgs::Init { features: cfgs[i].features.clone(), remote_network_address: None });
2111+
nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &msgs::Init { features: cfgs[j].features.clone(), remote_network_address: None }).unwrap();
2112+
nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &msgs::Init { features: cfgs[i].features.clone(), remote_network_address: None }).unwrap();
21132113
}
21142114
}
21152115

@@ -2383,9 +2383,9 @@ macro_rules! handle_chan_reestablish_msgs {
23832383
/// pending_htlc_adds includes both the holding cell and in-flight update_add_htlcs, whereas
23842384
/// for claims/fails they are separated out.
23852385
pub fn reconnect_nodes<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, send_channel_ready: (bool, bool), pending_htlc_adds: (i64, i64), pending_htlc_claims: (usize, usize), pending_htlc_fails: (usize, usize), pending_cell_htlc_claims: (usize, usize), pending_cell_htlc_fails: (usize, usize), pending_raa: (bool, bool)) {
2386-
node_a.node.peer_connected(&node_b.node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
2386+
node_a.node.peer_connected(&node_b.node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
23872387
let reestablish_1 = get_chan_reestablish_msgs!(node_a, node_b);
2388-
node_b.node.peer_connected(&node_a.node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
2388+
node_b.node.peer_connected(&node_a.node.get_our_node_id(), &msgs::Init { features: InitFeatures::known(), remote_network_address: None }).unwrap();
23892389
let reestablish_2 = get_chan_reestablish_msgs!(node_b, node_a);
23902390

23912391
if send_channel_ready.0 {

0 commit comments

Comments
 (0)