]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Merge pull request #19 from TheBlueMatt/2018-04-error-handling-framework
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 2 Apr 2018 23:43:46 +0000 (19:43 -0400)
committerGitHub <noreply@github.com>
Mon, 2 Apr 2018 23:43:46 +0000 (19:43 -0400)
Clean up peer_handler error handling a bit, clean up channel error handling a ton

fuzz/travis-fuzz.sh
src/ln/chan_utils.rs
src/ln/channel.rs
src/ln/channelmanager.rs
src/ln/msgs.rs
src/ln/peer_channel_encryptor.rs
src/ln/peer_handler.rs

index dfe03d151cec99fe3b59424a184435eeb91e2a4e..5129799ae649bcbef9254c6310a5de002e2675d7 100755 (executable)
@@ -1,6 +1,6 @@
 #!/bin/bash
-cargo install honggfuzz
-set +e
+set -e
+cargo install --force honggfuzz
 for TARGET in fuzz_targets/*; do
     FILENAME=$(basename $TARGET)
        FILE="${FILENAME%.*}"
index eb85c0e1f3e1a985d39be9161c3d9e670ed50a4e..dfe17c5911e711e3601d90670879718b24ecbcd8 100644 (file)
@@ -230,6 +230,6 @@ pub fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, a
 /// note here that 'a_revocation_key' is generated using b_revocation_basepoint and a's
 /// commitment secret. 'htlc' does *not* need to have its previous_output_index filled.
 #[inline]
-pub fn get_htlc_redeemscript(htlc: &HTLCOutputInCommitment, keys: &TxCreationKeys, offered: bool) -> Script {
-       get_htlc_redeemscript_with_explicit_keys(htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key, offered)
+pub fn get_htlc_redeemscript(htlc: &HTLCOutputInCommitment, keys: &TxCreationKeys) -> Script {
+       get_htlc_redeemscript_with_explicit_keys(htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key, htlc.offered)
 }
index 61fb8a3e05ed4bd794bd3848be5a3afe5204f457..b8a5bb66c5d39279f2e322d37ea7e4dfe4861703 100644 (file)
@@ -247,21 +247,20 @@ const SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT: u64 = 79; // prevout: 36, nSequence: 4
 const B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT: u64 = 104; // prevout: 40, nSequence: 4, script len: 1, witness lengths: 3/4, sig: 73/4, pubkey: 33/4, output: 31 (TODO: Wrong? Useless?)
 
 macro_rules! secp_call {
-       ( $res : expr ) => {
+       ( $res: expr, $err: expr ) => {
                match $res {
                        Ok(key) => key,
                        //TODO: make the error a parameter
-                       Err(_) => return Err(HandleError{err: "Secp call failed - probably bad signature or evil data generated a bad pubkey/privkey", msg: None})
+                       Err(_) => return Err(HandleError{err: $err, msg: Some(msgs::ErrorAction::DisconnectPeer{})})
                }
        };
 }
 
-macro_rules! get_key {
-       ( $ctx : expr, $slice : expr ) => {
-               secp_call! (SecretKey::from_slice($ctx, $slice))
-       };
+macro_rules! secp_derived_key {
+       ( $res: expr ) => {
+               secp_call!($res, "Derived invalid key, peer is maliciously selecting parameters")
+       }
 }
-
 impl Channel {
        // Convert constants + channel value to limits:
        fn get_our_max_htlc_value_in_flight_msat(channel_value_satoshis: u64) -> u64 {
@@ -360,48 +359,50 @@ impl Channel {
 
        fn check_remote_fee(fee_estimator: &FeeEstimator, feerate_per_kw: u32) -> Result<(), HandleError> {
                if (feerate_per_kw as u64) < fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Background) * 250 {
-                       return Err(HandleError{err: "Peer's feerate much too low", msg: None});
+                       return Err(HandleError{err: "Peer's feerate much too low", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::HighPriority) * 375 { // 375 = 250 * 1.5x
-                       return Err(HandleError{err: "Peer's feerate much too high", msg: None});
+                       return Err(HandleError{err: "Peer's feerate much too high", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                Ok(())
        }
 
        /// Creates a new channel from a remote sides' request for one.
        /// Assumes chain_hash has already been checked and corresponds with what we expect!
+       /// Generally prefers to take the DisconnectPeer action on failure, as a notice to the sender
+       /// that we're rejecting the new channel.
        pub fn new_from_req(fee_estimator: &FeeEstimator, their_node_id: PublicKey, msg: &msgs::OpenChannel, user_id: u64, announce_publicly: bool) -> Result<Channel, HandleError> {
                // Check sanity of message fields:
                if msg.funding_satoshis >= (1 << 24) {
-                       return Err(HandleError{err: "funding value > 2^24", msg: None});
+                       return Err(HandleError{err: "funding value > 2^24", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                if msg.funding_satoshis > 21000000 * 100000000 {
-                       return Err(HandleError{err: "More funding_satoshis than there are satoshis!", msg: None});
+                       return Err(HandleError{err: "More funding_satoshis than there are satoshis!", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                if msg.channel_reserve_satoshis > msg.funding_satoshis {
-                       return Err(HandleError{err: "Bogus channel_reserve_satoshis", msg: None});
+                       return Err(HandleError{err: "Bogus channel_reserve_satoshis", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                if msg.push_msat > (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 {
-                       return Err(HandleError{err: "push_msat more than highest possible value", msg: None});
+                       return Err(HandleError{err: "push_msat more than highest possible value", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
-               if msg.dust_limit_satoshis > 21000000 * 100000000 {
-                       return Err(HandleError{err: "Peer never wants payout outputs?", msg: None});
+               if msg.dust_limit_satoshis > msg.funding_satoshis {
+                       return Err(HandleError{err: "Peer never wants payout outputs?", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                if msg.max_htlc_value_in_flight_msat > msg.funding_satoshis * 1000 {
-                       return Err(HandleError{err: "Bogus max_htlc_value_in_flight_satoshis", msg: None});
+                       return Err(HandleError{err: "Bogus max_htlc_value_in_flight_satoshis", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                if msg.htlc_minimum_msat >= (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 {
-                       return Err(HandleError{err: "Minimum htlc value is full channel value", msg: None});
+                       return Err(HandleError{err: "Minimum htlc value is full channel value", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                Channel::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
                if msg.to_self_delay > MAX_LOCAL_BREAKDOWN_TIMEOUT {
-                       return Err(HandleError{err: "They wanted our payments to be delayed by a needlessly long period", msg: None});
+                       return Err(HandleError{err: "They wanted our payments to be delayed by a needlessly long period", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                if msg.max_accepted_htlcs < 1 {
-                       return Err(HandleError{err: "0 max_accpted_htlcs makes for a useless channel", msg: None});
+                       return Err(HandleError{err: "0 max_accpted_htlcs makes for a useless channel", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                if (msg.channel_flags & 254) != 0 {
-                       return Err(HandleError{err: "unknown channel_flags", msg: None});
+                       return Err(HandleError{err: "unknown channel_flags", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
 
                // Convert things into internal flags and prep our state:
@@ -484,9 +485,9 @@ impl Channel {
 
        // Utilities to derive keys:
 
-       fn build_local_commitment_secret(&self, idx: u64) -> Result<SecretKey, HandleError> {
+       fn build_local_commitment_secret(&self, idx: u64) -> SecretKey {
                let res = chan_utils::build_commitment_secret(self.local_keys.commitment_seed, idx);
-               Ok(get_key!(&self.secp_ctx, &res))
+               SecretKey::from_slice(&self.secp_ctx, &res).unwrap()
        }
 
        // Utilities to build transactions:
@@ -527,7 +528,7 @@ impl Channel {
        /// generated by the peer which proposed adding the HTLCs, and thus we need to understand both
        /// which peer generated this transaction and "to whom" this transaction flows.
        #[inline]
-       fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool) -> Result<(Transaction, Vec<HTLCOutputInCommitment>), HandleError> {
+       fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool) -> (Transaction, Vec<HTLCOutputInCommitment>) {
                let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ commitment_number;
 
                let txins = {
@@ -554,7 +555,7 @@ impl Channel {
                                        if htlc.amount_msat / 1000 >= dust_limit_satoshis + (self.feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
                                                let htlc_in_tx = htlc.get_in_commitment(true);
                                                txouts.push((TxOut {
-                                                       script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys, true).to_v0_p2wsh(),
+                                                       script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
                                                        value: htlc.amount_msat / 1000
                                                }, Some(htlc_in_tx)));
                                        }
@@ -562,7 +563,7 @@ impl Channel {
                                        if htlc.amount_msat / 1000 >= dust_limit_satoshis + (self.feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
                                                let htlc_in_tx = htlc.get_in_commitment(false);
                                                txouts.push((TxOut { // "received HTLC output"
-                                                       script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys, false).to_v0_p2wsh(),
+                                                       script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
                                                        value: htlc.amount_msat / 1000
                                                }, Some(htlc_in_tx)));
                                        }
@@ -615,12 +616,12 @@ impl Channel {
                        }
                }
 
-               Ok((Transaction {
+               (Transaction {
                        version: 2,
                        lock_time: ((0x20 as u32) << 8*3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32),
                        input: txins,
                        output: outputs,
-               }, htlcs_used))
+               }, htlcs_used)
        }
 
        #[inline]
@@ -699,11 +700,11 @@ impl Channel {
        /// The result is a transaction which we can revoke ownership of (ie a "local" transaction)
        /// TODO Some magic rust shit to compile-time check this?
        fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, HandleError> {
-               let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number)?).unwrap();
+               let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number)).unwrap();
                let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.delayed_payment_base_key).unwrap();
                let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.htlc_base_key).unwrap();
 
-               Ok(secp_call!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &self.their_revocation_basepoint, &self.their_payment_basepoint, &self.their_htlc_basepoint)))
+               Ok(secp_derived_key!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &self.their_revocation_basepoint, &self.their_payment_basepoint, &self.their_htlc_basepoint)))
        }
 
        #[inline]
@@ -716,7 +717,7 @@ impl Channel {
                let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.revocation_base_key).unwrap();
                let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.htlc_base_key).unwrap();
 
-               Ok(secp_call!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point, &self.their_delayed_payment_basepoint, &self.their_htlc_basepoint, &revocation_basepoint, &payment_basepoint, &htlc_basepoint)))
+               Ok(secp_derived_key!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point, &self.their_delayed_payment_basepoint, &self.their_htlc_basepoint, &revocation_basepoint, &payment_basepoint, &htlc_basepoint)))
        }
 
        /// Gets the redeemscript for the funding transaction output (ie the funding transaction output
@@ -811,11 +812,11 @@ impl Channel {
                        panic!("Tried to re-sign HTLC transaction");
                }
 
-               let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys, htlc.offered);
+               let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
 
-               let our_htlc_key = secp_call!(chan_utils::derive_private_key(&self.secp_ctx, &keys.per_commitment_point, &self.local_keys.htlc_base_key));
-               let sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]));
-               let our_sig = secp_call!(self.secp_ctx.sign(&sighash, &our_htlc_key));
+               let our_htlc_key = secp_derived_key!(chan_utils::derive_private_key(&self.secp_ctx, &keys.per_commitment_point, &self.local_keys.htlc_base_key));
+               let sighash = Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
+               let our_sig = self.secp_ctx.sign(&sighash, &our_htlc_key).unwrap();
 
                let local_tx = PublicKey::from_secret_key(&self.secp_ctx, &our_htlc_key).unwrap() == keys.a_htlc_key;
 
@@ -843,8 +844,12 @@ impl Channel {
        }
 
        pub fn get_update_fulfill_htlc(&mut self, payment_preimage: [u8; 32]) -> Result<msgs::UpdateFulfillHTLC, HandleError> {
+               // Either ChannelFunded got set (which means it wont bet unset) or there is no way any
+               // caller thought we could have something claimed (cause we wouldn't have accepted in an
+               // incoming HTLC anyway). If we got to ShutdownComplete, callers aren't allowed to call us,
+               // either.
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
-                       return Err(HandleError{err: "Was asked to fulfill an HTLC when channel was not in an operational state", msg: None});
+                       panic!("Was asked to fulfill an HTLC when channel was not in an operational state");
                }
                assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0);
 
@@ -970,18 +975,18 @@ impl Channel {
                let funding_script = self.get_funding_redeemscript();
 
                let remote_keys = self.build_remote_transaction_keys()?;
-               let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false)?.0;
-               let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]));
+               let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false).0;
+               let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
 
                let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
-               let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false)?.0;
-               let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]));
+               let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false).0;
+               let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
 
                // They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish.
-               secp_call!(self.secp_ctx.verify(&local_sighash, &sig, &self.their_funding_pubkey));
+               secp_call!(self.secp_ctx.verify(&local_sighash, &sig, &self.their_funding_pubkey), "Invalid funding_created signature from peer");
 
                // We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
-               Ok((remote_initial_commitment_tx, secp_call!(self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key))))
+               Ok((remote_initial_commitment_tx, self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key).unwrap()))
        }
 
        pub fn funding_created(&mut self, msg: &msgs::FundingCreated) -> Result<msgs::FundingSigned, HandleError> {
@@ -1036,11 +1041,11 @@ impl Channel {
                let funding_script = self.get_funding_redeemscript();
 
                let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
-               let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false)?.0;
-               let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]));
+               let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false).0;
+               let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
 
                // They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish.
-               secp_call!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey));
+               secp_call!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey), "Invalid funding_signed signature from peer");
 
                self.channel_state = ChannelState::FundingSent as u32;
 
@@ -1258,10 +1263,10 @@ impl Channel {
                let funding_script = self.get_funding_redeemscript();
 
                let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
-               let local_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false)?;
+               let local_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false);
                let local_commitment_txid = local_commitment_tx.0.txid();
-               let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]));
-               secp_call!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey));
+               let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
+               secp_call!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey), "Invalid commitment tx signature from peer");
 
                if msg.htlc_signatures.len() != local_commitment_tx.1.len() {
                        return Err(HandleError{err: "Got wrong number of HTLC signatures from remote", msg: None});
@@ -1269,12 +1274,12 @@ impl Channel {
 
                for (idx, ref htlc) in local_commitment_tx.1.iter().enumerate() {
                        let htlc_tx = self.build_htlc_transaction(&local_commitment_txid, htlc, true, &local_keys)?;
-                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys, htlc.offered);
-                       let htlc_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]));
-                       secp_call!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key));
+                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
+                       let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
+                       secp_call!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx siganture from peer");
                }
 
-               let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number - 1)?).unwrap();
+               let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number - 1)).unwrap();
                let per_commitment_secret = chan_utils::build_commitment_secret(self.local_keys.commitment_seed, self.cur_local_commitment_transaction_number);
 
                //TODO: Store htlc keys in our channel_watcher
@@ -1307,7 +1312,7 @@ impl Channel {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        return Err(HandleError{err: "Got revoke/ACK message when channel was not in an operational state", msg: None});
                }
-               if PublicKey::from_secret_key(&self.secp_ctx, &get_key!(&self.secp_ctx, &msg.per_commitment_secret)).unwrap() != self.their_cur_commitment_point {
+               if PublicKey::from_secret_key(&self.secp_ctx, &secp_call!(SecretKey::from_slice(&self.secp_ctx, &msg.per_commitment_secret), "Peer provided an invalid per_commitment_secret")).unwrap() != self.their_cur_commitment_point {
                        return Err(HandleError{err: "Got a revoke commitment secret which didn't correspond to their current pubkey", msg: None});
                }
                self.channel_monitor.provide_secret(self.cur_remote_commitment_transaction_number, msg.per_commitment_secret)?;
@@ -1379,9 +1384,9 @@ impl Channel {
 
                        let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
                        let funding_redeemscript = self.get_funding_redeemscript();
-                       let sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]));
+                       let sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
 
-                       (Some(proposed_feerate), Some(total_fee_satoshis), Some(secp_call!(self.secp_ctx.sign(&sighash, &self.local_keys.funding_key))))
+                       (Some(proposed_feerate), Some(total_fee_satoshis), Some(self.secp_ctx.sign(&sighash, &self.local_keys.funding_key).unwrap()))
                } else { (None, None, None) };
 
                // From here on out, we may not fail!
@@ -1441,7 +1446,7 @@ impl Channel {
                if used_total_fee != msg.fee_satoshis {
                        return Err(HandleError{err: "Remote sent us a closing_signed with a fee greater than the value they can claim", msg: None});
                }
-               let mut sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]));
+               let mut sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
 
                match self.secp_ctx.verify(&sighash, &msg.signature, &self.their_funding_pubkey) {
                        Ok(_) => {},
@@ -1449,8 +1454,8 @@ impl Channel {
                                // The remote end may have decided to revoke their output due to inconsistent dust
                                // limits, so check for that case by re-checking the signature here.
                                closing_tx = self.build_closing_transaction(msg.fee_satoshis, true).0;
-                               sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]));
-                               secp_call!(self.secp_ctx.verify(&sighash, &msg.signature, &self.their_funding_pubkey));
+                               sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
+                               secp_call!(self.secp_ctx.verify(&sighash, &msg.signature, &self.their_funding_pubkey), "Invalid closing tx signature from peer");
                        },
                };
 
@@ -1466,7 +1471,7 @@ impl Channel {
                        ($new_feerate: expr) => {
                                let closing_tx_max_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap());
                                let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate * closing_tx_max_weight / 4, false);
-                               sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]));
+                               sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
                                let our_sig = self.secp_ctx.sign(&sighash, &self.local_keys.funding_key).unwrap();
                                self.last_sent_closing_fee = Some(($new_feerate, used_total_fee));
                                return Ok((Some(msgs::ClosingSigned {
@@ -1623,11 +1628,7 @@ impl Channel {
                                        //as otherwise we will have a commitment transaction that they can't revoke (well, kinda,
                                        //they can by sending two revoke_and_acks back-to-back, but not really). This appears to be
                                        //a protocol oversight, but I assume I'm just missing something.
-                                       let next_per_commitment_secret = match self.build_local_commitment_secret(self.cur_local_commitment_transaction_number) {
-                                               Ok(secret) => secret,
-                                               Err(_) => return None
-                                       };
-
+                                       let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
                                        let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret).unwrap();
                                        return Some(msgs::FundingLocked {
                                                channel_id: self.channel_id,
@@ -1686,7 +1687,7 @@ impl Channel {
                        panic!("Tried to send an open_channel for a channel that has already advanced");
                }
 
-               let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number)?;
+               let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 
                Ok(msgs::OpenChannel {
                        chain_hash: chain_hash,
@@ -1722,7 +1723,7 @@ impl Channel {
                        panic!("Tried to send an accept_channel for a channel that has already advanced");
                }
 
-               let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number)?;
+               let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 
                Ok(msgs::AcceptChannel {
                        temporary_channel_id: self.channel_id,
@@ -1747,11 +1748,11 @@ impl Channel {
                let funding_script = self.get_funding_redeemscript();
 
                let remote_keys = self.build_remote_transaction_keys()?;
-               let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false)?.0;
-               let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]));
+               let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false).0;
+               let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
 
                // We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
-               Ok(secp_call!(self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key)))
+               Ok(self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key).unwrap())
        }
 
        /// Updates channel state with knowledge of the funding transaction's txid/index, and generates
@@ -1826,7 +1827,7 @@ impl Channel {
                };
 
                let msghash = Message::from_slice(&Sha256dHash::from_data(&msg.encode()[..])[..]).unwrap();
-               let sig = secp_call!(self.secp_ctx.sign(&msghash, &self.local_keys.funding_key));
+               let sig = self.secp_ctx.sign(&msghash, &self.local_keys.funding_key).unwrap();
 
                Ok((msg, sig))
        }
@@ -1925,19 +1926,19 @@ impl Channel {
                let funding_script = self.get_funding_redeemscript();
 
                let remote_keys = self.build_remote_transaction_keys()?;
-               let remote_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, true)?;
+               let remote_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, true);
                let remote_commitment_txid = remote_commitment_tx.0.txid();
-               let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]));
-               let our_sig = secp_call!(self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key));
+               let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
+               let our_sig = self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key).unwrap();
 
                let mut htlc_sigs = Vec::new();
 
                for ref htlc in remote_commitment_tx.1.iter() {
                        let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys)?;
-                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys, htlc.offered);
-                       let htlc_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]));
-                       let our_htlc_key = secp_call!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key));
-                       htlc_sigs.push(secp_call!(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key)));
+                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
+                       let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
+                       let our_htlc_key = secp_derived_key!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key));
+                       htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key).unwrap());
                }
 
                // Update state now that we've passed all the can-fail calls...
@@ -2071,7 +2072,7 @@ mod tests {
 
                macro_rules! test_commitment {
                        ( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => {
-                               unsigned_tx = chan.build_commitment_transaction(42, &keys, true, false).unwrap();
+                               unsigned_tx = chan.build_commitment_transaction(42, &keys, true, false);
                                let their_signature = Signature::from_der(&secp_ctx, &hex_bytes($their_sig_hex).unwrap()[..]).unwrap();
                                let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0.input[0], &chan.get_funding_redeemscript(), chan.channel_value_satoshis)[..]).unwrap();
                                secp_ctx.verify(&sighash, &their_signature, &chan.their_funding_pubkey).unwrap();
@@ -2089,7 +2090,7 @@ mod tests {
 
                                let ref htlc = unsigned_tx.1[$htlc_idx];
                                let mut htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys).unwrap();
-                               let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys, htlc.offered);
+                               let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
                                let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
                                secp_ctx.verify(&htlc_sighash, &remote_signature, &keys.b_htlc_key).unwrap();
 
index 43e0d59f4faa357a5bddab02906844bb4b7f700d..d2854ca741145eaeaefc11ab673cbe53c71d3f2c 100644 (file)
@@ -1121,7 +1121,7 @@ impl ChannelMessageHandler for ChannelManager {
                        ($msg: expr, $err_code: expr, $data: expr) => {
                                return Err(msgs::HandleError {
                                        err: $msg,
-                                       msg: Some(msgs::ErrorMessage::UpdateFailHTLC {
+                                       msg: Some(msgs::ErrorAction::UpdateFailHTLC {
                                                msg: msgs::UpdateFailHTLC {
                                                        channel_id: msg.channel_id,
                                                        htlc_id: msg.htlc_id,
@@ -1481,6 +1481,37 @@ impl ChannelMessageHandler for ChannelManager {
                pending_events.push(events::Event::BroadcastChannelAnnouncement { msg: chan_announcement, update_msg: chan_update });
                Ok(())
        }
+
+       fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool) {
+               let mut channel_state_lock = self.channel_state.lock().unwrap();
+               let channel_state = channel_state_lock.borrow_parts();
+               let short_to_id = channel_state.short_to_id;
+               if no_connection_possible {
+                       channel_state.by_id.retain(move |_, chan| {
+                               if chan.get_their_node_id() == *their_node_id {
+                                       match chan.get_short_channel_id() {
+                                               Some(short_id) => {
+                                                       short_to_id.remove(&short_id);
+                                               },
+                                               None => {},
+                                       }
+                                       //TODO: get the latest commitment tx, any HTLC txn built on top of it, etc out
+                                       //of the channel and throw those into the announcement blackhole.
+                                       false
+                               } else {
+                                       true
+                               }
+                       });
+               } else {
+                       for chan in channel_state.by_id {
+                               if chan.1.get_their_node_id() == *their_node_id {
+                                       //TODO: mark channel disabled (and maybe announce such after a timeout). Also
+                                       //fail and wipe any uncommitted outbound HTLCs as those are considered after
+                                       //reconnect.
+                               }
+                       }
+               }
+       }
 }
 
 #[cfg(test)]
index 312fea206e44d5702f02e4065e7d260f034381bf..e85f25ac78a23248690d520a586af5cb1cca9fb5 100644 (file)
@@ -342,16 +342,19 @@ pub struct ChannelUpdate {
 }
 
 /// Used to put an error message in a HandleError
-pub enum ErrorMessage {
+pub enum ErrorAction {
+       /// Indicates an inbound HTLC add resulted in a failure, and the UpdateFailHTLC provided in msg
+       /// should be sent back to the sender.
        UpdateFailHTLC {
                msg: UpdateFailHTLC
        },
+       /// The peer took some action which made us think they were useless. Disconnect them.
        DisconnectPeer {},
 }
 
 pub struct HandleError { //TODO: rename me
        pub err: &'static str,
-       pub msg: Option<ErrorMessage>, //TODO: Move into an Action enum and require it!
+       pub msg: Option<ErrorAction>, //TODO: Make this required and rename it
 }
 
 /// A trait to describe an object which can receive channel messages. Messages MAY be called in
@@ -381,6 +384,13 @@ pub trait ChannelMessageHandler : events::EventsProvider {
 
        // Channel-to-announce:
        fn handle_announcement_signatures(&self, their_node_id: &PublicKey, msg: &AnnouncementSignatures) -> Result<(), HandleError>;
+
+       // Informational:
+       /// Indicates a connection to the peer failed/an existing connection was lost. If no connection
+       /// is believed to be possible in the future (eg they're sending us messages we don't
+       /// understand or indicate they require unknown feature bits), no_connection_possible is set
+       /// and any outstanding channels should be failed.
+       fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool);
 }
 
 pub trait RoutingMessageHandler {
index 67b0a14cb168910bc97e472dce26ba2c95e4645f..3ec6241fb373fd58b415fbec2f548912a0cf9b96 100644 (file)
@@ -147,7 +147,7 @@ impl PeerChannelEncryptor {
 
                let mut chacha = ChaCha20Poly1305RFC::new(key, &nonce, h);
                if !chacha.decrypt(&cyphertext[0..cyphertext.len() - 16], res, &cyphertext[cyphertext.len() - 16..]) {
-                       return Err(HandleError{err: "Bad MAC", msg: Some(msgs::ErrorMessage::DisconnectPeer{})});
+                       return Err(HandleError{err: "Bad MAC", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
                Ok(())
        }
@@ -195,11 +195,11 @@ impl PeerChannelEncryptor {
                assert_eq!(act.len(), 50);
 
                if act[0] != 0 {
-                       return Err(HandleError{err: "Unknown handshake version number", msg: Some(msgs::ErrorMessage::DisconnectPeer{})});
+                       return Err(HandleError{err: "Unknown handshake version number", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                }
 
                let their_pub = match PublicKey::from_slice(secp_ctx, &act[1..34]) {
-                       Err(_) => return Err(HandleError{err: "Invalid public key", msg: Some(msgs::ErrorMessage::DisconnectPeer{})}),
+                       Err(_) => return Err(HandleError{err: "Invalid public key", msg: Some(msgs::ErrorAction::DisconnectPeer{})}),
                        Ok(key) => key,
                };
 
@@ -349,14 +349,14 @@ impl PeerChannelEncryptor {
                                                        panic!("Requested act at wrong step");
                                                }
                                                if act_three[0] != 0 {
-                                                       return Err(HandleError{err: "Unknown handshake version number", msg: Some(msgs::ErrorMessage::DisconnectPeer{})});
+                                                       return Err(HandleError{err: "Unknown handshake version number", msg: Some(msgs::ErrorAction::DisconnectPeer{})});
                                                }
 
                                                let mut their_node_id = [0; 33];
                                                PeerChannelEncryptor::decrypt_with_ad(&mut their_node_id, 1, &temp_k2.unwrap(), &bidirectional_state.h, &act_three[1..50])?;
                                                self.their_node_id = Some(match PublicKey::from_slice(&self.secp_ctx, &their_node_id) {
                                                        Ok(key) => key,
-                                                       Err(_) => return Err(HandleError{err: "Bad node_id from peer", msg: Some(msgs::ErrorMessage::DisconnectPeer{})}),
+                                                       Err(_) => return Err(HandleError{err: "Bad node_id from peer", msg: Some(msgs::ErrorAction::DisconnectPeer{})}),
                                                });
 
                                                let mut sha = Sha256::new();
index 173467dfce88bac83f6bb1593bc85fd62ccd39b3..01081845ae8630f9c6540fe30f706f6dbe4238c0 100644 (file)
@@ -40,16 +40,21 @@ pub trait SocketDescriptor : cmp::Eq + hash::Hash + Clone {
 /// generate no further read/write_events for the descriptor, only triggering a single
 /// disconnect_event (unless it was provided in response to a new_*_connection event, in which case
 /// no such disconnect_event must be generated and the socket be silently disconencted).
-pub struct PeerHandleError {}
+pub struct PeerHandleError {
+       no_connection_possible: bool,
+}
 impl fmt::Debug for PeerHandleError {
        fn fmt(&self, formatter: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-               formatter.write_str("Peer Send Invalid Data")
+               formatter.write_str("Peer Sent Invalid Data")
        }
 }
 
 struct Peer {
        channel_encryptor: PeerChannelEncryptor,
+       outbound: bool,
        their_node_id: Option<PublicKey>,
+       their_global_features: Option<msgs::GlobalFeatures>,
+       their_local_features: Option<msgs::LocalFeatures>,
 
        pending_outbound_buffer: LinkedList<Vec<u8>>,
        pending_outbound_buffer_first_msg_offset: usize,
@@ -112,7 +117,10 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                let mut peers = self.peers.lock().unwrap();
                if peers.peers.insert(descriptor, Peer {
                        channel_encryptor: peer_encryptor,
+                       outbound: true,
                        their_node_id: Some(their_node_id),
+                       their_global_features: None,
+                       their_local_features: None,
 
                        pending_outbound_buffer: LinkedList::new(),
                        pending_outbound_buffer_first_msg_offset: 0,
@@ -141,7 +149,10 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                let mut peers = self.peers.lock().unwrap();
                if peers.peers.insert(descriptor, Peer {
                        channel_encryptor: peer_encryptor,
+                       outbound: false,
                        their_node_id: None,
+                       their_global_features: None,
+                       their_local_features: None,
 
                        pending_outbound_buffer: LinkedList::new(),
                        pending_outbound_buffer_first_msg_offset: 0,
@@ -212,7 +223,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                match self.do_read_event(peer_descriptor, data) {
                        Ok(res) => Ok(res),
                        Err(e) => {
-                               self.disconnect_event(peer_descriptor);
+                               self.disconnect_event_internal(peer_descriptor, e.no_connection_possible);
                                Err(e)
                        }
                }
@@ -227,38 +238,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                        assert!(peer.pending_read_buffer.len() > 0);
                                        assert!(peer.pending_read_buffer.len() > peer.pending_read_buffer_pos);
 
-                                       macro_rules! try_potential_handleerror {
-                                               ($thing: expr) => {
-                                                       match $thing {
-                                                               Ok(x) => x,
-                                                               Err(_e) => {
-                                                                       //TODO: Handle e appropriately!
-                                                                       return Err(PeerHandleError{});
-                                                               }
-                                                       };
-                                               }
-                                       }
-
-                                       macro_rules! try_potential_decodeerror {
-                                               ($thing: expr) => {
-                                                       match $thing {
-                                                               Ok(x) => x,
-                                                               Err(_e) => {
-                                                                       //TODO: Handle e?
-                                                                       return Err(PeerHandleError{});
-                                                               }
-                                                       };
-                                               }
-                                       }
-
-                                       macro_rules! encode_and_send_msg {
-                                               ($msg: expr, $msg_code: expr) => {
-                                                       peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!($msg, $msg_code)[..]));
-                                               }
-                                       }
-
                                        let mut insert_node_id = None;
-
                                        let mut read_pos = 0;
                                        while read_pos < data.len() {
                                                {
@@ -267,7 +247,52 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                        read_pos += data_to_copy;
                                                        peer.pending_read_buffer_pos += data_to_copy;
                                                }
+
                                                if peer.pending_read_buffer_pos == peer.pending_read_buffer.len() {
+                                                       peer.pending_read_buffer_pos = 0;
+
+                                                       macro_rules! encode_and_send_msg {
+                                                               ($msg: expr, $msg_code: expr) => {
+                                                                       peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!($msg, $msg_code)[..]));
+                                                               }
+                                                       }
+
+                                                       macro_rules! try_potential_handleerror {
+                                                               ($thing: expr) => {
+                                                                       match $thing {
+                                                                               Ok(x) => x,
+                                                                               Err(e) => {
+                                                                                       // TODO: Log e.err
+                                                                                       if let Some(action) = e.msg {
+                                                                                               match action {
+                                                                                                       msgs::ErrorAction::UpdateFailHTLC { msg } => {
+                                                                                                               encode_and_send_msg!(msg, 131);
+                                                                                                               continue;
+                                                                                                       },
+                                                                                                       msgs::ErrorAction::DisconnectPeer {} => {
+                                                                                                               return Err(PeerHandleError{ no_connection_possible: false });
+                                                                                                       },
+                                                                                               }
+                                                                                       } else {
+                                                                                               return Err(PeerHandleError{ no_connection_possible: false });
+                                                                                       }
+                                                                               }
+                                                                       };
+                                                               }
+                                                       }
+
+                                                       macro_rules! try_potential_decodeerror {
+                                                               ($thing: expr) => {
+                                                                       match $thing {
+                                                                               Ok(x) => x,
+                                                                               Err(_e) => {
+                                                                                       //TODO: Handle e?
+                                                                                       return Err(PeerHandleError{ no_connection_possible: false });
+                                                                               }
+                                                                       };
+                                                               }
+                                                       }
+
                                                        let next_step = peer.channel_encryptor.get_noise_step();
                                                        match next_step {
                                                                NextNoiseStep::ActOne => {
@@ -300,27 +325,41 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                                                peer.pending_read_buffer = Vec::with_capacity(msg_len as usize + 16);
                                                                                peer.pending_read_buffer.resize(msg_len as usize + 16, 0);
                                                                                if msg_len < 2 { // Need at least the message type tag
-                                                                                       return Err(PeerHandleError{});
+                                                                                       return Err(PeerHandleError{ no_connection_possible: false });
                                                                                }
                                                                                peer.pending_read_is_header = false;
                                                                        } else {
                                                                                let msg_data = try_potential_handleerror!(peer.channel_encryptor.decrypt_message(&peer.pending_read_buffer[..]));
                                                                                assert!(msg_data.len() >= 2);
 
+                                                                               // Reset read buffer
+                                                                               peer.pending_read_buffer = [0; 18].to_vec();
+                                                                               peer.pending_read_is_header = true;
+
                                                                                let msg_type = byte_utils::slice_to_be16(&msg_data[0..2]);
+                                                                               if msg_type != 16 && peer.their_global_features.is_none() {
+                                                                                       // Need an init message as first message
+                                                                                       return Err(PeerHandleError{ no_connection_possible: false });
+                                                                               }
                                                                                match msg_type {
                                                                                        // Connection control:
                                                                                        16 => {
                                                                                                let msg = try_potential_decodeerror!(msgs::Init::decode(&msg_data[2..]));
                                                                                                if msg.global_features.requires_unknown_bits() {
-                                                                                                       return Err(PeerHandleError{});
+                                                                                                       return Err(PeerHandleError{ no_connection_possible: true });
                                                                                                }
                                                                                                if msg.local_features.requires_unknown_bits() {
-                                                                                                       return Err(PeerHandleError{});
+                                                                                                       return Err(PeerHandleError{ no_connection_possible: true });
+                                                                                               }
+                                                                                               peer.their_global_features = Some(msg.global_features);
+                                                                                               peer.their_local_features = Some(msg.local_features);
+
+                                                                                               if !peer.outbound {
+                                                                                                       encode_and_send_msg!(msgs::Init {
+                                                                                                               global_features: msgs::GlobalFeatures::new(),
+                                                                                                               local_features: msgs::LocalFeatures::new(),
+                                                                                                       }, 16);
                                                                                                }
-                                                                                               //TODO: Store features (and check that we've
-                                                                                               //received Init prior to any other messages)!
-                                                                                               //TODO: Respond to Init with Init if we're inbound.
                                                                                        },
                                                                                        17 => {
                                                                                                // Error msg
@@ -439,18 +478,13 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                                                        },
                                                                                        _ => {
                                                                                                if (msg_type & 1) == 0 {
-                                                                                                       //TODO: Fail all channels. Kill the peer!
-                                                                                                       return Err(PeerHandleError{});
+                                                                                                       return Err(PeerHandleError{ no_connection_possible: true });
                                                                                                }
                                                                                        },
                                                                                }
-
-                                                                               peer.pending_read_buffer = [0; 18].to_vec();
-                                                                               peer.pending_read_is_header = true;
                                                                        }
                                                                }
                                                        }
-                                                       peer.pending_read_buffer_pos = 0;
                                                }
                                        }
 
@@ -602,18 +636,22 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
        /// but must NOT be called if a PeerHandleError was provided out of a new_*_connection event!
        /// Panics if the descriptor was not previously registered in a successful new_*_connection event.
        pub fn disconnect_event(&self, descriptor: &Descriptor) {
+               self.disconnect_event_internal(descriptor, false);
+       }
+
+       fn disconnect_event_internal(&self, descriptor: &Descriptor, no_connection_possible: bool) {
                let mut peers = self.peers.lock().unwrap();
                let peer_option = peers.peers.remove(descriptor);
                match peer_option {
                        None => panic!("Descriptor for disconnect_event is not already known to PeerManager"),
                        Some(peer) => {
                                match peer.their_node_id {
-                                       Some(node_id) => { peers.node_id_to_descriptor.remove(&node_id); },
+                                       Some(node_id) => {
+                                               peers.node_id_to_descriptor.remove(&node_id);
+                                               self.message_handler.chan_handler.peer_disconnected(&node_id, no_connection_possible);
+                                       },
                                        None => {}
                                }
-                               //TODO: Notify the chan_handler that this node disconnected, and do something about
-                               //handling response messages that were queued for sending (maybe the send buffer
-                               //needs to be unencrypted?)
                        }
                };
        }