Avoid force-closing 0-conf channels when funding is reorg'd #4231
Conversation
|
👋 Thanks for assigning @tankyleo as a reviewer! |
When we see a funding transaction for one of our chanels reorg'd out, we worry that its possible we've been double-spent and immediately force-close the channel to avoid accepting any more HTLCs on it. This isn't ideal, but is mostly fine as most nodes require 6 confirmations and 6 block reorgs are exceedingly rare. However, this isn't so okay for 0-conf channels - in that case we elected to trust the funder anyway, so reorgs shouldn't worry us. Still, to handle this correctly we needed to track the old SCID and ensure our logic is safe across an SCID change. Luckily, we did that work for splices, and can now take advantage of it here. Fixes lightningdevkit#3836.
TheBlueMatt
force-pushed
the
2025-11-0conf-no-reorg-close
branch
from
83e1ae9 to
b4f7fdf
Compare
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
|
✅ Added second reviewer: @tankyleo |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4231 +/- ##
==========================================
+ Coverage 89.33% 89.34% +0.01%
==========================================
Files 180 180
Lines 138353 139177 +824
Branches 138353 139177 +824
==========================================
+ Hits 123598 124353 +755
- Misses 12132 12200 +68
- Partials 2623 2624 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lightning/src/ln/channel.rs
Outdated
| } else { | ||
| debug_assert!(false); | ||
| } | ||
| if self.context.minimum_depth(&self.funding).expect("set for a ready channel") > 1 { |
There was a problem hiding this comment.
Did we also not want to force close for 1-conf minimum channels ? Zero-conf channels have min depth 0 iiuc
There was a problem hiding this comment.
Mmm, yea, when i was writing the patch I was thinking we should also not FC for 1 confs but then thought better of it cause we'd need a channel-freeze option. Good catch.
TheBlueMatt
force-pushed
the
2025-11-0conf-no-reorg-close
branch
from
51dde37 to
faf6e00
Compare
TheBlueMatt
force-pushed
the
2025-11-0conf-no-reorg-close
branch
2 times, most recently
from
9312dfb to
850aef5
Compare
TheBlueMatt
force-pushed
the
2025-11-0conf-no-reorg-close
branch
from
850aef5 to
37d5a21
Compare
|
🔔 1st Reminder Hey @tankyleo! This PR has been waiting for your review. |
tankyleo
left a comment
tankyleo
left a comment
There was a problem hiding this comment.
LGTM @TheBlueMatt good to squash thanks
4 participants
When we see a funding transaction for one of our chanels reorg'd out, we worry that its possible we've been double-spent and immediately force-close the channel to avoid accepting any more HTLCs on it. This isn't ideal, but is mostly fine as most nodes require 6 confirmations and 6 block reorgs are exceedingly rare.
However, this isn't so okay for 0-conf channels - in that case we elected to trust the funder anyway, so reorgs shouldn't worry us. Still, to handle this correctly we needed to track the old SCID and ensure our logic is safe across an SCID change. Luckily, we did that work for splices, and can now take advantage of it here.
Fixes #3836.