staticaddr: surface mempool deposits immediately#1104
staticaddr: surface mempool deposits immediately#1104hieblmi wants to merge 10 commits intolightninglabs:masterfrom
Conversation
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.
hieblmi
force-pushed
the
dyn-conf-tracker
branch
2 times, most recently
from
43803c4 to
77cfd21
Compare
hieblmi
force-pushed
the
dyn-conf-tracker
branch
3 times, most recently
from
38c90ee to
f61d02f
Compare
hieblmi
force-pushed
the
dyn-conf-tracker
branch
3 times, most recently
from
b4fe625 to
5bba57d
Compare
hieblmi
force-pushed
the
dyn-conf-tracker
branch
from
5bba57d to
88fbde3
Compare
hieblmi
force-pushed
the
dyn-conf-tracker
branch
2 times, most recently
from
14d52be to
6babc2d
Compare
hieblmi
force-pushed
the
dyn-conf-tracker
branch
11 times, most recently
from
6372ed8 to
09ecf87
Compare
hieblmi
force-pushed
the
dyn-conf-tracker
branch
from
09ecf87 to
be1b86f
Compare
Surface static-address deposits as soon as they appear in the wallet instead of waiting for the old six-confirmation readiness threshold. Reconcile the wallet view on startup, on each block, and on the polling ticker so mempool deposits are created immediately. Backfill the first confirmation height once those outputs confirm, protect unconfirmed deposits from expiry, and mark vanished unconfirmed outpoints as Replaced so RBFed-away deposits stop showing up in RPCs. Expose the new state through static-address RPCs by deriving availability and summary totals from stored deposit state, reporting sensible expiry data for unconfirmed outputs, and hiding Replaced records from normal listings.
Allow static loop-ins to select unconfirmed deposits because their CSV timeout has not started yet, while still preferring confirmed outputs during automatic selection. Keep confirmed-input requirements for channel opens and withdrawals now that Deposited includes mempool outputs. Filter unconfirmed deposits out of automatic selection for those flows and fail manual requests that reference them, so the client does not build PSBTs or withdrawal attempts with unusable inputs. Treat deposit.MinConfs as the legacy readiness threshold rather than the single source of truth for all flows. Loop-in readiness is now governed by server confirmation-risk policy, while withdrawals and channel opens keep their confirmed-input checks.
Remove the old "no confirmed deposits available" error now that mempool deposits are listed immediately and can be selected for static loop-ins. Reproduce the server static-address deposit selection order in the CLI using the already-returned deposit metadata. This keeps the low-confirmation warning focused on the deposits auto-selection would actually choose, so users only see it when the swap payment may wait for the server confirmation-risk policy.
If InitHtlcAction creates the private swap invoice but fails before the loop-in is stored, the retry path otherwise leaves behind a live orphan invoice. Cancel that invoice on the early error path with a detached, timeout-limited context, and reuse the same helper when tearing down the monitor path. This keeps failed initialization attempts from leaving invoices that no local swap can complete.
FinalizeDepositAction only needs to tell the manager to remove the FSM from its active set, but the old synchronous send was still tied to the caller context and could race with request cancellation or a busy manager loop. Send the cleanup notification asynchronously and tie it to the FSM lifetime instead. Withdrawal completion no longer blocks while deposit locks are held just because the original request context was canceled.
Keep replacement UTXOs as fresh deposits while preserving the original deposit record and selected outpoint snapshot for pending swaps. Before signing a static loop-in HTLC, check each original selected outpoint with GetTxOut(..., includeMempool=true). Cancel the pending invoice only when that check reports an original outpoint unavailable; lookup errors fail the action without canceling so transient chain backend errors do not incorrectly abandon the swap. Keep recovered loop-ins using their stored outpoint snapshot and cover replacement discovery and cancellation in tests.
ListUnspentDeposits now reports only wallet UTXOs that have an active Deposited record. That matches the static loop-in admission path and avoids exposing wallet-seen outputs that are not ready for loop-in selection. Make local notification fan-out non-blocking for best-effort categories so a slow subscriber cannot stall the notification manager while it holds the subscriber lock. Static loop-in sweep signing requests remain blocking because they are work requests required for sweepbatcher presigning and must not be dropped.
hieblmi
force-pushed
the
dyn-conf-tracker
branch
from
be1b86f to
16eaa1b
Compare
Wait for the server's static loop-in risk-accepted notification before starting the client payment deadline. The server may intentionally hold the swap at the confirmation-risk gate after HTLC signing, and the client deadline should not run while that server-side wait is still in progress. Cache risk-accepted notifications by swap hash inside the local notification manager and replay them to the per-swap subscriber. This covers both reconnects and the internal race where the global notification stream receives the server event before the static loop-in FSM registers its waiter.
Add client handling for the server's static loop-in risk-rejected notification. If the server aborts confirmation-risk waiting before payment, the client fails the local swap instead of waiting for a payment deadline that will never start. Cache rejected notifications by swap hash using the same replay path as accepted notifications, and clear the opposite cached state when a final risk decision is received. This keeps reconnect and subscription-order races from stranding the client in the risk wait.
hieblmi
force-pushed
the
dyn-conf-tracker
branch
from
16eaa1b to
4b7d5c9
Compare
4 participants
This PR does the following:
settles.