-
Notifications
You must be signed in to change notification settings - Fork 492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gossip: delay considering a channel edge deleted for 12-blocks #1004
gossip: delay considering a channel edge deleted for 12-blocks #1004
Conversation
@@ -249,6 +249,10 @@ optional) features will have _odd_ feature bits, while incompatible features | |||
will have _even_ feature bits | |||
(["It's OK to be odd!"](00-introduction.md#glossary-and-terminology-guide)). | |||
|
|||
A delay of 12-blocks is used when forgetting a channel on funding output spend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a hard time parsing "channel on funding output spend". What is exactly meant by that?
Also, nit: s/12-blocks/12 blocks/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See L226:
- once its funding output has been spent OR reorganized out:
I'd like to spend more time exploring the design space for splicing before acking this, sorry if it takes some time before I can comment! |
No worries! Thought I'd go ahead an make a PR for it though. |
What's the worry with Just Doing This? It seems good to get moving on this a ways before it's needed. |
IMO in practice the signal will be pretty unreliable due to strictly relying on timely gossip message propagation. There also isn't a good way to track the actual observance of this, since it's more or less just a guideline. I wrote more about this here, along with a suggestion for a primary chain signal, with the gossip signal (w/e that looks like) being secondary: https://lists.linuxfoundation.org/pipermail/lightning-dev/2022-June/003616.html |
That's what I had in mind, my thinking was that the splicing tx would be somehow identifiable as a splicing tx so that we don't need this 12-blocks delay at all. But the issue is that we'd like implementations to deploy code to identify this signal before we actually deploy splicing... We can also choose to do both: delay considering a channel edge deleted for 12 blocks, which implementations can do today and will help splicing when it happens, even though it's not a perfect solution, and then implement a signal in the splicing tx to improve reliability, that would work for me - but it's only useful if all implementations agree to ship this 12 blocks delay in the short term. |
Here's a thought (also posted to ML): if you're in possession of a This is nice because it uses the on-chain signaling that @Roasbeef proposed, but doesn't require new protocol design, just changing the priority/scope of which you gossip a subset of messages. This is nicely spam compatible. Splice gossip has a natural rate-limit/real world cost in that triggering the close onchain costs sats; you can't generate a lot of urgent gossip messages w/o sending money to the L1 miners. |
I honestly don't really understand this discussion. Gossip is never reliable, it was never meant to be reliable, and its always been best-effort. All nodes cooperate to try to make sure they all have as much a view of the routing table as possible, to ensure they have all the information they can when routing. If you're splicing, you're always going to have some users running today's software that will not be happy seeing the splice on-chain, this is true no matter what we do. If the concern is that 12 blocks isn't enough, well, then we can simply increase the constant, easy-peasy. Making sure splices are identifiable on-chain is wasteful and doesn't solve the issue as stated here except for new nodes, just like waiting to remove channels. Please, lets not over-complicate things... |
Agreed with @TheBlueMatt: the gossip is intended as a best effort system, and striving for completeness or timeliness is likely a far deeper Rabbit Hole than we're willing to tackle (now or potentially ever). As for the actual number, 12 blocks seems like plenty: 6 blocks to make the New version announceable, plus 6 blocks for propagation (see #1001 for a back of the envelope calculation). Do we also want onchain signaling? I'm not sure we want to make transactions identifiable beyond what we already have, but it might not be a huge loss here (we're gossiping about these outpoints anyway) and a belts and suspenders variant might be sensible. This is all going to change when we detach gossip from onchain outpoints anyway, so maybe better not to overthink it here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
Shall we merge this now? The previous spec meeting had acks from eclair, cln and ldk and @Roasbeef wasn't opposed to it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🍫
Solution is now to wait for 12-blocks for all channel closes as per: lightning#1004
Changelog-Added: gossip: delay considering a channel edge deleted for 12-blocks propose by lightning/bolts#1004 Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-Added: gossip: delay considering a channel edge deleted for 12-blocks propose by lightning/bolts#1004 Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
We introduced a 12-blocks delay for channel closing in lightning#1004, but we didn't update the requirement in the section about pruning.
We introduced a 12-blocks delay for channel closing in #1004, but we didn't update the requirement in the section about pruning.
Allows us time to propagate a splice update (new channel_announcement)
Suggested-By: @t-bast
Co-Authored: @ddustin