Monitor should panic on receiving buggy update sequences

[rust-lightning] / lightning / src / ln / channelmonitor.rs

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,