Skip to content

Commit a3a2f70

Browse files
committed
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.
1 parent fba204b commit a3a2f70

File tree

4 files changed

+43
-13
lines changed

4 files changed

+43
-13
lines changed

lightning/src/ln/channel.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use bitcoin::secp256k1;
2525
use ln::features::{ChannelFeatures, InitFeatures};
2626
use ln::msgs;
2727
use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
28-
use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
28+
use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
2929
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};
3030
use ln::chan_utils;
3131
use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
@@ -3359,6 +3359,10 @@ impl<Signer: Sign> Channel<Signer> {
33593359
self.config.fee_proportional_millionths
33603360
}
33613361

3362+
pub fn get_cltv_expiry_delta(&self) -> u16 {
3363+
cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA)
3364+
}
3365+
33623366
#[cfg(test)]
33633367
pub fn get_feerate(&self) -> u32 {
33643368
self.feerate_per_kw

lightning/src/ln/channelmanager.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -523,11 +523,16 @@ pub const BREAKDOWN_TIMEOUT: u16 = 6 * 24;
523523
pub(crate) const MAX_LOCAL_BREAKDOWN_TIMEOUT: u16 = 2 * 6 * 24 * 7;
524524

525525
/// The minimum number of blocks between an inbound HTLC's CLTV and the corresponding outbound
526-
/// HTLC's CLTV. This should always be a few blocks greater than channelmonitor::CLTV_CLAIM_BUFFER,
527-
/// ie the node we forwarded the payment on to should always have enough room to reliably time out
528-
/// the HTLC via a full update_fail_htlc/commitment_signed dance before we hit the
529-
/// CLTV_CLAIM_BUFFER point (we static assert that it's at least 3 blocks more).
530-
const CLTV_EXPIRY_DELTA: u16 = 6 * 12; //TODO?
526+
/// HTLC's CLTV.
527+
///
528+
/// This can be increased (but not decreased) through [`ChannelConfig::cltv_expiry_delta`]
529+
///
530+
/// [`ChannelConfig::cltv_expiry_delta`]: crate::util::config::ChannelConfig::cltv_expiry_delta
531+
// This should always be a few blocks greater than channelmonitor::CLTV_CLAIM_BUFFER,
532+
// ie the node we forwarded the payment on to should always have enough room to reliably time out
533+
// the HTLC via a full update_fail_htlc/commitment_signed dance before we hit the
534+
// CLTV_CLAIM_BUFFER point (we static assert that it's at least 3 blocks more).
535+
pub const MIN_CLTV_EXPIRY_DELTA: u16 = 6 * 6;
531536
pub(super) const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO?
532537

533538
// 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?
538543
// LATENCY_GRACE_PERIOD_BLOCKS.
539544
#[deny(const_err)]
540545
#[allow(dead_code)]
541-
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;
546+
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;
542547

543548
// Check for ability of an attacker to make us fail on-chain by delaying inbound claim. See
544549
// ChannelMontior::would_broadcast_at_height for a description of why this is needed.
545550
#[deny(const_err)]
546551
#[allow(dead_code)]
547-
const CHECK_CLTV_EXPIRY_SANITY_2: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER;
552+
const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER;
548553

549554
/// Details of a channel, as returned by ChannelManager::list_channels and ChannelManager::list_usable_channels
550555
#[derive(Clone)]
@@ -1271,7 +1276,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
12711276
if fee.is_none() || msg.amount_msat < fee.unwrap() || (msg.amount_msat - fee.unwrap()) < *amt_to_forward { // fee_insufficient
12721277
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())));
12731278
}
1274-
if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + CLTV_EXPIRY_DELTA as u64 { // incorrect_cltv_expiry
1279+
if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + chan.get_cltv_expiry_delta() as u64 { // incorrect_cltv_expiry
12751280
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())));
12761281
}
12771282
let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1;
@@ -1329,7 +1334,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
13291334
short_channel_id,
13301335
timestamp: chan.get_update_time_counter(),
13311336
flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1),
1332-
cltv_expiry_delta: CLTV_EXPIRY_DELTA,
1337+
cltv_expiry_delta: chan.get_cltv_expiry_delta(),
13331338
htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(),
13341339
htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()),
13351340
fee_base_msat: chan.get_holder_fee_base_msat(&self.fee_estimator),

lightning/src/ln/functional_tests.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2966,7 +2966,11 @@ fn test_htlc_on_chain_timeout() {
29662966

29672967
let chanmon_cfgs = create_chanmon_cfgs(3);
29682968
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
2969-
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
2969+
let mut cfg = UserConfig::default();
2970+
// This test was written when the CLTV_EXPIRY_DELTA was fixed at 12*6, and expects it.
2971+
cfg.channel_options.cltv_expiry_delta = 12*6;
2972+
cfg.channel_options.announced_channel = true;
2973+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(cfg.clone()), Some(cfg.clone()), Some(cfg.clone())]);
29702974
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
29712975

29722976
// Create some intial channels

lightning/src/util/config.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
//! Various user-configurable channel limits and settings which ChannelManager
1111
//! applies for you.
1212
13-
use ln::channelmanager::{BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
13+
use ln::channelmanager::{BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
1414

1515
/// Configuration we set when applicable.
1616
///
@@ -161,6 +161,21 @@ pub struct ChannelConfig {
161161
///
162162
/// Default value: 0.
163163
pub fee_proportional_millionths: u32,
164+
/// The difference in the CLTV value between incoming HTLCs and an outbound HTLC forwarded over
165+
/// the channel this config applies to.
166+
///
167+
/// This is analogous to [`ChannelHandshakeConfig::our_to_self_delay`] but applies to in-flight
168+
/// HTLC balance when a channel appears on-chain whereas
169+
/// [`ChannelHandshakeConfig::our_to_self_delay`] applies to the remaining
170+
/// (non-HTLC-encumbered) balance.
171+
///
172+
/// Thus, for HTLC-encumbered balances to be enforced on-chain, when a channel is force-closed,
173+
/// we (or one of our watchtowers) MUST be online to check for broadcast of the current
174+
/// commitment transaction at least once per this many blocks.
175+
///
176+
/// Default value: [`MIN_CLTV_EXPIRY_DELTA`] (currently 36), which we enforce as a minimum so
177+
/// you can change the config to ask for more security, not less.
178+
pub cltv_expiry_delta: u16,
164179
/// Set to announce the channel publicly and notify all nodes that they can route via this
165180
/// channel.
166181
///
@@ -192,15 +207,17 @@ impl Default for ChannelConfig {
192207
fn default() -> Self {
193208
ChannelConfig {
194209
fee_proportional_millionths: 0,
210+
cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA,
195211
announced_channel: false,
196212
commit_upfront_shutdown_pubkey: true,
197213
}
198214
}
199215
}
200216

201217
//Add write and readable traits to channelconfig
202-
impl_writeable!(ChannelConfig, 8+1+1, {
218+
impl_writeable!(ChannelConfig, 8+2+1+1, {
203219
fee_proportional_millionths,
220+
cltv_expiry_delta,
204221
announced_channel,
205222
commit_upfront_shutdown_pubkey
206223
});

0 commit comments

Comments
 (0)