Merge pull request #1618 from wpaulino/gossip-sync-constructors
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 62dc48607f6a17ecb96ccf7ba268903e7d16c7a5..80cd9cb9d45958629e1789513190a4de63b87e7b 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;
@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
        // deserialization
        current_holder_commitment_number: u64,
 
+       /// The set of payment hashes from inbound payments for which we know the preimage. Payment
+       /// preimages that are not included in any unrevoked local commitment transaction or unrevoked
+       /// remote commitment transactions are automatically removed when commitment transactions are
+       /// revoked.
        payment_preimages: HashMap<PaymentHash, PaymentPreimage>,
 
        // Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
@@ -718,6 +722,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...
 }
 
@@ -865,6 +872,9 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
                        writer.write_all(&txid[..])?;
                        writer.write_all(&byte_utils::be64_to_array(htlc_infos.len() as u64))?;
                        for &(ref htlc_output, ref htlc_source) in htlc_infos.iter() {
+                               debug_assert!(htlc_source.is_none() || Some(**txid) == self.current_counterparty_commitment_txid
+                                               || Some(**txid) == self.prev_counterparty_commitment_txid,
+                                       "HTLC Sources for all revoked commitment transactions should be none!");
                                serialize_htlc_in_commitment!(htlc_output);
                                htlc_source.as_ref().map(|b| b.as_ref()).write(writer)?;
                        }
@@ -947,6 +957,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(())
@@ -960,7 +971,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                          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());
@@ -1050,6 +1061,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                                htlcs_resolved_on_chain: Vec::new(),
 
                                best_block,
+                               counterparty_node_id: Some(counterparty_node_id),
 
                                secp_ctx,
                        }),
@@ -1085,13 +1097,14 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
        }
 
-       #[cfg(test)]
+       /// This is used to provide payment preimage(s) out-of-band during startup without updating the
+       /// off-chain state with a new commitment transaction.
        pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
                &self,
                payment_hash: &PaymentHash,
                payment_preimage: &PaymentPreimage,
                broadcaster: &B,
-               fee_estimator: &F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: &L,
        ) where
                B::Target: BroadcasterInterface,
@@ -1287,8 +1300,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.
@@ -1308,8 +1322,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
@@ -1332,8 +1347,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.
@@ -1632,6 +1648,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 
                res
        }
+
+       pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
+               self.inner.lock().unwrap().payment_preimages.clone()
+       }
 }
 
 /// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
