Document exactly our CLTV sanitization policy for final incoming HTLCs

We want to avoid a third-party channel closure, where a random node
by sending us a payment expiring at current height, would trigger our
onchain logic to close the channel due to a near-expiration.

PaymentReceived and unknown HTLC cancellation must happen before
LATENCY_GRACE_PERIOD_BLOCKS.

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 374d74f6c18b578b8123e4f2344375164abff704..ffa65209710a6f6b3b51e14cf7f36e9a2ceb7bad 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -1711,8 +1711,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Close("Remote provided CLTV expiry in seconds instead of block height"));
                }
 
-               //TODO: Check msg.cltv_expiry further? Do this in channel manager?
-
                if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 {
                        if let PendingHTLCStatus::Forward(_) = pending_forward_state {
                                panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing");

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 4f71f406bd7872c01aed72d45b28f16105d06a1d..2ea6ff2b0d7c5020ca72a04dce0ad5de508846e8 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1039,6 +1039,10 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 
                                // OUR PAYMENT!
                                // final_expiry_too_soon
+                               // Final node can't rely on checking a CLTV_EXPIRY_DELTA which enforces by its own length CLTV_CLAIM_BUFFER so make sure
+                               // we don't accept incoming HTLC we wouldn't have time to claim with a worst-case broadcast scenario
+                               // Also, in case of unknown payment hash, makes sure payment logic has time to fail backward HTLC before our onchain
+                               // logic trigger a channel closure.
                                if (msg.cltv_expiry as u64) < self.latest_block_height.load(Ordering::Acquire) as u64 + (CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS) as u64 {
                                        return_err!("The final CLTV expiry is too soon to handle", 17, &[0;0]);
                                }

diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs
index 2c00a133575d60d9923f1e9afb0501b9cc4fa706..bfb29df2460b32180825e5b5d2da1c581ac2b210 100644 (file)
--- a/lightning/src/util/events.rs
+++ b/lightning/src/util/events.rs
@@ -52,7 +52,8 @@ pub enum Event {
        /// Indicates we've received money! Just gotta dig out that payment preimage and feed it to
        /// ChannelManager::claim_funds to get it....
        /// Note that if the preimage is not known or the amount paid is incorrect, you must call
-       /// ChannelManager::fail_htlc_backwards to free up resources for this HTLC.
+       /// ChannelManager::fail_htlc_backwards to free up resources for this HTLC before
+       /// LATENCY_GRACE_PERIOD_BLOCKS to avoid any channel-closure by onchain monitoring.
        /// The amount paid should be considered 'incorrect' when it is less than or more than twice
        /// the amount expected.
        PaymentReceived {