Skip to content

staticaddr: reconcile deposits after wallet indexing #1067

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
hieblmi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

staticaddr: reconcile deposits after wallet indexing #1067

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions staticaddr/deposit/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ func (m *Manager) Run(ctx context.Context, initChan chan struct{}) error {

// Reconcile new deposits that might have just gotten
// confirmed.
// Before reconciling our in-mem view of deposits with
// lnd we give the wallet extra time to index
// transactions of interest in the new block. This
// mitigates a race between wallet indexing and
// reconciling deposits.
time.Sleep(500 * time.Millisecond)
Copy link

@gemini-code-assist gemini-code-assist bot Jan 15, 2026

Choose a reason for hiding this comment

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

medium

Using a hardcoded 'magic number' like 500 for the sleep duration can make the code harder to understand and maintain. It's better to define this value as a named constant at the package level (e.g., walletIndexDelay = 500 * time.Millisecond). This improves readability by giving the value a clear meaning and makes it easier to adjust in the future.

Additionally, relying on a fixed sleep duration to resolve a race condition is inherently fragile, as the required delay might vary depending on system load. Have you investigated if lnd provides a more deterministic signal for when wallet indexing is complete for a new block? If not, this workaround is understandable, but its potential flakiness is worth noting.

Copy link
Collaborator

@starius starius Jan 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

+1

Can we wait for some observable event instead of sleeping?

IIUC, the root cause is that the output of ListUnspent WalletKit API can be outdated and reconcileDeposits uses ListUnspent to detect deposits. There is no way to detect this condition from ListUnspentResponse.

Maybe we should fix this in LND, adding the current height and block hash to ListUnspentResponse? Then we can retry ListUnspent until we get up-to-date results.

We are notified about new block in ChainNotifier component of LND, but track deposits using its WalletKit component which is also a root cause of the mismatch. Future idea: can ChainNotifier have some API to track deposits? Something similar to RegisterSpendNtfn but where you provide a pkScript and it notifies you when funds arrive to that pkScript. We can use RegisterConfirmationsNtfn without specifying txid to track any deposit to specific pkScript (e.g. static address in Loop). Limitation: repeated deposits to the same script won't trigger additional notifications because the notifier caches the first confirmation and treats subsequent reuse as duplicate. I remember this limitation was addressed somewhere, but I can't find where.

Copy link
Collaborator Author

@hieblmi hieblmi Jan 16, 2026

Choose a reason for hiding this comment

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

+1

Can we wait for some observable event instead of sleeping?

IIUC, the root cause is that the output of ListUnspent WalletKit API can be outdated and reconcileDeposits uses ListUnspent to detect deposits. There is no way to detect this condition from ListUnspentResponse.

Yes!

Maybe we should fix this in LND, adding the current height and block hash to ListUnspentResponse? Then we can retry ListUnspent until we get up-to-date results.

The wallet controller has a method IsSynced which compares the wallet tip to the chain backend. I think we can use the result in the ListUnspent call to see if indexing is finished.

We are notified about new block in ChainNotifier component of LND, but track deposits using its WalletKit component which is also a root cause of the mismatch. Future idea: can ChainNotifier have some API to track deposits? Something similar to RegisterSpendNtfn but where you provide a pkScript and it notifies you when funds arrive to that pkScript. We can use RegisterConfirmationsNtfn without specifying txid to track any deposit to specific pkScript (e.g. static address in Loop). Limitation: repeated deposits to the same script won't trigger additional notifications because the notifier caches the first confirmation and treats subsequent reuse as duplicate. I remember this limitation was addressed somewhere, but I can't find where.

We use ImportTaprootScript to add the pkscript to the wallet, then poll ListUnspent for changes. This hacks around the RegisterConfirmationsNtfn limitation.

if err = m.reconcileDeposits(ctx); err != nil {
log.Errorf("unable to reconcile deposits: %v",
err)
Expand Down
Loading