Repository navigation
Conversation
A block height that the deposit manager could not deliver to the deposit FSMs, because lnd's wallet tip was changing or unsynced or the wallet query failed, is currently dropped until the next block. Add the interval in which such a height will be delivered again, configurable per manager so tests can shorten it. It is not used yet.
A deposit whose expiry block arrives while lnd's wallet tip is changing or unsynced, or while the wallet query fails, must expire once lnd recovers, without waiting for another block. If no further block arrives, the expiry sweep is never published. The static address deposit expiry integration test hits this when the wallet tip is still moving for the last blocks it mines. The test fails until deferred notifications are retried.
The deposit manager skips delivering a block height to the deposit FSMs when reconciliation fails or lnd's wallet tip is changing or unsynced, so that deposits don't make expiry decisions from stale confirmation data. The skipped height was only delivered with the next block, which delays an expiry sweep by a block interval, or forever if no block follows. The periodic deposit poll reconciles, but never notifies. Retry the reconciliation and notification of the latest block height every ExpiryRetryInterval until lnd provides a stable wallet view. The retry runs in the manager's event loop, so notifications stay serialized. Failed retries are logged at debug level to avoid flooding the log while lnd is unavailable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since #1219, the deposit manager doesn't deliver a block height to the deposit FSMs when reconciliation fails or lnd's wallet tip is changing or unsynced, so deposits don't make expiry decisions from stale confirmation data. The skipped height was only delivered with the next block. The periodic deposit poll reconciles but never notifies the deposits. So an expiry sweep was delayed by a block interval, or forever if no block followed. This was raised as a non-blocking review comment on #1219.
A test failed because of that: when the wallet tip is still moving for the last blocks the test mines, the second set of deposits never expires and the test times out with "deposit expiries were not swept".
This PR retries the reconciliation and notification of the latest block height every second (
ExpiryRetryInterval) until lnd provides a stable wallet view. The retry runs in the manager's event loop, so notifications stay serialized, and it stops once a height is delivered. A newer block supersedes the pending height and rearms the retry if reconciliation still fails. Failed retries are logged at debug level so a persistent wallet failure doesn't flood the log.TestManagerRetriesDeferredExpiryNotificationcovers an unsynced wallet tip at startup, and an unsynced tip or a failing wallet query at a later block after a healthy startup. In each case nothing is signed while lnd is unhealthy, and the expiry sweep is signed and published once lnd recovers, without another block. The test fails without the fix. The existing tests that check no expiry happens while lnd is catching up or the wallet query fails still pass, because retries never act until the wallet view is stable.Leaving this in draft state for now, since it may conflict with more fundamental deposits rework.
Pull Request Checklist
docs/release-notes/release-notes-next.md, or apply theno-changeloglabel (required by CI)