lightninglabs / lightninglabs/loop
staticaddr/loopin: enforce immutable selected-deposit invariant
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 595
- Forks
- 135
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 12
Description
Problem
A static-address loop-in currently carries two representations of its funding set:
DepositOutpoints: the immutable, ordered outpoints selected for the swap and sent to the Loop server.Deposits: deposit rows reconstructed through current database associations and manager state.
These representations are intended to describe the same inputs, but that invariant is not enforced. The recovered rows can differ in cardinality, order, or outpoint.
One concrete way to lose the association is deposit reuse: MapDepositToSwap updates the deposit row’s single swap_hash, so an older swap can no longer reconstruct that deposit through DepositsForSwapHash. In addition, the generic deposit update path currently persists tx_hash and out_index, even though production callers do not intentionally change a deposit outpoint.
The current pre-signing checks reject a mismatched or unavailable input set. After signatures have been handed to the server, however, recovery and timeout sweeping must remain tied to the exact original inputs. HTLC transaction reconstruction uses Deposits; a wrong or incomplete set produces a different funding transaction/txid and can prevent the client from automatically timeout-sweeping the real HTLC output. The resulting worst case is funds remaining locked until manual recovery, rather than an unauthorized-spend path.
Required invariant
For every loop-in with a persisted selected-outpoint snapshot:
DepositOutpointsremains the immutable, ordered set sent to the server.Depositscontains exactly the deposit records for those outpoints, with the same cardinality and order.- Deposit confirmation and wallet availability may change, but funding identity cannot.
- An unavailable or spent original outpoint is represented as status on a separate wallet/chain view; another deposit record is never substituted.
Proposed implementation
-
Make a deposit funding outpoint immutable after creation:
- Remove
tx_hashandout_indexfrom the generic mutable-field update. - Reject an update when the supplied outpoint differs from the stored outpoint.
- Keep confirmation height, state, and withdrawal metadata independently mutable.
- Remove
-
Reconstruct the selected deposit set from
DepositOutpoints:- Fetch each deposit by outpoint in snapshot order.
- Do not rely on the mutable
swap_hashassociation as the source of funding identity. - Preserve explicit compatibility behavior for legacy swaps without an outpoint snapshot.
-
Enforce the invariant at persistence and FSM boundaries:
- Align records to snapshot order.
- Reject nil, missing, duplicate, invalid, or extra records.
- Validate exact cardinality and outpoint equality whenever a swap is created or recovered and before actions consume its deposits.
-
Separate identity from current availability:
- Query whether each selected outpoint is currently confirmed, unconfirmed, unavailable, or spent without changing the selected
Depositscollection.
- Query whether each selected outpoint is currently confirmed, unconfirmed, unavailable, or spent without changing the selected
-
Add regression coverage for:
- Reordered rows and cardinality mismatches.
- Missing, nil, duplicate, invalid, and extra records.
- Rejected outpoint mutation while other deposit metadata still updates.
- Recovery of an older swap after its deposit is reused/reassociated.
- Restart and timeout-sweep reconstruction when an original input is no longer in the active wallet view.
- SQLite and PostgreSQL persistence behavior.
Related work
This is related to, but separate from, #1171. This issue ensures recovery and sweeping always use the original inputs; #1171 covers how long the client must keep monitoring an already-armed HTLC after a swap aborts.
Raised from the invariant concern in the PR #1141 review thread.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing MapDepositToSwap, DepositsForSwapHash, and the loop-in persistence and FSM boundaries described in the issue. Compare SQLite and PostgreSQL behavior, then add the listed regression coverage for ordering, mismatches, immutable outpoints, reused deposits, restart, and timeout sweeping. Done means recovery and sweeping use the original ordered outpoints while availability changes do not replace them.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, postgresql, sqlite
- Domain
- backend, databases, payments, testing-qa
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100