staticaddr: multi-address support#1139
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 introduces multi-address support for static-address deposits, significantly increasing the flexibility of loop-in and withdrawal operations. By persisting per-deposit address parameters and implementing a generation-based recovery flow, the system ensures that funds remain accessible even after local data loss. Additionally, the PR includes robust CLI updates for address funding and improved deposit reconciliation logic to handle mempool-level changes and chain reorgs. 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 implements a robust recovery system for Loop's static addresses and L402 tokens, utilizing encrypted immutable backups. Key additions include a new recover CLI command, a dedicated recovery package for backup orchestration, and significant updates to the address and deposit managers to support multi-address derivation and mempool-aware reconciliation. The loop-in FSM is also enhanced to handle fractional swaps with static change outputs and server-side risk notifications. Feedback focuses on improving the clarity of change amount logic, optimizing the performance of multiset matching during change verification, and making the retry limit for stable block height lookups configurable.
There was a problem hiding this comment.
Code Review
This pull request introduces static address recovery for Loop, enabling the restoration of L402 and static-address state from encrypted backups. It also adds support for fractional loop-ins by implementing change address derivation and handling, along with improved deposit reconciliation and validation logic. My review identified several critical issues regarding notification replay logic, performance concerns with wallet transaction scanning, and potential address leakage during retries.
hieblmi
force-pushed
the
multi-address
branch
15 times, most recently
from
bc06754 to
242a3e1
Compare
|
@hieblmi, remember to re-request review from reviewers when ready |
Keep deposits locked when the server publishes the loop-in HTLC without paying the invoice. This lets the client sweep through the HTLC timeout path instead of making the same outputs available for another action.
Return early when block notifications reach deposits that already moved into a terminal state. This prevents final deposits from retrying expiry handling after recovery or while their FSM is still draining block updates.
Reject duplicate static-address deposit outpoints before creating withdrawal, loop-in, or channel-open requests. Use the shared outpoint duplicate helper so each flow reports the same input validation failure.
Extract the monitor invoice update semantics into a helper and cover the existing state mapping with a dedicated test.
Treat closed invoice update channels as terminal for the monitor loop. This avoids spinning when lnd closes the subscription after invoice cancellation or shutdown.
Add an explicit Stop method for deposit FSM block-notification loops. Call it when the manager removes a finalized active deposit so stale FSM goroutines stop consuming block updates.
Store an independent snapshot of the outpoints selected for a static loop-in. Recovered swaps remain tied to the original funding outputs even if deposit records later change confirmation or replacement metadata. Avoid decoding an empty database outpoint string as a synthetic outpoint.
Add a TxOutChecker interface for checking whether a selected deposit outpoint is still available before signing the HTLC transaction. Back the implementation with lnd wallet transaction data so known confirmed and mempool spends mark the outpoint unavailable.
Document the lock-order invariant between Manager.mu and individual deposit locks. Later changes need both locks in the same path, so make the rule explicit before the locking surface grows.
Reject duplicate static-address deposit outpoints before creating withdrawal, loop-in, or channel-open requests. Use the shared outpoint duplicate helper so each flow reports the same input validation failure.
Retain static-address deposits as soon as lnd reports the UTXO, even when the output is still unconfirmed. Store the first confirmation height once the output confirms. Replay the startup block to recovered deposit FSMs so expiry handling can run immediately after restart. Derive confirmation heights from a stable wallet view because lnd reports confirmation counts.
Build list and summary responses from tracked deposit records instead of raw wallet UTXOs so RPC clients see the manager availability state. Split unconfirmed value from confirmed deposited value in summaries. Keep withdrawal and channel-open flows on confirmed inputs by rejecting unconfirmed selected deposits in those paths.
Static address deposits with no confirmation height have not started their CSV timeout yet, so keep them eligible for loop-in selection instead of treating them as already near expiry. Prefer confirmed deposits before unconfirmed ones during automatic selection, and share the remaining-lifetime calculation used by the selector.
Use the shared deposit-expiry helper when building autoloop DP candidates so unconfirmed deposits do not look like the earliest-expiring options. This keeps the no-change selector's expiry tie-break aligned with the generic loop-in deposit selection rules.
Treat lnd wallet view as the source of spendable static-address outputs while keeping historical deposit records in the DB. Reconcile active FSMs against the current wallet view and reactivate known deposits when their outpoints are visible again. Refresh deposits before selection, withdrawal, loop-in, and channel-open paths, and filter list and summary responses through the live active set so stale Deposited records are not exposed as available funds.
Check the originally selected deposit outpoints before signing a static loop-in HTLC transaction. If any selected outpoint is no longer available, cancel the swap invoice and fail the signing action instead of producing signatures for stale inputs. Wire the lnd-backed checker through loopd and make invoice-monitoring handle closed subscription channels without spinning.
Subscribe to static loop-in confirmation-risk notifications before starting the payment deadline. Start that deadline only after server acceptance or the legacy confirmation fallback, and cancel the swap invoice when the server rejects the risk wait. Refresh selected deposits before the legacy fallback so recovered monitors use current confirmation heights.
Store server confirmation-risk decisions with static loop-in swaps and recover accepted payment-deadline timers after restart. Wire notification persistence through loopd so recovered swaps do not lose pending risk state. Deduplicate notification fanout cache entries by swap hash.
Warn before dispatching a static loop-in that selects deposits below the conservative six-confirmation threshold. Mirror automatic coin selection before prompting so the warning reflects both manual and auto-selected deposits. Cover manual and auto-selected warning paths in CLI tests.
Refresh static loop-in replay sessions for the low-confirmation warning and payment-timeout prompts. Add replay coverage for the warning prompt and update fee and payment-timeout variants for the new interaction sequence.
Allow golangci-lint more time for the larger static-address test suite. The dynamic-confirmation stack adds several focused tests, and the default timeout is tight on slower CI workers.
hieblmi
force-pushed
the
multi-address
branch
3 times, most recently
from
2f9b251 to
c9f9b14
Compare
Add SendCoins-backed funding through loop static deposit, keeping loop static new focused on address creation. Wire the nested lnd SendCoins request through the NewStaticAddress RPC and return the SendCoins response. Validate cheap SendCoins errors before address creation and require execute permissions for the funding-capable RPC. Regenerate CLI docs and looprpc artifacts, and add validation coverage for the static address funding request.
3 participants
Summary
This PR adds multi-address support for static-address deposits.
Static-address deposits now retain the concrete address parameters they were sent to, instead of assuming every deposit belongs to a single reusable output script. This allows loop-ins and withdrawals to spend deposits that were received across multiple derived static addresses.
Key Changes
SendCoins.Recovery
The recovery flow now restores multi-address static deposits by deriving receive and change address candidates from the immutable static-address backup, scanning wallet-visible UTXOs, and reconciling matching deposits back into loopd.
Testing
This branch adds coverage for multi-address deposit persistence, recovery, loop-in signing, generated change outputs, server input proofs, and the new static-address funding flow.