Conversation
a1b5ec5 to
c74ac6c
Compare
ee1bbba to
5b4bd5b
Compare
5b4bd5b to
c49a43e
Compare
|
@starius: review reminder |
c49a43e to
56816b7
Compare
2f594a8 to
205077e
Compare
|
@starius: review reminder |
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. Persist, import, and activate each script before returning it to callers. Rebuild the active address index on startup and serialize issuance without blocking address reads. Import only scripts missing from lnd, and accept duplicate-import errors only when they identify the expected Taproot output key.
Look up each newly discovered wallet UTXO by script and persist the matching active-address parameters on the deposit. Reject unknown scripts before allocating the timeout sweep address. Use the per-deposit parameters when constructing the FSM, sign descriptor, and unilateral expiry sweep so derived-address recovery uses its owning script and key.
Associate fractional loop-ins with their operation-specific static change address so recovery restores the descriptor needed to reconstruct signed transactions. Backfill legacy fractional swaps to the original address.
bd323ff to
10c9f28
Compare
Create a fresh static address for fractional loop-in change and send its descriptor to the server. Reconstruct signed HTLCs with the persisted parameters and verify cooperative batch change by output script.
Multi-address loop-ins sign and construct transactions from the parameters attached to each deposit and their dedicated change address. The legacy root address fields therefore became write-only, but populating them could still abort signing, sweep handling, or recovery when the root lookup failed. Remove those fields and lookups, select the FSM from the protocol version persisted with the swap, and set that version before constructing new state machines. Keep the root-parameter lookup used by autoloop expiry calculation and add regression coverage for recovery and unsupported persisted versions.
Create a fresh static address for partial-withdrawal change and identify it in the confirmed transaction through its active change-family script, without assuming output order or count. Record withdrawn and change amounts by script identity. Keep all withdrawal outputs in the PSBT without separate signing metadata while preserving full-withdrawal behavior.
Let loop static deposit create and fund a fresh receive address through lnd SendCoins. Validate funding arguments before address creation and require explicit confirmation unless --force is set, including for non-interactive and first-use deposits. Allow NewStaticAddress RPC callers to fund a requested existing static address by resolving it through the active script index. Expose the nested request through the client RPC, require swap:execute permission, and cover the CLI new-address and daemon existing-address funding paths. Regenerate RPC and CLI documentation.
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. Calculate blocks until expiry from each deposit owner instead of the legacy root address, and reject deposits whose owning parameters are missing. Centralize deposit response conversion and update generated RPC artifacts, regression coverage, and command replay fixtures.
The CLI previously recognized an uninitialized static-address seed by searching arbitrary gRPC error text. Any wrapping or wording change could suppress the L402 backup warning before a user funded a newly derived address. Map ErrNoStaticAddress to codes.NotFound at the RPC boundary and classify that status in the CLI. Retain compatibility with older daemons only for an exact Unknown-status message, avoiding the broad substring match, and cover both sides with regression tests.
A static-address account can now receive deposits across multiple derived addresses, so the singular summary field can no longer describe the current receive address. Removing or repurposing field 1 would break existing clients. Keep the wire value as the legacy/root derivation address, formally deprecate it, document the expiry as the shared CSV delay, and direct CLI users to derive a fresh receive address. Rename the server locals to make the compatibility behavior explicit and regenerate protobuf and Swagger artifacts.
Cover per-deposit address ownership and operation-specific change outputs across the shared SQL persistence boundary. Reconstruct the deposit, loop-in, and withdrawal stores to verify ownership and change metadata survive restart.
The sweep request handler only compared the number of prevouts the server sent against the number of sweep inputs. A list of the right length could still contain duplicate outpoints or reference outpoints the sweep doesn't spend. The prevout fetcher then returns nil for an input and NewTxSigHashes panics on the nil dereference, taking down loopd on a malformed server request. Reject duplicate prevouts while building the prevout map and require a prevout for every sweep input before computing sighashes.
Rename the legacy key family and address-manager parameter alias, use root terminology consistently, and consolidate wallet UTXO listing. Keep key-family values and the underlying script parameters unchanged. Document script map keys, issuance helpers, and test invariants. Include the output script in missing-parameter errors, preallocate address store results, and remove the constant-value test. Update CLI and RPC docs.
Keep NewStaticAddress address-only and move wallet funding into the new FundStaticAddress RPC. Require wallet:fund alongside swap:execute and loop:in so existing swap or address-URI macaroons cannot authorize wallet funding. Explicit grants for the new RPC remain supported. Preserve new-address and existing-address funding behavior, route the static deposit CLI through the new RPC, and regenerate all bindings. Reserve the removed experimental fields and document macaroon upgrades. Exercise the production interceptor with signed and expired macaroons, and cover address-only calls, funding validation, request preservation, existing-address funding, and recovery after a wallet broadcast failure.
Require exactly one legacy static address when deposits exist before migration 22 assigns their address IDs. Use a temporary CHECK constraint shared by SQLite and PostgreSQL; databases without deposits remain valid. Cover valid backfills, missing and ambiguous ownership, unchanged schema and data on rejection, and retry after repair and migration-version reset. The existing framework still marks a rejected migration as dirty.
Encode listing addresses directly from their canonical P2TR scripts, validating the script shape and using the configured network. This avoids rebuilding Taproot trees and aggregating keys for every record without introducing a cache. Check encoding against full reconstruction on three Bitcoin networks and reject malformed or non-Taproot scripts.
Quote selected deposits with one exact active-set lookup, retaining presence, state, duplicate and expiry checks without rendering history. Refresh deposits before taking the unspent response snapshot and filter against active Deposited records. Historical database rows cannot revive outputs removed during reconciliation or return stale confirmations.
Keep the lowest-ID active address alongside the script index so receive and change issuance can find the root without scanning all addresses. Publish the cached root only after successful activation and update it under the same mutex as the index. Cover restart recovery and repeated import failures so caching cannot make an unavailable root appear ready.
Read wallet UTXOs outside the map lock, then match their scripts directly against the active address index under a short lock. This removes the full-index allocation and copy on each poll while preserving confirmation bounds and allowing address activation during the wallet RPC. Test filtering across addresses and verify wallet RPCs do not hold the map lock.
Load persisted addresses in bounded pages using an ascending ID cursor during startup and root recovery. Read wallet watches once and publish the complete runtime index only after every page and import succeeds. The existing bulk-read API remains available for compatibility. Test sparse IDs, page boundaries, failed-page recovery and restart behavior, including the SQL query on SQLite and PostgreSQL.
Replace the issuance mutex with a context-aware gate so queued callers can cancel without disturbing the current issuer. Keep root creation, recovery and derived issuance serialized and retryable. A private mutex now explicitly guards the active script index and root. Test canceled root and derived waiters, subsequent progress and concurrent first callers sharing one root.
Remove the blank line before the closing brace flagged by CI. This is formatting only; test behavior is unchanged.
26983e9 to
ad14247
Compare
Compare lnd's next legacy/root, receive and change indices with the highest persisted Loop keys before activating addresses. The legacy family also holds every static loop-in HTLC key, so its target covers the highest persisted HTLC key index as well. Advance only lagging counters, verify the restored wallet against the family's persisted address key first, and leave equal or ahead counters untouched without creating any Loop addresses. Allow startup reconciliation to outlive the fixed manager deadline while preserving RPC timeouts, cancellation and error propagation. Add recovery, restart and failure coverage plus documentation.
The deposit expiry sweep passed only the client pubkey to lnd. After a wallet restore lnd has not yet re-derived the static-address key, so its pubkey lookup misses and falls back to locator (0, 0), producing an invalid timeout-path signature. Pass the deposit address' key locator so lnd derives the signing key directly for root, receive, and change deposits.
Document fresh receive-address derivation, lazy seed initialization, funding-address lookup hardening, and the swap:execute permission required by address creation. Regenerate the CLI, gRPC, Swagger, and man-page documentation and add feature, breaking-change, and recovery release notes.
ad14247 to
5b76079
Compare
| // | ||
| // The server receives this proof material with swap and withdrawal requests and | ||
| // verifies it against the L402's server key and expiry before co-signing any | ||
| // input. |
There was a problem hiding this comment.
Are root and multi addresses treated differently here? Can the Loop server distinguish which is which?
Could you add these details to the godoc, please?
| var filteredUtxos []*lnwallet.Utxo | ||
| for _, utxo := range utxos { | ||
| if bytes.Equal(utxo.PkScript, staticAddress.PkScript) { | ||
| if _, ok := m.activeStaticAddresses[string(utxo.PkScript)]; ok { |
There was a problem hiding this comment.
I tested many scenarios of multi-address and identified an interesting property. This is not pre-existing.
Restoring an older Loop DB (with lnd's wallet unchanged) hides deposits at receive/change addresses issued after the backup: this filter only knows the restored DB's address rows, and counter reconciliation doesn’t rebuild missing ones. I confirmed empirically that some funds are still unspent past CSV maturity, with no automatic sweep. The single-address version could rediscover later deposits through its retained root.
Could we add recovery of the missing 42061/42062 addresses, or explicitly agree on the weaker backup guarantees before release?
| if hasChange { | ||
| changeAmount := f.loopIn.ExpectedChangeAmount() | ||
| f.loopIn.ChangeAddressParams, err = | ||
| f.cfg.AddressManager.NewChangeAddress(ctx) |
There was a problem hiding this comment.
If the Loop server does not support multi-address yet, we have a problem if this code actually runs.
Every partial loop-in now requests derived change, but the loop server rejects any ChangeOutput, so even root-only partial loop-ins stop working. I confirmed empirically that root-only partial withdrawals succeed but leave derived change the server refuses to co-sign (CSV recovery still works).
Could we gate these operations on server support, or explicitly require multi-address enabled before rolling out this client? If someone runs this as-is from master branch, we can get this problem.
| changeAmount += txOut.Value | ||
| continue | ||
| } | ||
|
|
||
| withdrawnAmount += txOut.Value |
There was a problem hiding this comment.
let's rewrite it in more readable way using else instead of continue
|
|
||
| var depositStaticAddressCommand = &cli.Command{ | ||
| Name: "deposit", | ||
| Usage: "Create and fund a new static loop in address.", |
There was a problem hiding this comment.
Do we really need this command? Could loop static new just print a command for lncli to fund this new address? IMHO embedding this flow here is an extra complexity and extra braveness of the loop command itself.
| params := addressManager.GetParameters(txOut.PkScript) | ||
| if params == nil || int32(params.KeyLocator.Family) != | ||
| swap.StaticAddressChangeKeyFamily { | ||
|
|
||
| continue | ||
| } |
There was a problem hiding this comment.
A pending partial withdrawal created before upgrading has change at the legacy root (family 42060). The new change detector only recognizes family 42062, so it returns no change script. The new store loop (SqlStore.UpdateWithdrawal) then counts both outputs as withdrawn, with zero change. The old two-output logic in SqlStore.UpdateWithdrawal classified the second output as change correctly.
For example, a confirmed transaction paying 100,000 sat to the destination and 49,000 sat back to the root gets recorded as 149,000 withdrawn, zero change. This affects history/accounting, not ownership of the funds.
I'd either retain the existing one/two-output handling, or fix legacy change recognition before keeping the new loop.
| " \"id\": \"68262a104c9ec325de6bec37b8e31bd875bbd2f5f0b9ce2da20cf0bd636fc448\",\n", | ||
| " \"outpoint\": \"edcdab8f0b1138d853a453b8b7a5ac3c694bd53ad38b7ccf062e45f99440e6e6:0\",\n", | ||
| " \"state\": \"WITHDRAWING\",\n", | ||
| " \"static_address\": \"\",\n", |
There was a problem hiding this comment.
Why is it empty in all the fixtures? Maybe we need to re-generate them? I guess AI can do it easily with minimum blast radius.
| if err := validateExpirySpend( | ||
| confirmedTx, f.deposit.OutPoint, | ||
| f.deposit.TimeOutSweepPkScript, | ||
| ); err != nil { | ||
| return f.HandleError(err) | ||
| } |
| confChan, confErrChan, err := | ||
| f.cfg.ChainNotifier.RegisterConfirmationsNtfn( | ||
| ctx, &spendingTxID, | ||
| f.deposit.TimeOutSweepPkScript, | ||
| DefaultConfTarget, heightHint, | ||
| ) |
There was a problem hiding this comment.
Can we handle spend, reorg and confirmation events in one loop (with WithReOrgChan on the spend subscription)? If sweep A confirms once, gets reorged out, and replacement B confirms, we currently keep waiting for A’s txid. On a spend reorg, we should cancel the old confirmation watch and follow the deposit’s next spender, then wait for its three confirmations. Otherwise B can confirm while the deposit stays stuck until loopd restarts.
| func (m *Manager) GetTaprootAddressFromScript(pkScript []byte) ( | ||
| *btcutil.AddressTaproot, error) { |
There was a problem hiding this comment.
I propose to make this a free function, not a method. Currently it looks like the manager has something to do with the address (e.g. makes sure the address belongs to it).
ChainParams can be passed explicitly as an argument.
This is PR 1 of 3 in the Static Address multi-address stack.
Static Address previously treated one legacy/root address as the owner of every
deposit. This PR introduces fresh receive and operation-specific change
addresses while preserving the address parameters that own each deposit.
Loop-ins and withdrawals can consequently spend deposits received across
multiple derived addresses, with each input signed and proven using its actual
address parameters.
Key Changes
across restarts.
address.
loop-ins and partial withdrawals.
errors for the expected Taproot output key.
of reconstructing and scanning every persisted address.
paying the same destination script cannot finalize a deposit.
loop static depositsupport for creating and optionally funding a freshaddress through lnd
SendCoins.RPC and Compatibility Changes
NewStaticAddressnow derives a fresh receive address on every request andnever funds it. Callers must not assume repeated requests are idempotent or
return the same address.
Wallet funding is handled by the separate
FundStaticAddressRPC, exposed asPOST /v1/staticaddr/fund. It creates and funds a fresh address whensend_coins_request.addris empty, or funds a known existing static addresswhen it is set. The
loop static depositcommand uses this endpoint.Funding requires
wallet:fundin addition toswap:executeandloop:in,or an explicit URI grant for
FundStaticAddress. Existing execute/in andNewStaticAddressURI macaroons cannot authorize funding. The default localLoop macaroon is regenerated on startup when permissions change; distributed
or custom funding macaroons must be explicitly updated.
Because address creation mutates wallet and database state, the RPC permission
changes from
swap:readtoswap:execute. Operators using custom scopedmacaroons must rebake them accordingly.
StaticAddressSummaryResponse.static_addressremains populated with thelegacy/root address for wire compatibility, but is deprecated and must not be
treated as the current receive address. Call
NewStaticAddressto derive afresh address.
Database and Recovery
The database now records:
Migrations backfill existing records using the legacy/root address. Store
reconstruction restores deposit ownership and loop-in and withdrawal change
metadata after restart.
Stacked PRs
pull/1215)
validates and follows the transactions that actually replace multi-address
withdrawals.
persists the confirmed HTLC output so recovery can rebuild the exact
server-published transaction.
The address startup and lookup hardening previously isolated in
#1214 has been folded into
this PR, and #1214 is now closed.
Testing
go test ./...go vet ./...go test -race ./staticaddr/address ./loopdmake docs-checkgit diff --checkRelease Notes
Release notes document the new address behavior, RPC permission change,
deprecated summary field, migration compatibility, and address lookup
hardening.