Skip to content
Open
Changes from all commits
Commits
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
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)

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

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

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