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
|
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.
bc06754 to
242a3e1
Compare
|
@hieblmi, remember to re-request review from reviewers when ready |
bcefc9f to
75123f2
Compare
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. Derive confirmation heights from the current wallet view because lnd reports confirmation counts instead of first-confirmation heights.
The first block epoch is consumed before recovered deposit FSMs exist. Replay that startup height after recovery so already-expired deposits can run expiry handling immediately after restart.
Treat lnd's wallet view as the source of spendable static-address outpoints while keeping historical deposit records in the DB. Reconcile active FSMs against the current wallet view, reactivate known deposits that reappear, and hide stale Deposited records from the visible deposit set.
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, and reject manual loop-in quotes for selected deposits that are not currently Deposited.
Refresh the active static-address deposit set against lnd's wallet view before quote, loop-in, withdrawal, channel-open, and autoloop selection paths. This prevents stale persisted Deposited records from being selected after replacement, reorg, or an external spend.
Use the deposit manager's visible-deposit view for normal list and summary RPCs so historical Deposited rows whose outpoints vanished from lnd's wallet view 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.
Add cached per-swap notification fanout for static loop-in confirmation-risk acceptance and rejection notifications so loop-in FSMs can subscribe by swap hash and receive decisions that arrived before subscription.
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.
Add schema, sqlc queries, store fields, and SqlStore support for recording server confirmation-risk decisions with static loop-in swaps. Store the decision timestamp so payment-deadline recovery can reconstruct elapsed time after restart.
Persist static loop-in confirmation-risk decisions before fanout when a persistence callback is configured. Keep unpersisted decisions cached for replay so notification delivery is not lost if the swap row is not available yet.
Create the static loop-in SQL store before the notification manager and pass a persistence callback so server confirmation-risk decisions are durably recorded before fanout.
Track whether the invoice was canceled for non-payment while monitoring the HTLC. If the HTLC never confirms before timeout, unlock the deposits; if it did confirm, transition them to the HTLC-timeout sweep state without issuing duplicate transitions.
Record replayed server risk decisions through the loop-in store, recover accepted payment-deadline timers using the persisted decision time, and handle persisted rejections on restart. This lets recovered static loop-ins keep pending confirmation-risk state instead of restarting payment timing from scratch.
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.
Reserve separate key families for static receive and change addresses. This keeps derived keys out of the legacy static-address and HTLC key streams.
Associate every deposit with the static address parameters that created it. This lets restored deposits recover the correct script and signing keys instead of assuming the legacy root address.
Create receive and change addresses from locally derived client keys while reusing the server key and expiry from the legacy seed. Import, persist, and activate each script before returning it to callers.
Filter wallet UTXOs against every active static-address script and attach the matching address parameters to new deposits. This makes deposits to derived receive addresses visible to the deposit manager.
Build timeout sweeps from each deposit own script, expiry, and key locator. Derived-address deposits can now use their unilateral recovery path without falling back to the legacy root parameters.
Construct cooperative MuSig2 sessions from the address parameters stored on each deposit. Loop-ins and withdrawals can therefore combine inputs owned by different derived static addresses.
Carry each deposit client derivation key and an optional generated change descriptor in static-address loop-in and withdrawal requests. The server can use these fields to validate scripts it does not store directly.
Map every selected outpoint to the client key that derived its static address and include those proofs in loop-in requests. Keep MuSig2 signing indexed by outpoint so request ordering cannot select the wrong key.
Include the derivation key for every withdrawal input in the server request. This lets the server validate and sign withdrawals that combine deposits from multiple derived addresses.
Join each selected deposit with its persisted static-address row during loop-in recovery. Hydrate legacy rows as needed so restored swaps retain the scripts and key locators required for signing.
Create a fresh static change address for fractional loop-ins and persist its key locator with the selected HTLC outpoint. Recovery reconstructs the same change output instead of returning funds to the legacy root address.
Create a fresh static address for partial-withdrawal change and include its descriptor in the signing request. Build and validate the withdrawal against that address while preserving full-withdrawal behavior.
Let loop static deposit create a fresh receive address and optionally fund it through lnd SendCoins. Validate funding arguments before address creation and expose the nested request through the client RPC. Regenerate the RPC artifacts, CLI documentation, and replay fixtures for the new command behavior.
Include the owning static address in every deposit RPC response and CLI listing. Users can distinguish deposits created by different receive and change addresses without reconstructing scripts externally. Update generated RPC artifacts and command replay fixtures for the new field.
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.