multi: integrate rbf changes from staging branch #9610
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
In this commit, we change LinkDirection to be a type alias. This makes creating wrapper structs/interfaces easier, as we don't need to leak htlcswitch.LinkDirection, instead we can accept a bool.
In this commit, we add an implementation of a new interface the rbf coop state machine needs. We take care to accept interfaces everywhere, to make this easier to test, and decouple from the concrete types we'll end up using elsewhere.
Similar to the last commit, in this commit, we make a daemon adapter impl for lnd. We'll use this for any future protofsm based state machine.
We don't return an error on broadcast fail as the broadcast might have failed due to insufficient fees, or inability to be replaced, which may happen when one side attempts to unnecessarily bump their coop close fee.
With this commit, we make sure we set the right height hint, even if the channel is a zero conf channel.
In this commit, we update `chooseDeliveryScript` to generate a new script if needed. This allows us to fold in a few other lines that always followed this function into this expanded function. The tests have been updated accordingly.
This'll be useful to communicate what the new fee rate is to an RPC caller.
If we go to close while the channel is already flushed, we might get an extra event, so we can safely ignore it and do a self state transition.
Both these messages now carry the address of both parties, so you can update an address without needing to send shutdown again.
In this commit, we implement the latest version of the RBF loop as described in the spec. We remove the self loop back based on sending or receiving shutdown. Instead, from the ClosePending state, we can trigger a new loop by sending SendOfferEvent (we bump), or OfferReceivedEvent (they bump). We also update the rbf state machine w/ the new close addr logic. This log ensures that the remote party always sends our current address, and that if they send a new address, we'll update our view of it, and counter sign the correct transaction. We also add a CloseErr state. With this new state, we can ensure that we're able to properly report errors back to the RPC client, and also optionally force a reconnection or send a warning to the remote party.
In this commit, we implement a special case for OP_RETURN scripts outlined in the spec. If a party decides that its output will be too small even after the dust check, then they can opt to set it to zero by sending an `OP_RETURN` as their script.
In this commit, we update the RBF state machine to handle early offer cases. This can happen if after we send out shutdown (to kick things off), the remote party sends their offer early. This can also happen if their outgoing shutdown (to ACK ours) was delayed for w/e reason, and we get their offer first. The alternative was to modify the state machine itself, but we feel that handling this early case is better in line with the Robustness principle.
In this commit, we add an upfront check for `SendWhen` predicates before deciding to launch a goroutine. This ensures that when a message comes along that is already ready to send, we do the send in a synchronous manner.
In the next commit, we'll start checking feature bits to decide how to init the chan closer. In the future, we can move the current chan closer handling logic into an `MsgEndpoint`, which'll allow us to get rid of the explicit chan closer map and direct handling.
In this commit, we use the interfaces we created in the prior commit to make a new method capable of spinning up the new rbf coop closer.
In this commit, we add a new composite chanCloserFsm type. This'll allow us to store a single value that might be a negotiator or and rbf-er. In a follow up commit, we'll use this to conditionally create the new rbf closer.
In this commit, we fully integrate the new RBF close state machine into the peer. For the restart case after shutdown, we can short circuit the existing logic as the new FSM will handle retransmitting the shutdown message itself, and doesn't need to delegate that duty to the link. Unlike the existing state machine, we're able to restart the flow to sign a coop close with a new higher fee rate. In this case, we can now send multiple updates to the RPC caller, one for each newly singed coop close transaction. To implement the async flush case, we'll launch a new goroutine to wait until the state machine reaches the `ChannelFlushing` state, then we'll register the hook. We don't do this at start up, as otherwise the channel may _already_ be flushed, triggering an invalid state transition.
For now, we disallow the option to be used with the taproot chans option, as the new flow hasn't yet been updated for nonce usage.
This fixes some existing race conditions, as the `finalizeChanClosure` function was being called from outside the main event loop.
If we hit an error, we want to wipe the state machine state, which also includes removing the old endpoint.
This'll allow us to notify the caller each time a new coop close transaction with a higher fee rate is signed.
Resp is always nil, so we actually need to log event.Update here.
In this commit, we extend `CloseChannelAssertPending` with new args that returns the raw close status update (as we have more things we'd like to assert), and also allows us to pass in a custom fee rate.
We'll properly handle a protocol error due to user input by halting, and sending the error back to the user. When a user goes to issue a new update, based on which state we're in, we'll either kick off the shutdown, or attempt a new offer. This matches the new spec update where we'll only send `Shutdown` once per connection.
In this commit, we alter the existing co-op close flow to enable RBF bumps after re connection. With the new RBF close flow, it's possible that after a success round _and_ a re connection, either side wants to do another fee bump. Typically we route these requests through the switch, but in this case, the link no longer exists in the switch, so any requests to fee bump again would find that the link doesn't exist. In this commit, we implement a work around wherein if we have an RBF chan closer active, and the link isn't in the switch, then we just route the request directly to the chan closer via the peer. Once we have the chan closer, we can use the exact same flow as prior.
The itest has both sides try to close multiple times, each time with increasing fee rates. We also test the reconnection case, bad RBF updates, and instances where the local party can't actually pay for fees.
With this commit, we make sure we set the right height hint, even if the channel is a zero conf channel.
This fixes an issue in the itests in the restart case. We'd see an error like: ``` 2025-03-12 23:41:10.754 [ERR] PFSM state_machine.go:661: FSM(rbf_chan_closer(2f20725d9004f7fda7ef280f77dd8d419fd6669bda1a5231dd58d6f6597066e0:0)): Unable to apply event err="invalid state transition: received *chancloser.SendOfferEvent while in ClosingNegotiation(local=LocalOfferSent(proposed_fee=0.00000193 BTC), remote=ClosePending(txid=07229915459cb439bdb8ad4f5bf112dc6f42fca0192ea16a7d6dd05e607b92ae, party=Remote, fee_rate=1 sat/vb))" ``` We resolve this by waiting to send in the new request unil the old one has been completed.
yyforyongyu
approved these changes
Mar 19, 2025
twofaktor
reviewed
May 23, 2025
| ; Set to disable experimental endorsement signaling. | ||
| ; protocol.no-experimental-endorsement=false | ||
|
|
||
| ; Set to disable support for RBF based coop close. |
There was a problem hiding this comment.
*Set to "enable" support for RBF based coop close.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.