Merge pull request #259 from TheBlueMatt/2018-11-256-redux
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Fri, 23 Nov 2018 20:21:35 +0000 (15:21 -0500)
committerGitHub <noreply@github.com>
Fri, 23 Nov 2018 20:21:35 +0000 (15:21 -0500)
Add test_claim_on_remote_revoked_sizeable_push_msat

Cargo.toml
README.md
fuzz/Cargo.toml
src/ln/channel.rs
src/ln/channelmanager.rs
src/ln/channelmonitor.rs
src/ln/peer_channel_encryptor.rs
src/ln/peer_handler.rs

index 9ecd10fb2bede052abc6d0ddf2f2683e1c6b543f..b3c5f3ac0356ddb4de3cd5f5e6fd7e572c1721b1 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.0.6"
+version = "0.0.7"
 authors = ["Matt Corallo"]
 license = "Apache-2.0"
 repository = "https://github.com/rust-bitcoin/rust-lightning/"
index 6a9369ea9b8ca8a7e35ace4a261f5cc0e0d38090..5b84449249050102d6a01f99163921f3179a3c86 100644 (file)
--- a/README.md
+++ b/README.md
@@ -1,6 +1,6 @@
 Rust-Lightning, not Rusty's Lightning!
 
-Currently somewhere near 10% towards usable, published to see if there is any
+Currently somewhere near 15% towards usable, published to see if there is any
 real interest from folks in using a lightning rust library.
 
 The goal is to provide a full-featured but also incredibly flexible lightning
index ecf3809c59b3ec8cc88ca349b3ad1eff3b9cb7b0..c8ed4b613cf5d795eae7047a4377f0e937d510bb 100644 (file)
@@ -31,6 +31,10 @@ cc = "1.0"
 [workspace]
 members = ["."]
 
+[profile.release]
+lto = true
+codegen-units = 1
+
 [[bin]]
 name = "peer_crypt_target"
 path = "fuzz_targets/peer_crypt_target.rs"
index e60f5628e53c029249e80f63ff4ef8392deaa65b..e0fdcaf9080627a3dae94786adadc080707aca82 100644 (file)
@@ -633,7 +633,6 @@ impl Channel {
                                                              &chan_keys.htlc_base_key, &chan_keys.payment_base_key, &keys_provider.get_shutdown_pubkey(), BREAKDOWN_TIMEOUT,
                                                              keys_provider.get_destination_script(), logger.clone());
                channel_monitor.set_their_base_keys(&msg.htlc_basepoint, &msg.delayed_payment_basepoint);
-               channel_monitor.provide_their_next_revocation_point(Some((INITIAL_COMMITMENT_NUMBER, msg.first_per_commitment_point)));
                channel_monitor.set_their_to_self_delay(msg.to_self_delay);
 
                let mut chan = Channel {
@@ -1358,7 +1357,6 @@ impl Channel {
                }
 
                self.channel_monitor.set_their_base_keys(&msg.htlc_basepoint, &msg.delayed_payment_basepoint);
-               self.channel_monitor.provide_their_next_revocation_point(Some((INITIAL_COMMITMENT_NUMBER, msg.first_per_commitment_point)));
 
                self.their_dust_limit_satoshis = msg.dust_limit_satoshis;
                self.their_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
@@ -1434,7 +1432,7 @@ impl Channel {
 
                // Now that we're past error-generating stuff, update our local state:
 
-               self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number);
+               self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
                self.last_local_commitment_txn = vec![local_initial_commitment_tx.clone()];
                self.channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx, local_keys, self.feerate_per_kw, Vec::new());
                self.channel_state = ChannelState::FundingSent as u32;
@@ -1506,7 +1504,6 @@ impl Channel {
                        return Err(ChannelError::Close("Peer sent a funding_locked at a strange time"));
                }
 
-               self.channel_monitor.provide_their_next_revocation_point(Some((INITIAL_COMMITMENT_NUMBER - 1 , msg.next_per_commitment_point)));
                self.their_prev_commitment_point = self.their_cur_commitment_point;
                self.their_cur_commitment_point = Some(msg.next_per_commitment_point);
                Ok(())
