Skip to content

sweepbatcher: fixes for presigned mode #952

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

starius
Copy link
Collaborator

@starius starius commented Jun 12, 2025

Fixed TODO left from #891 : re-add sweeps to new batches only after fully confirmed.
In case of a reorg sweeps should not go to another batch but stay in the current batch until it is fully confirmed. Only after that the remaining sweeps are re-added to another batch. Field sweep.completed is now set to true only for fully-confirmed sweeps.

Fixed OnChainFeePortion values. There were two mistakes. In case of a swap with multiple sweeps only the fee of the first sweep of a swap was accounted. Another issue is that 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.

Pull Request Checklist

  • Update release_notes.md if your PR contains major features, breaking changes or bugfixes

starius added 14 commits June 12, 2025 15:48
This is needed to have multiple spending registrations running and to send
a notification to a specific spending registration.
In case of a reorg sweeps should not go to another batch but stay in the current
batch until it is fully confirmed. Only after that the remaining sweeps are
re-added to another batch.

Field sweep.completed is now set to true only for fully-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.
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.
This is needed because sweepbatcher can use this channel in multiple
select statements to unblock itself if the caller cancels.
It doesn't need loopdb, so remove that code.
Make sure that broadcasted tx has feeRate >= minRelayFee.
Make sure that feeRate of broadcasted tx doesn't decrease.
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.
For presigned possible remaining groups, the destination address of the current
batch was used instead of the destination address of an expected future batch.

TODO: reproduce in unit test "purged". For this, each swap should have a
separate destination address.
@starius starius marked this pull request as ready for review June 12, 2025 19:15
@starius starius requested review from bhandras, sputn1ck and hieblmi June 13, 2025 03:17
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant