Fix pre-noise peer disconnect panic on non-Err disconnect

366e796 fixed the same crash for
Errs that come up during handshake, but was incomplete and should
have just dropped the node_id being different based on
inbound/outbound. This patch does so and actually fixes the issue.

Found by fuzzer.

Author: TheBlueMatt

src/ln/peer_channel_encryptor.rs

@@ -279,7 +279,7 @@ impl PeerChannelEncryptor {
 		self.process_act_one_with_ephemeral_key(act_one, our_node_secret, our_ephemeral_key)
 	}
 
-	pub fn process_act_two(&mut self, act_two: &[u8], our_node_secret: &SecretKey) -> Result<[u8; 66], HandleError> {
+	pub fn process_act_two(&mut self, act_two: &[u8], our_node_secret: &SecretKey) -> Result<([u8; 66], PublicKey), HandleError> {
 		assert_eq!(act_two.len(), 50);
 
 		let mut final_hkdf = [0; 64];
@@ -334,7 +334,7 @@ impl PeerChannelEncryptor {
 			rck: ck,
 		};
 
-		Ok(res)
+		Ok((res, self.their_node_id.unwrap().clone()))
 	}
 
 	pub fn process_act_three(&mut self, act_three: &[u8]) -> Result<PublicKey, HandleError> {
@@ -537,7 +537,7 @@ mod tests {
 			let mut outbound_peer = get_outbound_peer_for_initiator_test_vectors();
 
 			let act_two = hex::decode("0002466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f276e2470b93aac583c9ef6eafca3f730ae").unwrap().to_vec();
-			assert_eq!(outbound_peer.process_act_two(&act_two[..], &our_node_id).unwrap()[..], hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap()[..]);
+			assert_eq!(outbound_peer.process_act_two(&act_two[..], &our_node_id).unwrap().0[..], hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap()[..]);
 
 			match outbound_peer.noise_state {
 				NoiseState::Finished { sk, sn, sck, rk, rn, rck } => {
@@ -694,7 +694,7 @@ mod tests {
 			let our_node_id = SecretKey::from_slice(&secp_ctx, &hex::decode("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap();
 
 			let act_two = hex::decode("0002466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f276e2470b93aac583c9ef6eafca3f730ae").unwrap().to_vec();
-			assert_eq!(outbound_peer.process_act_two(&act_two[..], &our_node_id).unwrap()[..], hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap()[..]);
+			assert_eq!(outbound_peer.process_act_two(&act_two[..], &our_node_id).unwrap().0[..], hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap()[..]);
 
 			match outbound_peer.noise_state {
 				NoiseState::Finished { sk, sn, sck, rk, rn, rck } => {

src/ln/peer_handler.rs

@@ -234,7 +234,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 		if peers.peers.insert(descriptor, Peer {
 			channel_encryptor: peer_encryptor,
 			outbound: true,
-			their_node_id: Some(their_node_id),
+			their_node_id: None,
 			their_global_features: None,
 			their_local_features: None,
 
@@ -438,9 +438,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 
 							macro_rules! try_potential_handleerror {
 								($thing: expr) => {
-									try_potential_handleerror!($thing, false);
-								};
-								($thing: expr, $pre_noise: expr) => {
 									match $thing {
 										Ok(x) => x,
 										Err(e) => {
@@ -449,9 +446,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 													msgs::ErrorAction::DisconnectPeer { msg: _ } => {
 														//TODO: Try to push msg
 														log_trace!(self, "Got Err handling message, disconnecting peer because {}", e.err);
-														if $pre_noise {
-															peer.their_node_id = None; // Unset so that we don't generate a peer_disconnected event
-														}
 														return Err(PeerHandleError{ no_connection_possible: false });
 													},
 													msgs::ErrorAction::IgnoreError => {
@@ -517,16 +511,17 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 							let next_step = peer.channel_encryptor.get_noise_step();
 							match next_step {
 								NextNoiseStep::ActOne => {
-									let act_two = try_potential_handleerror!(peer.channel_encryptor.process_act_one_with_key(&peer.pending_read_buffer[..], &self.our_node_secret), true).to_vec();
+									let act_two = try_potential_handleerror!(peer.channel_encryptor.process_act_one_with_key(&peer.pending_read_buffer[..], &self.our_node_secret)).to_vec();
 									peer.pending_outbound_buffer.push_back(act_two);
 									peer.pending_read_buffer = [0; 66].to_vec(); // act three is 66 bytes long
 								},
 								NextNoiseStep::ActTwo => {
-									let act_three = try_potential_handleerror!(peer.channel_encryptor.process_act_two(&peer.pending_read_buffer[..], &self.our_node_secret), true).to_vec();
-									peer.pending_outbound_buffer.push_back(act_three);
+									let (act_three, their_node_id) = try_potential_handleerror!(peer.channel_encryptor.process_act_two(&peer.pending_read_buffer[..], &self.our_node_secret));
+									peer.pending_outbound_buffer.push_back(act_three.to_vec());
 									peer.pending_read_buffer = [0; 18].to_vec(); // Message length header is 18 bytes
 									peer.pending_read_is_header = true;
 
+									peer.their_node_id = Some(their_node_id);
 									insert_node_id!();
 									let mut local_features = msgs::LocalFeatures::new();
 									if self.initial_syncs_sent.load(Ordering::Acquire) < INITIAL_SYNCS_TO_SEND {
@@ -539,7 +534,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 									}, 16);
 								},
 								NextNoiseStep::ActThree => {
-									let their_node_id = try_potential_handleerror!(peer.channel_encryptor.process_act_three(&peer.pending_read_buffer[..]), true);
+									let their_node_id = try_potential_handleerror!(peer.channel_encryptor.process_act_three(&peer.pending_read_buffer[..]));
 									peer.pending_read_buffer = [0; 18].to_vec(); // Message length header is 18 bytes
 									peer.pending_read_is_header = true;
 									peer.their_node_id = Some(their_node_id);