The code inside require.Eventually runs in parallel with the event loops
of the batcher and its batches. Accessing fields of the batcher and batches
must be done within an event loop.
To address this, testRunInEventLoop methods were added to the Batcher and batch
types. Unit tests were then rewritten to use this approach when accessing
batcher and batch fields.
Additionally, in many cases, receive operations from RegisterSpendChannel
were moved before require.Eventually. This prevents testRunInEventLoop from
getting stuck in an event loop while blocked on a RegisterSpendChannel send
operation.
Mixed batches approach covers the case where all the inputs are cooperative
(function publishBatchCoop) and it is better than fully non-cooperative case
(function publishBatch), but it can also create such transaction if needed, i.e.
if all the sweeps are non-cooperative. So we can remove functions publishBatch,
publishBatchCoop, option WithMixedBatch and associated code in greedy batch
selection algorithm.
Prevent a crash with "a height hint greater than 0 must be provided" error when
monitorSpend starts at the beginning of batch.Run.
The timer timerChan is now initialized at the start, because it was previously
initialized after the first block (the current tip) was read from blockChan and
now the first block is read before the main for-select loop to fill the field
currentHeight in advance.
Previous behaviour was to overwrite batch's feerate with minFeeRate of
its primary sweep, which could be lower that previus batch's feerate or
lower that feerate of some other sweep.
Instead, batch's feerate only grows and never declines and is at least as high
as the highest feerate of its sweeps.
Added a test to verify this.
This is needed to avoid non-standard batch transactions (larger than 400k wu).
A non-cooperative input is 393 wu, so 1000 inputs are still under 400k wu.
Option WithMixedBatch instructs sweepbatcher to create mixed batches with regard
to cooperativeness. Such a batch can include both sweeps signed both
cooperatively and non-cooperatively. If cooperative signing fails for a sweep,
transaction is updated to sign that sweep non-cooperatively and another round of
cooperative signing runs on the remaining sweeps. The remaining sweeps are
signed in non-cooperative (more expensive) way. If the whole procedure fails for
whatever reason, the batch is signed non-cooperatively (the fallback).
Previously sweepbatcher used loop/labels.LoopOutBatchSweepSuccess to assign
transactions labels. The value is "BatchOutSweepSuccess -- $batch_id", which
does not fit use cases outside of loop-out. Now there is option WithTxLabeler
which sets a function used to generate the label.
WithInitialDelay instructs sweepbatcher to wait for the duration provided
after new batch creation before it is first published. This facilitates
better grouping. It only affects newly created batches, not batches loaded from
DB, so publishing does happen in case of a daemon restart (especially important
in case of a crashloop). Defaults to 0s. If a sweep is about to expire (time
until timeout is less that 2x initialDelay), then waiting is skipped.
Use customFeeRate if it is provided, otherwise use wallet's EstimateFeeRate.
Added flag SkipNextBump to rbfCache to avoid extra bumping upon updating
fee rate externally.
Fix test TestSweepBatcherCloseDuringAdding, it didn't have confTarget and
failed in wallet.EstimateFeeRate call.
Use SigHashDefault instead of SigHashAll in weight estimations.
Actual code uses SigHashDefault, not SigHashAll. SigHashAll is 1wu larger.
Use the actual destination address in weight estimator, not taproot always.
In the test the type of address is P2WPKH, not taproot.
Updated testSweepFetcher to calculate fee, fee rate and weight accurately and
to compare the data observed in the published transaction with the estimates.
If custom fee rates are used, the algorithm is tried. It selects a batch for the
sweep using the greedy algorithm, which minimizes costs, and adds the sweep to
the batch. If it fails, old algorithm is used, trying to add to any batch, or
starting a new batch.
The reason is because we want to know in advance if fee rates come from
an external source or are determined by sweepbatcher internally.
Also remove WithNoBumping option. It is now a part of WithCustomFeeRate:
if WithCustomFeeRate is passed, automatic fee bumping by sweepbatcher
is disabled.
MinFeeRate is minimum fee rate that must be used by a batch of
the sweep. If it is specified, confTarget is ignored.
This is useful for external source of fees.
The race was detected in CI and locally when running with -race.
It happened between the following calls:
WARNING: DATA RACE
Write at 0x00c0003e6638 by goroutine 1374:
runtime.racewrite()
<autogenerated>:1 +0x1e
github.com/lightninglabs/loop/sweepbatcher.(*batch).Wait()
sweepbatcher/sweep_batch.go:463 +0x6e
github.com/lightninglabs/loop/sweepbatcher.(*Batcher).Run.func1()
sweepbatcher/sweep_batcher.go:272 +0x10e
Previous read at 0x00c0003e6638 by goroutine 1388:
runtime.raceread()
<autogenerated>:1 +0x1e
github.com/lightninglabs/loop/sweepbatcher.(*batch).monitorConfirmations()
sweepbatcher/sweep_batch.go:1144 +0x285
github.com/lightninglabs/loop/sweepbatcher.(*batch).handleSpend()
sweepbatcher/sweep_batch.go:1309 +0x10e4
github.com/lightninglabs/loop/sweepbatcher.(*batch).Run()
sweepbatcher/sweep_batch.go:526 +0xb04
github.com/lightninglabs/loop/sweepbatcher.(*Batcher).spinUpBatch.func1()
sweepbatcher/sweep_batcher.go:455 +0xbd
The race was caused because wg.Add(1) and wg.Wait() were running from different
goroutines (one goroutine was running batch.Run() and another - batcher.Run()).
To avoid this scenario, wg.Wait() call was moved into batch.Run() call, so it
waits itself for its children goroutines, after which the channel b.finished
is closed, and it serves a signal for external waiters (the batcher, calling
batch.Wait()).
Also the channel batch.stopped was renamed to batch.stopping to better reflect
its nature.
Added TestSweepBatcherCloseDuringAdding to make sure adding a sweep during
shutting down does not cause a crash. The test did not catch the original
race condition.
It used to be set to default (defaultBatchConfTarget = 12) which
could in theory affect fee rate if updateRbfRate() and publish()
were not called before the batch was saved. (Unlikely scenario.)
Previously storing an empty batch would make the batcher fail to start
as spinning up a restored batch assumes that there's a primary sweep
added already. As there's no point in spinning up such batch we can just
skip over it.
Furthermore we'll ensure that we won't try to ever publish an empty
batch to avoid setting the fee rate too early.
Previously we'd report the fees per sweep as the total sweep cost of a
batch. With this change the reported cost will be the proportional fee
which should be equal for all sweeps except if there's any rounding
difference in which case that is paid by the sweep belonging to the
first input of the batch tx.
