Skip to content

Commit 487b496

Browse files
committed
Correct spliced-stale SCID expiry for upgrades from pre-0.2 HTLC
If an HTLC was forwarded in 0.1, but waiting to be failed back, it will ultimately be failed by adding it to the `ChannelManager::pending_forwards` map with the channel's original SCID. If that channel is spliced between when the HTLC was forwarded (on 0.1) and when the HTLC is failed back (on 0.2), that SCID may no longer exist, causing the HTLC fail-back to be lost. Luckily, delaying when an SCID is expired is cheap - its just storing an extra `u64` or two and generating one requires an on-chain splice, so we simply delay removal of SCIDs for two months at which point any incoming HTLCs should have been expired for six weeks and the counterparty should have force-closed anyway. Backport of d12c6a3
1 parent 815b4b5 commit 487b496

File tree

1 file changed

+21
-0
lines changed

1 file changed

+21
-0
lines changed

lightning/src/ln/channel.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,6 +1372,27 @@ pub(crate) const COINBASE_MATURITY: u32 = 100;
13721372
/// The number of blocks to wait for a channel_announcement to propagate such that payments using an
13731373
/// older SCID can still be relayed. Once the spend of the previous funding transaction has reached
13741374
/// this number of confirmations, the corresponding SCID will be forgotten.
1375+
///
1376+
/// Because HTLCs added prior to 0.1 which were waiting to be failed may reference a channel's
1377+
/// pre-splice SCID, we need to ensure this is at least the maximum number of blocks before an HTLC
1378+
/// gets failed-back due to a time-out. Luckily, in LDK prior to 0.2, this is enforced directly
1379+
/// when checking the incoming HTLC, and compared against `CLTV_FAR_FAR_AWAY` (which prior to LDK
1380+
/// 0.2, and still at the time of writing, is 14 * 24 * 6, i.e. two weeks).
1381+
///
1382+
/// Here we use four times that value to give us more time to fail an HTLC back (which does require
1383+
/// the user call [`ChannelManager::process_pending_htlc_forwards`]) just in case (if an HTLC has
1384+
/// been expired for 3 * 2 weeks our counterparty really should have closed the channel by now).
1385+
/// Holding on to stale SCIDs doesn't really cost us much as each one costs an on-chain splice to
1386+
/// generate anyway, so we might as well make this nearly arbitrarily long.
1387+
///
1388+
/// [`ChannelManager::process_pending_htlc_forwards`]: crate::ln::channelmanager::ChannelManager::process_pending_htlc_forwards
1389+
#[cfg(not(test))]
1390+
pub(crate) const CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY: u32 = 14 * 24 * 6 * 4;
1391+
1392+
/// In test (not `_test_utils`, though, since that tests actual upgrading), we deliberately break
1393+
/// the above condition so that we can ensure that HTLCs forwarded in 0.2 or later are handled
1394+
/// correctly even if this constant is reduced and an HTLC can outlive the original channel's SCID.
1395+
#[cfg(test)]
13751396
pub(crate) const CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY: u32 = 144;
13761397

13771398
struct PendingChannelMonitorUpdate {

0 commit comments

Comments
 (0)