Monitor should panic on receiving buggy update sequences

Channel shouldn't send a ChannelForceClosed update followed by
a LatestLocalCommitmentTxInfo as it would be a programming error
leading to risk of money loss. Force-closing the channel will
broadcast the local commitment transaction, if the revocation
secret for this one is released after its broadcast, it would
allow remote party to claim outputs on this transaction using
the revocation path.

diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs
index d180d3bbdc095501f686e86f5f433a0373210b7e..cba28982d4872dc39aca0c7025c816d88209adab 100644 (file)
--- a/lightning/src/ln/channelmonitor.rs
+++ b/lightning/src/ln/channelmonitor.rs
@@ -794,6 +794,9 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
        #[cfg(not(test))]
        onchain_tx_handler: OnchainTxHandler<ChanSigner>,
 
+       // Used to detect programming bug due to unsafe monitor update sequence { ChannelForceClosed, LatestLocalCommitmentTXInfo }
+       lockdown_from_offchain: bool,
+
        // We simply modify last_block_hash in Channel's block_connected so that serialization is
        // consistent but hopefully the users' copy handles block_connected in a consistent way.
        // (we do *not*, however, update them in update_monitor to ensure any local user copies keep
@@ -1053,6 +1056,8 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
                }
                self.onchain_tx_handler.write(writer)?;
 
+               self.lockdown_from_offchain.write(writer)?;
+
                Ok(())
        }
 
@@ -1136,6 +1141,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
                        onchain_tx_handler: OnchainTxHandler::new(destination_script.clone(), keys, funding_redeemscript, their_to_self_delay, logger.clone()),
 
+                       lockdown_from_offchain: false,
+
                        last_block_hash: Default::default(),
                        secp_ctx: Secp256k1::new(),
                        logger,
@@ -1305,8 +1312,10 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        pub(super) fn update_monitor_ooo(&mut self, mut updates: ChannelMonitorUpdate) -> Result<(), MonitorUpdateError> {
                for update in updates.updates.drain(..) {
                        match update {
-                               ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo { commitment_tx, local_keys, feerate_per_kw, htlc_outputs } =>
-                                       self.provide_latest_local_commitment_tx_info(commitment_tx, local_keys, feerate_per_kw, htlc_outputs)?,
+                               ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo { commitment_tx, local_keys, feerate_per_kw, htlc_outputs } => {
+                                       if self.lockdown_from_offchain { panic!(); }
+                                       self.provide_latest_local_commitment_tx_info(commitment_tx, local_keys, feerate_per_kw, htlc_outputs)?
+                               },
                                ChannelMonitorUpdateStep::LatestRemoteCommitmentTXInfo { unsigned_commitment_tx, htlc_outputs, commitment_number, their_revocation_point } =>
                                        self.provide_latest_remote_commitment_tx_info(&unsigned_commitment_tx, htlc_outputs, commitment_number, their_revocation_point),
                                ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } =>
@@ -1334,8 +1343,10 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                }
                for update in updates.updates.drain(..) {
                        match update {
-                               ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo { commitment_tx, local_keys, feerate_per_kw, htlc_outputs } =>
-                                       self.provide_latest_local_commitment_tx_info(commitment_tx, local_keys, feerate_per_kw, htlc_outputs)?,
+                               ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo { commitment_tx, local_keys, feerate_per_kw, htlc_outputs } => {
+                                       if self.lockdown_from_offchain { panic!(); }
+                                       self.provide_latest_local_commitment_tx_info(commitment_tx, local_keys, feerate_per_kw, htlc_outputs)?
+                               },
                                ChannelMonitorUpdateStep::LatestRemoteCommitmentTXInfo { unsigned_commitment_tx, htlc_outputs, commitment_number, their_revocation_point } =>
                                        self.provide_latest_remote_commitment_tx_info(&unsigned_commitment_tx, htlc_outputs, commitment_number, their_revocation_point),
                                ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } =>
@@ -1345,6 +1356,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                ChannelMonitorUpdateStep::RescueRemoteCommitmentTXInfo { their_current_per_commitment_point } =>
                                        self.provide_rescue_remote_commitment_tx_info(their_current_per_commitment_point),
                                ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => {
+                                       self.lockdown_from_offchain = true;
                                        if should_broadcast {
                                                self.broadcast_latest_local_commitment_txn(broadcaster);
                                        } else {
@@ -2485,6 +2497,8 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
                }
                let onchain_tx_handler = ReadableArgs::read(reader, logger.clone())?;
 
+               let lockdown_from_offchain = Readable::read(reader)?;
+
                Ok((last_block_hash.clone(), ChannelMonitor {
                        latest_update_id,
                        commitment_transaction_number_obscure_factor,
@@ -2523,6 +2537,8 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
 
                        onchain_tx_handler,
 
+                       lockdown_from_offchain,
+
                        last_block_hash,
                        secp_ctx: Secp256k1::new(),
                        logger,