Use Writeable for ChannelMonitor instead of a specific function.
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 425baeddf75e3933c309f4723c4706db999d5a07..5278376126228bcc467a80278c61128e44bc1013 100644 (file)
@@ -27,7 +27,6 @@ use bitcoin::blockdata::transaction::{TxOut,Transaction};
 use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
 use bitcoin::blockdata::script::{Script, Builder};
 use bitcoin::blockdata::opcodes;
-use bitcoin::consensus::encode;
 
 use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
@@ -39,7 +38,7 @@ use bitcoin::secp256k1;
 
 use ln::msgs::DecodeError;
 use ln::chan_utils;
-use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, HTLCType};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCType, ChannelTransactionParameters, HolderCommitmentTransaction};
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
 use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
@@ -179,7 +178,7 @@ pub enum ChannelMonitorUpdateErr {
 pub struct MonitorUpdateError(pub &'static str);
 
 /// An event to be processed by the ChannelManager.
-#[derive(PartialEq)]
+#[derive(Clone, PartialEq)]
 pub enum MonitorEvent {
        /// A monitor event containing an HTLCUpdate.
        HTLCEvent(HTLCUpdate),
@@ -252,6 +251,7 @@ pub(crate) const ANTI_REORG_DELAY: u32 = 6;
 /// end up force-closing the channel on us to claim it.
 pub(crate) const HTLC_FAIL_BACK_BUFFER: u32 = CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS;
 
+// TODO(devrandom) replace this with HolderCommitmentTransaction
 #[derive(Clone, PartialEq)]
 struct HolderSignedTx {
        /// txid of the transaction in tx, just used to make comparison faster
@@ -493,7 +493,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
                htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
        },
        LatestCounterpartyCommitmentTXInfo {
-               unsigned_commitment_tx: Transaction, // TODO: We should actually only need the txid here
+               commitment_txid: Txid,
                htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
                commitment_number: u64,
                their_revocation_point: PublicKey,
@@ -527,9 +527,9 @@ impl Writeable for ChannelMonitorUpdateStep {
                                        source.write(w)?;
                                }
                        }
-                       &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { ref unsigned_commitment_tx, ref htlc_outputs, ref commitment_number, ref their_revocation_point } => {
+                       &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, ref htlc_outputs, ref commitment_number, ref their_revocation_point } => {
                                1u8.write(w)?;
-                               unsigned_commitment_tx.write(w)?;
+                               commitment_txid.write(w)?;
                                commitment_number.write(w)?;
                                their_revocation_point.write(w)?;
                                (htlc_outputs.len() as u64).write(w)?;
@@ -573,7 +573,7 @@ impl Readable for ChannelMonitorUpdateStep {
                        },
                        1u8 => {
                                Ok(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
-                                       unsigned_commitment_tx: Readable::read(r)?,
+                                       commitment_txid: Readable::read(r)?,
                                        commitment_number: Readable::read(r)?,
                                        their_revocation_point: Readable::read(r)?,
                                        htlc_outputs: {
@@ -617,6 +617,12 @@ impl Readable for ChannelMonitorUpdateStep {
 /// get_and_clear_pending_monitor_events or get_and_clear_pending_events are serialized to disk and
 /// reloaded at deserialize-time. Thus, you must ensure that, when handling events, all events
 /// gotten are fully handled before re-serializing the new state.
+///
+/// Note that the deserializer is only implemented for (Sha256dHash, ChannelMonitor), which
+/// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along
+/// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the
+/// returned block hash and the the current chain and then reconnecting blocks to get to the
+/// best chain) upon deserializing the object!
 pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        latest_update_id: u64,
        commitment_transaction_number_obscure_factor: u64,
@@ -626,7 +632,8 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        counterparty_payment_script: Script,
        shutdown_script: Script,
 
-       keys: ChanSigner,
+       key_derivation_params: (u64, u64),
+       holder_revocation_basepoint: PublicKey,
        funding_info: (OutPoint, Script),
        current_counterparty_commitment_txid: Option<Txid>,
        prev_counterparty_commitment_txid: Option<Txid>,
@@ -721,7 +728,8 @@ impl<ChanSigner: ChannelKeys> PartialEq for ChannelMonitor<ChanSigner> {
                        self.destination_script != other.destination_script ||
                        self.broadcasted_holder_revokable_script != other.broadcasted_holder_revokable_script ||
                        self.counterparty_payment_script != other.counterparty_payment_script ||
-                       self.keys.pubkeys() != other.keys.pubkeys() ||
+                       self.key_derivation_params != other.key_derivation_params ||
+                       self.holder_revocation_basepoint != other.holder_revocation_basepoint ||
                        self.funding_info != other.funding_info ||
                        self.current_counterparty_commitment_txid != other.current_counterparty_commitment_txid ||
                        self.prev_counterparty_commitment_txid != other.prev_counterparty_commitment_txid ||
@@ -753,15 +761,8 @@ impl<ChanSigner: ChannelKeys> PartialEq for ChannelMonitor<ChanSigner> {
        }
 }
 
-impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
-       /// Writes this monitor into the given writer, suitable for writing to disk.
-       ///
-       /// Note that the deserializer is only implemented for (Sha256dHash, ChannelMonitor), which
-       /// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along
-       /// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the
-       /// returned block hash and the the current chain and then reconnecting blocks to get to the
-       /// best chain) upon deserializing the object!
-       pub fn serialize_for_disk<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
+impl<ChanSigner: ChannelKeys> Writeable for ChannelMonitor<ChanSigner> {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
                //TODO: We still write out all the serialization here manually instead of using the fancy
                //serialization framework we have, we should migrate things over to it.
                writer.write_all(&[SERIALIZATION_VERSION; 1])?;
@@ -785,7 +786,8 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
                self.counterparty_payment_script.write(writer)?;
                self.shutdown_script.write(writer)?;
 
-               self.keys.write(writer)?;
+               self.key_derivation_params.write(writer)?;
+               self.holder_revocation_basepoint.write(writer)?;
                writer.write_all(&self.funding_info.0.txid[..])?;
                writer.write_all(&byte_utils::be16_to_array(self.funding_info.0.index))?;
                self.funding_info.1.write(writer)?;
@@ -948,11 +950,11 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
 
 impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        pub(crate) fn new(keys: ChanSigner, shutdown_pubkey: &PublicKey,
-                       on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
-                       counterparty_htlc_base_key: &PublicKey, counterparty_delayed_payment_base_key: &PublicKey,
-                       on_holder_tx_csv: u16, funding_redeemscript: Script, channel_value_satoshis: u64,
-                       commitment_transaction_number_obscure_factor: u64,
-                       initial_holder_commitment_tx: HolderCommitmentTransaction) -> ChannelMonitor<ChanSigner> {
+                         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) -> ChannelMonitor<ChanSigner> {
 
                assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
                let our_channel_close_key_hash = WPubkeyHash::hash(&shutdown_pubkey.serialize());
@@ -960,21 +962,34 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize());
                let counterparty_payment_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_key_hash[..]).into_script();
 
-               let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key: *counterparty_delayed_payment_base_key, counterparty_htlc_base_key: *counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() };
-
-               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), on_holder_tx_csv);
-
-               let holder_tx_sequence = initial_holder_commitment_tx.unsigned_tx.input[0].sequence as u64;
-               let holder_tx_locktime = initial_holder_commitment_tx.unsigned_tx.lock_time as u64;
-               let holder_commitment_tx = HolderSignedTx {
-                       txid: initial_holder_commitment_tx.txid(),
-                       revocation_key: initial_holder_commitment_tx.keys.revocation_key,
-                       a_htlc_key: initial_holder_commitment_tx.keys.broadcaster_htlc_key,
-                       b_htlc_key: initial_holder_commitment_tx.keys.countersignatory_htlc_key,
-                       delayed_payment_key: initial_holder_commitment_tx.keys.broadcaster_delayed_payment_key,
-                       per_commitment_point: initial_holder_commitment_tx.keys.per_commitment_point,
-                       feerate_per_kw: initial_holder_commitment_tx.feerate_per_kw,
-                       htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
+               let counterparty_channel_parameters = channel_parameters.counterparty_parameters.as_ref().unwrap();
+               let counterparty_delayed_payment_base_key = counterparty_channel_parameters.pubkeys.delayed_payment_basepoint;
+               let counterparty_htlc_base_key = counterparty_channel_parameters.pubkeys.htlc_basepoint;
+               let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key, counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() };
+
+               let key_derivation_params = keys.key_derivation_params();
+               let holder_revocation_basepoint = keys.pubkeys().revocation_basepoint;
+               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys, channel_parameters.clone());
+
+               let secp_ctx = Secp256k1::new();
+
+               // block for Rust 1.34 compat
+               let (holder_commitment_tx, current_holder_commitment_number) = {
+                       let trusted_tx = initial_holder_commitment_tx.trust();
+                       let txid = trusted_tx.txid();
+
+                       let tx_keys = trusted_tx.keys();
+                       let holder_commitment_tx = HolderSignedTx {
+                               txid,
+                               revocation_key: tx_keys.revocation_key,
+                               a_htlc_key: tx_keys.broadcaster_htlc_key,
+                               b_htlc_key: tx_keys.countersignatory_htlc_key,
+                               delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
+                               per_commitment_point: tx_keys.per_commitment_point,
+                               feerate_per_kw: trusted_tx.feerate_per_kw(),
+                               htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
+                       };
+                       (holder_commitment_tx, trusted_tx.commitment_number())
                };
                onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_tx);
 
@@ -990,7 +1005,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        counterparty_payment_script,
                        shutdown_script,
 
-                       keys,
+                       key_derivation_params,
+                       holder_revocation_basepoint,
                        funding_info,
                        current_counterparty_commitment_txid: None,
                        prev_counterparty_commitment_txid: None,
@@ -1000,7 +1016,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        channel_value_satoshis,
                        their_cur_revocation_points: None,
 
-                       on_holder_tx_csv,
+                       on_holder_tx_csv: counterparty_channel_parameters.selected_contest_delay,
 
                        commitment_secrets: CounterpartyCommitmentSecrets::new(),
                        counterparty_claimable_outpoints: HashMap::new(),
@@ -1010,7 +1026,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        prev_holder_signed_commitment_tx: None,
                        current_holder_commitment_tx: holder_commitment_tx,
                        current_counterparty_commitment_number: 1 << 48,
-                       current_holder_commitment_number: 0xffff_ffff_ffff - ((((holder_tx_sequence & 0xffffff) << 3*8) | (holder_tx_locktime as u64 & 0xffffff)) ^ commitment_transaction_number_obscure_factor),
+                       current_holder_commitment_number,
 
                        payment_preimages: HashMap::new(),
                        pending_monitor_events: Vec::new(),
@@ -1025,7 +1041,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        holder_tx_signed: false,
 
                        last_block_hash: Default::default(),
-                       secp_ctx: Secp256k1::new(),
+                       secp_ctx,
                }
        }
 
@@ -1084,7 +1100,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        /// The monitor watches for it to be broadcasted and then uses the HTLC information (and
        /// possibly future revocation/preimage information) to claim outputs where possible.
        /// We cache also the mapping hash:commitment number to lighten pruning of old preimages by watchtowers.
-       pub(crate) fn provide_latest_counterparty_commitment_tx_info<L: Deref>(&mut self, unsigned_commitment_tx: &Transaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>, commitment_number: u64, their_revocation_point: PublicKey, logger: &L) where L::Target: Logger {
+       pub(crate) fn provide_latest_counterparty_commitment_tx<L: Deref>(&mut self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>, commitment_number: u64, their_revocation_point: PublicKey, logger: &L) where L::Target: Logger {
                // TODO: Encrypt the htlc_outputs data with the single-hash of the commitment transaction
                // so that a remote monitor doesn't learn anything unless there is a malicious close.
                // (only maybe, sadly we cant do the same for local info, as we need to be aware of
@@ -1093,12 +1109,10 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        self.counterparty_hash_commitment_number.insert(htlc.payment_hash, commitment_number);
                }
 
-               let new_txid = unsigned_commitment_tx.txid();
-               log_trace!(logger, "Tracking new counterparty commitment transaction with txid {} at commitment number {} with {} HTLC outputs", new_txid, commitment_number, htlc_outputs.len());
-               log_trace!(logger, "New potential counterparty commitment transaction: {}", encode::serialize_hex(unsigned_commitment_tx));
+               log_trace!(logger, "Tracking new counterparty commitment transaction with txid {} at commitment number {} with {} HTLC outputs", txid, commitment_number, htlc_outputs.len());
                self.prev_counterparty_commitment_txid = self.current_counterparty_commitment_txid.take();
-               self.current_counterparty_commitment_txid = Some(new_txid);
-               self.counterparty_claimable_outpoints.insert(new_txid, htlc_outputs.clone());
+               self.current_counterparty_commitment_txid = Some(txid);
+               self.counterparty_claimable_outpoints.insert(txid, htlc_outputs.clone());
                self.current_counterparty_commitment_number = commitment_number;
                //TODO: Merge this into the other per-counterparty-transaction output storage stuff
                match self.their_cur_revocation_points {
@@ -1125,7 +1139,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                htlcs.push(htlc.0);
                        }
                }
-               self.counterparty_tx_cache.per_htlc.insert(new_txid, htlcs);
+               self.counterparty_tx_cache.per_htlc.insert(txid, htlcs);
        }
 
        /// Informs this monitor of the latest holder (ie broadcastable) commitment transaction. The
@@ -1133,22 +1147,25 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        /// is important that any clones of this channel monitor (including remote clones) by kept
        /// up-to-date as our holder commitment transaction is updated.
        /// Panics if set_on_holder_tx_csv has never been called.
-       fn provide_latest_holder_commitment_tx_info(&mut self, commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
-               let txid = commitment_tx.txid();
-               let sequence = commitment_tx.unsigned_tx.input[0].sequence as u64;
-               let locktime = commitment_tx.unsigned_tx.lock_time as u64;
-               let mut new_holder_commitment_tx = HolderSignedTx {
-                       txid,
-                       revocation_key: commitment_tx.keys.revocation_key,
-                       a_htlc_key: commitment_tx.keys.broadcaster_htlc_key,
-                       b_htlc_key: commitment_tx.keys.countersignatory_htlc_key,
-                       delayed_payment_key: commitment_tx.keys.broadcaster_delayed_payment_key,
-                       per_commitment_point: commitment_tx.keys.per_commitment_point,
-                       feerate_per_kw: commitment_tx.feerate_per_kw,
-                       htlc_outputs,
+       fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
+               // block for Rust 1.34 compat
+               let mut new_holder_commitment_tx = {
+                       let trusted_tx = holder_commitment_tx.trust();
+                       let txid = trusted_tx.txid();
+                       let tx_keys = trusted_tx.keys();
+                       self.current_holder_commitment_number = trusted_tx.commitment_number();
+                       HolderSignedTx {
+                               txid,
+                               revocation_key: tx_keys.revocation_key,
+                               a_htlc_key: tx_keys.broadcaster_htlc_key,
+                               b_htlc_key: tx_keys.countersignatory_htlc_key,
+                               delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
+                               per_commitment_point: tx_keys.per_commitment_point,
+                               feerate_per_kw: trusted_tx.feerate_per_kw(),
+                               htlc_outputs,
+                       }
                };
-               self.onchain_tx_handler.provide_latest_holder_tx(commitment_tx);
-               self.current_holder_commitment_number = 0xffff_ffff_ffff - ((((sequence & 0xffffff) << 3*8) | (locktime as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
+               self.onchain_tx_handler.provide_latest_holder_tx(holder_commitment_tx);
                mem::swap(&mut new_holder_commitment_tx, &mut self.current_holder_commitment_tx);
                self.prev_holder_signed_commitment_tx = Some(new_holder_commitment_tx);
                if self.holder_tx_signed {
@@ -1165,6 +1182,41 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                    L::Target: Logger,
        {
                self.payment_preimages.insert(payment_hash.clone(), payment_preimage.clone());
+
+               // If the channel is force closed, try to claim the output from this preimage.
+               // First check if a counterparty commitment transaction has been broadcasted:
+               macro_rules! claim_htlcs {
+                       ($commitment_number: expr, $txid: expr) => {
+                               let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs($commitment_number, $txid, None);
+                               self.onchain_tx_handler.update_claims_view(&Vec::new(), htlc_claim_reqs, None, broadcaster, fee_estimator, logger);
+                       }
+               }
+               if let Some(txid) = self.current_counterparty_commitment_txid {
+                       if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
+                               claim_htlcs!(*commitment_number, txid);
+                               return;
+                       }
+               }
+               if let Some(txid) = self.prev_counterparty_commitment_txid {
+                       if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
+                               claim_htlcs!(*commitment_number, txid);
+                               return;
+                       }
+               }
+
+               // Then if a holder commitment transaction has been seen on-chain, broadcast transactions
+               // claiming the HTLC output from each of the holder commitment transactions.
+               // Note that we can't just use `self.holder_tx_signed`, because that only covers the case where
+               // *we* sign a holder commitment transaction, not when e.g. a watchtower broadcasts one of our
+               // holder commitment transactions.
+               if self.broadcasted_holder_revokable_script.is_some() {
+                       let (claim_reqs, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx);
+                       self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, None, broadcaster, fee_estimator, logger);
+                       if let Some(ref tx) = self.prev_holder_signed_commitment_tx {
+                               let (claim_reqs, _) = self.get_broadcasted_holder_claims(&tx);
+                               self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, None, broadcaster, fee_estimator, logger);
+                       }
+               }
        }
 
        pub(crate) fn broadcast_latest_holder_commitment_txn<B: Deref, L: Deref>(&mut self, broadcaster: &B, logger: &L)
@@ -1204,11 +1256,11 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
                                        if self.lockdown_from_offchain { panic!(); }
-                                       self.provide_latest_holder_commitment_tx_info(commitment_tx.clone(), htlc_outputs.clone())?
-                               },
-                               ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { unsigned_commitment_tx, htlc_outputs, commitment_number, their_revocation_point } => {
+                                       self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone())?
+                               }
+                               ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_revocation_point } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
-                                       self.provide_latest_counterparty_commitment_tx_info(&unsigned_commitment_tx, htlc_outputs.clone(), *commitment_number, *their_revocation_point, logger)
+                                       self.provide_latest_counterparty_commitment_tx(*commitment_txid, htlc_outputs.clone(), *commitment_number, *their_revocation_point, logger)
                                },
                                ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } => {
                                        log_trace!(logger, "Updating ChannelMonitor with payment preimage");
@@ -1326,7 +1378,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        let secret = self.get_secret(commitment_number).unwrap();
                        let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret));
                        let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
-                       let revocation_pubkey = ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &self.keys.pubkeys().revocation_basepoint));
+                       let revocation_pubkey = ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &self.holder_revocation_basepoint));
                        let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.counterparty_tx_cache.counterparty_delayed_payment_base_key));
 
                        let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.counterparty_tx_cache.on_counterparty_tx_csv, &delayed_key);
@@ -1463,39 +1515,55 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                check_htlc_fails!(txid, "previous", 'prev_loop);
                        }
 
+                       let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs(commitment_number, commitment_txid, Some(tx));
+                       for req in htlc_claim_reqs {
+                               claimable_outpoints.push(req);
+                       }
+
+               }
+               (claimable_outpoints, (commitment_txid, watch_outputs))
+       }
+
+       fn get_counterparty_htlc_output_claim_reqs(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>) -> Vec<ClaimRequest> {
+               let mut claims = Vec::new();
+               if let Some(htlc_outputs) = self.counterparty_claimable_outpoints.get(&commitment_txid) {
                        if let Some(revocation_points) = self.their_cur_revocation_points {
                                let revocation_point_option =
+                                       // If the counterparty commitment tx is the latest valid state, use their latest
+                                       // per-commitment point
                                        if revocation_points.0 == commitment_number { Some(&revocation_points.1) }
                                        else if let Some(point) = revocation_points.2.as_ref() {
+                                               // If counterparty commitment tx is the state previous to the latest valid state, use
+                                               // their previous per-commitment point (non-atomicity of revocation means it's valid for
+                                               // them to temporarily have two valid commitment txns from our viewpoint)
                                                if revocation_points.0 == commitment_number + 1 { Some(point) } else { None }
                                        } else { None };
                                if let Some(revocation_point) = revocation_point_option {
-                                       self.counterparty_payment_script = {
-                                               // Note that the Network here is ignored as we immediately drop the address for the
-                                               // script_pubkey version
-                                               let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
-                                               Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
-                                       };
-
-                                       // Then, try to find htlc outputs
-                                       for (_, &(ref htlc, _)) in per_commitment_data.iter().enumerate() {
+                                       for (_, &(ref htlc, _)) in htlc_outputs.iter().enumerate() {
                                                if let Some(transaction_output_index) = htlc.transaction_output_index {
-                                                       if transaction_output_index as usize >= tx.output.len() ||
-                                                                       tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
-                                                               return (claimable_outpoints, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user
+                                                       if let Some(transaction) = tx {
+                                                               if transaction_output_index as usize >= transaction.output.len() ||
+                                                                       transaction.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
+                                                                               return claims; // Corrupted per_commitment_data, fuck this user
+                                                                       }
                                                        }
-                                                       let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None };
+                                                       let preimage =
+                                                               if htlc.offered {
+                                                                       if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) {
+                                                                               Some(*p)
+                                                                       } else { None }
+                                                               } else { None };
                                                        let aggregable = if !htlc.offered { false } else { true };
                                                        if preimage.is_some() || !htlc.offered {
                                                                let witness_data = InputMaterial::CounterpartyHTLC { per_commitment_point: *revocation_point, counterparty_delayed_payment_base_key: self.counterparty_tx_cache.counterparty_delayed_payment_base_key, counterparty_htlc_base_key: self.counterparty_tx_cache.counterparty_htlc_base_key, preimage, htlc: htlc.clone() };
-                                                               claimable_outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
+                                                               claims.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
                                                        }
                                                }
                                        }
                                }
                        }
                }
