Previously, we passed the current best block height as the height hint to
RegisterConfirmationsNtfn. If Loop was shut down when a sweep was confirmed
and restarted later, it would use the current best height as the hint. This
caused the confirmation to be missed, since it had already occurred before
that height.
This commit fixes the issue by passing the initiation height of the swap as the
height hint instead. This is consistent with what we do in other places where
we use RegisterConfirmationsNtfn.
Since LND caches previously passed height hints for RegisterConfirmationsNtfn
and uses maximum passed value, to actually recover in such a condition, one has
to restart LND passing the option --height-hint-cache-query-disable and then
restart Loop applying this fix. If a pruned bitcoind backend is used, this might
not work.
Option WithInitialDelay now accepts a function returning initialDelay
depending on sweep's data.
This is needed to be able to wait longer for sweeps with low priority, but
still sweeping high priority sweeps soon.
SwapHash used to be a key in the sweeps table. Now the key is outpoint which
replaces columns outpoint_txid and outpoint_index. In-memory structures and unit
tests were also updated to use outpoint as key. Outpoint is truly unique.
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.
Previously, if a completed batch was visited after a batch to which the
sweep was added, it was not deleted because the function returned early.
This has been separated into two loops: the first one removes completed batches,
and the second one adds the sweep to a batch.
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.
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.
WithPublishDelay sets the delay of batch publishing that is applied in the
beginning, after the appearance of a new block in the network or after the
end of initial delay (see WithInitialDelay). It is needed to prevent
unnecessary transaction publishments when a spend is detected on that block.
Default value depends on the network: 5 seconds in mainnet, 0.5s in testnet.
For batches recovered from DB this value is always 0s.
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.
Renamed defaultPublishDelay to defaultTestnetPublishDelay. Its godoc was already
for defaultTestnetPublishDelay, but the const was named defaultPublishDelay.
According to AddTapscriptInput's godoc, the first argument (leafWitnessSize)
must include not the whole size of witness, but only the size of data consumed
by the revealed script. Previously the value was too high, because it also
included the following extra elements: number_of_witness_elements (1 byte),
witness script and its size, control block and its size.
Tests for greedy_batch_selection were affected by this change.
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.
Previously, when handling a sweep we assumed that if a sweep status
was completed, the parent batch was also finished. However, since the
batch confirmation status depends on three on-chain confirmations, it
is possible that a spend notifier was started for a sweep of an active
batch. The notifier would fetch the parent batch from the database, but
because we incorrectly assumed that the parent was confirmed (when it
was not), the DB call would fail with a 'no rows returned' error.
This failure would cause the sweep to fail and the sweep batcher to
stop, resulting in a permanent failure state.
