Skip to content

Commit 1ded225

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 1a6becc commit 1ded225

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};
@@ -3339,6 +3339,10 @@ impl<Signer: Sign> Channel<Signer> {
33393339
self.config.fee_proportional_millionths
33403340
}
33413341

3342+
pub fn get_cltv_expiry_delta(&self) -> u16 {
3343+
cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA)
3344+
}
3345+
33423346
#[cfg(test)]
33433347
pub fn get_feerate(&self) -> u32 {
33443348
self.feerate_per_kw

lightning/src/ln/channelmanager.rs

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

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

530535
// Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + ANTI_REORG_DELAY + LATENCY_GRACE_PERIOD_BLOCKS,
@@ -535,13 +540,13 @@ pub(super) const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO?
535540
// LATENCY_GRACE_PERIOD_BLOCKS.
536541
#[deny(const_err)]
537542
#[allow(dead_code)]
538-
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;
543+
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;
539544

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

546551
/// Details of a channel, as returned by ChannelManager::list_channels and ChannelManager::list_usable_channels
547552
#[derive(Clone)]
@@ -1263,7 +1268,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
12631268
if fee.is_none() || msg.amount_msat < fee.unwrap() || (msg.amount_msat - fee.unwrap()) < *amt_to_forward { // fee_insufficient
12641269
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())));
12651270
}
1266-
if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + CLTV_EXPIRY_DELTA as u64 { // incorrect_cltv_expiry
1271+
if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + chan.get_cltv_expiry_delta() as u64 { // incorrect_cltv_expiry
12671272
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())));
12681273
}
12691274
let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1;
@@ -1321,7 +1326,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
13211326
short_channel_id,
13221327
timestamp: chan.get_update_time_counter(),
13231328
flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1),
1324-
cltv_expiry_delta: CLTV_EXPIRY_DELTA,
1329+
cltv_expiry_delta: chan.get_cltv_expiry_delta(),
13251330
htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(),
13261331
htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()),
13271332
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)