Make ChannelMonitor clonable again
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 3f7a1055b6dc3f1dba4d21387051cbe0751293e9..6daab991922f160be0d4062f5cb392545f69789e 100644 (file)
@@ -40,7 +40,7 @@ use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLC
 use ln::channelmanager::HTLCSource;
 use chain;
 use chain::{BestBlock, WatchedOutput};
-use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
+use chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
 use chain::transaction::{OutPoint, TransactionData};
 use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface};
 use chain::onchaintx::OnchainTxHandler;
@@ -256,7 +256,7 @@ impl_writeable_tlv_based!(HolderSignedTx, {
 
 /// We use this to track static counterparty commitment transaction data and to generate any
 /// justice or 2nd-stage preimage/timeout transactions.
-#[derive(PartialEq)]
+#[derive(Clone, PartialEq)]
 struct CounterpartyCommitmentParameters {
        counterparty_delayed_payment_base_key: PublicKey,
        counterparty_htlc_base_key: PublicKey,
@@ -310,7 +310,7 @@ impl Readable for CounterpartyCommitmentParameters {
 /// transaction causing it.
 ///
 /// Used to determine when the on-chain event can be considered safe from a chain reorganization.
-#[derive(PartialEq)]
+#[derive(Clone, PartialEq)]
 struct OnchainEventEntry {
        txid: Txid,
        height: u32,
@@ -346,7 +346,7 @@ impl OnchainEventEntry {
 
 /// Upon discovering of some classes of onchain tx by ChannelMonitor, we may have to take actions on it
 /// once they mature to enough confirmations (ANTI_REORG_DELAY)
-#[derive(PartialEq)]
+#[derive(Clone, PartialEq)]
 enum OnchainEvent {
        /// An outbound HTLC failing after a transaction is confirmed. Used
        ///  * when an outbound HTLC output is spent by us after the HTLC timed out
@@ -566,7 +566,7 @@ pub enum Balance {
 }
 
 /// An HTLC which has been irrevocably resolved on-chain, and has reached ANTI_REORG_DELAY.
-#[derive(PartialEq)]
+#[derive(Clone, PartialEq)]
 struct IrrevocablyResolvedHTLC {
        commitment_tx_output_idx: u32,
        /// Only set if the HTLC claim was ours using a payment preimage
@@ -601,6 +601,13 @@ pub struct ChannelMonitor<Signer: Sign> {
        inner: Mutex<ChannelMonitorImpl<Signer>>,
 }
 
+impl<Signer: Sign> Clone for ChannelMonitor<Signer> {
+       fn clone(&self) -> Self {
+               Self { inner: Mutex::new(self.inner.lock().unwrap().clone()) }
+       }
+}
+
+#[derive(Clone)]
 pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
        latest_update_id: u64,
        commitment_transaction_number_obscure_factor: u64,
@@ -722,6 +729,9 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
        // the full block_connected).
        best_block: BestBlock,
 
+       /// The node_id of our counterparty
+       counterparty_node_id: Option<PublicKey>,
+
        secp_ctx: Secp256k1<secp256k1::All>, //TODO: dedup this a bit...
 }
 
@@ -954,6 +964,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
                        (3, self.htlcs_resolved_on_chain, vec_type),
                        (5, self.pending_monitor_events, vec_type),
                        (7, self.funding_spend_seen, required),
+                       (9, self.counterparty_node_id, option),
                });
 
                Ok(())
@@ -961,13 +972,20 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 }
 
 impl<Signer: Sign> ChannelMonitor<Signer> {
+       /// For lockorder enforcement purposes, we need to have a single site which constructs the
+       /// `inner` mutex, otherwise cases where we lock two monitors at the same time (eg in our
+       /// PartialEq implementation) we may decide a lockorder violation has occurred.
+       fn from_impl(imp: ChannelMonitorImpl<Signer>) -> Self {
+               ChannelMonitor { inner: Mutex::new(imp) }
+       }
+
        pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, keys: Signer, shutdown_script: Option<Script>,
                          on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
                          channel_parameters: &ChannelTransactionParameters,
                          funding_redeemscript: Script, channel_value_satoshis: u64,
                          commitment_transaction_number_obscure_factor: u64,
                          initial_holder_commitment_tx: HolderCommitmentTransaction,
-                         best_block: BestBlock) -> ChannelMonitor<Signer> {
+                         best_block: BestBlock, counterparty_node_id: PublicKey) -> ChannelMonitor<Signer> {
 
                assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
                let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize());
@@ -1008,59 +1026,58 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                let mut outputs_to_watch = HashMap::new();
                outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
 
-               ChannelMonitor {
-                       inner: Mutex::new(ChannelMonitorImpl {
-                               latest_update_id: 0,
-                               commitment_transaction_number_obscure_factor,
+               Self::from_impl(ChannelMonitorImpl {
+                       latest_update_id: 0,
+                       commitment_transaction_number_obscure_factor,
 
-                               destination_script: destination_script.clone(),
-                               broadcasted_holder_revokable_script: None,
-                               counterparty_payment_script,
-                               shutdown_script,
+                       destination_script: destination_script.clone(),
+                       broadcasted_holder_revokable_script: None,
+                       counterparty_payment_script,
+                       shutdown_script,
 
-                               channel_keys_id,
-                               holder_revocation_basepoint,
-                               funding_info,
-                               current_counterparty_commitment_txid: None,
-                               prev_counterparty_commitment_txid: None,
+                       channel_keys_id,
+                       holder_revocation_basepoint,
+                       funding_info,
+                       current_counterparty_commitment_txid: None,
+                       prev_counterparty_commitment_txid: None,
 
-                               counterparty_commitment_params,
-                               funding_redeemscript,
-                               channel_value_satoshis,
-                               their_cur_per_commitment_points: None,
+                       counterparty_commitment_params,
+                       funding_redeemscript,
+                       channel_value_satoshis,
+                       their_cur_per_commitment_points: None,
 
-                               on_holder_tx_csv: counterparty_channel_parameters.selected_contest_delay,
+                       on_holder_tx_csv: counterparty_channel_parameters.selected_contest_delay,
 
-                               commitment_secrets: CounterpartyCommitmentSecrets::new(),
-                               counterparty_claimable_outpoints: HashMap::new(),
-                               counterparty_commitment_txn_on_chain: HashMap::new(),
-                               counterparty_hash_commitment_number: HashMap::new(),
+                       commitment_secrets: CounterpartyCommitmentSecrets::new(),
+                       counterparty_claimable_outpoints: HashMap::new(),
+                       counterparty_commitment_txn_on_chain: HashMap::new(),
+                       counterparty_hash_commitment_number: HashMap::new(),
 
-                               prev_holder_signed_commitment_tx: None,
-                               current_holder_commitment_tx: holder_commitment_tx,
-                               current_counterparty_commitment_number: 1 << 48,
-                               current_holder_commitment_number,
+                       prev_holder_signed_commitment_tx: None,
+                       current_holder_commitment_tx: holder_commitment_tx,
+                       current_counterparty_commitment_number: 1 << 48,
+                       current_holder_commitment_number,
 
-                               payment_preimages: HashMap::new(),
-                               pending_monitor_events: Vec::new(),
-                               pending_events: Vec::new(),
+                       payment_preimages: HashMap::new(),
+                       pending_monitor_events: Vec::new(),
+                       pending_events: Vec::new(),
 
-                               onchain_events_awaiting_threshold_conf: Vec::new(),
-                               outputs_to_watch,
+                       onchain_events_awaiting_threshold_conf: Vec::new(),
+                       outputs_to_watch,
 
-                               onchain_tx_handler,
+                       onchain_tx_handler,
 
-                               lockdown_from_offchain: false,
-                               holder_tx_signed: false,
-                               funding_spend_seen: false,
-                               funding_spend_confirmed: None,
-                               htlcs_resolved_on_chain: Vec::new(),
+                       lockdown_from_offchain: false,
+                       holder_tx_signed: false,
+                       funding_spend_seen: false,
+                       funding_spend_confirmed: None,
+                       htlcs_resolved_on_chain: Vec::new(),
 
-                               best_block,
+                       best_block,
+                       counterparty_node_id: Some(counterparty_node_id),
 
-                               secp_ctx,
-                       }),
-               }
+                       secp_ctx,
+               })
        }
 
        #[cfg(test)]
@@ -1099,7 +1116,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                payment_hash: &PaymentHash,
                payment_preimage: &PaymentPreimage,
                broadcaster: &B,
-               fee_estimator: &F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: &L,
        ) where
                B::Target: BroadcasterInterface,
@@ -1129,7 +1146,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                &self,
                updates: &ChannelMonitorUpdate,
                broadcaster: &B,
-               fee_estimator: &F,
+               fee_estimator: F,
                logger: &L,
        ) -> Result<(), ()>
        where
@@ -1204,6 +1221,14 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                self.inner.lock().unwrap().get_cur_holder_commitment_number()
        }
 
+       /// Gets the `node_id` of the counterparty for this channel.
+       ///
+       /// Will be `None` for channels constructed on LDK versions prior to 0.0.110 and always `Some`
+       /// otherwise.
+       pub fn get_counterparty_node_id(&self) -> Option<PublicKey> {
+               self.inner.lock().unwrap().counterparty_node_id
+       }
+
        /// Used by ChannelManager deserialization to broadcast the latest holder state if its copy of
        /// the Channel was out-of-date. You may use it to get a broadcastable holder toxic tx in case of
        /// fallen-behind, i.e when receiving a channel_reestablish with a proof that our counterparty side knows
@@ -1295,8 +1320,9 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                F::Target: FeeEstimator,
                L::Target: Logger,
        {
+               let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
                self.inner.lock().unwrap().transactions_confirmed(
-                       header, txdata, height, broadcaster, fee_estimator, logger)
+                       header, txdata, height, broadcaster, &bounded_fee_estimator, logger)
        }
 
        /// Processes a transaction that was reorganized out of the chain.
@@ -1316,8 +1342,9 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                F::Target: FeeEstimator,
                L::Target: Logger,
        {
+               let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
                self.inner.lock().unwrap().transaction_unconfirmed(
-                       txid, broadcaster, fee_estimator, logger);
+                       txid, broadcaster, &bounded_fee_estimator, logger);
        }
 
        /// Updates the monitor with the current best chain tip, returning new outputs to watch. See
@@ -1340,8 +1367,9 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                F::Target: FeeEstimator,
                L::Target: Logger,
        {
+               let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
                self.inner.lock().unwrap().best_block_updated(
-                       header, height, broadcaster, fee_estimator, logger)
+                       header, height, broadcaster, &bounded_fee_estimator, logger)
        }
 
        /// Returns the set of txids that should be monitored for re-organization out of the chain.
@@ -1729,6 +1757,26 @@ macro_rules! fail_unbroadcast_htlcs {
        } }
 }
 
+// In the `test_invalid_funding_tx` test, we need a bogus script which matches the HTLC-Accepted
+// witness length match (ie is 136 bytes long). We generate one here which we also use in some
+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {
+       let mut ret = [opcodes::all::OP_NOP.into_u8(); 136];
+       ret[131] = opcodes::all::OP_DROP.into_u8();
+       ret[132] = opcodes::all::OP_DROP.into_u8();
+       ret[133] = opcodes::all::OP_DROP.into_u8();
+       ret[134] = opcodes::all::OP_DROP.into_u8();
+       ret[135] = opcodes::OP_TRUE.into_u8();
+       Vec::from(&ret[..])
+}
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness() -> Vec<Vec<u8>> {
+       vec![Vec::new(), Vec::new(), Vec::new(), Vec::new(), deliberately_bogus_accepted_htlc_witness_program().into()].into()
+}
+
 impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither
        /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen
@@ -1857,7 +1905,9 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 
        /// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
        /// commitment_tx_infos which contain the payment hash have been revoked.
-       fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(&mut self, payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B, fee_estimator: &F, logger: &L)
+       fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
+               &mut self, payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B,
+               fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L)
        where B::Target: BroadcasterInterface,
                    F::Target: FeeEstimator,
                    L::Target: Logger,
