Fixes a crash window where handleConf updated the batch row to confirmed but
failed before marking sweeps complete, so re-added sweeps spawned a duplicate
batch or kept retrying. Batch confirmation and sweep completion are now
persisted inside a single DB transaction ConfirmBatchWithSweeps, and handleConf
uses the helper to atomically store the batch and the set of confirmed sweeps.
Added TestSweepBatcherConfirmedBatchIncompleteSweeps that runs against the real
loopdb backend, injects a failure mid-transaction, and verifies the database
never ends up with confirmed=true batches paired with completed=false sweeps.
Presigned sweeps that produce a change output misreported the on-chain fee.
The fee portion was derived from the total swept amount minus only the
first transaction output, so any change output was treated as additional fee.
Update getFeePortionForSweep to subtract the value of every tx output so
the fee portion reflects only the actual miner fee paid. Add regression
coverage that sweeps a presigned input with change and asserts the spend
and confirmation notifications report the corrected fee.
- ensurePresigned: use passed minRelayFeeRate instead of chainfee.FeePerKwFloor
- presign: use minRelayFeeRate for start and minRelayFee
- presign: make sure minRelayFeeRate is set
- add tests for presign to test this new behavior; make sure the number of
transactions is lower if minRelayFeeRate is higher
- update error message in constructUnsignedTx: use <, not <= (more accurate)
- use utils.DustLimitForPkScript instead of lnwallet.DustLimitForSize in tests
- in tests adjust amounts to edge values, add controls
Presigning sweeps takes change outputs into account.
Each primary deposit id of a sweep group points to
an optional change output.
sweepbatcher.presign scans all passed sweeps for
change outputs and passes them to constructUnsignedTx.
Optional change of a swap is encoded in its sweeps
as a pointer to the same change output. This change
is taken into account when constructing the unsigned
batch transaction when it comes to tx weight and
outputs.
AddSweep may not be called after getting the first confirmation, but feerate
updates are still needed in case of reorg.
Update test TestFeeRateGrows not to call AddSweep again and make sure feerate
is updated itself.
The reorg channel is now passed to RegisterSpendNtfn, and waiting for spend
notifications remains active even after the transaction receives its first
confirmation. The dedicated goroutine previously used to wait for the spend
is no longer needed, as we now handle both spend and potential reorg events
in the main event loop while the batch is running.
Following this change, RegisterConfirmationsNtfn runs without a reorg channel,
as it would only detect deep reorgs that undo the final confirmation - something
we can't handle anyway. We can't track fully confirmed swaps indefinitely to
guard against such rare reorgs; instead, we can mitigate the risk by increasing
the required confirmation depth.
Method Presign is not as reliable as SignTx, because it checks transaction by
txid and can miss for example if LockTime is different. SignTx can do everything
Presign was used for.
There were two mistakes.
In case of a swap with multiple sweeps only the fee of the first sweep
of a swap was accounted.
Rounding diff (the remainder) was attributed to all the sweeps rather than to
the first (primary) sweep of the batch. The sweep to attribute the remainder
was chosen by comparing SignatureScript which is always empty. New approach is
to find the primary sweep and to compare its outpoint directly.
In case of a reorg sweeps should not go to another batch but stay in the current
batch until it is reorg-safely confirmed. Only after that the remaining sweeps
are re-added to another batch.
Field sweep.completed is now set to true only for reorg-safely confirmed sweeps.
In handleConf we now use batch.persist() (i.e. store.UpdateSweepBatch) instead
of ConfirmBatch, because we set not only Confirmed flag, but also batchTxid.
Added option WithSkippedTxns, which has one historical problematic tx by default.
Sweeps originating from these transactions are omitted when reading from DB.
loopdb: add column sweep_batches.cancelled and replaced DropBatch with
CancelBatch. It is needed, because sweep.batch_id is a foreign key to batch.
Changed StoreMock.InsertSweepBatch not to reuse batch_id. This is needed by
the test, which checks that new batch has fresh ID.
Add fields ConfChan and ConfErrChan to SpendNotifier type which is a part of
SweepRequest passed to AddSweep method.
This is needed to reuse confirmation notifications on the calling side the same
way it is done for spending notifications.
Previously, dbBatch had a State field (enum: Open, Closed, Confirmed), but in
the database it is represented as a boolean Confirmed. The Closed state was
stored the same way as Open. This wasn't an issue in practice, since an Open
batch is quickly transitioned to Closed after startup.
However, the in-memory mock stores plain dbBatch instances, leading to
inconsistent behavior between the mock and the real DB-backed store. This
commit updates dbBatch to match the database representation by replacing
the State field with a Confirmed boolean.
In this mode sweepbatcher uses transactions provided by the Presigned helper.
Transactions are signed upon adding an input to a batch.
A single Batcher instance can handle both presigned and regular batches.
Currently presigned and non-presigned sweeps never appear in the same batch.
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.
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).
