From c8d4536b3e4baa94590ca43bc39cff2cc85e96e1 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 17 Mar 2021 12:49:49 -0400 Subject: [PATCH] Make cltv_expiry_delta configurable and reduce the min/default some We allow users to configure the to_self_delay, which is analogous to the cltv_expiry_delta in terms of its security context, so we should allow users to specify both. We similarly bound it on the lower end, but reduce that bound somewhat now that it is configurable. --- lightning/src/ln/channel.rs | 6 +++++- lightning/src/ln/channelmanager.rs | 23 ++++++++++++++--------- lightning/src/util/config.rs | 24 +++++++++++++++++++++++- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 16a882946..ca1887d8b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -25,7 +25,7 @@ use bitcoin::secp256k1; use ln::features::{ChannelFeatures, InitFeatures}; use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; -use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT}; +use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor}; use ln::chan_utils; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; @@ -3359,6 +3359,10 @@ impl Channel { self.config.fee_proportional_millionths } + pub fn get_cltv_expiry_delta(&self) -> u16 { + cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) + } + #[cfg(test)] pub fn get_feerate(&self) -> u32 { self.feerate_per_kw diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index dd3fac1b6..bb9dbaba7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -523,11 +523,16 @@ pub const BREAKDOWN_TIMEOUT: u16 = 6 * 24; pub(crate) const MAX_LOCAL_BREAKDOWN_TIMEOUT: u16 = 2 * 6 * 24 * 7; /// The minimum number of blocks between an inbound HTLC's CLTV and the corresponding outbound -/// HTLC's CLTV. This should always be a few blocks greater than channelmonitor::CLTV_CLAIM_BUFFER, -/// ie the node we forwarded the payment on to should always have enough room to reliably time out -/// the HTLC via a full update_fail_htlc/commitment_signed dance before we hit the -/// CLTV_CLAIM_BUFFER point (we static assert that it's at least 3 blocks more). -const CLTV_EXPIRY_DELTA: u16 = 6 * 12; //TODO? +/// HTLC's CLTV. The current default represents roughly six hours of blocks at six blocks/hour. +/// +/// This can be increased (but not decreased) through [`ChannelConfig::cltv_expiry_delta`] +/// +/// [`ChannelConfig::cltv_expiry_delta`]: crate::util::config::ChannelConfig::cltv_expiry_delta +// This should always be a few blocks greater than channelmonitor::CLTV_CLAIM_BUFFER, +// i.e. the node we forwarded the payment on to should always have enough room to reliably time out +// the HTLC via a full update_fail_htlc/commitment_signed dance before we hit the +// CLTV_CLAIM_BUFFER point (we static assert that it's at least 3 blocks more). +pub const MIN_CLTV_EXPIRY_DELTA: u16 = 6 * 6; pub(super) const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO? // Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + ANTI_REORG_DELAY + LATENCY_GRACE_PERIOD_BLOCKS, @@ -538,13 +543,13 @@ pub(super) const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO? // LATENCY_GRACE_PERIOD_BLOCKS. #[deny(const_err)] #[allow(dead_code)] -const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - CLTV_CLAIM_BUFFER - ANTI_REORG_DELAY - LATENCY_GRACE_PERIOD_BLOCKS; +const CHECK_CLTV_EXPIRY_SANITY: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - CLTV_CLAIM_BUFFER - ANTI_REORG_DELAY - LATENCY_GRACE_PERIOD_BLOCKS; // Check for ability of an attacker to make us fail on-chain by delaying inbound claim. See // ChannelMontior::would_broadcast_at_height for a description of why this is needed. #[deny(const_err)] #[allow(dead_code)] -const CHECK_CLTV_EXPIRY_SANITY_2: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER; +const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER; /// Details of a channel, as returned by ChannelManager::list_channels and ChannelManager::list_usable_channels #[derive(Clone)] @@ -1271,7 +1276,7 @@ impl ChannelMana if fee.is_none() || msg.amount_msat < fee.unwrap() || (msg.amount_msat - fee.unwrap()) < *amt_to_forward { // fee_insufficient break Some(("Prior hop has deviated from specified fees parameters or origin node has obsolete ones", 0x1000 | 12, Some(self.get_channel_update(chan).unwrap()))); } - if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + CLTV_EXPIRY_DELTA as u64 { // incorrect_cltv_expiry + if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + chan.get_cltv_expiry_delta() as u64 { // incorrect_cltv_expiry break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update(chan).unwrap()))); } let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1; @@ -1329,7 +1334,7 @@ impl ChannelMana short_channel_id, timestamp: chan.get_update_time_counter(), flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1), - cltv_expiry_delta: CLTV_EXPIRY_DELTA, + cltv_expiry_delta: chan.get_cltv_expiry_delta(), htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(), htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()), fee_base_msat: chan.get_holder_fee_base_msat(&self.fee_estimator), diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 6c46421a9..a288caf46 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -161,6 +161,26 @@ pub struct ChannelConfig { /// /// Default value: 0. pub fee_proportional_millionths: u32, + /// The difference in the CLTV value between incoming HTLCs and an outbound HTLC forwarded over + /// the channel this config applies to. + /// + /// This is analogous to [`ChannelHandshakeConfig::our_to_self_delay`] but applies to in-flight + /// HTLC balance when a channel appears on-chain whereas + /// [`ChannelHandshakeConfig::our_to_self_delay`] applies to the remaining + /// (non-HTLC-encumbered) balance. + /// + /// Thus, for HTLC-encumbered balances to be enforced on-chain when a channel is force-closed, + /// we (or one of our watchtowers) MUST be online to check for broadcast of the current + /// commitment transaction at least once per this many blocks (minus some margin to allow us + /// enough time to broadcast and confirm a transaction, possibly with time in between to RBF + /// the spending transaction). + /// + /// Default value: 72 (12 hours at an average of 6 blocks/hour). + /// Minimum value: [`MIN_CLTV_EXPIRY_DELTA`], any values less than this will be treated as + /// [`MIN_CLTV_EXPIRY_DELTA`] instead. + /// + /// [`MIN_CLTV_EXPIRY_DELTA`]: crate::ln::channelmanager::MIN_CLTV_EXPIRY_DELTA + pub cltv_expiry_delta: u16, /// Set to announce the channel publicly and notify all nodes that they can route via this /// channel. /// @@ -192,6 +212,7 @@ impl Default for ChannelConfig { fn default() -> Self { ChannelConfig { fee_proportional_millionths: 0, + cltv_expiry_delta: 6 * 12, // 6 blocks/hour * 12 hours announced_channel: false, commit_upfront_shutdown_pubkey: true, } @@ -199,8 +220,9 @@ impl Default for ChannelConfig { } //Add write and readable traits to channelconfig -impl_writeable!(ChannelConfig, 8+1+1, { +impl_writeable!(ChannelConfig, 8+2+1+1, { fee_proportional_millionths, + cltv_expiry_delta, announced_channel, commit_upfront_shutdown_pubkey }); -- 2.39.5