staticaddr: dyn-conf-tracker#1141
Open
hieblmi wants to merge 24 commits into
Open
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 reliability and responsiveness of static-address deposits. By allowing immediate visibility of mempool deposits while enforcing strict confirmation requirements for high-risk operations like channel opens and withdrawals, the system balances user experience with security. The changes also include robust handling of deposit replacement, durable persistence of risk decisions for recovery, and comprehensive regression testing to ensure stable behavior across the deposit lifecycle. Highlights
New Features🧠 You can now enable Memory (public preview) 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 the 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 counterproductive. 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 introduces support for unconfirmed deposits in static address loop-ins, enabling faster swap initiation while managing confirmation risk. It includes client-side warnings for low-confirmation outputs, server-side persistence of risk decisions, and logic to handle deposits that vanish from the wallet view due to mempool replacement or reorgs. The notification manager was also enhanced to support asynchronous risk acceptance/rejection events. Feedback was provided regarding the duplication of coin selection logic between the CLI warning path and the loop-in manager, recommending a refactor into a shared utility for better maintainability.
hieblmi
force-pushed
the
dyn-conf-tracker-staging
branch
4 times, most recently
from
7bee626 to
ec42d48
Compare
hieblmi
force-pushed
the
dyn-conf-tracker-staging
branch
2 times, most recently
from
f8f474e to
d424169
Compare
hieblmi
force-pushed
the
dyn-conf-tracker-staging
branch
from
d424169 to
d630c2e
Compare
hieblmi
force-pushed
the
dyn-conf-tracker-staging
branch
3 times, most recently
from
295daee to
49241e1
Compare
What: - Cancel a private swap invoice if init fails before storage. - Use a detached timeout context for invoice cleanup. - Reuse the helper for monitor-driven invoice cancellation. - Add tests for early-init invoice cancellation. Why: - A failed pre-storage swap cannot be recovered on restart. - Leaving its invoice active can accumulate orphan invoices.
What: - Send finalization notifications from a goroutine. - Stop blocking the deposit FSM on the manager receive loop. - Add tests for blocked manager and shutdown races. Why: - Final states should not stall while deposit locks are held. - Startup recovery can handle shutdown before notification delivery.
What: - Fix state-list query formatting for final loop-in states. - Add a failed loop-in to the final-state store test. Why: - The previous formatting excluded the last state in the list. - Failed swaps must remain visible to callers querying final states.
What: - Make reservation fanout nonblocking for slow subscribers. - Keep recovery and sweep notifications blocking until delivered. - Make required fanout cancellation-aware. - Cover slow and required subscribers in manager tests. Why: - A slow optional subscriber must not stall the manager lock. - Required work requests still need reliable local delivery.
hieblmi
force-pushed
the
dyn-conf-tracker-staging
branch
from
49241e1 to
f6caba3
Compare
What: - Add per-subscriber queues for must-deliver notifications. - Use queued delivery for sweep requests and unfinished swaps. - Match subscribers by receive channel when removing them. - Add tests for queued delivery and cleanup. Why: - Required fanout should not block the manager lock. - Subscribers still need ordered delivery after brief backpressure.
What: - Add ChainKit access to the deposit manager. - Derive first confirmation heights from lnd wallet confirmations. - Cover unconfirmed, confirmed, and invalid height cases. Why: - Avoid blocking on confirmation notifications for listed UTXOs. - Keep stored confirmation heights tied to a stable chain tip.
What: - Discover static-address deposits from zero confirmations. - Store unconfirmed deposits with confirmation height zero. - Sync confirmation heights when wallet-visible deposits change. - Keep unconfirmed deposits from expiring before CSV starts. Why: - Static loop-ins need to see mempool deposits immediately. - Reorged deposits must not keep stale confirmation heights.
What: - Add the Replaced deposit state. - Mark Deposited outpoints replaced after repeated wallet misses. - Revive a Replaced record when the same outpoint reappears. - Stop removed FSMs and reject stale transitions from final states. Why: - RBF, eviction, and deep reorgs can remove tracked outpoints. - Historical records should remain while unavailable funds disappear.
What: - Base unspent-deposit availability on tracked deposit state. - Hide Replaced deposits from normal lists and summary totals. - Report full CSV expiry for unconfirmed deposits. - Reject manual quotes for deposits that are not Deposited. Why: - RPC callers should not select wallet outputs Loop cannot spend. - Replaced history must not inflate available balances.
What: - Remember the initial block epoch consumed during startup. - Replay that height to active recovered deposit FSMs. - Share normal and startup block fanout through one helper. Why: - Recovered deposits at expiry should act before another block arrives. - Startup should not hide an already-open expiry path.
What: - Treat unconfirmed deposits as swappable for static loop-ins. - Prefer confirmed deposits during automatic deposit selection. - Treat MinConfs as a legacy readiness threshold. Why: - A mempool deposit has not started its CSV timeout yet. - Confirmed deposits should still be selected first when available.
What: - Reject unconfirmed deposits for withdrawals. - Filter unconfirmed deposits from automatic channel opens. - Fail manual channel-open requests that select mempool deposits. - Reject withdraw-all when any deposited output is unconfirmed. Why: - Withdrawal and channel funding transactions need confirmed inputs. - The all-deposits path must not silently downgrade to a subset.
What: - Add a warning for selected static deposits below six confirmations. - Mirror automatic static loop-in selection for warning output. - Add CLI tests for manual and auto-selected warning cases. Why: - Low-confirmation deposits may wait on server risk policy. - The CLI should surface that delay before users confirm a swap.
What: - Copy selected deposit outpoints when creating loop-ins. - Recover the stored outpoint snapshot separately from deposits. - Keep empty stored outpoint lists from producing a blank outpoint. Why: - Replacement handling can change deposit records after initiation. - Recovery must keep signing checks tied to the original inputs.
What: - Add an lnd-backed txout checker for original deposit inputs. - Check selected outpoints before signing the HTLC transaction. - Cancel the swap invoice when an original input vanished. - Handle closed invoice monitor channels without zero-value updates. - Copy mock invoices when tests read and seed them. Why: - The client must not sign an HTLC for inputs it can no longer spend. - Replaced or evicted deposits should fail before server publication. - Closed subscription channels should not spin the monitor loop.
What: - Return only wallet UTXOs that have active Deposited records. - Stop reporting unknown wallet outputs as selectable deposits. - Update unspent-deposit tests for the stricter rule. Why: - Static loop-in initiation already requires tracked deposits. - RPC output should not race ahead of deposit manager readiness.
What: - Subscribe loop-in FSMs to risk notification updates. - Wait after HTLC signing until the server accepts risk. - Time out waiting swaps and cancel their invoices. - Add notification and FSM tests for acceptance waiting. Why: - Low-confirmation deposits may require asynchronous server review. - The client must not monitor an HTLC before risk is accepted.
What: - Route server risk-rejection notifications to waiting loop-ins. - Cancel the swap invoice when risk is rejected. - Fail the loop-in with the server rejection reason. - Add notification and FSM tests for rejection delivery. Why: - A rejected low-confirmation swap should stop before monitoring. - Users need the rejection surfaced as the swap failure reason.
What: - Add DB columns for static loop-in risk decisions and time. - Generate sqlc accessors for recording risk decisions. - Add loop-in fields and store API for persisted decisions. - Cover persistence and missing-swap errors in store tests. Why: - Recovery needs durable accepted or rejected risk state. - The decision timestamp is the source for rebuilt timeouts.
What: - Add a callback for durable static loop-in risk decisions. - Persist accepted and rejected risk notifications before fanout. - Wire the notification manager to the static loop-in store. - Keep replaying cached notifications if persistence fails. Why: - A restart should not lose a server risk decision. - Early notifications can arrive before the loop-in row exists.
What: - Rebuild payment deadlines from persisted risk-accepted times. - Treat persisted risk rejections as terminal after restart. - Persist replayed risk decisions when the store was previously missing. - Cover live replay and restart recovery in monitor tests. Why: - The payment timeout should include time before a restart. - Cached server notifications may replay after the swap row exists.
What: - Share risk accepted and rejected notification handling. - Reuse best-effort drop handling for risk subscribers. - Keep accepted and rejected caches mutually exclusive. - Update risk fanout tests for the shared path. Why: - The two risk paths have identical persistence and fanout rules. - One implementation reduces drift between accepted and rejected cases.
What: - Request the static address summary only after quote validation. - Add replay fixtures for static loop-in warning prompts. - Update replay flow for payment-timeout and fee variants. Why: - The replay sessions need to reflect the new confirmation warning. - Avoiding an early summary request keeps prompt order stable.
What: - Reuse loop-in expiry math for autoloop deposit candidates. - Treat unconfirmed deposits as not yet aging in DP tie-breaks. - Add a selector test for confirmed versus unconfirmed ties. Why: - Unconfirmed deposits have no running CSV timer. - Autoloop should not prefer mempool deposits as earliest expiring.
hieblmi
force-pushed
the
dyn-conf-tracker-staging
branch
from
f6caba3 to
0669702
Compare
|
@hieblmi, remember to re-request review from reviewers when ready |
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
3 participants
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.
This PR surfaces static-address deposits as soon as they appear in the wallet, including mempool deposits, instead of waiting for the old confirmation readiness
threshold.
It also updates the static-address flows around those low-confirmation deposits: