Skip to content

multi: add labels for static addresses#1178

Open
GustavoStingelin wants to merge 6 commits into
lightninglabs:masterfrom
GustavoStingelin:feat/static-address-label
Open

multi: add labels for static addresses#1178
GustavoStingelin wants to merge 6 commits into
lightninglabs:masterfrom
GustavoStingelin:feat/static-address-label

Conversation

@GustavoStingelin

Copy link
Copy Markdown

Adds persistent, locally managed labels for static addresses.

  • Stores labels in LoopDB
  • Exposes labels through RPC responses and CLI commands
  • Supports updating and clearing labels
  • Associates labels with the address and UTXOs

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Comment on lines +134 to +139
// TODO: When multi-address support lands, resolve an existing address
// by label or create a new one when no matching label exists.
params := addresses[0]
clientPubKey := params.ClientPubkey
serverPubKey := params.ServerPubkey
expiry := int64(params.Expiry)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: reviewers, pls, pay attention here.

@GustavoStingelin
GustavoStingelin force-pushed the feat/static-address-label branch from ae23a9d to 92270e0 Compare July 17, 2026 21:32
@starius

starius commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

I propose to include the current state of #1139 into this PR, so own label commits go after @hieblmi's commits. Add a note to the PR description not to review the multi-address commits.

I attempted to do a rebase using AI, here is my attempt: https://github.com/starius/loop/commits/staticaddr-combined/ It is not functional.

Some semantic rebase is needed. For example:

  • PR 1178 adds 000022_static_address_label, while PR 1139 already uses migrations 22–24 for multi-address persistence.
  • PR 1178 assigns protobuf field 2 in NewStaticAddressRequest to label; PR 1139 assigns field 2 to send_coins.
  • PR 1178 assigns protobuf field 3 in NewStaticAddressResponse to label; PR 1139 assigns field 3 to send_coins_response.

Found some issues in that multi-address + labels branch that I got:

  • the multi-address manager returns an internal *Parameters pointer after releasing its lock, while label update mutates that same object. go test -race reports a real concurrent read/write race. repro
  • after multi-address is applied, static new returns a derived receive address, but static summary still reads the legacy/root seed. The two commands no longer describe the same address. repro
  • every generated change address is stored with an empty label, regardless of the labels on the consumed deposits. I think labels should be inherited by change - that is a common practice among wallets
  • testing gap: the SQL tests read the label back through the same open database/store instance. They do not close and reopen the database or rebuild the manager cache.
  • CLI: the shared --label help text says "label for this swap" and lacks a space, while static deposit --label labels the lnd funding transaction and static new --label labels the address.
  • ListUnspentDeposits includes the label, but historical deposit/list responses carry the address without the corresponding label - needs unification

Some of them will be resolved by a proper rebase, but some seem genuine.

@GustavoStingelin

Copy link
Copy Markdown
Author

Okay, if we agree to rebase this on top of #1139, I’d prefer to wait until @hieblmi has finished it and its CI is green. This should avoid premature conflicts and duplicated work from having to address #1139’s current race failures in the combined branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants