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.
Tested scenarios:
1. For a regular sweep newly added it waits for initialDelay + publishDelay
before publishing a transaction.
2. For a sweep recovered from DB it does not wait before publishing tx.
3. If a sweep is about to expire, initialDelay 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.
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.
This is needed to cover the code of SQLStore with tests.
To achieve compatibility with loopdb (SQLite), the following changes were done:
- DestAddr is filled to avoid crash in SQL layer
- Preimage is filled to avoid uniqueness checks by the DB
- the code working with batch IDs was changed to work correctly
when batch_id starts with 1 instead of 0
- SQL swap store has to have a swap with swap_hash from sweep
to satisfy foreign key constraint of the DB
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.
Changed argument of function NewBatcher from LoopOutFetcher to SweepFetcher
(returning new public type SweepInfo).
This change is backwards-incompatible on the package layer, but nobody seems
to use the package outside of Loop.
To use NewBatcher inside Loop, turn loopdb into SweepFetcher using
function NewSweepFetcherFromSwapStore.
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.)
Method sweepbatcher.Store.FetchBatchSweeps (implementation using real DB) runs
JOIN query to load LoopOut from swaps table. Now the mock does the same.
It is needed to test store and load scenarios in tests.
If the sweep was successfully updated in the batch, no need to
try to add it to all other batches.
Added a test reproducing adding a sweep to both batches without this change.
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.
