diff --git a/codecov.yml b/codecov.yml index 88751cdb15f..c55a5357d88 100644 --- a/codecov.yml +++ b/codecov.yml @@ -3,6 +3,11 @@ coverage: project: default: target: auto - threshold: 2% + threshold: 1% base: auto informational: false + patch: + default: + target: auto + threshold: 100% + base: auto diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 16a8829468e..ca1887d8b25 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 dd3fac1b6dc..bb9dbaba774 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 6c46421a97b..4a9f300638a 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -23,18 +23,21 @@ pub struct ChannelHandshakeConfig { /// /// Default value: 6. pub minimum_depth: u32, - /// Set to the amount of time we require our counterparty to wait to claim their money. + /// Set to the number of blocks we require our counterparty to wait to claim their money (ie + /// the number of blocks we have to punish our counterparty if they broadcast a revoked + /// transaction). /// - /// It's one of the main parameter of our security model. We (or one of our watchtowers) MUST - /// be online to check for peer having broadcast a revoked transaction to steal our funds - /// at least once every our_to_self_delay blocks. + /// This is one of the main parameters of our security model. We (or one of our watchtowers) MUST + /// be online to check for revoked transactions on-chain at least once every our_to_self_delay + /// 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). /// /// Meanwhile, asking for a too high delay, we bother peer to freeze funds for nothing in /// case of an honest unilateral channel close, which implicitly decrease the economic value of /// our channel. /// - /// Default value: [`BREAKDOWN_TIMEOUT`] (currently 144), we enforce it as a minimum at channel - /// opening so you can tweak config to ask for more security, not less. + /// Default value: [`BREAKDOWN_TIMEOUT`], we enforce it as a minimum at channel opening so you + /// can tweak config to ask for more security, not less. pub our_to_self_delay: u16, /// Set to the smallest value HTLC we will accept to process. /// @@ -161,6 +164,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 +215,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 +223,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 });