multi: add new rbf coop close actor for RPC server fee bumps by Roasbeef · Pull Request #9821 · lightningnetwork/lnd

Roasbeef · 2025-05-17T00:52:04Z

In this PR, we create a new rbfCloseActor wrapper struct. This will
wrap the RPC operations to trigger a new RBF close bump within a new
actor. In the next commit, we'll now register this actor, and clean up
the call graph from the rpc server to this actor.

We then register the rbfCloseActor when we create the rbf
chan closer state machine. Now the RPC server no longer neesd to
traverse a series of maps and pointers (rpcServer -> server -> peer ->
activeCloseMap -> rbf chan closer) to trigger a new fee bump.

Instead, it just creates the service key that it knows that the closer
can be reached at, and sends a message to it using the returned
actorRef/router. We also hide additional details re the various methods
in play, as we only care about the type of message we expect to send and
receive.

Along the way we add some helper types to enable any protofsm state
machine to function as an actor in this framework.

Depends on #9820

coderabbitai · 2025-05-17T00:52:10Z

Important

Review skipped

Auto reviews are limited to specific labels.

🏷️ Labels to auto review (1)

llm-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

github-actions · 2025-05-17T00:53:21Z

Pull reviewers stats

Stats of the last 30 days for lnd:

User	Total reviews	Time to review	Total comments
guggero 🥇	24 ▀▀▀	23h 49m	47 ▀▀▀
ziggie1984 🥈	13 ▀	12h 35m	35 ▀▀
bhandras 🥉	11 ▀	4h 34m	12 ▀
yyforyongyu	10 ▀	1d 3h 57m	16 ▀
Roasbeef	7 ▀	9h 39m	4
ellemouton	5 ▀	1d 6h 18m	5
bitromortac	5 ▀	1h 41m	6
morehouse	3	1d 1h 19m	3
ffranr	2	18m	0
mohamedawnallah	2	6d 14h 50m ▀▀	11 ▀
NishantBansal2003	2	5d 15h 32m ▀▀	0
sputn1ck	1	23h 39m	2
GeorgeTsagk	1	3d 36m ▀	0
saubyk	1	20h 37m	0
MPins	1	8d 14h 1m ▀▀▀	3

erickcestari

Can you rebase? I think this PR is going to be way easier to review after it.

In this commit, we create a new rbfCloseActor wrapper struct. This will wrap the RPC operations to trigger a new RBF close bump within a new actor. In the next commit, we'll now register this actor, and clean up the call graph from the rpc server to this actor.

In this commit, we now register the rbfCloseActor when we create the rbf chan closer state machine. Now the RPC server no longer neesd to traverse a series of maps and pointers (rpcServer -> server -> peer -> activeCloseMap -> rbf chan closer) to trigger a new fee bump. Instead, it just creates the service key that it knows that the closer can be reached at, and sends a message to it using the returned actorRef/router. We also hide additional details re the various methods in play, as we only care about the type of message we expect to send and receive.

In this commit, we implement the actor.ActorBehavior interface for StateMachine. This enables the state machine executor to be registered as an actor, and have messages be sent to it via a unique ServiceKey that a concrete instance will set.

This can be used to allow any system to send a message to the RBF chan closer if it knows the proper service key. In the future, we can use this to redo the msgmux.Router in terms of the new actor abstractions.

Roasbeef · 2026-03-18T01:57:53Z

@erickcestari rebased!

erickcestari

Nice refactor! Routing fee bumps through the actor system with a service key lookup is a clean simplification over the old rpcServer -> server -> peer chain with the DB fetch + peer map lookup.

erickcestari · 2026-03-18T13:29:32Z

server.go

 	peerAccessMan *accessMan

+	// actors is the central registry for the set of active actors.
+	actors *actor.ActorSystem


Is the separation intentional for isolation, or should these be consolidated into a single actor system?

erickcestari · 2026-03-18T13:41:39Z

peer/rbf_close_wrapper_actor.go

+	// We only want to have a single actor instance for this rbf
+	// closer, so we'll now attempt to unregister any other
+	// instances.
+	_ = actorKey.UnregisterAll(r.actors)
+
+	// Now that we know that no instances of the actor are present,
+	// let's register a new instance. We don't actually need the ref
+	// though, as any interested parties can look up the actor via
+	// the service key.
+	actorID := fmt.Sprintf(
+		"PeerWrapper(RbfChanCloser(%s))", r.chanPoint,
+	)
+	_, _ = actorKey.Spawn(r.actors, actorID, r)


Shouldn't we handle the error from calling UnregisterAll and Spawn? At least may have a log here?

erickcestari · 2026-03-18T13:48:45Z

peer/rbf_close_wrapper_actor.go

+// reach an RBF chan closer, via an active peer.
+//
+//nolint:ll
+func NewRbfCloserServiceKey(op wire.OutPoint) RbfCloseActorServiceKey {


nit:

Suggested change

func NewRbfCloserServiceKey(op wire.OutPoint) RbfCloseActorServiceKey {

func NewRbfCloserPeerServiceKey(op wire.OutPoint) RbfCloseActorServiceKey {

erickcestari · 2026-03-18T13:59:37Z

server.go

 	peerAccessMan *accessMan

+	// actors is the central registry for the set of active actors.
+	actors *actor.ActorSystem


Also we should probably shutdown both actors when the server stops

erickcestari · 2026-03-18T14:02:06Z

peer/rbf_close_wrapper_actor.go

+// rbfCloseMessage is a message type that is used to trigger a cooperative fee
+// bump, or initiate a close for the first time.
+type rbfCloseMessage struct {
+	actor.Message


Here it should embeds the actor.BaseMessage (struct) instead of the actor.Message (interface)

Suggested change

actor.Message

actor.BaseMessage

erickcestari · 2026-03-18T14:28:59Z

peer/rbf_close_wrapper_actor.go

+
+	type retType = *CoopCloseUpdates
+
+	// If RBF coop close isn't permitted, then we'll an error.


nit:

Suggested change

// If RBF coop close isn't permitted, then we'll an error.

// If RBF coop close isn't permitted, then we'll return an error.

erickcestari · 2026-03-18T14:31:32Z

peer/rbf_close_wrapper_actor.go

+// nolint:ll
+type RbfCloseActorServiceKey = actor.ServiceKey[rbfCloseMessage, *CoopCloseUpdates]
+
+// NewRbfCloserPeerServiceKey returns a new service key that can be used to


nit:

Suggested change

// NewRbfCloserPeerServiceKey returns a new service key that can be used to

// NewRbfCloserServiceKey returns a new service key that can be used to

erickcestari · 2026-03-18T14:36:51Z

peer/rbf_close_wrapper_actor.go

+	opStr := op.String()
+
+	// Now that even just using the channel point here would be enough, as
+	// we have a unique type here ChanCloserActorMsg which will handle the


nit:

Suggested change

// we have a unique type here ChanCloserActorMsg which will handle the

// we have a unique type here rbfCloseMessage which will handle the

erickcestari · 2026-03-18T14:39:04Z

peer/brontide.go

+	// Actors enables the peer to send messages to the set of actors, and
+	// also register new actors itself.
+
+	Actors *actor.ActorSystem


nit:

Suggested change

// Actors enables the peer to send messages to the set of actors, and

// also register new actors itself.

Actors *actor.ActorSystem

// Actors enables the peer to send messages to the set of actors, and

// also register new actors itself.

Actors *actor.ActorSystem

erickcestari · 2026-03-18T16:59:10Z

peer/rbf_close_wrapper_actor.go

+// bump, or initiate a close for the first time.
+type rbfCloseMessage struct {
+	actor.Message
+


The ctx passed to Receive is the actor's internal context (a.ctx), not the caller's RPC stream context. This means observeRbfCloseUpdates can never detect RPC client disconnection via closeReq.Ctx.Done(), leaking the observer goroutine. Propagating the caller's context through the message restores the old TriggerCoopCloseRbfBump behavior.

Suggested change

// Ctx is the caller's context (e.g., the RPC stream context), used to detect client disconnection.

Ctx context.Context

yyforyongyu · 2026-03-31T12:27:14Z

go.mod

 // allows us to specify that as an option.
 replace google.golang.org/protobuf => github.com/lightninglabs/protobuf-go-hex-display v1.33.0-hex-display

+replace github.com/lightningnetwork/lnd/actor => ./actor


unnecessary change? also the TODO is removed

yyforyongyu · 2026-03-31T13:00:30Z

peer/rbf_close_wrapper_actor.go

+		DeliveryScript: msg.DeliveryScript,
+		Updates:        closeUpdates.UpdateChan,
+		Err:            closeUpdates.ErrChan,
+		Ctx:            ctx,


this seems to be the wrong ctx to inherit? The ctx here is a bit difficult to follow - but my understanding is that the Receive stores the actor lifecycle context in ChanClose.Ctx. Once this path is wired in, canceling the RPC no longer cancels the close-update observer, because the observer tears down on closeReq.Ctx.Done(), but that context now belongs to the actor, not the RPC caller.

yyforyongyu · 2026-03-31T13:02:03Z

peer/rbf_close_wrapper_actor.go

+	// bypassing the switch entirely.
+	closeReq := htlcswitch.ChanClose{
+		CloseType:      contractcourt.CloseRegular,
+		ChanPoint:      &msg.ChanPoint,


should we check r.chanPoint != msg.ChanPoint? otherwise it just forwards blindly.

yyforyongyu · 2026-03-31T13:02:49Z

peer/rbf_close_wrapper_actor.go

+	// We only want to have a single actor instance for this rbf
+	// closer, so we'll now attempt to unregister any other
+	// instances.
+	_ = actorKey.UnregisterAll(r.actors)


let's log the error here? can imagine it will be easier for future debugging

yyforyongyu · 2026-03-31T13:05:44Z

peer/brontide.go

+		p.log.Infof("Registering RBF actor for channel %v",
+			channel.ChannelPoint())
+
+		actorWrapper := newRbfCloseActor(


Should we add a cleanup or unregister logic when the peer disconnects? otherwise we would have stale RBF actors.

yyforyongyu · 2026-03-31T13:07:31Z

peer/brontide.go


+	// In addition to the message router, we'll register the state machine
+	// with the actor system.
+	if p.cfg.Actors != nil {


We prolly need to move this block after p.activeChanCloses.Store(chanID, makeRbfCloser(&chanCloser)), so we store the closer first, then register the actor.

yyforyongyu · 2026-03-31T13:08:16Z

rpcserver.go

 			rpcsLog.Infof("Bypassing Switch to do fee bump "+
 				"for ChannelPoint(%v)", chanPoint)

-			closeUpdates, err := r.server.AttemptRBFCloseUpdate(


yyforyongyu · 2026-03-31T13:22:01Z

lnwallet/chancloser/rbf_coop_states.go

+// RBF chan closer.
+//
+//nolint:ll
+func NewRbfCloserServiceKey(op wire.OutPoint) actor.ServiceKey[ChanCloserActorMsg, bool] {


we already have peer.NewRbfCloserServiceKey tho, the same name can be confusing.

lightninglabs-deploy · 2026-04-07T13:31:08Z

@gijswijs: review reminder
@Roasbeef, remember to re-request review from reviewers when ready

Roasbeef added rbf actors labels May 17, 2025

Roasbeef mentioned this pull request Jul 11, 2025

[epic]: master tracking issue for prod taproot channels spec PRs #10069

Open

6 tasks

Roasbeef added the taproot label Sep 10, 2025

Roasbeef force-pushed the actor branch from 9319e4c to 88328f1 Compare September 17, 2025 00:40

Roasbeef force-pushed the actor branch 2 times, most recently from bca5ca0 to dc5d57f Compare October 31, 2025 22:58

Roasbeef force-pushed the actor branch 2 times, most recently from 3134546 to 8781daa Compare February 7, 2026 03:27

saubyk assigned Roasbeef Feb 23, 2026

saubyk requested review from gijswijs and yyforyongyu February 24, 2026 17:55

erickcestari reviewed Mar 17, 2026

View reviewed changes

Roasbeef changed the base branch from actor to master March 18, 2026 01:44

Roasbeef added 5 commits March 17, 2026 18:46

build: add temp replace to point to new actor package

98db091

lnwallet/chancloser: create unique ServiceKey for the RBF chan closer

d1736eb

This can be used to allow any system to send a message to the RBF chan closer if it knows the proper service key. In the future, we can use this to redo the msgmux.Router in terms of the new actor abstractions.

Roasbeef force-pushed the coop-close-actor branch from 2be2d3b to d1736eb Compare March 18, 2026 01:57

erickcestari requested changes Mar 18, 2026

View reviewed changes

saubyk added this to the v0.21.0 milestone Mar 24, 2026

saubyk added this to v0.21 Mar 24, 2026

saubyk moved this to In review in v0.21 Mar 24, 2026

yyforyongyu requested changes Mar 31, 2026

View reviewed changes

	func NewRbfCloserServiceKey(op wire.OutPoint) RbfCloseActorServiceKey {
	func NewRbfCloserPeerServiceKey(op wire.OutPoint) RbfCloseActorServiceKey {


		type retType = *CoopCloseUpdates

		// If RBF coop close isn't permitted, then we'll an error.

	// NewRbfCloserPeerServiceKey returns a new service key that can be used to
	// NewRbfCloserServiceKey returns a new service key that can be used to

	// we have a unique type here ChanCloserActorMsg which will handle the
	// we have a unique type here rbfCloseMessage which will handle the


	// Ctx is the caller's context (e.g., the RPC stream context), used to detect client disconnection.
	Ctx context.Context

Conversation

Roasbeef commented May 17, 2025

Uh oh!

coderabbitai bot commented May 17, 2025

Review skipped

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

github-actions bot commented May 17, 2025

Pull reviewers stats

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

Roasbeef commented Mar 18, 2026

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lightninglabs-deploy commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CodeRabbit Configuration File (`.coderabbit.yaml`)