Skip to content

Conversation

@starius
Copy link
Collaborator

@starius starius commented Apr 5, 2025

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.

Pull Request Checklist

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

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.
@starius starius changed the title [WIP] sweepbatcher: fix height hint for confirmations sweepbatcher: fix height hint for confirmations Apr 5, 2025
@starius starius marked this pull request as ready for review April 5, 2025 21:32
@starius starius requested review from bhandras, hieblmi and sputn1ck April 5, 2025 21:32
Copy link
Collaborator

@hieblmi hieblmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! LGTM.

Copy link
Member

@bhandras bhandras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!! LGTM 🎉

@starius starius merged commit 84820f2 into lightninglabs:master Apr 6, 2025
4 checks passed
@starius starius deleted the register-conf-initiation-height branch April 6, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants