Merge pull request #1957 from TheBlueMatt/2022-01-mon-ref-lockorder
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Tue, 17 Jan 2023 23:09:05 +0000 (23:09 +0000)
committerGitHub <noreply@github.com>
Tue, 17 Jan 2023 23:09:05 +0000 (23:09 +0000)
Pass MonitorUpdates by ref and tweak manager lockorder

1  2 
fuzz/src/chanmon_consistency.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/test_utils.rs

index 67e7ab1b781bc6d03de28f1ff98f2fb3d0157341,a5f14fdcfa5c45a406b2c1c4aaadd0fb2d1236e3..3c174f3093f2e20da4ec78f71558367af5b715f1
@@@ -61,7 -61,7 +61,7 @@@ use bitcoin::secp256k1::Secp256k1
  
  use std::mem;
  use std::cmp::{self, Ordering};
 -use std::collections::{HashSet, hash_map, HashMap};
 +use hashbrown::{HashSet, hash_map, HashMap};
  use std::sync::{Arc,Mutex};
  use std::sync::atomic;
  use std::io::Cursor;
@@@ -152,7 -152,7 +152,7 @@@ impl chain::Watch<EnforcingSigner> for 
                self.chain_monitor.watch_channel(funding_txo, monitor)
        }
  
-       fn update_channel(&self, funding_txo: OutPoint, update: channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus {
+       fn update_channel(&self, funding_txo: OutPoint, update: &channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus {
                let mut map_lock = self.latest_monitors.lock().unwrap();
                let mut map_entry = match map_lock.entry(funding_txo) {
                        hash_map::Entry::Occupied(entry) => entry,
                };
                let deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingSigner>)>::
                        read(&mut Cursor::new(&map_entry.get().1), (&*self.keys, &*self.keys)).unwrap().1;
-               deserialized_monitor.update_monitor(&update, &&TestBroadcaster{}, &FuzzEstimator { ret_val: atomic::AtomicU32::new(253) }, &self.logger).unwrap();
+               deserialized_monitor.update_monitor(update, &&TestBroadcaster{}, &FuzzEstimator { ret_val: atomic::AtomicU32::new(253) }, &self.logger).unwrap();
                let mut ser = VecWriter(Vec::new());
                deserialized_monitor.write(&mut ser).unwrap();
                map_entry.insert((update.update_id, ser.0));
@@@ -482,7 -482,7 +482,7 @@@ pub fn do_test<Out: Output>(data: &[u8]
                                } else { panic!("Wrong event type"); }
                        };
  
 -                      $dest.handle_open_channel(&$source.get_our_node_id(), $source.init_features(), &open_channel);
 +                      $dest.handle_open_channel(&$source.get_our_node_id(), &open_channel);
                        let accept_channel = {
                                let events = $dest.get_and_clear_pending_msg_events();
                                assert_eq!(events.len(), 1);
                                } else { panic!("Wrong event type"); }
                        };
  
 -                      $source.handle_accept_channel(&$dest.get_our_node_id(), $dest.init_features(), &accept_channel);
 +                      $source.handle_accept_channel(&$dest.get_our_node_id(), &accept_channel);
                        let funding_output;
                        {
                                let events = $source.get_and_clear_pending_events();
index 24f0ffdcd4922d9a6743b67aa9e3c98dfa092e29,29903cbbcf0b8ece381540d1b552b433eb68d1c5..385f5b5353ca5f697880c441061de805cfbc2d28
@@@ -147,9 -147,9 +147,9 @@@ fn test_monitor_and_persister_update_fa
                        // Check that even though the persister is returning a InProgress,
                        // because the update is bogus, ultimately the error that's returned
                        // should be a PermanentFailure.
-                       if let ChannelMonitorUpdateStatus::PermanentFailure = chain_mon.chain_monitor.update_channel(outpoint, update.clone()) {} else { panic!("Expected monitor error to be permanent"); }
+                       if let ChannelMonitorUpdateStatus::PermanentFailure = chain_mon.chain_monitor.update_channel(outpoint, &update) {} else { panic!("Expected monitor error to be permanent"); }
                        logger.assert_log_regex("lightning::chain::chainmonitor".to_string(), regex::Regex::new("Persistence of ChannelMonitorUpdate for channel [0-9a-f]* in progress").unwrap(), 1);
-                       assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, update), ChannelMonitorUpdateStatus::Completed);
+                       assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
                } else { assert!(false); }
        }
  
@@@ -1832,8 -1832,8 +1832,8 @@@ fn do_during_funding_monitor_fail(confi
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
  
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 43, None).unwrap();
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
  
        let (temporary_channel_id, funding_tx, funding_output) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
  
@@@ -2547,10 -2547,10 +2547,10 @@@ fn test_temporary_error_during_shutdown
        chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
  
        nodes[0].node.close_channel(&channel_id, &nodes[1].node.get_our_node_id()).unwrap();
 -      nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &nodes[0].node.init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()));
 +      nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()));
        check_added_monitors!(nodes[1], 1);
  
 -      nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &nodes[1].node.init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()));
 +      nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()));
        check_added_monitors!(nodes[0], 1);
  
        assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
@@@ -2622,7 -2622,7 +2622,7 @@@ fn test_permanent_error_during_handling
  
        assert!(nodes[0].node.close_channel(&channel_id, &nodes[1].node.get_our_node_id()).is_ok());
        let shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id());
 -      nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &nodes[0].node.init_features(), &shutdown);
 +      nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &shutdown);
        check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 2);
        check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
@@@ -2747,7 -2747,7 +2747,7 @@@ fn do_test_outbound_reload_without_init
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
  
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 43, None).unwrap();
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
  
        let events = nodes[1].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
                _ => panic!("Unexpected event"),
        };
  
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
  
        let (temporary_channel_id, funding_tx, ..) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
  
@@@ -2836,7 -2836,7 +2836,7 @@@ fn do_test_inbound_reload_without_init_
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
  
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 43, None).unwrap();
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
  
        let events = nodes[1].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
                _ => panic!("Unexpected event"),
        };
  
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
  
        let (temporary_channel_id, funding_tx, ..) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
  
index b599034eed710458d539c6243db206c23eed73cd,78d77f121348bccfd8874584d86de06630b763c6..5b312f04e4a13ba987539a8bde2481f42cd20550
@@@ -577,13 -577,13 +577,13 @@@ pub type SimpleRefChannelManager<'a, 'b
  //  |   |
  //  |   |__`pending_intercepted_htlcs`
  //  |
- //  |__`pending_inbound_payments`
+ //  |__`per_peer_state`
  //  |   |
- //  |   |__`claimable_payments`
- //  |   |
- //  |   |__`pending_outbound_payments` // This field's struct contains a map of pending outbounds
+ //  |   |__`pending_inbound_payments`
+ //  |       |
+ //  |       |__`claimable_payments`
  //  |       |
- //  |       |__`per_peer_state`
+ //  |       |__`pending_outbound_payments` // This field's struct contains a map of pending outbounds
  //  |           |
  //  |           |__`peer_state`
  //  |               |
@@@ -1709,7 -1709,7 +1709,7 @@@ wher
  
                                        // Update the monitor with the shutdown script if necessary.
                                        if let Some(monitor_update) = monitor_update {
-                                               let update_res = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update);
+                                               let update_res = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), &monitor_update);
                                                let (result, is_permanent) =
                                                        handle_monitor_update_res!(self, update_res, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, chan_entry.key(), NO_UPDATE);
                                                if is_permanent {
                        // force-closing. The monitor update on the required in-memory copy should broadcast
                        // the latest local state, which is the best we can do anyway. Thus, it is safe to
                        // ignore the result here.
-                       let _ = self.chain_monitor.update_channel(funding_txo, monitor_update);
+                       let _ = self.chain_monitor.update_channel(funding_txo, &monitor_update);
                }
        }
  
                        // short_channel_id is non-0 in any ::Forward.
                        if let &PendingHTLCRouting::Forward { ref short_channel_id, .. } = routing {
                                if let Some((err, mut code, chan_update)) = loop {
 -                                      let id_option = self.short_to_chan_info.read().unwrap().get(&short_channel_id).cloned();
 +                                      let id_option = self.short_to_chan_info.read().unwrap().get(short_channel_id).cloned();
                                        let forwarding_chan_info_opt = match id_option {
                                                None => { // unknown_next_peer
                                                        // Note that this is likely a timing oracle for detecting whether an scid is a
                                                chan)
                                } {
                                        Some((update_add, commitment_signed, monitor_update)) => {
-                                               let update_err = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update);
+                                               let update_err = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &monitor_update);
                                                let chan_id = chan.get().channel_id();
                                                match (update_err,
                                                        handle_monitor_update_res!(self, update_err, chan,
                let per_peer_state = self.per_peer_state.read().unwrap();
                let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
                if let None = peer_state_mutex_opt {
 -                      return Err(APIError::APIMisuseError { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })
 +                      return Err(APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })
                }
  
                let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
                                BackgroundEvent::ClosingMonitorUpdate((funding_txo, update)) => {
                                        // The channel has already been closed, so no use bothering to care about the
                                        // monitor updating completing.
-                                       let _ = self.chain_monitor.update_channel(funding_txo, update);
+                                       let _ = self.chain_monitor.update_channel(funding_txo, &update);
                                },
                        }
                }
                        // Ensure that no peer state channel storage lock is not held when calling this
                        // function.
                        // This ensures that future code doesn't introduce a lock_order requirement for
-                       // `forward_htlcs` to be locked after the `per_peer_state` locks, which calling this
-                       // function with the `per_peer_state` aquired would.
-                       assert!(self.per_peer_state.try_write().is_ok());
+                       // `forward_htlcs` to be locked after the `per_peer_state` peer locks, which calling
+                       // this function with any `per_peer_state` peer lock aquired would.
+                       let per_peer_state = self.per_peer_state.read().unwrap();
+                       for (_, peer) in per_peer_state.iter() {
+                               assert!(peer.try_lock().is_ok());
+                       }
                }
  
                //TODO: There is a timing attack here where if a node fails an HTLC back to us they can
                                match chan.get_mut().get_update_fulfill_htlc_and_commit(prev_hop.htlc_id, payment_preimage, &self.logger) {
                                        Ok(msgs_monitor_option) => {
                                                if let UpdateFulfillCommitFetch::NewClaim { msgs, htlc_value_msat, monitor_update } = msgs_monitor_option {
-                                                       match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
+                                                       match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &monitor_update) {
                                                                ChannelMonitorUpdateStatus::Completed => {},
                                                                e => {
                                                                        log_given_level!(self.logger, if e == ChannelMonitorUpdateStatus::PermanentFailure { Level::Error } else { Level::Debug },
                                                }
                                        },
                                        Err((e, monitor_update)) => {
-                                               match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) {
+                                               match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &monitor_update) {
                                                        ChannelMonitorUpdateStatus::Completed => {},
                                                        e => {
                                                                // TODO: This needs to be handled somehow - if we receive a monitor update
                        };
                        // We update the ChannelMonitor on the backward link, after
                        // receiving an `update_fulfill_htlc` from the forward link.
-                       let update_res = self.chain_monitor.update_channel(prev_hop.outpoint, preimage_update);
+                       let update_res = self.chain_monitor.update_channel(prev_hop.outpoint, &preimage_update);
                        if update_res != ChannelMonitorUpdateStatus::Completed {
                                // TODO: This needs to be handled somehow - if we receive a monitor update
                                // with a preimage we *must* somehow manage to propagate it to the upstream
                Ok(())
        }
  
 -      fn internal_open_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> {
 +      fn internal_open_channel(&self, counterparty_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> {
                if msg.chain_hash != self.genesis_hash {
                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(), msg.temporary_channel_id.clone()));
                }
                let user_channel_id = u128::from_be_bytes(random_bytes);
  
                let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
 +              let per_peer_state = self.per_peer_state.read().unwrap();
 +              let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
 +              if let None = peer_state_mutex_opt {
 +                      return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id.clone()))
 +              }
 +              let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
 +              let peer_state = &mut *peer_state_lock;
                let mut channel = match Channel::new_from_req(&self.fee_estimator, &self.entropy_source, &self.signer_provider,
 -                      counterparty_node_id.clone(), &their_features, msg, user_channel_id, &self.default_configuration,
 +                      counterparty_node_id.clone(), &peer_state.latest_features, msg, user_channel_id, &self.default_configuration,
                        self.best_block.read().unwrap().height(), &self.logger, outbound_scid_alias)
                {
                        Err(e) => {
                        },
                        Ok(res) => res
                };
 -              let per_peer_state = self.per_peer_state.read().unwrap();
 -              let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
 -              if let None = peer_state_mutex_opt {
 -                      return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id.clone()))
 -              }
 -              let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
 -              let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(channel.channel_id()) {
                        hash_map::Entry::Occupied(_) => {
                                self.outbound_scid_aliases.lock().unwrap().remove(&outbound_scid_alias);
                Ok(())
        }
  
 -      fn internal_accept_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> {
 +      fn internal_accept_channel(&self, counterparty_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> {
                let (value, output_script, user_id) = {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.temporary_channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
 -                                      try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration.channel_handshake_limits, &their_features), chan);
 +                                      try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration.channel_handshake_limits, &peer_state.latest_features), chan);
                                        (chan.get().get_value_satoshis(), chan.get().get_funding_redeemscript().to_v0_p2wsh(), chan.get().get_user_id())
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.temporary_channel_id))
                }
        }
  
 -      fn internal_shutdown(&self, counterparty_node_id: &PublicKey, their_features: &InitFeatures, msg: &msgs::Shutdown) -> Result<(), MsgHandleErrInternal> {
 +      fn internal_shutdown(&self, counterparty_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(), MsgHandleErrInternal> {
                let mut dropped_htlcs: Vec<(HTLCSource, PaymentHash)>;
                let result: Result<(), _> = loop {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                                                        if chan_entry.get().sent_shutdown() { " after we initiated shutdown" } else { "" });
                                        }
  
 -                                      let (shutdown, monitor_update, htlcs) = try_chan_entry!(self, chan_entry.get_mut().shutdown(&self.signer_provider, &their_features, &msg), chan_entry);
 +                                      let (shutdown, monitor_update, htlcs) = try_chan_entry!(self, chan_entry.get_mut().shutdown(&self.signer_provider, &peer_state.latest_features, &msg), chan_entry);
                                        dropped_htlcs = htlcs;
  
                                        // Update the monitor with the shutdown script if necessary.
                                        if let Some(monitor_update) = monitor_update {
-                                               let update_res = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update);
+                                               let update_res = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), &monitor_update);
                                                let (result, is_permanent) =
                                                        handle_monitor_update_res!(self, update_res, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, chan_entry.key(), NO_UPDATE);
                                                if is_permanent {
                                                Err((None, e)) => try_chan_entry!(self, Err(e), chan),
                                                Err((Some(update), e)) => {
                                                        assert!(chan.get().is_awaiting_monitor_update());
-                                                       let _ = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), update);
+                                                       let _ = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &update);
                                                        try_chan_entry!(self, Err(e), chan);
                                                        unreachable!();
                                                },
                                                Ok(res) => res
                                        };
-                               let update_res = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update);
+                               let update_res = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &monitor_update);
                                if let Err(e) = handle_monitor_update_res!(self, update_res, chan, RAACommitmentOrder::RevokeAndACKFirst, true, commitment_signed.is_some()) {
                                        return Err(e);
                                }
                                        let raa_updates = break_chan_entry!(self,
                                                chan.get_mut().revoke_and_ack(&msg, &self.logger), chan);
                                        htlcs_to_fail = raa_updates.holding_cell_failed_htlcs;
-                                       let update_res = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), raa_updates.monitor_update);
+                                       let update_res = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &raa_updates.monitor_update);
                                        if was_paused_for_mon_update {
                                                assert!(update_res != ChannelMonitorUpdateStatus::Completed);
                                                assert!(raa_updates.commitment_update.is_none());
                                                                ));
                                                        }
                                                        if let Some((commitment_update, monitor_update)) = commitment_opt {
-                                                               match self.chain_monitor.update_channel(chan.get_funding_txo().unwrap(), monitor_update) {
+                                                               match self.chain_monitor.update_channel(chan.get_funding_txo().unwrap(), &monitor_update) {
                                                                        ChannelMonitorUpdateStatus::Completed => {
                                                                                pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
                                                                                        node_id: chan.get_counterparty_node_id(),
@@@ -5976,14 -5979,14 +5979,14 @@@ wher
        R::Target: Router,
        L::Target: Logger,
  {
 -      fn handle_open_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel) {
 +      fn handle_open_channel(&self, counterparty_node_id: &PublicKey, msg: &msgs::OpenChannel) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 -              let _ = handle_error!(self, self.internal_open_channel(counterparty_node_id, their_features, msg), *counterparty_node_id);
 +              let _ = handle_error!(self, self.internal_open_channel(counterparty_node_id, msg), *counterparty_node_id);
        }
  
 -      fn handle_accept_channel(&self, counterparty_node_id: &PublicKey, their_features: InitFeatures, msg: &msgs::AcceptChannel) {
 +      fn handle_accept_channel(&self, counterparty_node_id: &PublicKey, msg: &msgs::AcceptChannel) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 -              let _ = handle_error!(self, self.internal_accept_channel(counterparty_node_id, their_features, msg), *counterparty_node_id);
 +              let _ = handle_error!(self, self.internal_accept_channel(counterparty_node_id, msg), *counterparty_node_id);
        }
  
        fn handle_funding_created(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingCreated) {
                let _ = handle_error!(self, self.internal_channel_ready(counterparty_node_id, msg), *counterparty_node_id);
        }
  
 -      fn handle_shutdown(&self, counterparty_node_id: &PublicKey, their_features: &InitFeatures, msg: &msgs::Shutdown) {
 +      fn handle_shutdown(&self, counterparty_node_id: &PublicKey, msg: &msgs::Shutdown) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 -              let _ = handle_error!(self, self.internal_shutdown(counterparty_node_id, their_features, msg), *counterparty_node_id);
 +              let _ = handle_error!(self, self.internal_shutdown(counterparty_node_id, msg), *counterparty_node_id);
        }
  
        fn handle_closing_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::ClosingSigned) {
@@@ -6739,6 -6742,8 +6742,8 @@@ wher
                        }
                }
  
+               let per_peer_state = self.per_peer_state.write().unwrap();
                let pending_inbound_payments = self.pending_inbound_payments.lock().unwrap();
                let claimable_payments = self.claimable_payments.lock().unwrap();
                let pending_outbound_payments = self.pending_outbound_payments.pending_outbound_payments.lock().unwrap();
                        htlc_purposes.push(purpose);
                }
  
-               let per_peer_state = self.per_peer_state.write().unwrap();
                (per_peer_state.len() as u64).write(writer)?;
                for (peer_pubkey, peer_state_mutex) in per_peer_state.iter() {
                        peer_pubkey.write(writer)?;
@@@ -7116,7 -7120,7 +7120,7 @@@ wher
                        }
                }
  
 -              for (ref funding_txo, ref mut monitor) in args.channel_monitors.iter_mut() {
 +              for (funding_txo, monitor) in args.channel_monitors.iter_mut() {
                        if !funding_txo_set.contains(funding_txo) {
                                log_info!(args.logger, "Broadcasting latest holder commitment transaction for closed channel {}", log_bytes!(funding_txo.to_channel_id()));
                                monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
@@@ -8019,9 -8023,9 +8023,9 @@@ mod tests 
  
                nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 1_000_000, 500_000_000, 42, None).unwrap();
                let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
 -              nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_channel);
 +              nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel);
                let accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
 -              nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &accept_channel);
 +              nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
  
                let (temporary_channel_id, tx, _funding_output) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 1_000_000, 42);
                let channel_id = &tx.txid().into_inner();
                update_nodes_with_chan_announce(&nodes, 0, 1, &announcement, &nodes_0_update, &nodes_1_update);
  
                nodes[0].node.close_channel(channel_id, &nodes[1].node.get_our_node_id()).unwrap();
 -              nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &nodes[0].node.init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()));
 +              nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()));
                let nodes_1_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id());
 -              nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &nodes[1].node.init_features(), &nodes_1_shutdown);
 +              nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &nodes_1_shutdown);
  
                let closing_signed_node_0 = get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id());
                nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &closing_signed_node_0);
                // creating dummy ones.
                nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 1_000_000, 500_000_000, 42, None).unwrap();
                let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
 -              nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_channel_msg);
 +              nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
                let accept_channel_msg = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
  
                // Dummy values
                // Test the API functions and message handlers.
                check_not_connected_to_peer_error(nodes[0].node.create_channel(unkown_public_key, 1_000_000, 500_000_000, 42, None), unkown_public_key);
  
 -              nodes[1].node.handle_open_channel(&unkown_public_key, nodes[0].node.init_features(), &open_channel_msg);
 +              nodes[1].node.handle_open_channel(&unkown_public_key, &open_channel_msg);
  
 -              nodes[0].node.handle_accept_channel(&unkown_public_key, nodes[1].node.init_features(), &accept_channel_msg);
 +              nodes[0].node.handle_accept_channel(&unkown_public_key, &accept_channel_msg);
  
                check_unkown_peer_error(nodes[0].node.accept_inbound_channel(&open_channel_msg.temporary_channel_id, &unkown_public_key, 42), unkown_public_key);
  
  
                check_unkown_peer_error(nodes[0].node.update_channel_config(&unkown_public_key, &[channel_id], &ChannelConfig::default()), unkown_public_key);
  
 -              nodes[0].node.handle_shutdown(&unkown_public_key, &nodes[1].node.init_features(), &shutdown_msg);
 +              nodes[0].node.handle_shutdown(&unkown_public_key, &shutdown_msg);
  
                nodes[1].node.handle_closing_signed(&unkown_public_key, &closing_signed_msg);
  
@@@ -8384,8 -8388,8 +8388,8 @@@ pub mod bench 
                node_a.peer_connected(&node_b.get_our_node_id(), &Init { features: node_b.init_features(), remote_network_address: None }).unwrap();
                node_b.peer_connected(&node_a.get_our_node_id(), &Init { features: node_a.init_features(), remote_network_address: None }).unwrap();
                node_a.create_channel(node_b.get_our_node_id(), 8_000_000, 100_000_000, 42, None).unwrap();
 -              node_b.handle_open_channel(&node_a.get_our_node_id(), node_a.init_features(), &get_event_msg!(node_a_holder, MessageSendEvent::SendOpenChannel, node_b.get_our_node_id()));
 -              node_a.handle_accept_channel(&node_b.get_our_node_id(), node_b.init_features(), &get_event_msg!(node_b_holder, MessageSendEvent::SendAcceptChannel, node_a.get_our_node_id()));
 +              node_b.handle_open_channel(&node_a.get_our_node_id(), &get_event_msg!(node_a_holder, MessageSendEvent::SendOpenChannel, node_b.get_our_node_id()));
 +              node_a.handle_accept_channel(&node_b.get_our_node_id(), &get_event_msg!(node_b_holder, MessageSendEvent::SendAcceptChannel, node_a.get_our_node_id()));
  
                let tx;
                if let Event::FundingGenerationReady { temporary_channel_id, output_script, .. } = get_event!(node_a_holder, Event::FundingGenerationReady) {
index be8dfcc925f4429bcd6d501d10f66a3dc7de8f54,c50c3c75aa411f1dbd3d03833dfbc6b641373e1c..fb95c122bf4a48d5d5008d18015480b60dae8b0b
@@@ -87,7 -87,7 +87,7 @@@ fn test_insane_channel_opens() 
        // Test helper that asserts we get the correct error string given a mutator
        // that supposedly makes the channel open message insane
        let insane_open_helper = |expected_error_str: &str, message_mutator: fn(msgs::OpenChannel) -> msgs::OpenChannel| {
 -              nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &message_mutator(open_channel_message.clone()));
 +              nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &message_mutator(open_channel_message.clone()));
                let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
                assert_eq!(msg_events.len(), 1);
                let expected_regex = regex::Regex::new(expected_error_str).unwrap();
@@@ -165,7 -165,7 +165,7 @@@ fn do_test_counterparty_no_reserve(send
                open_channel_message.channel_reserve_satoshis = 0;
                open_channel_message.max_htlc_value_in_flight_msat = 100_000_000;
        }
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_channel_message);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_message);
  
        // Extract the channel accept message from node1 to node0
        let mut accept_channel_message = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
                accept_channel_message.channel_reserve_satoshis = 0;
                accept_channel_message.max_htlc_value_in_flight_msat = 100_000_000;
        }
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &accept_channel_message);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel_message);
        {
                let sender_node = if send_from_initiator { &nodes[1] } else { &nodes[0] };
                let counterparty_node = if send_from_initiator { &nodes[0] } else { &nodes[1] };
@@@ -519,11 -519,11 +519,11 @@@ fn do_test_sanity_on_in_flight_opens(st
        let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
  
        if steps & 0x0f == 1 { return; }
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_channel);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel);
        let accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
  
        if steps & 0x0f == 2 { return; }
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &accept_channel);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
  
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
  
@@@ -1633,7 -1633,7 +1633,7 @@@ fn test_chan_init_feerate_unaffordabili
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, push_amt, 42, None).unwrap();
        let mut open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
        open_channel_msg.push_msat += 1;
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_channel_msg);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
  
        let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
        assert_eq!(msg_events.len(), 1);
@@@ -3529,9 -3529,9 +3529,9 @@@ fn test_peer_disconnected_before_fundin
        // broadcasted, even though it's created by `nodes[0]`.
        let expected_temporary_channel_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 1_000_000, 500_000_000, 42, None).unwrap();
        let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_channel);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel);
        let accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &accept_channel);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
  
        let (temporary_channel_id, tx, _funding_output) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 1_000_000, 42);
        assert_eq!(temporary_channel_id, expected_temporary_channel_id);
@@@ -5517,7 -5517,7 +5517,7 @@@ fn bolt2_open_channel_sending_node_chec
        let push_msat=10001;
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), channel_value_satoshis, push_msat, 42, None).unwrap();
        let node0_to_1_send_open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &node0_to_1_send_open_channel);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &node0_to_1_send_open_channel);
        get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
  
        // Create a second channel with the same random values. This used to panic due to a colliding
@@@ -5584,7 -5584,7 +5584,7 @@@ fn bolt2_open_channel_sane_dust_limit(
        node0_to_1_send_open_channel.dust_limit_satoshis = 547;
        node0_to_1_send_open_channel.channel_reserve_satoshis = 100001;
  
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &node0_to_1_send_open_channel);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &node0_to_1_send_open_channel);
        let events = nodes[1].node.get_and_clear_pending_msg_events();
        let err_msg = match events[0] {
                MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id: _ } => {
@@@ -6872,10 -6872,10 +6872,10 @@@ fn test_user_configurable_csv_delay() 
  
        // We test msg.to_self_delay <= config.their_to_self_delay is enforced in Chanel::accept_channel()
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
        let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
        accept_channel.to_self_delay = 200;
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[0].node.init_features(), &accept_channel);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
        let reason_msg;
        if let MessageSendEvent::HandleError { ref action, .. } = nodes[0].node.get_and_clear_pending_msg_events()[0] {
                match action {
@@@ -7608,7 -7608,7 +7608,7 @@@ fn test_override_0msat_htlc_minimum() 
        let res = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
        assert_eq!(res.htlc_minimum_msat, 1);
  
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &res);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &res);
        let res = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
        assert_eq!(res.htlc_minimum_msat, 1);
  }
@@@ -7677,7 -7677,7 +7677,7 @@@ fn test_manually_accept_inbound_channel
        let temp_channel_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(manually_accept_conf)).unwrap();
        let res = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
  
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &res);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &res);
  
        // Assert that `nodes[1]` has no `MessageSendEvent::SendAcceptChannel` in `msg_events` before
        // accepting the inbound channel request.
@@@ -7727,7 -7727,7 +7727,7 @@@ fn test_manually_reject_inbound_channel
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(manually_accept_conf)).unwrap();
        let res = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
  
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &res);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &res);
  
        // Assert that `nodes[1]` has no `MessageSendEvent::SendAcceptChannel` in `msg_events` before
        // rejecting the inbound channel request.
@@@ -7771,7 -7771,7 +7771,7 @@@ fn test_reject_funding_before_inbound_c
        let res = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
        let temp_channel_id = res.temporary_channel_id;
  
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &res);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &res);
  
        // Assert that `nodes[1]` has no `MessageSendEvent::SendAcceptChannel` in the `msg_events`.
        assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                let channel =  get_channel_ref!(&nodes[1], nodes[0], node_1_per_peer_lock, node_1_peer_state_lock, temp_channel_id);
                channel.get_accept_channel_message()
        };
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &accept_chan_msg);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_chan_msg);
  
        let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
  
@@@ -7828,7 -7828,7 +7828,7 @@@ fn test_can_not_accept_inbound_channel_
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(manually_accept_conf)).unwrap();
        let res = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
  
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &res);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &res);
  
        // Assert that `nodes[1]` has no `MessageSendEvent::SendAcceptChannel` in `msg_events` before
        // accepting the inbound channel request.
@@@ -8135,8 -8135,8 +8135,8 @@@ fn test_update_err_monitor_lockdown() 
                let mut node_0_peer_state_lock;
                let mut channel = get_channel_ref!(nodes[0], nodes[1], node_0_per_peer_lock, node_0_peer_state_lock, chan_1.2);
                if let Ok((_, _, update)) = channel.commitment_signed(&updates.commitment_signed, &node_cfgs[0].logger) {
-                       assert_eq!(watchtower.chain_monitor.update_channel(outpoint, update.clone()), ChannelMonitorUpdateStatus::PermanentFailure);
-                       assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, update), ChannelMonitorUpdateStatus::Completed);
+                       assert_eq!(watchtower.chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::PermanentFailure);
+                       assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
                } else { assert!(false); }
        }
        // Our local monitor is in-sync and hasn't processed yet timeout
@@@ -8230,9 -8230,9 +8230,9 @@@ fn test_concurrent_monitor_claim() 
                let mut channel = get_channel_ref!(nodes[0], nodes[1], node_0_per_peer_lock, node_0_peer_state_lock, chan_1.2);
                if let Ok((_, _, update)) = channel.commitment_signed(&updates.commitment_signed, &node_cfgs[0].logger) {
                        // Watchtower Alice should already have seen the block and reject the update
-                       assert_eq!(watchtower_alice.chain_monitor.update_channel(outpoint, update.clone()), ChannelMonitorUpdateStatus::PermanentFailure);
-                       assert_eq!(watchtower_bob.chain_monitor.update_channel(outpoint, update.clone()), ChannelMonitorUpdateStatus::Completed);
-                       assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, update), ChannelMonitorUpdateStatus::Completed);
+                       assert_eq!(watchtower_alice.chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::PermanentFailure);
+                       assert_eq!(watchtower_bob.chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
+                       assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
                } else { assert!(false); }
        }
        // Our local monitor is in-sync and hasn't processed yet timeout
@@@ -8282,9 -8282,9 +8282,9 @@@ fn test_pre_lockin_no_chan_closed_updat
        // Create an initial channel
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap();
        let mut open_chan_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_chan_msg);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_chan_msg);
        let accept_chan_msg = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &accept_chan_msg);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_chan_msg);
  
        // Move the first channel through the funding flow...
        let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
@@@ -8557,7 -8557,7 +8557,7 @@@ fn test_duplicate_temporary_channel_id_
  
        // Assert that `nodes[0]` can accept both `OpenChannel` requests, even though they use the same
        // `temporary_channel_id` as they are from different peers.
 -      nodes[0].node.handle_open_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &open_chan_msg_chan_1_0);
 +      nodes[0].node.handle_open_channel(&nodes[1].node.get_our_node_id(), &open_chan_msg_chan_1_0);
        {
                let events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
                }
        }
  
 -      nodes[0].node.handle_open_channel(&nodes[2].node.get_our_node_id(), nodes[2].node.init_features(), &open_chan_msg_chan_2_0);
 +      nodes[0].node.handle_open_channel(&nodes[2].node.get_our_node_id(), &open_chan_msg_chan_2_0);
        {
                let events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
@@@ -8601,12 -8601,12 +8601,12 @@@ fn test_duplicate_chan_id() 
        // Create an initial channel
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap();
        let mut open_chan_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_chan_msg);
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_chan_msg);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
  
        // Try to create a second channel with the same temporary_channel_id as the first and check
        // that it is rejected.
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_chan_msg);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_chan_msg);
        {
                let events = nodes[1].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
        // Technically this is allowed by the spec, but we don't support it and there's little reason
        // to. Still, it shouldn't cause any other issues.
        open_chan_msg.temporary_channel_id = channel_id;
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_chan_msg);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_chan_msg);
        {
                let events = nodes[1].node.get_and_clear_pending_msg_events();
                assert_eq!(events.len(), 1);
        // Now try to create a second channel which has a duplicate funding output.
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap();
        let open_chan_2_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_chan_2_msg);
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_chan_2_msg);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
        create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); // Get and check the FundingGenerationReady event
  
        let funding_created = {
@@@ -8809,8 -8809,8 +8809,8 @@@ fn test_invalid_funding_tx() 
        let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
  
        nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 10_000, 42, None).unwrap();
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()));
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()));
  
        let (temporary_channel_id, mut tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100_000, 42);
  
@@@ -9325,9 -9325,9 +9325,9 @@@ fn do_test_max_dust_htlc_exposure(dust_
        if on_holder_tx {
                open_channel.dust_limit_satoshis = 546;
        }
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_channel);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel);
        let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &accept_channel);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
  
        let opt_anchors = false;
  
@@@ -9472,9 -9472,9 +9472,9 @@@ fn test_non_final_funding_tx() 
  
        let temp_channel_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None).unwrap();
        let open_channel_message = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
 -      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), nodes[0].node.init_features(), &open_channel_message);
 +      nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_message);
        let accept_channel_message = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
 -      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[1].node.init_features(), &accept_channel_message);
 +      nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel_message);
  
        let best_height = nodes[0].node.best_block.read().unwrap().height();
  
index 1838b1078a98d42adbd96f15af3ab93cd01f9426,b0187f0201344ebee1379435a50a915e0d58350f..f528ae2d547c61f78c09fbbe64b2a3e3da6aa173
@@@ -184,12 -184,12 +184,12 @@@ impl<'a> chain::Watch<EnforcingSigner> 
                self.chain_monitor.watch_channel(funding_txo, new_monitor)
        }
  
-       fn update_channel(&self, funding_txo: OutPoint, update: channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus {
+       fn update_channel(&self, funding_txo: OutPoint, update: &channelmonitor::ChannelMonitorUpdate) -> chain::ChannelMonitorUpdateStatus {
                // Every monitor update should survive roundtrip
                let mut w = TestVecWriter(Vec::new());
                update.write(&mut w).unwrap();
                assert!(channelmonitor::ChannelMonitorUpdate::read(
-                               &mut io::Cursor::new(&w.0)).unwrap() == update);
+                               &mut io::Cursor::new(&w.0)).unwrap() == *update);
  
                self.monitor_updates.lock().unwrap().entry(funding_txo.to_channel_id()).or_insert(Vec::new()).push(update.clone());
  
                }
  
                self.latest_monitor_update_id.lock().unwrap().insert(funding_txo.to_channel_id(),
-                       (funding_txo, update.update_id, MonitorUpdateId::from_monitor_update(&update)));
+                       (funding_txo, update.update_id, MonitorUpdateId::from_monitor_update(update)));
                let update_res = self.chain_monitor.update_channel(funding_txo, update);
                // At every point where we get a monitor update, we should be able to send a useful monitor
                // to a watchtower and disk...
@@@ -254,7 -254,7 +254,7 @@@ impl<Signer: keysinterface::Sign> chain
                chain::ChannelMonitorUpdateStatus::Completed
        }
  
-       fn update_persisted_channel(&self, funding_txo: OutPoint, update: &Option<channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<Signer>, update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
+       fn update_persisted_channel(&self, funding_txo: OutPoint, update: Option<&channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<Signer>, update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
                let mut ret = chain::ChannelMonitorUpdateStatus::Completed;
                if let Some(update_ret) = self.update_rets.lock().unwrap().pop_front() {
                        ret = update_ret;
@@@ -337,10 -337,10 +337,10 @@@ impl Drop for TestChannelMessageHandle
  }
  
  impl msgs::ChannelMessageHandler for TestChannelMessageHandler {
 -      fn handle_open_channel(&self, _their_node_id: &PublicKey, _their_features: InitFeatures, msg: &msgs::OpenChannel) {
 +      fn handle_open_channel(&self, _their_node_id: &PublicKey, msg: &msgs::OpenChannel) {
                self.received_msg(wire::Message::OpenChannel(msg.clone()));
        }
 -      fn handle_accept_channel(&self, _their_node_id: &PublicKey, _their_features: InitFeatures, msg: &msgs::AcceptChannel) {
 +      fn handle_accept_channel(&self, _their_node_id: &PublicKey, msg: &msgs::AcceptChannel) {
                self.received_msg(wire::Message::AcceptChannel(msg.clone()));
        }
        fn handle_funding_created(&self, _their_node_id: &PublicKey, msg: &msgs::FundingCreated) {
        fn handle_channel_ready(&self, _their_node_id: &PublicKey, msg: &msgs::ChannelReady) {
                self.received_msg(wire::Message::ChannelReady(msg.clone()));
        }
 -      fn handle_shutdown(&self, _their_node_id: &PublicKey, _their_features: &InitFeatures, msg: &msgs::Shutdown) {
 +      fn handle_shutdown(&self, _their_node_id: &PublicKey, msg: &msgs::Shutdown) {
                self.received_msg(wire::Message::Shutdown(msg.clone()));
        }
        fn handle_closing_signed(&self, _their_node_id: &PublicKey, msg: &msgs::ClosingSigned) {