Commit message · Olaoluwa Osuntokunpeer+lnwallet/chancloser: advance the legacy closer from one goroutine
In this commit, we give the legacy ChanCloser a single owner, rather than
letting two goroutines advance it. The peer's channelManager drives the state
machine for the Shutdown and ClosingSigned messages that come off the wire, and
for local close requests. The link drives it as well: while we wait for the
channel to drain we register a flush hook, and the link invokes that hook from
its own goroutine, where it called BeginNegotiation directly. Nothing kept the
two apart, so the state field, the priorFeeOffers map, and the signing step
could all be touched at once. Under `go test -race` this shows up as a data race
on the state field.
Rather than reach for a lock, we route the flush through the channelManager. The
hook now only reports the channel ID over a new chanCloseFlushed channel, and
handleChanFlushed picks it up next to the close messages. Every transition, the
cached offer processing, the fee map, and the signing then happen on the one
goroutine, so the closer needs no synchronization of its own. We spell that out
on the type, since it's an invariant a new caller can break from the outside.
The report goes out from a fresh goroutine, which matters more than it looks.
The link may well be holding its own lock while it invokes the hook, and
channelManager reaches for that same lock in DisableAdds, so blocking on the
handoff would trade the race for a deadlock. The `go` in front of RemoveLink
just above it is there for the same reason.
We look the closer up with a plain map load rather than through
fetchActiveChanCloser, as that one builds a fresh closer when it doesn't find
an existing one, and a flush that lands after the negotiation was torn down has
no business starting a new negotiation.
One behavior change falls out of the move: the flush path now runs the same
finalization tail as the message path. It skipped that before, so a responder
that drained a cached offer would reach closeFinished and broadcast, but nothing
ran finalizeChanClosure until the next close message showed up, and having
already sent its final signature, there may not be one. The link == nil path
already ran the tail, so this makes all three paths agree.
The new test drives a close with a link that hands us the flush hook instead of
running it inline, so we can check that negotiation waits on the report, and
that a report for a channel we have no closer for is dropped.
88/100 · StrongMessage clarity
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification✓ Names security-relevant behavior explicitly
signing boundarysigning or wallet path
AI analysis · Moderate 60/100This commit fixes a concurrency bug in how LND negotiates cooperative channel closures. Previously, two different goroutines could update the same channel-closing state machine at the same time, which could corrupt internal data or cause a crash. The fix routes all updates through a single goroutine and adds a test to confirm the behavior.
