staticaddr: surface mempool deposits immediately#1104
staticaddr: surface mempool deposits immediately#1104hieblmi wants to merge 8 commits intolightninglabs:masterfrom
Conversation
Replace the blocking RegisterConfirmationsNtfn-based getBlockHeight with a lightweight confirmationHeightForUtxo that derives the first confirmation height from the wallet UTXO confirmation count and the currently tracked block height. This removes the MinConfs gating from reconcileDeposits so that mempool deposits are detected immediately with ConfirmationHeight=0. Add updateDepositConfirmations to backfill confirmation heights once previously-unconfirmed deposits get mined. The manager now stores the current block height via an atomic and reconciles deposits on every new block.
Unconfirmed deposits (ConfirmationHeight <= 0) cannot have started their CSV timer, so IsExpired now returns false for them. Update the Deposited state and OnStart event documentation to reflect that deposits are detected from the mempool rather than after reaching a confirmation threshold.
Remove the MinConfs-based bifurcation from ListUnspentDeposits. All wallet UTXOs are now checked against the deposit store: known deposits in the Deposited state are available, unknown outpoints are new deposits and also available, and all other known states are filtered out. The mock deposit store now returns ErrDepositNotFound for missing outpoints to match the real store behavior, and tests are updated to verify availability by deposit state rather than confirmation depth.
Extract depositBlocksUntilExpiry helper that returns 0 for unconfirmed deposits (ConfirmationHeight <= 0) since their CSV has not started. Use it in ListStaticAddressSwaps and populateBlocksUntilExpiry. In GetStaticAddressSummary, derive the unconfirmed value from deposits in the Deposited state with no confirmation height instead of issuing a separate wallet ListUnspent call.
Unconfirmed deposits (ConfirmationHeight == 0) are considered swappable because their CSV timeout has not started yet. Extract blocksUntilDepositExpiry helper that returns MaxUint32 for unconfirmed deposits and use it in SelectDeposits sorting and IsSwappable.
Now that Deposited includes mempool outputs, channel opens and withdrawals must explicitly reject unconfirmed deposits (ConfirmationHeight <= 0) since both operations require confirmed inputs.
Update the loop-in CLI error message to no longer reference a minimum confirmation count now that mempool deposits are surfaced immediately. Remove the unused MinConfs constant from the deposit package.
Unconfirmed deposits still have the full CSV window ahead of them, so depositBlocksUntilExpiry should return the full expiry value rather than 0, which would misleadingly suggest the deposit is about to expire.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the user experience for static-address deposits by making them available for certain operations, specifically loop-ins, as soon as they are detected in the mempool. This change reduces the waiting time for users by removing the previous requirement for a minimum number of confirmations before a deposit could be utilized. The underlying logic has been updated to correctly track and manage the state and expiry of these newly visible unconfirmed deposits, while ensuring that more sensitive operations like withdrawals and channel opens still adhere to confirmation requirements. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the static address deposit management by decoupling the 'Deposited' state from a minimum confirmation count. Deposits are now considered 'Deposited' as soon as they are detected, even if unconfirmed, and their actual confirmation height is tracked. This change impacts how unspent deposits are listed, how expiry is calculated (introducing depositBlocksUntilExpiry and confirmationHeightForUtxo helpers), and how IsExpired and IsSwappable functions behave for unconfirmed deposits. While unconfirmed deposits are now available for loop-ins, explicit checks have been added to openchannel and withdraw managers to ensure only confirmed deposits are used for these operations. The deposit.Manager now updates currentHeight and reconciles deposits on every new block, making the system more responsive. A review comment suggests that the pollDeposits function might be redundant given that reconcileDeposits is now called on every new block, proposing its removal for efficiency and logic simplification.
| case height := <-newBlockChan: | ||
| m.currentHeight.Store(uint32(height)) | ||
|
|
||
| err := m.reconcileDeposits(ctx) |
There was a problem hiding this comment.
Now that reconcileDeposits is called on every new block, the periodic polling started by pollDeposits seems redundant. Reconciling on new blocks is more efficient and responsive than polling on a fixed interval.
Consider removing the pollDeposits function and its call on line 127 to avoid unnecessary work and simplify the logic.
1 participant
This PR updates static-address deposit handling so deposits become visible and
usable as soon as they are detected in the wallet, instead of waiting for the
historical confirmed-only threshold.
What changed
Depositedimmediately on detectionConfirmationHeight=0started
deposits