Fix XXXs that slipped into router and handle HTLCFailCHannelUpdates
[rust-lightning] / src / ln / channelmonitor.rs
index acd63da79e1bd63b511bfc7ec49ed29bb3dbcebb..60cb9c91583d604fa8fa464bd0b0f60790c48005 100644 (file)
@@ -1,8 +1,10 @@
 //! The logic to monitor for on-chain transactions and create the relevant claim responses lives
 //! here.
+//!
 //! ChannelMonitor objects are generated by ChannelManager in response to relevant
 //! messages/actions, and MUST be persisted to disk (and, preferably, remotely) before progress can
 //! be made in responding to certain messages, see ManyChannelMonitor for more.
+//!
 //! Note that ChannelMonitors are an important part of the lightning trust model and a copy of the
 //! latest ChannelMonitor must always be actively monitoring for chain updates (and no out-of-date
 //! ChannelMonitors should do so). Thus, if you're building rust-lightning into an HSM or other
@@ -37,13 +39,31 @@ use std::sync::{Arc,Mutex};
 use std::{hash,cmp};
 
 /// An error enum representing a failure to persist a channel monitor update.
+#[derive(Clone)]
 pub enum ChannelMonitorUpdateErr {
        /// Used to indicate a temporary failure (eg connection to a watchtower failed, but is expected
        /// to succeed at some point in the future).
+       ///
        /// Such a failure will "freeze" a channel, preventing us from revoking old states or
        /// submitting new commitment transactions to the remote party.
        /// ChannelManager::test_restore_channel_monitor can be used to retry the update(s) and restore
        /// the channel to an operational state.
+       ///
+       /// Note that continuing to operate when no copy of the updated ChannelMonitor could be
+       /// persisted is unsafe - if you failed to store the update on your own local disk you should
+       /// instead return PermanentFailure to force closure of the channel ASAP.
+       ///
+       /// Even when a channel has been "frozen" updates to the ChannelMonitor can continue to occur
+       /// (eg if an inbound HTLC which we forwarded was claimed upstream resulting in us attempting
+       /// to claim it on this channel) and those updates must be applied wherever they can be. At
+       /// least one such updated ChannelMonitor must be persisted otherwise PermanentFailure should
+       /// be returned to get things on-chain ASAP using only the in-memory copy. Obviously updates to
+       /// the channel which would invalidate previous ChannelMonitors are not made when a channel has
+       /// been "frozen".
+       ///
+       /// Note that even if updates made after TemporaryFailure succeed you must still call
+       /// test_restore_channel_monitor to ensure you have the latest monitor and re-enable normal
+       /// channel operation.
        TemporaryFailure,
        /// Used to indicate no further channel monitor updates will be allowed (eg we've moved on to a
        /// different watchtower and cannot update with all watchtowers that were previously informed
@@ -55,12 +75,14 @@ pub enum ChannelMonitorUpdateErr {
 /// them. Generally should be implemented by keeping a local SimpleManyChannelMonitor and passing
 /// events to it, while also taking any add_update_monitor events and passing them to some remote
 /// server(s).
+///
 /// Note that any updates to a channel's monitor *must* be applied to each instance of the
 /// channel's monitor everywhere (including remote watchtowers) *before* this function returns. If
 /// an update occurs and a remote watchtower is left with old state, it may broadcast transactions
 /// which we have revoked, allowing our counterparty to claim all funds in the channel!
 pub trait ManyChannelMonitor: Send + Sync {
        /// Adds or updates a monitor for the given `funding_txo`.
+       ///
        /// Implementor must also ensure that the funding_txo outpoint is registered with any relevant
        /// ChainWatchInterfaces such that the provided monitor receives block_connected callbacks with
        /// any spends of it.
@@ -69,10 +91,13 @@ pub trait ManyChannelMonitor: Send + Sync {
 
 /// A simple implementation of a ManyChannelMonitor and ChainListener. Can be used to create a
 /// watchtower or watch our own channels.
+///
 /// Note that you must provide your own key by which to refer to channels.
+///
 /// If you're accepting remote monitors (ie are implementing a watchtower), you must verify that
 /// users cannot overwrite a given channel by providing a duplicate key. ie you should probably
 /// index by a PublicKey which is required to sign any updates.
+///
 /// If you're using this for local monitoring of your own channels, you probably want to use
 /// `OutPoint` as the key, which will give you a ManyChannelMonitor implementation.
 pub struct SimpleManyChannelMonitor<Key> {
@@ -147,7 +172,13 @@ impl ManyChannelMonitor for SimpleManyChannelMonitor<OutPoint> {
 const CLTV_SHARED_CLAIM_BUFFER: u32 = 12;
 /// If an HTLC expires within this many blocks, force-close the channel to broadcast the
 /// HTLC-Success transaction.
-const CLTV_CLAIM_BUFFER: u32 = 6;
+/// In other words, this is an upper bound on how many blocks we think it can take us to get a
+/// transaction confirmed (and we use it in a few more, equivalent, places).
+pub(crate) const CLTV_CLAIM_BUFFER: u32 = 6;
+/// Number of blocks by which point we expect our counterparty to have seen new blocks on the
+/// network and done a full update_fail_htlc/commitment_signed dance (+ we've updated all our
+/// copies of ChannelMonitors, including watchtowers).
+pub(crate) const HTLC_FAIL_TIMEOUT_BLOCKS: u32 = 3;
 
 #[derive(Clone, PartialEq)]
 enum KeyStorage {
@@ -180,6 +211,7 @@ const MIN_SERIALIZATION_VERSION: u8 = 1;
 
 /// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates
 /// on-chain transactions to ensure no loss of funds occurs.
+///
 /// You MUST ensure that no ChannelMonitors for a given channel anywhere contain out-of-date
 /// information and are actively monitoring the chain.
 pub struct ChannelMonitor {
@@ -750,7 +782,7 @@ impl ChannelMonitor {
                                        ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &htlc_base_key)))
                                },
                        };
-                       let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.delayed_payment_base_key));
+                       let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap()));
                        let a_htlc_key = match self.their_htlc_base_key {
                                None => return (txn_to_broadcast, (commitment_txid, watch_outputs)),
                                Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &their_htlc_base_key)),
@@ -847,7 +879,7 @@ impl ChannelMonitor {
                                                };
                                                let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
                                                sign_input!(sighash_parts, single_htlc_tx.input[0], Some(idx), htlc.amount_msat / 1000);
-                                               txn_to_broadcast.push(single_htlc_tx); // TODO: This is not yet tested in ChannelManager!
+                                               txn_to_broadcast.push(single_htlc_tx);
                                        }
                                }
                        }
@@ -1002,7 +1034,9 @@ impl ChannelMonitor {
 
        /// Attempst to claim a remote HTLC-Success/HTLC-Timeout s outputs using the revocation key
        fn check_spend_remote_htlc(&self, tx: &Transaction, commitment_number: u64) -> Option<Transaction> {
-               let htlc_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
+               if tx.input.len() != 1 || tx.output.len() != 1 {
+                       return None;
+               }
 
                macro_rules! ignore_error {
                        ( $thing : expr ) => {
@@ -1030,6 +1064,7 @@ impl ChannelMonitor {
                };
                let redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.their_to_self_delay.unwrap(), &delayed_key);
                let revokeable_p2wsh = redeemscript.to_v0_p2wsh();
+               let htlc_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
 
                let mut inputs = Vec::new();
                let mut amount = 0;
@@ -1172,16 +1207,7 @@ impl ChannelMonitor {
                        }
                }
                if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
-                       let mut needs_broadcast = false;
-                       for &(ref htlc, _, _) in cur_local_tx.htlc_outputs.iter() {
-                               if htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER {
-                                       if htlc.offered || self.payment_preimages.contains_key(&htlc.payment_hash) {
-                                               needs_broadcast = true;
-                                       }
-                               }
-                       }
-
-                       if needs_broadcast {
+                       if self.would_broadcast_at_height(height) {
                                broadcaster.broadcast_transaction(&cur_local_tx.tx);
                                for tx in self.broadcast_by_local_state(&cur_local_tx) {
                                        broadcaster.broadcast_transaction(&tx);
@@ -1194,10 +1220,29 @@ impl ChannelMonitor {
        pub(super) fn would_broadcast_at_height(&self, height: u32) -> bool {
                if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
                        for &(ref htlc, _, _) in cur_local_tx.htlc_outputs.iter() {
-                               if htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER {
-                                       if htlc.offered || self.payment_preimages.contains_key(&htlc.payment_hash) {
-                                               return true;
-                                       }
+                               // For inbound HTLCs which we know the preimage for, we have to ensure we hit the
+                               // chain with enough room to claim the HTLC without our counterparty being able to
+                               // time out the HTLC first.
+                               // For outbound HTLCs which our counterparty hasn't failed/claimed, our primary
+                               // concern is being able to claim the corresponding inbound HTLC (on another
+                               // channel) before it expires. In fact, we don't even really care if our
+                               // counterparty here claims such an outbound HTLC after it expired as long as we
+                               // can still claim the corresponding HTLC. Thus, to avoid needlessly hitting the
+                               // chain when our counterparty is waiting for expiration to off-chain fail an HTLC
+                               // we give ourselves a few blocks of headroom after expiration before going
+                               // on-chain for an expired HTLC.
+                               // Note that, to avoid a potential attack whereby a node delays claiming an HTLC
+                               // from us until we've reached the point where we go on-chain with the
+                               // corresponding inbound HTLC, we must ensure that outbound HTLCs go on chain at
+                               // least CLTV_CLAIM_BUFFER blocks prior to the inbound HTLC.
+                               //  aka outbound_cltv + HTLC_FAIL_TIMEOUT_BLOCKS == height - CLTV_CLAIM_BUFFER
+                               //      inbound_cltv == height + CLTV_CLAIM_BUFFER
+                               //      outbound_cltv + HTLC_FAIL_TIMEOUT_BLOCKS + CLTV_CLAIM_BUFER <= inbound_cltv - CLTV_CLAIM_BUFFER
+                               //      HTLC_FAIL_TIMEOUT_BLOCKS + 2*CLTV_CLAIM_BUFER <= inbound_cltv - outbound_cltv
+                               //      HTLC_FAIL_TIMEOUT_BLOCKS + 2*CLTV_CLAIM_BUFER <= CLTV_EXPIRY_DELTA
+                               if ( htlc.offered && htlc.cltv_expiry + HTLC_FAIL_TIMEOUT_BLOCKS <= height) ||
+                                  (!htlc.offered && htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER && self.payment_preimages.contains_key(&htlc.payment_hash)) {
+                                       return true;
                                }
                        }
                }