@@ -1914,10 +1964,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
        }
 
-       pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), ()>
+       pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: F, logger: &L) -> Result<(), ()>
        where B::Target: BroadcasterInterface,
-                   F::Target: FeeEstimator,
-                   L::Target: Logger,
+               F::Target: FeeEstimator,
+               L::Target: Logger,
        {
                log_info!(logger, "Applying update to monitor {}, bringing update_id from {} to {} with {} changes.",
                        log_funding_info!(self), self.latest_update_id, updates.update_id, updates.updates.len());
@@ -1955,7 +2005,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                },
                                ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } => {
                                        log_trace!(logger, "Updating ChannelMonitor with payment preimage");
-                                       self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()), &payment_preimage, broadcaster, fee_estimator, logger)
+                                       let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&*fee_estimator);
+                                       self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()), &payment_preimage, broadcaster, &bounded_fee_estimator, logger)
                                },
                                ChannelMonitorUpdateStep::CommitmentSecret { idx, secret } => {
                                        log_trace!(logger, "Updating ChannelMonitor with commitment secret");
@@ -2381,7 +2432,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                let block_hash = header.block_hash();
                self.best_block = BestBlock::new(block_hash, height);
 
-               self.transactions_confirmed(header, txdata, height, broadcaster, fee_estimator, logger)
+               let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
+               self.transactions_confirmed(header, txdata, height, broadcaster, &bounded_fee_estimator, logger)
        }
 
        fn best_block_updated<B: Deref, F: Deref, L: Deref>(
@@ -2389,7 +2441,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                header: &BlockHeader,
                height: u32,
                broadcaster: B,
-               fee_estimator: F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: L,
        ) -> Vec<TransactionOutputs>
        where
@@ -2416,7 +2468,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                txdata: &TransactionData,
                height: u32,
                broadcaster: B,
-               fee_estimator: F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: L,
        ) -> Vec<TransactionOutputs>
        where
@@ -2513,7 +2565,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                mut watch_outputs: Vec<TransactionOutputs>,
                mut claimable_outpoints: Vec<PackageTemplate>,
                broadcaster: &B,
-               fee_estimator: &F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: &L,
        ) -> Vec<TransactionOutputs>
        where
@@ -2651,7 +2703,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                //- maturing spendable output has transaction paying us has been disconnected
                self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height < height);
 
-               self.onchain_tx_handler.block_disconnected(height, broadcaster, fee_estimator, logger);
+               let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
+               self.onchain_tx_handler.block_disconnected(height, broadcaster, &bounded_fee_estimator, logger);
 
                self.best_block = BestBlock::new(header.prev_blockhash, height - 1);
        }
@@ -2660,7 +2713,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                &mut self,
                txid: &Txid,
                broadcaster: B,
-               fee_estimator: F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: L,
        ) where
                B::Target: BroadcasterInterface,
@@ -2701,14 +2754,21 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        if *idx == input.previous_output.vout {
                                                #[cfg(test)]
                                                {
-                                                       // If the expected script is a known type, check that the witness
-                                                       // appears to be spending the correct type (ie that the match would
-                                                       // actually succeed in BIP 158/159-style filters).
-                                                       if _script_pubkey.is_v0_p2wsh() {
-                                                               assert_eq!(&bitcoin::Address::p2wsh(&Script::from(input.witness.last().unwrap().to_vec()), bitcoin::Network::Bitcoin).script_pubkey(), _script_pubkey);
-                                                       } else if _script_pubkey.is_v0_p2wpkh() {
-                                                               assert_eq!(&bitcoin::Address::p2wpkh(&bitcoin::PublicKey::from_slice(&input.witness.last().unwrap()).unwrap(), bitcoin::Network::Bitcoin).unwrap().script_pubkey(), _script_pubkey);
-                                                       } else { panic!(); }
+                                                       // If the expected script is a known type, check that the witness
+                                                       // appears to be spending the correct type (ie that the match would
+                                                       // actually succeed in BIP 158/159-style filters).
+                                                       if _script_pubkey.is_v0_p2wsh() {
+                                                               if input.witness.last().unwrap().to_vec() == deliberately_bogus_accepted_htlc_witness_program() {
+                                                                       // In at least one test we use a deliberately bogus witness
+                                                                       // script which hit an old panic. Thus, we check for that here
+                                                                       // and avoid the assert if its the expected bogus script.
+                                                                       return true;
+                                                               }
+
+                                                               assert_eq!(&bitcoin::Address::p2wsh(&Script::from(input.witness.last().unwrap().to_vec()), bitcoin::Network::Bitcoin).script_pubkey(), _script_pubkey);
+                                                       } else if _script_pubkey.is_v0_p2wpkh() {
+                                                               assert_eq!(&bitcoin::Address::p2wpkh(&bitcoin::PublicKey::from_slice(&input.witness.last().unwrap()).unwrap(), bitcoin::Network::Bitcoin).unwrap().script_pubkey(), _script_pubkey);
+                                                       } else { panic!(); }
                                                }
                                                return true;
                                        }