@@ -1901,7 +1898,6 @@ impl Channel {
                }
                self.channel_monitor.provide_secret(self.cur_remote_commitment_transaction_number + 1, msg.per_commitment_secret)
                        .map_err(|e| ChannelError::Close(e.0))?;
-               self.channel_monitor.provide_their_next_revocation_point(Some((self.cur_remote_commitment_transaction_number - 1, msg.next_per_commitment_point)));
 
                // Update state now that we've passed all the can-fail calls...
                // (note that we may still fail to generate the new commitment_signed message, but that's
@@ -3002,7 +2998,7 @@ impl Channel {
                let temporary_channel_id = self.channel_id;
 
                // Now that we're past error-generating stuff, update our local state:
-               self.channel_monitor.provide_latest_remote_commitment_tx_info(&commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number);
+               self.channel_monitor.provide_latest_remote_commitment_tx_info(&commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
                self.channel_state = ChannelState::FundingCreated as u32;
                self.channel_id = funding_txo.to_channel_id();
                self.cur_remote_commitment_transaction_number -= 1;
@@ -3214,7 +3210,7 @@ impl Channel {
                match self.send_commitment_no_state_update() {
                        Ok((res, remote_commitment_tx)) => {
                                // Update state now that we've passed all the can-fail calls...
-                               self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_commitment_tx.0, remote_commitment_tx.1, self.cur_remote_commitment_transaction_number);
+                               self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_commitment_tx.0, remote_commitment_tx.1, self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
                                self.channel_state |= ChannelState::AwaitingRemoteRevoke as u32;
                                Ok((res, self.channel_monitor.clone()))
                        },
index 561495bb47cd948a73469a9de3c6b7f70b077acf..6933198c608e0e56f59ffe510d2b2b6583fb3dde 100644 (file)
@@ -1988,10 +1988,20 @@ impl ChannelManager {
                                        // but if we've sent a shutdown and they haven't acknowledged it yet, we just
                                        // want to reject the new HTLC and fail it backwards instead of forwarding.
                                        if let PendingHTLCStatus::Forward(PendingForwardHTLCInfo { incoming_shared_secret, .. }) = pending_forward_info {
+                                               let chan_update = self.get_channel_update(chan);
                                                pending_forward_info = PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
                                                        channel_id: msg.channel_id,
                                                        htlc_id: msg.htlc_id,
-                                                       reason: ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &self.get_channel_update(chan).unwrap().encode_with_len()[..]),
+                                                       reason: if let Ok(update) = chan_update {
+                                                               ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &update.encode_with_len()[..])
+                                                       } else {
+                                                               // This can only happen if the channel isn't in the fully-funded
+                                                               // state yet, implying our counterparty is trying to route payments
+                                                               // over the channel back to themselves (cause no one else should
+                                                               // know the short_id is a lightning channel yet). We should have no
+                                                               // problem just calling this unknown_next_peer
+                                                               ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x4000|10, &[])
+                                                       },
                                                }));
                                        }
                                }
index d50c73f6dcd5de61da0b82093188e722c1f39460..6ce4b22d5a8c63109765be350c53883ad9d91855 100644 (file)
@@ -461,37 +461,11 @@ impl ChannelMonitor {
                Ok(())
        }
 
-       /// Tracks the next revocation point which may be required to claim HTLC outputs which we know
-       /// the preimage of in case the remote end force-closes using their latest state. When called at
-       /// channel opening revocation point is the CURRENT one used for first commitment tx. Needed in case of sizeable push_msat.
-       pub(super) fn provide_their_next_revocation_point(&mut self, their_next_revocation_point: Option<(u64, PublicKey)>) {
-               if let Some(new_revocation_point) = their_next_revocation_point {
-                       match self.their_cur_revocation_points {
-                               Some(old_points) => {
-                                       if old_points.0 == new_revocation_point.0 + 1 {
-                                               self.their_cur_revocation_points = Some((old_points.0, old_points.1, Some(new_revocation_point.1)));
-                                       } else if old_points.0 == new_revocation_point.0 + 2 {
-                                               if let Some(old_second_point) = old_points.2 {
-                                                       self.their_cur_revocation_points = Some((old_points.0 - 1, old_second_point, Some(new_revocation_point.1)));
-                                               } else {
-                                                       self.their_cur_revocation_points = Some((new_revocation_point.0, new_revocation_point.1, None));
-                                               }
-                                       } else {
-                                               self.their_cur_revocation_points = Some((new_revocation_point.0, new_revocation_point.1, None));
-                                       }
-                               },
-                               None => {
-                                       self.their_cur_revocation_points = Some((new_revocation_point.0, new_revocation_point.1, None));
-                               }
-                       }
-               }
-       }
-
        /// Informs this monitor of the latest remote (ie non-broadcastable) commitment transaction.
        /// The monitor watches for it to be broadcasted and then uses the HTLC information (and
        /// possibly future revocation/preimage information) to claim outputs where possible.
        /// We cache also the mapping hash:commitment number to lighten pruning of old preimages by watchtowers.