@@ -1650,7 +1670,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 /// as long as we examine both the current counterparty commitment transaction and, if it hasn't
 /// been revoked yet, the previous one, we we will never "forget" to resolve an HTLC.
 macro_rules! fail_unbroadcast_htlcs {
-       ($self: expr, $commitment_tx_type: expr, $commitment_tx_conf_height: expr, $confirmed_htlcs_list: expr, $logger: expr) => { {
+       ($self: expr, $commitment_tx_type: expr, $commitment_txid_confirmed: expr,
+        $commitment_tx_conf_height: expr, $confirmed_htlcs_list: expr, $logger: expr) => { {
                macro_rules! check_htlc_fails {
                        ($txid: expr, $commitment_tx: expr) => {
                                if let Some(ref latest_outpoints) = $self.counterparty_claimable_outpoints.get($txid) {
@@ -1666,9 +1687,14 @@ macro_rules! fail_unbroadcast_htlcs {
                                                        // cannot currently change after channel initialization, so we don't
                                                        // need to here.
                                                        let confirmed_htlcs_iter: &mut Iterator<Item = (&HTLCOutputInCommitment, Option<&HTLCSource>)> = &mut $confirmed_htlcs_list;
+
                                                        let mut matched_htlc = false;
                                                        for (ref broadcast_htlc, ref broadcast_source) in confirmed_htlcs_iter {
-                                                               if broadcast_htlc.transaction_output_index.is_some() && Some(&**source) == *broadcast_source {
+                                                               if broadcast_htlc.transaction_output_index.is_some() &&
+                                                                       (Some(&**source) == *broadcast_source ||
+                                                                        (broadcast_source.is_none() &&
+                                                                         broadcast_htlc.payment_hash == htlc.payment_hash &&
+                                                                         broadcast_htlc.amount_msat == htlc.amount_msat)) {
                                                                        matched_htlc = true;
                                                                        break;
                                                                }
@@ -1684,7 +1710,7 @@ macro_rules! fail_unbroadcast_htlcs {
                                                                }
                                                        });
                                                        let entry = OnchainEventEntry {
-                                                               txid: *$txid,
+                                                               txid: $commitment_txid_confirmed,
                                                                height: $commitment_tx_conf_height,
                                                                event: OnchainEvent::HTLCUpdate {
                                                                        source: (**source).clone(),
@@ -1693,8 +1719,9 @@ macro_rules! fail_unbroadcast_htlcs {
                                                                        commitment_tx_output_idx: None,
                                                                },
                                                        };
-                                                       log_trace!($logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of {} commitment transaction, waiting for confirmation (at height {})",
-                                                               log_bytes!(htlc.payment_hash.0), $commitment_tx, $commitment_tx_type, entry.confirmation_threshold());
+                                                       log_trace!($logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of {} commitment transaction {}, waiting for confirmation (at height {})",
+                                                               log_bytes!(htlc.payment_hash.0), $commitment_tx, $commitment_tx_type,
+                                                               $commitment_txid_confirmed, entry.confirmation_threshold());
                                                        $self.onchain_events_awaiting_threshold_conf.push(entry);
                                                }
                                        }
@@ -1710,6 +1737,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
@@ -1838,7 +1885,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,
@@ -1936,7 +1985,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");
@@ -2091,7 +2141,16 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                }
                                self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
-                               fail_unbroadcast_htlcs!(self, "revoked counterparty", height, [].iter().map(|a| *a), logger);
+                               if let Some(per_commitment_data) = per_commitment_option {
+                                       fail_unbroadcast_htlcs!(self, "revoked_counterparty", commitment_txid, height,
+                                               per_commitment_data.iter().map(|(htlc, htlc_source)|
+                                                       (htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
+                                               ), logger);
+                               } else {
+                                       debug_assert!(false, "We should have per-commitment option for any recognized old commitment txn");
+                                       fail_unbroadcast_htlcs!(self, "revoked counterparty", commitment_txid, height,
+                                               [].iter().map(|reference| *reference), logger);
+                               }
                        }
                } else if let Some(per_commitment_data) = per_commitment_option {
                        // While this isn't useful yet, there is a potential race where if a counterparty
@@ -2107,7 +2166,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
                        log_info!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid);
-                       fail_unbroadcast_htlcs!(self, "counterparty", height, per_commitment_data.iter().map(|(a, b)| (a, b.as_ref().map(|b| b.as_ref()))), logger);
+                       fail_unbroadcast_htlcs!(self, "counterparty", commitment_txid, height,
+                               per_commitment_data.iter().map(|(htlc, htlc_source)|
+                                       (htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
+                               ), logger);
 
                        let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs(commitment_number, commitment_txid, Some(tx));
                        for req in htlc_claim_reqs {
@@ -2263,7 +2325,9 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height);
                        let mut to_watch = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, tx);
                        append_onchain_update!(res, to_watch);
-                       fail_unbroadcast_htlcs!(self, "latest holder", height, self.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger);
+                       fail_unbroadcast_htlcs!(self, "latest holder", commitment_txid, height,
+                               self.current_holder_commitment_tx.htlc_outputs.iter()
+                               .map(|(htlc, _, htlc_source)| (htlc, htlc_source.as_ref())), logger);
                } else if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx {
                        if holder_tx.txid == commitment_txid {
                                is_holder_tx = true;
@@ -2271,7 +2335,9 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                let res = self.get_broadcasted_holder_claims(holder_tx, height);
                                let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_tx, tx);
                                append_onchain_update!(res, to_watch);
-                               fail_unbroadcast_htlcs!(self, "previous holder", height, holder_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger);
+                               fail_unbroadcast_htlcs!(self, "previous holder", commitment_txid, height,
+                                       holder_tx.htlc_outputs.iter().map(|(htlc, _, htlc_source)| (htlc, htlc_source.as_ref())),
+                                       logger);
                        }
                }
 
@@ -2346,7 +2412,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>(
@@ -2354,7 +2421,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                header: &BlockHeader,
                height: u32,
                broadcaster: B,
-               fee_estimator: F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: L,
        ) -> Vec<TransactionOutputs>
        where
@@ -2381,7 +2448,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                txdata: &TransactionData,
                height: u32,
                broadcaster: B,
-               fee_estimator: F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: L,
        ) -> Vec<TransactionOutputs>
        where
@@ -2478,7 +2545,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
@@ -2552,7 +2619,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                matured_htlcs.push(source.clone());
                                        }
 
-                                       log_debug!(logger, "HTLC {} failure update has got enough confirmations to be passed upstream", log_bytes!(payment_hash.0));
+                                       log_debug!(logger, "HTLC {} failure update in {} has got enough confirmations to be passed upstream",
+                                               log_bytes!(payment_hash.0), entry.txid);
                                        self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
                                                payment_hash,
                                                payment_preimage: None,
@@ -2615,7 +2683,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);
        }
@@ -2624,14 +2693,17 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                &mut self,
                txid: &Txid,
                broadcaster: B,
-               fee_estimator: F,
+               fee_estimator: &LowerBoundedFeeEstimator<F>,
                logger: L,
        ) where
                B::Target: BroadcasterInterface,
                F::Target: FeeEstimator,
                L::Target: Logger,
        {
-               self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.txid != *txid);
+               self.onchain_events_awaiting_threshold_conf.retain(|ref entry| if entry.txid == *txid {
+                       log_info!(logger, "Removing onchain event with txid {}", txid);
+                       false
+               } else { true });
                self.onchain_tx_handler.transaction_unconfirmed(txid, broadcaster, fee_estimator, logger);
        }
 
@@ -2662,14 +2734,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;
                                        }
@@ -2754,10 +2833,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);
 
@@ -3267,11 +3349,13 @@ 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();
@@ -3326,6 +3410,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
                                htlcs_resolved_on_chain: htlcs_resolved_on_chain.unwrap(),
 
                                best_block,
+                               counterparty_node_id,
 
                                secp_ctx,
                        }),
@@ -3351,6 +3436,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};
@@ -3472,7 +3559,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() };
@@ -3562,7 +3649,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();
@@ -3571,7 +3658,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