@@ -2793,10 +2853,13 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        let prev_last_witness_len = input.witness.second_to_last().map(|w| w.len()).unwrap_or(0);
                        let revocation_sig_claim = (witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && prev_last_witness_len == 33)
                                || (witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && prev_last_witness_len == 33);
-                       let accepted_preimage_claim = witness_items == 5 && htlctype == Some(HTLCType::AcceptedHTLC);
+                       let accepted_preimage_claim = witness_items == 5 && htlctype == Some(HTLCType::AcceptedHTLC)
+                               && input.witness.second_to_last().unwrap().len() == 32;
                        #[cfg(not(fuzzing))]
                        let accepted_timeout_claim = witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && !revocation_sig_claim;
-                       let offered_preimage_claim = witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && !revocation_sig_claim;
+                       let offered_preimage_claim = witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) &&
+                               !revocation_sig_claim && input.witness.second_to_last().unwrap().len() == 32;
+
                        #[cfg(not(fuzzing))]
                        let offered_timeout_claim = witness_items == 5 && htlctype == Some(HTLCType::OfferedHTLC);
 
@@ -3306,69 +3369,70 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
                let mut funding_spend_confirmed = None;
                let mut htlcs_resolved_on_chain = Some(Vec::new());
                let mut funding_spend_seen = Some(false);
+               let mut counterparty_node_id = None;
                read_tlv_fields!(reader, {
                        (1, funding_spend_confirmed, option),
                        (3, htlcs_resolved_on_chain, vec_type),
                        (5, pending_monitor_events, vec_type),
                        (7, funding_spend_seen, option),
+                       (9, counterparty_node_id, option),
                });
 
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
 
