-
Notifications
You must be signed in to change notification settings - Fork 2.3k
multi: fix SIMPLE_TAPROOT_FINAL acceptor and overlay RBF auto-enable #10763
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -670,11 +670,18 @@ func newServer(ctx context.Context, cfg *Config, listenAddrs []net.Addr, | |
| "in a standalone lnd build") | ||
| } | ||
|
|
||
| // If either taproot channel type is enabled, we also need to enable | ||
| // the RBF cooperative close protocol, as it is required for taproot | ||
| // channel interoperability. | ||
| if cfg.ProtocolOptions.TaprootChans || | ||
| cfg.ProtocolOptions.TaprootOverlayChans { | ||
| // If taproot channels are enabled, we also enable the RBF cooperative | ||
| // close protocol, as it is required for taproot channel | ||
| // interoperability. | ||
| // | ||
| // Exception: when taproot-overlay channels are enabled we do NOT | ||
| // auto-enable RBF, because the RBF coop close state machine does not | ||
| // yet thread through the AuxCloser hook that overlay channels rely on | ||
| // to build the aux-aware close transaction. Forcing RBF on for a | ||
| // node that holds overlay channels would silently break their coop | ||
| // closes. | ||
| if cfg.ProtocolOptions.TaprootChans && | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this mean if we have overlay channels the rbf protocol is also switch off for the normal taproot channels ?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but if the user sets it via the config it still breaks the coop close flow for overlay channels, we should prio this fix on the taproot assets side then.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes we don't want to auto-enable it for overlay chans the proper fix involves LND diff (integrating the AuxCloser in the RBF closer) and maybe some supporting code in taproot assets. Since we're already at the rc phase this should be enough. We control the flags in litd so for now we'll just try to protect & discourage the user from setting the flag (or even fail early in litd). |
||
| !cfg.ProtocolOptions.TaprootOverlayChans { | ||
|
|
||
| cfg.ProtocolOptions.RbfCoopClose = true | ||
| } | ||
|
|
||
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.
These four
casestatements forSIMPLE_TAPROOT_FINALare all setting the samecommitmentTypeand are quite repetitive. This could be simplified to improve readability and maintainability. A singlecasecan handle all combinations by checking for the base feature and ensuring no other unexpected features are present.This pattern of repeated
casestatements is also used for other commitment types in thisswitchblock and could be refactored similarly.