-               (claimable_outpoints, (commitment_txid, watch_outputs))
+               claims
        }
 
        /// Attempts to claim a counterparty HTLC-Success/HTLC-Timeout's outputs using the revocation key
@@ -1816,7 +1884,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        }
                }
 
-               self.onchain_tx_handler.block_connected(&txn_matched, claimable_outpoints, height, &*broadcaster, &*fee_estimator, &*logger);
+               self.onchain_tx_handler.update_claims_view(&txn_matched, claimable_outpoints, Some(height), &&*broadcaster, &&*fee_estimator, &&*logger);
                self.last_block_hash = block_hash;
 
                // Determine new outputs to watch by comparing against previously known outputs to watch,
@@ -2142,7 +2210,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                                per_commitment_point: broadcasted_holder_revokable_script.1,
                                                to_self_delay: self.on_holder_tx_csv,
                                                output: outp.clone(),
-                                               key_derivation_params: self.keys.key_derivation_params(),
+                                               key_derivation_params: self.key_derivation_params,
                                                revocation_pubkey: broadcasted_holder_revokable_script.2.clone(),
                                        });
                                        break;
@@ -2151,7 +2219,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                spendable_output = Some(SpendableOutputDescriptor::StaticOutputCounterpartyPayment {
                                        outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
                                        output: outp.clone(),
-                                       key_derivation_params: self.keys.key_derivation_params(),
+                                       key_derivation_params: self.key_derivation_params,
                                });
                                break;
                        } else if outp.script_pubkey == self.shutdown_script {
@@ -2267,7 +2335,8 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for (BlockHash, ChannelMonitor
                let counterparty_payment_script = Readable::read(reader)?;
                let shutdown_script = Readable::read(reader)?;
 
-               let keys = Readable::read(reader)?;
+               let key_derivation_params = Readable::read(reader)?;
+               let holder_revocation_basepoint = Readable::read(reader)?;
                // Technically this can fail and serialize fail a round-trip, but only for serialization of
                // barely-init'd ChannelMonitors that we can't do anything with.
                let outpoint = OutPoint {
@@ -2481,7 +2550,8 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for (BlockHash, ChannelMonitor
                        counterparty_payment_script,
                        shutdown_script,
 
-                       keys,
+                       key_derivation_params,
+                       holder_revocation_basepoint,
                        funding_info,
                        current_counterparty_commitment_txid,
                        prev_counterparty_commitment_txid,
@@ -2538,7 +2608,7 @@ mod tests {
        use ln::channelmanager::{PaymentPreimage, PaymentHash};
        use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
        use ln::chan_utils;
-       use ln::chan_utils::{HTLCOutputInCommitment, HolderCommitmentTransaction};
+       use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
        use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
        use bitcoin::secp256k1::key::{SecretKey,PublicKey};
        use bitcoin::secp256k1::Secp256k1;
@@ -2611,20 +2681,39 @@ mod tests {
                        (0, 0)
                );
 
+               let counterparty_pubkeys = ChannelPublicKeys {
+                       funding_pubkey: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
+                       revocation_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
+                       payment_point: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()),
+                       delayed_payment_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[47; 32]).unwrap()),
+                       htlc_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[48; 32]).unwrap())
+               };
+               let funding_outpoint = OutPoint { txid: Default::default(), index: u16::max_value() };
+               let channel_parameters = ChannelTransactionParameters {
+                       holder_pubkeys: keys.holder_channel_pubkeys.clone(),
+                       holder_selected_contest_delay: 66,
+                       is_outbound_from_holder: true,
+                       counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
+                               pubkeys: counterparty_pubkeys,
+                               selected_contest_delay: 67,
+                       }),
+                       funding_outpoint: Some(funding_outpoint),
+               };
                // Prune with one old state and a holder commitment tx holding a few overlaps with the
                // old state.
                let mut monitor = ChannelMonitor::new(keys,
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
-                       (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
-                       10, Script::new(), 46, 0, HolderCommitmentTransaction::dummy());
-
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap();
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger);
+                                                     &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
+                                                     (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
+                                                     &channel_parameters,
+                                                     Script::new(), 46, 0,
+                                                     HolderCommitmentTransaction::dummy());
+
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap();
+               let dummy_txid = dummy_tx.txid();
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key, &logger);
+               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);
                }
@@ -2646,7 +2735,7 @@ mod tests {
 
                // Now update holder commitment tx info, pruning only element 18 as we still care about the
                // previous commitment tx's preimages too
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..5])).unwrap();
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..5])).unwrap();
                secret[0..32].clone_from_slice(&hex::decode("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
                monitor.provide_secret(281474976710653, secret.clone()).unwrap();
                assert_eq!(monitor.payment_preimages.len(), 12);
@@ -2654,7 +2743,7 @@ mod tests {
                test_preimages_exist!(&preimages[18..20], monitor);
 
                // But if we do it again, we'll prune 5-10
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..3])).unwrap();
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..3])).unwrap();
                secret[0..32].clone_from_slice(&hex::decode("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
                monitor.provide_secret(281474976710652, secret.clone()).unwrap();
                assert_eq!(monitor.payment_preimages.len(), 5);