-               Ok((best_block.block_hash(), ChannelMonitor {
-                       inner: Mutex::new(ChannelMonitorImpl {
-                               latest_update_id,
-                               commitment_transaction_number_obscure_factor,
+               Ok((best_block.block_hash(), ChannelMonitor::from_impl(ChannelMonitorImpl {
+                       latest_update_id,
+                       commitment_transaction_number_obscure_factor,
 
-                               destination_script,
-                               broadcasted_holder_revokable_script,
-                               counterparty_payment_script,
-                               shutdown_script,
+                       destination_script,
+                       broadcasted_holder_revokable_script,
+                       counterparty_payment_script,
+                       shutdown_script,
 
-                               channel_keys_id,
-                               holder_revocation_basepoint,
-                               funding_info,
-                               current_counterparty_commitment_txid,
-                               prev_counterparty_commitment_txid,
+                       channel_keys_id,
+                       holder_revocation_basepoint,
+                       funding_info,
+                       current_counterparty_commitment_txid,
+                       prev_counterparty_commitment_txid,
 
-                               counterparty_commitment_params,
-                               funding_redeemscript,
-                               channel_value_satoshis,
-                               their_cur_per_commitment_points,
+                       counterparty_commitment_params,
+                       funding_redeemscript,
+                       channel_value_satoshis,
+                       their_cur_per_commitment_points,
 
-                               on_holder_tx_csv,
+                       on_holder_tx_csv,
 
-                               commitment_secrets,
-                               counterparty_claimable_outpoints,
-                               counterparty_commitment_txn_on_chain,
-                               counterparty_hash_commitment_number,
+                       commitment_secrets,
+                       counterparty_claimable_outpoints,
+                       counterparty_commitment_txn_on_chain,
+                       counterparty_hash_commitment_number,
 
-                               prev_holder_signed_commitment_tx,
-                               current_holder_commitment_tx,
-                               current_counterparty_commitment_number,
-                               current_holder_commitment_number,
+                       prev_holder_signed_commitment_tx,
+                       current_holder_commitment_tx,
+                       current_counterparty_commitment_number,
+                       current_holder_commitment_number,
 
-                               payment_preimages,
-                               pending_monitor_events: pending_monitor_events.unwrap(),
-                               pending_events,
+                       payment_preimages,
+                       pending_monitor_events: pending_monitor_events.unwrap(),
+                       pending_events,
 
-                               onchain_events_awaiting_threshold_conf,
-                               outputs_to_watch,
+                       onchain_events_awaiting_threshold_conf,
+                       outputs_to_watch,
 
-                               onchain_tx_handler,
+                       onchain_tx_handler,
 
-                               lockdown_from_offchain,
-                               holder_tx_signed,
-                               funding_spend_seen: funding_spend_seen.unwrap(),
-                               funding_spend_confirmed,
-                               htlcs_resolved_on_chain: htlcs_resolved_on_chain.unwrap(),
+                       lockdown_from_offchain,
+                       holder_tx_signed,
+                       funding_spend_seen: funding_spend_seen.unwrap(),
+                       funding_spend_confirmed,
+                       htlcs_resolved_on_chain: htlcs_resolved_on_chain.unwrap(),
 
-                               best_block,
+                       best_block,
+                       counterparty_node_id,
 
-                               secp_ctx,
-                       }),
-               }))
+                       secp_ctx,
+               })))
        }
 }
 
@@ -3390,6 +3454,8 @@ mod tests {
 
        use hex;
 
+       use crate::chain::chaininterface::LowerBoundedFeeEstimator;
+
        use super::ChannelMonitorUpdateStep;
        use ::{check_added_monitors, check_closed_broadcast, check_closed_event, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash, unwrap_send_err};
        use chain::{BestBlock, Confirm};
@@ -3486,7 +3552,7 @@ mod tests {
 
                let broadcaster = TestBroadcaster::new(Arc::clone(&nodes[1].blocks));
                assert!(
-                       pre_update_monitor.update_monitor(&replay_update, &&broadcaster, &&chanmon_cfgs[1].fee_estimator, &nodes[1].logger)
+                       pre_update_monitor.update_monitor(&replay_update, &&broadcaster, &chanmon_cfgs[1].fee_estimator, &nodes[1].logger)
                        .is_err());
                // Even though we error'd on the first update, we should still have generated an HTLC claim
                // transaction
@@ -3511,7 +3577,7 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let logger = Arc::new(TestLogger::new());
                let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))});
-               let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: Mutex::new(253) });
+               let fee_estimator = TestFeeEstimator { sat_per_kw: Mutex::new(253) };
 
                let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
@@ -3601,7 +3667,7 @@ mod tests {
                                                  (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
                                                  &channel_parameters,
                                                  Script::new(), 46, 0,
-                                                 HolderCommitmentTransaction::dummy(), best_block);
+                                                 HolderCommitmentTransaction::dummy(), best_block, dummy_key);
 
                monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap();
                let dummy_txid = dummy_tx.txid();
@@ -3610,7 +3676,8 @@ mod tests {
                monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger);
                monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger);
                for &(ref preimage, ref hash) in preimages.iter() {
-                       monitor.provide_payment_preimage(hash, preimage, &broadcaster, &fee_estimator, &logger);
+                       let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator);
+                       monitor.provide_payment_preimage(hash, preimage, &broadcaster, &bounded_fee_estimator, &logger);
                }
 
                // Now provide a secret, pruning preimages 10-15