Add a BIG lock to ChannelManager
[rust-lightning] / src / ln / channelmanager.rs
index c5356e291789215d98ebbdae6cfa676dcf6d01f7..f0fd7ad2d30f5d0673fb34fd9d6a82ddd54aa7ee 100644 (file)
@@ -45,7 +45,7 @@ use std::{ptr, mem};
 use std::collections::HashMap;
 use std::collections::hash_map;
 use std::io::Cursor;
-use std::sync::{Mutex,MutexGuard,Arc};
+use std::sync::{Arc, Mutex, MutexGuard, RwLock};
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::time::{Instant,Duration};
 
@@ -65,13 +65,12 @@ mod channel_held_info {
        use ln::msgs;
        use ln::router::Route;
        use secp256k1::key::SecretKey;
-       use secp256k1::ecdh::SharedSecret;
 
        /// Stores the info we will need to send when we want to forward an HTLC onwards
        #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
        pub struct PendingForwardHTLCInfo {
                pub(super) onion_packet: Option<msgs::OnionPacket>,
-               pub(super) incoming_shared_secret: SharedSecret,
+               pub(super) incoming_shared_secret: [u8; 32],
                pub(super) payment_hash: [u8; 32],
                pub(super) short_channel_id: u64,
                pub(super) amt_to_forward: u64,
@@ -96,7 +95,7 @@ mod channel_held_info {
        pub struct HTLCPreviousHopData {
                pub(super) short_channel_id: u64,
                pub(super) htlc_id: u64,
-               pub(super) incoming_packet_shared_secret: SharedSecret,
+               pub(super) incoming_packet_shared_secret: [u8; 32],
        }
 
        /// Tracks the inbound corresponding to an outbound HTLC
@@ -318,6 +317,10 @@ pub struct ChannelManager {
        our_network_key: SecretKey,
 
        pending_events: Mutex<Vec<events::Event>>,
+       /// Used when we have to take a BIG lock to make sure everything is self-consistent.
+       /// Essentially just when we're serializing ourselves out.
+       /// Taken first everywhere where we are making changes before any other locks.
+       total_consistency_lock: RwLock<()>,
 
        keys_manager: Arc<KeysInterface>,
 
@@ -419,6 +422,7 @@ impl ChannelManager {
                        our_network_key: keys_manager.get_node_secret(),
 
                        pending_events: Mutex::new(Vec::new()),
+                       total_consistency_lock: RwLock::new(()),
 
                        keys_manager,
 
@@ -443,6 +447,8 @@ impl ChannelManager {
        pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64) -> Result<(), APIError> {
                let channel = Channel::new_outbound(&*self.fee_estimator, &self.keys_manager, their_network_key, channel_value_satoshis, push_msat, self.announce_channels_publicly, user_id, Arc::clone(&self.logger))?;
                let res = channel.get_open_channel(self.genesis_hash.clone(), &*self.fee_estimator);
+
+               let _ = self.total_consistency_lock.read().unwrap();
                let mut channel_state = self.channel_state.lock().unwrap();
                match channel_state.by_id.entry(channel.channel_id()) {
                        hash_map::Entry::Occupied(_) => {
@@ -506,6 +512,8 @@ impl ChannelManager {
        ///
        /// May generate a SendShutdown message event on success, which should be relayed.
        pub fn close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> {
+               let _ = self.total_consistency_lock.read().unwrap();
+
                let (mut failed_htlcs, chan_option) = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = channel_state_lock.borrow_parts();
@@ -568,6 +576,8 @@ impl ChannelManager {
        /// Force closes a channel, immediately broadcasting the latest local commitment transaction to
        /// the chain and rejecting new HTLCs on the given channel.
        pub fn force_close_channel(&self, channel_id: &[u8; 32]) {
+               let _ = self.total_consistency_lock.read().unwrap();
+
                let mut chan = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = channel_state_lock.borrow_parts();
@@ -625,7 +635,8 @@ impl ChannelManager {
        }
 
        #[inline]
-       fn gen_rho_mu_from_shared_secret(shared_secret: &SharedSecret) -> ([u8; 32], [u8; 32]) {
+       fn gen_rho_mu_from_shared_secret(shared_secret: &[u8]) -> ([u8; 32], [u8; 32]) {
+               assert_eq!(shared_secret.len(), 32);
                ({
                        let mut hmac = Hmac::new(Sha256::new(), &[0x72, 0x68, 0x6f]); // rho
                        hmac.input(&shared_secret[..]);
@@ -643,7 +654,8 @@ impl ChannelManager {
        }
 
        #[inline]
-       fn gen_um_from_shared_secret(shared_secret: &SharedSecret) -> [u8; 32] {
+       fn gen_um_from_shared_secret(shared_secret: &[u8]) -> [u8; 32] {
+               assert_eq!(shared_secret.len(), 32);
                let mut hmac = Hmac::new(Sha256::new(), &[0x75, 0x6d]); // um
                hmac.input(&shared_secret[..]);
                let mut res = [0; 32];
@@ -652,7 +664,8 @@ impl ChannelManager {
        }
 
        #[inline]
-       fn gen_ammag_from_shared_secret(shared_secret: &SharedSecret) -> [u8; 32] {
+       fn gen_ammag_from_shared_secret(shared_secret: &[u8]) -> [u8; 32] {
+               assert_eq!(shared_secret.len(), 32);
                let mut hmac = Hmac::new(Sha256::new(), &[0x61, 0x6d, 0x6d, 0x61, 0x67]); // ammag
                hmac.input(&shared_secret[..]);
                let mut res = [0; 32];
@@ -691,7 +704,7 @@ impl ChannelManager {
                let mut res = Vec::with_capacity(route.hops.len());
 
                Self::construct_onion_keys_callback(secp_ctx, route, session_priv, |shared_secret, _blinding_factor, ephemeral_pubkey, _| {
-                       let (rho, mu) = ChannelManager::gen_rho_mu_from_shared_secret(&shared_secret);
+                       let (rho, mu) = ChannelManager::gen_rho_mu_from_shared_secret(&shared_secret[..]);
 
                        res.push(OnionKeys {
                                #[cfg(test)]
@@ -815,7 +828,7 @@ impl ChannelManager {
 
        /// Encrypts a failure packet. raw_packet can either be a
        /// msgs::DecodedOnionErrorPacket.encode() result or a msgs::OnionErrorPacket.data element.
-       fn encrypt_failure_packet(shared_secret: &SharedSecret, raw_packet: &[u8]) -> msgs::OnionErrorPacket {
+       fn encrypt_failure_packet(shared_secret: &[u8], raw_packet: &[u8]) -> msgs::OnionErrorPacket {
                let ammag = ChannelManager::gen_ammag_from_shared_secret(&shared_secret);
 
                let mut packet_crypted = Vec::with_capacity(raw_packet.len());
@@ -827,7 +840,8 @@ impl ChannelManager {
                }
        }
 
-       fn build_failure_packet(shared_secret: &SharedSecret, failure_type: u16, failure_data: &[u8]) -> msgs::DecodedOnionErrorPacket {
+       fn build_failure_packet(shared_secret: &[u8], failure_type: u16, failure_data: &[u8]) -> msgs::DecodedOnionErrorPacket {
+               assert_eq!(shared_secret.len(), 32);
                assert!(failure_data.len() <= 256 - 2);
 
                let um = ChannelManager::gen_um_from_shared_secret(&shared_secret);
@@ -858,7 +872,7 @@ impl ChannelManager {
        }
 
        #[inline]
-       fn build_first_hop_failure_packet(shared_secret: &SharedSecret, failure_type: u16, failure_data: &[u8]) -> msgs::OnionErrorPacket {
+       fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type: u16, failure_data: &[u8]) -> msgs::OnionErrorPacket {
                let failure_packet = ChannelManager::build_failure_packet(shared_secret, failure_type, failure_data);
                ChannelManager::encrypt_failure_packet(shared_secret, &failure_packet.encode()[..])
        }
@@ -886,7 +900,11 @@ impl ChannelManager {
                        })), self.channel_state.lock().unwrap());
                }
 
-               let shared_secret = SharedSecret::new(&self.secp_ctx, &msg.onion_routing_packet.public_key.unwrap(), &self.our_network_key);
+               let shared_secret = {
+                       let mut arr = [0; 32];
+                       arr.copy_from_slice(&SharedSecret::new(&self.secp_ctx, &msg.onion_routing_packet.public_key.unwrap(), &self.our_network_key)[..]);
+                       arr
+               };
                let (rho, mu) = ChannelManager::gen_rho_mu_from_shared_secret(&shared_secret);
 
                let mut channel_state = None;
@@ -963,7 +981,7 @@ impl ChannelManager {
                                        onion_packet: None,
                                        payment_hash: msg.payment_hash.clone(),
                                        short_channel_id: 0,
-                                       incoming_shared_secret: shared_secret.clone(),
+                                       incoming_shared_secret: shared_secret,
                                        amt_to_forward: next_hop_data.data.amt_to_forward,
                                        outgoing_cltv_value: next_hop_data.data.outgoing_cltv_value,
                                })
@@ -977,7 +995,7 @@ impl ChannelManager {
                                let blinding_factor = {
                                        let mut sha = Sha256::new();
                                        sha.input(&new_pubkey.serialize()[..]);
-                                       sha.input(&shared_secret[..]);
+                                       sha.input(&shared_secret);
                                        let mut res = [0u8; 32];
                                        sha.result(&mut res);
                                        match SecretKey::from_slice(&self.secp_ctx, &res) {
@@ -1003,7 +1021,7 @@ impl ChannelManager {
                                        onion_packet: Some(outgoing_packet),
                                        payment_hash: msg.payment_hash.clone(),
                                        short_channel_id: next_hop_data.data.short_channel_id,
-                                       incoming_shared_secret: shared_secret.clone(),
+                                       incoming_shared_secret: shared_secret,
                                        amt_to_forward: next_hop_data.data.amt_to_forward,
                                        outgoing_cltv_value: next_hop_data.data.outgoing_cltv_value,
                                })
@@ -1140,6 +1158,7 @@ impl ChannelManager {
                let (onion_payloads, htlc_msat, htlc_cltv) = ChannelManager::build_onion_payloads(&route, cur_height)?;
                let onion_packet = ChannelManager::construct_onion_packet(onion_payloads, onion_keys, &payment_hash);
 
+               let _ = self.total_consistency_lock.read().unwrap();
                let mut channel_state = self.channel_state.lock().unwrap();
 
                let id = match channel_state.short_to_id.get(&route.hops.first().unwrap().short_channel_id) {
@@ -1196,6 +1215,8 @@ impl ChannelManager {
        /// May panic if the funding_txo is duplicative with some other channel (note that this should
        /// be trivially prevented by using unique funding transaction keys per-channel).
        pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_txo: OutPoint) {
+               let _ = self.total_consistency_lock.read().unwrap();
+
                let (chan, msg, chan_monitor) = {
                        let mut channel_state = self.channel_state.lock().unwrap();
                        match channel_state.by_id.remove(temporary_channel_id) {
@@ -1261,6 +1282,8 @@ impl ChannelManager {
        /// Should only really ever be called in response to an PendingHTLCsForwardable event.
        /// Will likely generate further events.
        pub fn process_pending_htlc_forwards(&self) {
+               let _ = self.total_consistency_lock.read().unwrap();
+
                let mut new_events = Vec::new();
                let mut failed_forwards = Vec::new();
                {
@@ -1382,6 +1405,8 @@ impl ChannelManager {
 
        /// Indicates that the preimage for payment_hash is unknown or the received amount is incorrect after a PaymentReceived event.
        pub fn fail_htlc_backwards(&self, payment_hash: &[u8; 32], reason: PaymentFailReason) -> bool {
+               let _ = self.total_consistency_lock.read().unwrap();
+
                let mut channel_state = Some(self.channel_state.lock().unwrap());
                let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(payment_hash);
                if let Some(mut sources) = removed_source {
@@ -1477,6 +1502,8 @@ impl ChannelManager {
                let mut payment_hash = [0; 32];
                sha.result(&mut payment_hash);
 
+               let _ = self.total_consistency_lock.read().unwrap();
+
                let mut channel_state = Some(self.channel_state.lock().unwrap());
                let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(&payment_hash);
                if let Some(mut sources) = removed_source {
@@ -1555,6 +1582,7 @@ impl ChannelManager {
                let mut close_results = Vec::new();
                let mut htlc_forwards = Vec::new();
                let mut htlc_failures = Vec::new();
+               let _ = self.total_consistency_lock.read().unwrap();
 
                {
                        let mut channel_lock = self.channel_state.lock().unwrap();
@@ -1938,7 +1966,7 @@ impl ChannelManager {
                                let amt_to_forward = htlc_msat - route_hop.fee_msat;
                                htlc_msat = amt_to_forward;
 
-                               let ammag = ChannelManager::gen_ammag_from_shared_secret(&shared_secret);
+                               let ammag = ChannelManager::gen_ammag_from_shared_secret(&shared_secret[..]);
 
                                let mut decryption_tmp = Vec::with_capacity(packet_decrypted.len());
                                decryption_tmp.resize(packet_decrypted.len(), 0);
@@ -1949,7 +1977,7 @@ impl ChannelManager {
                                let is_from_final_node = route.hops.last().unwrap().pubkey == route_hop.pubkey;
 
                                if let Ok(err_packet) = msgs::DecodedOnionErrorPacket::read(&mut Cursor::new(&packet_decrypted)) {
-                                       let um = ChannelManager::gen_um_from_shared_secret(&shared_secret);
+                                       let um = ChannelManager::gen_um_from_shared_secret(&shared_secret[..]);
                                        let mut hmac = Hmac::new(Sha256::new(), &um);
                                        hmac.input(&err_packet.encode()[32..]);
                                        let mut calc_tag = [0u8; 32];
@@ -2359,6 +2387,7 @@ impl ChannelManager {
        /// Note: This API is likely to change!
        #[doc(hidden)]
        pub fn update_fee(&self, channel_id: [u8;32], feerate_per_kw: u64) -> Result<(), APIError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                let mut channel_state_lock = self.channel_state.lock().unwrap();
                let channel_state = channel_state_lock.borrow_parts();
 
@@ -2416,6 +2445,7 @@ impl events::EventsProvider for ChannelManager {
 
 impl ChainListener for ChannelManager {
        fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) {
+               let _ = self.total_consistency_lock.read().unwrap();
                let mut failed_channels = Vec::new();
                {
                        let mut channel_lock = self.channel_state.lock().unwrap();
@@ -2493,6 +2523,7 @@ impl ChainListener for ChannelManager {
 
        /// We force-close the channel without letting our counterparty participate in the shutdown
        fn block_disconnected(&self, header: &BlockHeader) {
+               let _ = self.total_consistency_lock.read().unwrap();
                let mut failed_channels = Vec::new();
                {
                        let mut channel_lock = self.channel_state.lock().unwrap();
@@ -2558,70 +2589,87 @@ macro_rules! handle_error {
 impl ChannelMessageHandler for ChannelManager {
        //TODO: Handle errors and close channel (or so)
        fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_open_channel(their_node_id, msg), their_node_id)
        }
 
        fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_accept_channel(their_node_id, msg), their_node_id)
        }
 
        fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_funding_created(their_node_id, msg), their_node_id)
        }
 
        fn handle_funding_signed(&self, their_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_funding_signed(their_node_id, msg), their_node_id)
        }
 
        fn handle_funding_locked(&self, their_node_id: &PublicKey, msg: &msgs::FundingLocked) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_funding_locked(their_node_id, msg), their_node_id)
        }
 
        fn handle_shutdown(&self, their_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_shutdown(their_node_id, msg), their_node_id)
        }
 
        fn handle_closing_signed(&self, their_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_closing_signed(their_node_id, msg), their_node_id)
        }
 
        fn handle_update_add_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateAddHTLC) -> Result<(), msgs::HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_update_add_htlc(their_node_id, msg), their_node_id)
        }
 
        fn handle_update_fulfill_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_update_fulfill_htlc(their_node_id, msg), their_node_id)
        }
 
        fn handle_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_update_fail_htlc(their_node_id, msg), their_node_id)
        }
 
        fn handle_update_fail_malformed_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailMalformedHTLC) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_update_fail_malformed_htlc(their_node_id, msg), their_node_id)
        }
 
        fn handle_commitment_signed(&self, their_node_id: &PublicKey, msg: &msgs::CommitmentSigned) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_commitment_signed(their_node_id, msg), their_node_id)
        }
 
        fn handle_revoke_and_ack(&self, their_node_id: &PublicKey, msg: &msgs::RevokeAndACK) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_revoke_and_ack(their_node_id, msg), their_node_id)
        }
 
        fn handle_update_fee(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFee) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_update_fee(their_node_id, msg), their_node_id)
        }
 
        fn handle_announcement_signatures(&self, their_node_id: &PublicKey, msg: &msgs::AnnouncementSignatures) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_announcement_signatures(their_node_id, msg), their_node_id)
        }
 
        fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), HandleError> {
+               let _ = self.total_consistency_lock.read().unwrap();
                handle_error!(self, self.internal_channel_reestablish(their_node_id, msg), their_node_id)
        }
 
        fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool) {
+               let _ = self.total_consistency_lock.read().unwrap();
                let mut failed_channels = Vec::new();
                let mut failed_payments = Vec::new();
                {
@@ -2677,6 +2725,7 @@ impl ChannelMessageHandler for ChannelManager {
        }
 
        fn peer_connected(&self, their_node_id: &PublicKey) {
+               let _ = self.total_consistency_lock.read().unwrap();
                let mut channel_state_lock = self.channel_state.lock().unwrap();
                let channel_state = channel_state_lock.borrow_parts();
                let pending_msg_events = channel_state.pending_msg_events;
@@ -2701,6 +2750,8 @@ impl ChannelMessageHandler for ChannelManager {
        }
 
        fn handle_error(&self, their_node_id: &PublicKey, msg: &msgs::ErrorMessage) {
+               let _ = self.total_consistency_lock.read().unwrap();
+
                if msg.channel_id == [0; 32] {
                        for chan in self.list_channels() {
                                if chan.remote_network_id == *their_node_id {
@@ -2889,22 +2940,22 @@ mod tests {
                // Returning Errors test vectors from BOLT 4
 
                let onion_keys = build_test_onion_keys();
-               let onion_error = ChannelManager::build_failure_packet(&onion_keys[4].shared_secret, 0x2002, &[0; 0]);
+               let onion_error = ChannelManager::build_failure_packet(&onion_keys[4].shared_secret[..], 0x2002, &[0; 0]);
                assert_eq!(onion_error.encode(), hex::decode("4c2fc8bc08510334b6833ad9c3e79cd1b52ae59dfe5c2a4b23ead50f09f7ee0b0002200200fe0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000").unwrap());
 
-               let onion_packet_1 = ChannelManager::encrypt_failure_packet(&onion_keys[4].shared_secret, &onion_error.encode()[..]);
+               let onion_packet_1 = ChannelManager::encrypt_failure_packet(&onion_keys[4].shared_secret[..], &onion_error.encode()[..]);
                assert_eq!(onion_packet_1.data, hex::decode("a5e6bd0c74cb347f10cce367f949098f2457d14c046fd8a22cb96efb30b0fdcda8cb9168b50f2fd45edd73c1b0c8b33002df376801ff58aaa94000bf8a86f92620f343baef38a580102395ae3abf9128d1047a0736ff9b83d456740ebbb4aeb3aa9737f18fb4afb4aa074fb26c4d702f42968888550a3bded8c05247e045b866baef0499f079fdaeef6538f31d44deafffdfd3afa2fb4ca9082b8f1c465371a9894dd8c243fb4847e004f5256b3e90e2edde4c9fb3082ddfe4d1e734cacd96ef0706bf63c9984e22dc98851bcccd1c3494351feb458c9c6af41c0044bea3c47552b1d992ae542b17a2d0bba1a096c78d169034ecb55b6e3a7263c26017f033031228833c1daefc0dedb8cf7c3e37c9c37ebfe42f3225c326e8bcfd338804c145b16e34e4").unwrap());
 
-               let onion_packet_2 = ChannelManager::encrypt_failure_packet(&onion_keys[3].shared_secret, &onion_packet_1.data[..]);
+               let onion_packet_2 = ChannelManager::encrypt_failure_packet(&onion_keys[3].shared_secret[..], &onion_packet_1.data[..]);
                assert_eq!(onion_packet_2.data, hex::decode("c49a1ce81680f78f5f2000cda36268de34a3f0a0662f55b4e837c83a8773c22aa081bab1616a0011585323930fa5b9fae0c85770a2279ff59ec427ad1bbff9001c0cd1497004bd2a0f68b50704cf6d6a4bf3c8b6a0833399a24b3456961ba00736785112594f65b6b2d44d9f5ea4e49b5e1ec2af978cbe31c67114440ac51a62081df0ed46d4a3df295da0b0fe25c0115019f03f15ec86fabb4c852f83449e812f141a9395b3f70b766ebbd4ec2fae2b6955bd8f32684c15abfe8fd3a6261e52650e8807a92158d9f1463261a925e4bfba44bd20b166d532f0017185c3a6ac7957adefe45559e3072c8dc35abeba835a8cb01a71a15c736911126f27d46a36168ca5ef7dccd4e2886212602b181463e0dd30185c96348f9743a02aca8ec27c0b90dca270").unwrap());
 
-               let onion_packet_3 = ChannelManager::encrypt_failure_packet(&onion_keys[2].shared_secret, &onion_packet_2.data[..]);
+               let onion_packet_3 = ChannelManager::encrypt_failure_packet(&onion_keys[2].shared_secret[..], &onion_packet_2.data[..]);
                assert_eq!(onion_packet_3.data, hex::decode("a5d3e8634cfe78b2307d87c6d90be6fe7855b4f2cc9b1dfb19e92e4b79103f61ff9ac25f412ddfb7466e74f81b3e545563cdd8f5524dae873de61d7bdfccd496af2584930d2b566b4f8d3881f8c043df92224f38cf094cfc09d92655989531524593ec6d6caec1863bdfaa79229b5020acc034cd6deeea1021c50586947b9b8e6faa83b81fbfa6133c0af5d6b07c017f7158fa94f0d206baf12dda6b68f785b773b360fd0497e16cc402d779c8d48d0fa6315536ef0660f3f4e1865f5b38ea49c7da4fd959de4e83ff3ab686f059a45c65ba2af4a6a79166aa0f496bf04d06987b6d2ea205bdb0d347718b9aeff5b61dfff344993a275b79717cd815b6ad4c0beb568c4ac9c36ff1c315ec1119a1993c4b61e6eaa0375e0aaf738ac691abd3263bf937e3").unwrap());
 
-               let onion_packet_4 = ChannelManager::encrypt_failure_packet(&onion_keys[1].shared_secret, &onion_packet_3.data[..]);
+               let onion_packet_4 = ChannelManager::encrypt_failure_packet(&onion_keys[1].shared_secret[..], &onion_packet_3.data[..]);
                assert_eq!(onion_packet_4.data, hex::decode("aac3200c4968f56b21f53e5e374e3a2383ad2b1b6501bbcc45abc31e59b26881b7dfadbb56ec8dae8857add94e6702fb4c3a4de22e2e669e1ed926b04447fc73034bb730f4932acd62727b75348a648a1128744657ca6a4e713b9b646c3ca66cac02cdab44dd3439890ef3aaf61708714f7375349b8da541b2548d452d84de7084bb95b3ac2345201d624d31f4d52078aa0fa05a88b4e20202bd2b86ac5b52919ea305a8949de95e935eed0319cf3cf19ebea61d76ba92532497fcdc9411d06bcd4275094d0a4a3c5d3a945e43305a5a9256e333e1f64dbca5fcd4e03a39b9012d197506e06f29339dfee3331995b21615337ae060233d39befea925cc262873e0530408e6990f1cbd233a150ef7b004ff6166c70c68d9f8c853c1abca640b8660db2921").unwrap());
 
-               let onion_packet_5 = ChannelManager::encrypt_failure_packet(&onion_keys[0].shared_secret, &onion_packet_4.data[..]);
+               let onion_packet_5 = ChannelManager::encrypt_failure_packet(&onion_keys[0].shared_secret[..], &onion_packet_4.data[..]);
                assert_eq!(onion_packet_5.data, hex::decode("9c5add3963fc7f6ed7f148623c84134b5647e1306419dbe2174e523fa9e2fbed3a06a19f899145610741c83ad40b7712aefaddec8c6baf7325d92ea4ca4d1df8bce517f7e54554608bf2bd8071a4f52a7a2f7ffbb1413edad81eeea5785aa9d990f2865dc23b4bc3c301a94eec4eabebca66be5cf638f693ec256aec514620cc28ee4a94bd9565bc4d4962b9d3641d4278fb319ed2b84de5b665f307a2db0f7fbb757366067d88c50f7e829138fde4f78d39b5b5802f1b92a8a820865af5cc79f9f30bc3f461c66af95d13e5e1f0381c184572a91dee1c849048a647a1158cf884064deddbf1b0b88dfe2f791428d0ba0f6fb2f04e14081f69165ae66d9297c118f0907705c9c4954a199bae0bb96fad763d690e7daa6cfda59ba7f2c8d11448b604d12d").unwrap());
        }
 
@@ -5500,25 +5551,27 @@ mod tests {
                        // Drop the payment_event messages, and let them get re-generated in reconnect_nodes!
                } else {
                        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
-                       nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap();
-                       check_added_monitors!(nodes[1], 1);
-                       let (bs_revoke_and_ack, bs_commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
-
                        if messages_delivered >= 3 {
-                               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap();
-                               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
-                               check_added_monitors!(nodes[0], 1);
+                               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap();
+                               check_added_monitors!(nodes[1], 1);
+                               let (bs_revoke_and_ack, bs_commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
 
                                if messages_delivered >= 4 {
-                                       nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_commitment_signed).unwrap();
-                                       let as_revoke_and_ack = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
-                                       // No commitment_signed so get_event_msg's assert(len == 1) passes
+                                       nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap();
+                                       assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                                        check_added_monitors!(nodes[0], 1);
 
                                        if messages_delivered >= 5 {
-                                               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap();
-                                               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
-                                               check_added_monitors!(nodes[1], 1);
+                                               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_commitment_signed).unwrap();
+                                               let as_revoke_and_ack = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+                                               // No commitment_signed so get_event_msg's assert(len == 1) passes
+                                               check_added_monitors!(nodes[0], 1);
+
+                                               if messages_delivered >= 6 {
+                                                       nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap();
+                                                       assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+                                                       check_added_monitors!(nodes[1], 1);
+                                               }
                                        }
                                }
                        }
@@ -5526,20 +5579,20 @@ mod tests {
 
                nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
                nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
-               if messages_delivered < 2 {
+               if messages_delivered < 3 {
                        // Even if the funding_locked messages get exchanged, as long as nothing further was
                        // received on either side, both sides will need to resend them.
                        reconnect_nodes(&nodes[0], &nodes[1], true, (0, 1), (0, 0), (0, 0), (0, 0), (false, false));
-               } else if messages_delivered == 2 {
+               } else if messages_delivered == 3 {
                        // nodes[0] still wants its RAA + commitment_signed
                        reconnect_nodes(&nodes[0], &nodes[1], false, (-1, 0), (0, 0), (0, 0), (0, 0), (true, false));
-               } else if messages_delivered == 3 {
+               } else if messages_delivered == 4 {
                        // nodes[0] still wants its commitment_signed
                        reconnect_nodes(&nodes[0], &nodes[1], false, (-1, 0), (0, 0), (0, 0), (0, 0), (false, false));
-               } else if messages_delivered == 4 {
+               } else if messages_delivered == 5 {
                        // nodes[1] still wants its final RAA
                        reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, true));
-               } else if messages_delivered == 5 {
+               } else if messages_delivered == 6 {
                        // Everything was delivered...
                        reconnect_nodes(&nodes[0], &nodes[1], false, (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
                }
@@ -5667,13 +5720,14 @@ mod tests {
                do_test_drop_messages_peer_disconnect(0);
                do_test_drop_messages_peer_disconnect(1);
                do_test_drop_messages_peer_disconnect(2);
+               do_test_drop_messages_peer_disconnect(3);
        }
 
        #[test]
        fn test_drop_messages_peer_disconnect_b() {
-               do_test_drop_messages_peer_disconnect(3);
                do_test_drop_messages_peer_disconnect(4);
                do_test_drop_messages_peer_disconnect(5);
+               do_test_drop_messages_peer_disconnect(6);
        }
 
        #[test]