CI started panicking in TestRbfChannelFlushingTransitions/early_offer /
TestRbfCloseClosingNegotiationRemote (see GitHub Actions run
https://github.com/lightningnetwork/lnd/actions/runs/19155841408/job/54756127218?pr=10352)
because the cached remote offer could fire before the test harness
registered its mock CloseSigner expectations. When that happened,
the mock complained that CreateCloseProposal was unexpected:
panic:
assert: mock: I don't know what to return because the method call was unexpected.
Fix this by having assertSingleRemoteRbfIteration take a sendEvent callback
that receives the context and initial offer, so tests can install expectations
first and then fire the event via SendEvent (or the early-offer test's custom
flush sender).
Reproduction (on master)
------------------------
1. Modify lnwallet/chancloser/rbf_coop_test.go
Add time.Sleep(10 * time.Millisecond) before the first call of
closeHarness.assertSingleRemoteRbfIteration (in function
TestRbfChannelFlushingTransitions).
2. go test ./lnwallet/chancloser -run TestRbfChannelFlushingTransitions/early_offer
3. The panic reproduces immediately.
We now execute the aux chan closer finalization within the chain
watcher. This is better as we don't need to rely on the remote party
being online and sending us a message. Instead we do the finalization
once the on-chain transaction has been confirmed.
The aux close types will soon be used by a different package that would
otherwise cause an import cycle if used directly from
lnwallet/chancloser. We now create a new sub-package lnwallet/types that
will be improrted from all users of these types.
In this commit, we replace all instances of spew.Sdump in the lnwallet
package and its subpackages with lnutils.SpewLogClosure. This change
ensures that expensive debug dump operations are only performed when the
log level actually requires them.
Use the new feature of Go 1.24, fix linter warnings.
This change was produced by:
- running golangci-lint run --fix
- sed 's/context.Background/t.Context/' -i `git grep -l context.Background | grep test.go`
- manually fixing broken tests
- itest, lntest: use ht.Context() where ht or hn is available
- in HarnessNode.Stop() we keep using context.Background(), because it is
called from a cleanup handler in which t.Context() is canceled already.
Fixes another flake in the unit-race test: Sometimes we miss startup
events if there's a high CPU load (in CI for example).
To avoid that, we register our subscriber before starting the state
machine.
These tests previously only worked because we didn't properly wait for
all transitions to come in. We fix them by correctly asserting the
expected number of state transitions.
This fixes the actual flake: We didn't wait for next iterations to _not_
come in, so depending on timing, we could read another transition in a
second run.
But making sure we expect all transitions at the same time and then make
sure no further transitions come in makes this not depend on time.
The param previously was unused so this was likely an oversight or
copy/paste error.
This does not change any behavior, as the method was always called with
the constant previously used. But it makes things more explicit.
In this commit, we fix a flake in the
`TestRbfCloseClosingNegotiationLocal/send_offer_rbf_wrong_local_script`
test.
This flake can happen if the test shuts down _before_ the state machine
is actually able to process the sent event. In this case, the
expectations are triggered, and we find that the error isn't sent.
To resolve this, we create a new wrapper function that'll use a sync
channel send to assert that the error has been sent before we exit the
test.
In this commit, we fix a flake in the rbf loop sub-test for the
TestRbfCloseClosingNegotiationLocal test case.
The fix here is that when we go from ClosePending for an RBF iteration
loop, we first transition to LocalCloseStart. However we only do this
extra transition if we're doing an iteration (starting from ClosePending).
To fix this, we add a new bool that tracks if this is an iteration or
not. We can then also eliminate the extra assertion at the end, as we'll
terminate in `ClosePending` which is checked by
`assertLocalClosePending()` in `assertSingleRbfIteration`.
Fixes https://github.com/lightningnetwork/lnd/issues/9526.
In this commit, we fix a flake in the
`TestRbfChannelFlushingTransitions/early_offer` test. The fix is simple:
this is actually an "iteration", as we have a self transition to the
ChannelNegotiation state first. We also don't need to send the
remoteOffer, so we set `sendInit` to false. The offer still needs to be
passed in to ensure that the assertions work however.
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 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 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 enable a custom payer for the rbf coop close. This
allows us to ensure that the party that started one side of the close
flow pays the fees.
In this commit, we add the state transitions for the new protofsm based
RBF chan closer. The underlying protocol is a new asymmetric co-op close
process, wherein either side can initiate a chan closer, and use their
settled funds to pay for fees within the channel.
In this commit, we add the initial set of states for the new protofsm
based rbf chan closer. A diagram outlining the new states and their
transitions can be found here:
https://gist.github.com/Roasbeef/acc4ff51b9dff127230228a05553cdfe.
Unlike the existing co-op close process, this co-op close can be
restarted at anytime if either side sends a shutdown message. From
there, we'll each obtain a new RBF'd version that can be re-broadcasted.
This commit creates the set of states, along with the environment that
our state machine will use to drive itself forward.
Find and replace all nolint instances refering to the `lll` linter and
replace with `ll` which is the name of our custom version of the `lll`
linter which can be used to ignore log lines during linting.
The next commit will do the configuration of the custom linter and
disable the default one.
In this commit, we move to add the internal key to the delivery addr. This way, we give the aux chan closer the extra information it may need to properly augment the normal co-op close process.
With this commit, the channel is now aware of if it's a musig2 channel, that also has a tapscript root. We'll need to always pass in the tapscript root each time we: make the funding output, sign a new state, and also verify a new state.
András Bánki-Horváth