-       pub(super) fn provide_latest_remote_commitment_tx_info(&mut self, unsigned_commitment_tx: &Transaction, htlc_outputs: Vec<HTLCOutputInCommitment>, commitment_number: u64) {
+       pub(super) fn provide_latest_remote_commitment_tx_info(&mut self, unsigned_commitment_tx: &Transaction, htlc_outputs: Vec<HTLCOutputInCommitment>, commitment_number: u64, their_revocation_point: PublicKey) {
                // TODO: Encrypt the htlc_outputs data with the single-hash of the commitment transaction
                // so that a remote monitor doesn't learn anything unless there is a malicious close.
                // (only maybe, sadly we cant do the same for local info, as we need to be aware of
@@ -501,6 +475,25 @@ impl ChannelMonitor {
                }
                self.remote_claimable_outpoints.insert(unsigned_commitment_tx.txid(), htlc_outputs);
                self.current_remote_commitment_number = commitment_number;
+               //TODO: Merge this into the other per-remote-transaction output storage stuff
+               match self.their_cur_revocation_points {
+                       Some(old_points) => {
+                               if old_points.0 == commitment_number + 1 {
+                                       self.their_cur_revocation_points = Some((old_points.0, old_points.1, Some(their_revocation_point)));
+                               } else if old_points.0 == commitment_number + 2 {
+                                       if let Some(old_second_point) = old_points.2 {
+                                               self.their_cur_revocation_points = Some((old_points.0 - 1, old_second_point, Some(their_revocation_point)));
+                                       } else {
+                                               self.their_cur_revocation_points = Some((commitment_number, their_revocation_point, None));
+                                       }
+                               } else {
+                                       self.their_cur_revocation_points = Some((commitment_number, their_revocation_point, None));
+                               }
+                       },
+                       None => {
+                               self.their_cur_revocation_points = Some((commitment_number, their_revocation_point, None));
+                       }
+               }
        }
 
        /// Informs this monitor of the latest local (ie broadcastable) commitment transaction. The
@@ -896,16 +889,18 @@ impl ChannelMonitor {
                if commitment_number >= self.get_min_seen_secret() {
                        let secret = self.get_secret(commitment_number).unwrap();
                        let per_commitment_key = ignore_error!(SecretKey::from_slice(&self.secp_ctx, &secret));
-                       let (revocation_pubkey, b_htlc_key) = match self.key_storage {
-                               KeyStorage::PrivMode { ref revocation_base_key, ref htlc_base_key, .. } => {
+                       let (revocation_pubkey, b_htlc_key, local_payment_key) = match self.key_storage {
+                               KeyStorage::PrivMode { ref revocation_base_key, ref htlc_base_key, ref payment_base_key, .. } => {
                                        let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
                                        (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &PublicKey::from_secret_key(&self.secp_ctx, &revocation_base_key))),
-                                       ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &PublicKey::from_secret_key(&self.secp_ctx, &htlc_base_key))))
+                                       ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &PublicKey::from_secret_key(&self.secp_ctx, &htlc_base_key))),
+                                       Some(ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &payment_base_key))))
                                },
                                KeyStorage::SigsMode { ref revocation_base_key, ref htlc_base_key, .. } => {
                                        let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
                                        (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &revocation_base_key)),
-                                       ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &htlc_base_key)))
+                                       ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &htlc_base_key)),
+                                       None)
                                },
                        };
                        let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap()));
