Correct spliced-stale SCID expiry for upgrades from pre-0.2 HTLC
What the developer wrote
Authored by Matt Corallo
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.
What changed, and why it matters
This patch fixes a bug where money-routing instructions (HTLCs) started in an older version of LDK could get lost if the channel was later upgraded via a process called 'splicing.' The old channel identifier was being deleted too soon, so when the software tried to fail the payment back to the sender, it couldn't find the channel anymore. The fix keeps those old identifiers around for about two months longer, giving the software time to properly return stuck payments.
Users running LDK nodes, especially those upgrading from pre-0.2 versions with active or pending HTLCs, should apply this patch. Monitor for any HTLCs stuck in pending_forwards after channel splices and ensure process_pending_htlc_forwards is called regularly. No immediate external action such as rotating keys is required.
Security signals we found
Potential loss of HTLC fail-back path leading to stuck funds
Race between SCID expiry and pending HTLC timeout handling
Upgrade compatibility issue from pre-0.2 to 0.2+ HTLC forwarding logic
Splicing interaction with stale channel identifier cleanup
Evidence from the diff
The commit changes the CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY constant in lightning/src/ln/channel.rs. For non-test builds, it increases the delay from 144 blocks to 14 * 24 * 6 * 4 blocks (about 8 weeks). This ensures that pre-0.2 HTLCs waiting to be failed back still reference a valid short channel ID (SCID) even if the channel was spliced while the HTLC was pending. The old SCID would otherwise expire before the HTLC timeout/fail-back path could resolve, causing the fail-back to be dropped. A separate test-only value of 144 blocks is retained to exercise the newer 0.2+ forwarding behavior under tighter conditions.
Changed components
lightning/src/ln/channel.rsCHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY constantChannelManager::pending_forwards handlingSplice-related SCID lifecycle managementInspect captured patch +21 / −0
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index dc966af..f5cb6c5 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -1373,6 +1373,27 @@ pub(crate) const COINBASE_MATURITY: u32 = 100;
/// The number of blocks to wait for a channel_announcement to propagate such that payments using an
/// older SCID can still be relayed. Once the spend of the previous funding transaction has reached
/// this number of confirmations, the corresponding SCID will be forgotten.
+///
+/// Because HTLCs added prior to 0.1 which were waiting to be failed may reference a channel's
+/// pre-splice SCID, we need to ensure this is at least the maximum number of blocks before an HTLC
+/// gets failed-back due to a time-out. Luckily, in LDK prior to 0.2, this is enforced directly
+/// when checking the incoming HTLC, and compared against `CLTV_FAR_FAR_AWAY` (which prior to LDK
+/// 0.2, and still at the time of writing, is 14 * 24 * 6, i.e. two weeks).
+///
+/// Here we use four times that value to give us more time to fail an HTLC back (which does require
+/// the user call [`ChannelManager::process_pending_htlc_forwards`]) just in case (if an HTLC has
+/// been expired for 3 * 2 weeks our counterparty really should have closed the channel by now).
+/// Holding on to stale SCIDs doesn't really cost us much as each one costs an on-chain splice to
+/// generate anyway, so we might as well make this nearly arbitrarily long.
+///
+/// [`ChannelManager::process_pending_htlc_forwards`]: crate::ln::channelmanager::ChannelManager::process_pending_htlc_forwards
+#[cfg(not(test))]
+pub(crate) const CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY: u32 = 14 * 24 * 6 * 4;
+
+/// In test (not `_test_utils`, though, since that tests actual upgrading), we deliberately break
+/// the above condition so that we can ensure that HTLCs forwarded in 0.2 or later are handled
+/// correctly even if this constant is reduced and an HTLC can outlive the original channel's SCID.
+#[cfg(test)]
pub(crate) const CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY: u32 = 144;
struct PendingChannelMonitorUpdate {
Why this scored 56/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.