@@ -917,6 +912,13 @@ impl ChannelMonitor {
                        let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.our_to_self_delay, &delayed_key);
                        let revokeable_p2wsh = revokeable_redeemscript.to_v0_p2wsh();
 
+                       let local_payment_p2wpkh = if let Some(payment_key) = local_payment_key {
+                               // Note that the Network here is ignored as we immediately drop the address for the
+                               // script_pubkey version.
+                               let payment_hash160 = Hash160::from_data(&PublicKey::from_secret_key(&self.secp_ctx, &payment_key).serialize());
+                               Some(Builder::new().push_opcode(opcodes::All::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script())
+                       } else { None };
+
                        let mut total_value = 0;
                        let mut values = Vec::new();
                        let mut inputs = Vec::new();
@@ -936,23 +938,12 @@ impl ChannelMonitor {
                                        htlc_idxs.push(None);
                                        values.push(outp.value);
                                        total_value += outp.value;
-                               } else if outp.script_pubkey.is_v0_p2wpkh() {
-                                       match self.key_storage {
-                                               KeyStorage::PrivMode { ref payment_base_key, .. } => {
-                                                       let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
-                                                       if let Ok(local_key) = chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &payment_base_key) {
-                                                               spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH {
-                                                                       outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 },
-                                                                       key: local_key,
-                                                                       output: outp.clone(),
-                                                               });
-                                                       }
-                                               }
-                                               KeyStorage::SigsMode { .. } => {
-                                                       //TODO: we need to ensure an offline client will generate the event when it
-                                                       // cames back online after only the watchtower saw the transaction
-                                               }
-                                       }
+                               } else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() {
+                                       spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH {
+                                               outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 },
+                                               key: local_payment_key.unwrap(),
+                                               output: outp.clone(),
+                                       });
                                }
                        }
 
@@ -1090,7 +1081,6 @@ impl ChannelMonitor {
                                                Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)),
                                        };
 
-
                                        for (idx, outp) in tx.output.iter().enumerate() {
                                                if outp.script_pubkey.is_v0_p2wpkh() {
                                                        match self.key_storage {
@@ -1102,11 +1092,8 @@ impl ChannelMonitor {
                                                                                        output: outp.clone(),
                                                                                });
                                                                        }
-                                                               }
-                                                               KeyStorage::SigsMode { .. } => {
-                                                                       //TODO: we need to ensure an offline client will generate the event when it
-                                                                       // cames back online after only the watchtower saw the transaction
-                                                               }
+                                                               },
+                                                               KeyStorage::SigsMode { .. } => {}
                                                        }
                                                        break; // Only to_remote ouput is claimable
                                                }
@@ -2161,10 +2148,10 @@ mod tests {
                let logger = Arc::new(TestLogger::new());
                let dummy_sig = Signature::from_der(&secp_ctx, &hex::decode("3045022100fa86fa9a36a8cd6a7bb8f06a541787d51371d067951a9461d5404de6b928782e02201c8b7c334c10aed8976a3a465be9a28abff4cb23acbf00022295b378ce1fa3cd").unwrap()[..]).unwrap();
 
+               let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap());
                macro_rules! dummy_keys {
                        () => {
                                {
-                                       let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap());
                                        TxCreationKeys {
                                                per_commitment_point: dummy_key.clone(),
                                                revocation_key: dummy_key.clone(),
@@ -2233,10 +2220,10 @@ mod tests {
                monitor.set_their_to_self_delay(10);
 
                monitor.provide_latest_local_commitment_tx_info(dummy_tx.clone(), dummy_keys!(), 0, preimages_to_local_htlcs!(preimages[0..10]));
-               monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655);
-               monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654);
-               monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653);
-               monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652);
+               monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key);
+               monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key);
+               monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key);
+               monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key);
                for &(ref preimage, ref hash) in preimages.iter() {
                        monitor.provide_payment_preimage(hash, preimage);
                }
index 6a2033e4532e9069f27f8e3a33c6193e7a5a7daa..eaf2f683279efe83ad1a84c2f37b9c2d9df76dae 100644 (file)
@@ -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 } => {
index 87a2e6d1c242b64f6e9b15fb0e9de9ef7cb8f74e..1ea825473a45b86eaf42236475ca206faaabdcf6 100644 (file)
@@ -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);