Skip to content

wallet+db+waddrmgr: prep for address-manager Store routing#1237

Open
yyforyongyu wants to merge 11 commits into
btcsuite:impl-account-manager-storefrom
yyforyongyu:prep-address-manager-store
Open

wallet+db+waddrmgr: prep for address-manager Store routing#1237
yyforyongyu wants to merge 11 commits into
btcsuite:impl-account-manager-storefrom
yyforyongyu:prep-address-manager-store

Conversation

@yyforyongyu
Copy link
Copy Markdown
Collaborator

@yyforyongyu yyforyongyu commented May 18, 2026

Summary

Prep stack that lands the foundation pieces the upcoming address-manager
Store/cache routing PR (#1214) needs, without touching
wallet/address_manager.go routing or the kvdb address implementation.

Base branch: impl-account-manager-store (PR #1215).
Follow-up PR (kvdb impl + manager routing): impl-address-manager-store
(#1214).

What's in scope

  • waddrmgr exports:
    • export ManagedPubKeyAddressHasPrivateKey so the kvdb address store can
      derive the spendable-vs-watch-only signal without reaching into private
      fields
    • export the taproot script codec used by imported tap addresses so kvdb and
      SQL backends can encode/decode via the same path
  • db contract structural changes:
    • align the address metadata contract by surfacing account number/name,
      key-scope, master fingerprint, pubkey, script marker, watch-only, and
      used-state on AddressInfo
    • thread that metadata through AddressInfoRow plumbing in both pg and
      sqlite adapters
    • add ADR 0011 and derive SQL address IsUsed from EXISTS queries over
      utxos rather than persisting an addresses.used column
    • project account metadata in SQL address read queries so list/read paths do
      not perform per-address account lookups
  • SQL address creation:
    • populate returned address metadata from data already available in the write
      transaction, keeping create operations all-or-nothing from the caller's
      perspective
  • mocks/tests:
    • bump wallet mocks and tests to expose the expanded address contract ahead
      of the routing PR's test rewrites

What's NOT in scope

  • routing wallet/address_manager.go through Store
  • kvdb address-store implementation work (derived-create, import, list, type
    metadata, secrets) -- these land in the follow-up
    impl-address-manager-store PR (wallet: migrate address manager to db.Store #1214)
  • a persisted SQL addresses.used column; ADR 0011 intentionally avoids that
    column and uses the utxos table as the SQL source of truth

Test plan

  • GOWORK=off go test ./wallet ./wallet/internal/db/...
  • GOWORK=off make itest-db db=sqlite
  • GOWORK=off make itest-db db=postgres
  • GOWORK=off make lint-check

@yyforyongyu yyforyongyu force-pushed the impl-account-manager-store branch from a48b346 to ace5899 Compare May 18, 2026 06:13
@yyforyongyu yyforyongyu force-pushed the prep-address-manager-store branch from 9a4b083 to a948f75 Compare May 18, 2026 06:13
@github-actions
Copy link
Copy Markdown

Coverage Report for CI Build 26016618506

Warning

No base build found for commit a48b346 on impl-account-manager-store.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 81.5%

Details

  • Patch coverage: 93 uncovered changes across 5 files (207 of 300 lines covered, 69.0%).

Uncovered Changes

File Changed Covered %
wallet/internal/db/addresses_common.go 108 75 69.44%
wallet/internal/db/pg/addresses.go 77 55 71.43%
wallet/internal/db/sqlite/addresses.go 75 55 73.33%
wallet/internal/db/pg/accounts.go 20 11 55.0%
wallet/internal/db/sqlite/accounts.go 20 11 55.0%

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 7735
Covered Lines: 6304
Line Coverage: 81.5%
Coverage Strength: 80.07 hits per line

💛 - Coveralls

@yyforyongyu yyforyongyu force-pushed the impl-account-manager-store branch from ace5899 to bad6f5a Compare May 18, 2026 06:44
@yyforyongyu yyforyongyu force-pushed the prep-address-manager-store branch 5 times, most recently from 6f34422 to 1ae2cdd Compare May 18, 2026 07:35
@yyforyongyu yyforyongyu added this to the Introduce SQL store milestone May 18, 2026
@yyforyongyu yyforyongyu self-assigned this May 18, 2026
Comment thread wallet/internal/sql/pg/sqlc/addresses.sql.go Outdated
Comment thread wallet/internal/sql/sqlite/sqlc/addresses.sql.go Outdated
Comment thread wallet/internal/db/addresses_common.go Outdated
Copy link
Copy Markdown
Collaborator Author

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 (openai/gpt-5.5)

Comment thread wallet/db_ops.go Outdated
Comment thread wallet/internal/db/pg/addresses.go Outdated
Comment thread wallet/internal/db/pg/addresses.go Outdated
Comment thread waddrmgr/tlv.go
Comment thread wallet/internal/db/kvdb/utxostore.go Outdated
@yyforyongyu yyforyongyu force-pushed the prep-address-manager-store branch 2 times, most recently from b80a2a4 to fe1a600 Compare May 18, 2026 15:21
@yyforyongyu yyforyongyu force-pushed the impl-account-manager-store branch from bad6f5a to 2293674 Compare May 18, 2026 15:22
@yyforyongyu yyforyongyu force-pushed the prep-address-manager-store branch from fe1a600 to 9980cad Compare May 18, 2026 15:35
@yyforyongyu yyforyongyu force-pushed the impl-account-manager-store branch from 2293674 to bb4442b Compare May 18, 2026 15:36
@yyforyongyu yyforyongyu force-pushed the prep-address-manager-store branch 2 times, most recently from 3c873d9 to 22e38c9 Compare May 18, 2026 16:40
@yyforyongyu yyforyongyu force-pushed the impl-account-manager-store branch from bb4442b to 323428d Compare May 18, 2026 17:11
@yyforyongyu yyforyongyu force-pushed the prep-address-manager-store branch from 22e38c9 to f2f8777 Compare May 18, 2026 17:11
@yyforyongyu yyforyongyu force-pushed the impl-account-manager-store branch from 323428d to ffaca1a Compare May 18, 2026 17:47
@yyforyongyu yyforyongyu force-pushed the prep-address-manager-store branch from f2f8777 to 46046c0 Compare May 18, 2026 17:47
@saubyk saubyk added this to lnd v0.22 May 18, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in lnd v0.22 May 18, 2026
@saubyk saubyk moved this from Backlog to In progress in lnd v0.22 May 18, 2026
@yyforyongyu yyforyongyu force-pushed the impl-account-manager-store branch 4 times, most recently from d8ffd1f to 1a3c7b4 Compare May 20, 2026 18:13
@yyforyongyu
Copy link
Copy Markdown
Collaborator Author

LGTM 🪶
(claude-opus-4-7[1m])

Comment thread wallet/internal/db/addresses_common.go Outdated
Comment thread wallet/internal/db/addresses_common.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Prepares the wallet/db/waddrmgr layers for the upcoming address-manager Store/cache routing work by expanding the address metadata contract, making derived-address creation return/persist pubkeys when available, and standardizing taproot script encoding/decoding for imported taproot script addresses across backends.

Changes:

  • Extended db.AddressInfo / address row plumbing to include account display metadata, HasScript, and IsUsed (SQL derives used-ness via EXISTS on utxos per ADR 0011).
  • Updated derived-address creation flow to allow/persist an optional derived pubkey (DerivedAddressData.PubKey) and to determine the effective scope address schema from the account row.
  • Exported waddrmgr taproot TLV codec (EncodeTaprootScript/DecodeTaprootScript) and a helper to detect presence of private-key material on managed pubkey addresses.

Reviewed changes

Copilot reviewed 16 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
wallet/internal/sql/sqlite/sqlc/addresses.sql.go Regenerated sqlc code to project account fingerprint/master key and derived is_used in address reads.
wallet/internal/sql/sqlite/queries/addresses.sql Updated SQLite address read queries to include derived is_used and extra account/wallet fields.
wallet/internal/sql/sqlite/migrations/000006_addresses.up.sql Added schema commentary pointing to ADR rationale for deriving used-ness.
wallet/internal/sql/pg/sqlc/addresses.sql.go Regenerated sqlc code to project derived is_used and account/wallet metadata in address reads.
wallet/internal/sql/pg/queries/addresses.sql Updated Postgres address read queries to include derived is_used and extra account/wallet fields.
wallet/internal/sql/pg/migrations/000006_addresses.up.sql Added schema commentary pointing to ADR rationale for deriving used-ness.
wallet/internal/db/sqlite/addresses.go Threads account metadata into returned AddressInfo; persists derived pubkey; derives IsUsed.
wallet/internal/db/sqlite/accounts.go Adds derived-account address schema extraction from (purpose, coin_type).
wallet/internal/db/pg/addresses.go Threads account metadata into returned AddressInfo; persists derived pubkey; derives IsUsed.
wallet/internal/db/pg/accounts.go Adds derived-account address schema extraction from (purpose, coin_type).
wallet/internal/db/interface.go Extends DerivedAddressData with optional PubKey; clarifies transitional kvdb derive semantics.
wallet/internal/db/data_types.go Extends AddressInfo with account metadata + HasScript and IsUsed.
wallet/internal/db/addresses_common.go Adds account metadata conversion helpers; threads HasScript/IsUsed; updates derived address flow to carry pubkey and schema.
wallet/internal/db/addresses_common_test.go Updates shared helper test for new derived-address inputs/outputs.
waddrmgr/tlv.go Exports taproot script TLV codec; refactors control block record encoding.
waddrmgr/address.go Exports helper to detect private-key material on managed pubkey addresses.
docs/developer/adr/README.md Adds ADR 0011 entry to ADR index.
docs/developer/adr/0011-no-addresses-used-column.md New ADR documenting “derive used-ness from utxos; no used column” decision.
Files not reviewed (2)
  • wallet/internal/sql/pg/sqlc/addresses.sql.go: Language not supported
  • wallet/internal/sql/sqlite/sqlc/addresses.sql.go: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wallet/internal/db/sqlite/addresses.go Outdated
Comment thread wallet/internal/db/pg/addresses.go Outdated
Comment thread wallet/internal/sql/sqlite/queries/addresses.sql Outdated
Comment thread wallet/internal/sql/sqlite/queries/addresses.sql Outdated
Comment thread waddrmgr/address.go
Comment thread docs/developer/adr/0011-no-addresses-used-column.md Outdated
@yyforyongyu
Copy link
Copy Markdown
Collaborator Author

LGTM 🧱
(claude-opus-4-7[1m])

Comment thread wallet/internal/db/pg/accounts.go
Comment thread wallet/internal/db/pg/accounts.go
Comment thread wallet/internal/db/sqlite/accounts.go
Comment thread wallet/internal/db/sqlite/accounts.go
Comment thread wallet/internal/db/sqlite/addresses.go
Comment thread wallet/internal/db/sqlite/accounts.go Outdated
@yyforyongyu
Copy link
Copy Markdown
Collaborator Author

yyforyongyu commented May 21, 2026

LGTM 🪶 (openai/gpt-5.5)

SQL backends derive AddressInfo.IsUsed via EXISTS(utxos) rather than
persisting a column. Implementation lands in later commits on this
branch.
Expose the taproot script TLV codec so wallet-side import routing can
persist encrypted script material through the store seam.
Expose whether a managed public-key address carries encrypted
private-key material. The kvdb adapter's address lookup uses this
in the watch-only address-info computation: an imported address
under a spendable wallet is watch-only at the address row when
its underlying `managedAddress` has no encrypted private key.

Extracted from the original `kvdb+waddrmgr: add address lookup`
commit so the waddrmgr export sits on prep-address-manager-store
and the kvdb consumer stays on impl-address-manager-store.
Disambiguates P2TR key-path vs script-path imports — output type
alone is ambiguous.
PubKey return on derivedAddressInput + DerivedAddressData. addrSchema
parameter so derivedAddressInput honors per-account overrides instead of
re-reading scope default. Adds GetAccountAddrSchema adapter field with
pg/sqlite scope-default implementations.
Adds AccountNumber, AccountName, KeyScope, MasterKeyFingerprint to
AddressInfo and AddressInfoRow. Introduces convertAccountMetadata +
convertAddressAccountMetadata + ApplyAddressAccountMetadata so SQL
adapters can populate these fields on create + read paths.
SELECT account_master_fingerprint + wallet_master_hd_pub_key from
the wallets/accounts JOIN already present in GetAddressByScriptPubKey
and ListAddressesByAccount. Regenerates sqlc.
Create + read paths now route through the new helper so AddressInfo
returns AccountNumber + AccountName + KeyScope + MasterKeyFingerprint
on both backends.
Move the shared SQL derived-address adapter helpers out of the mirrored pg/sqlite closures before wiring IsUsed query projections.

The account-schema helper uses the persisted key-scope address type IDs so SQL derived address creation honors stored scope schema overrides.
See ADR 0011 for SQL/kvdb derivation asymmetry.
Wires GetAddressByScriptPubKey + ListAddressesByAccount through EXISTS(SELECT 1 FROM utxos WHERE address_id = ?). The Store interface has no SQL MarkAddressUsed method: the tx-record path already writes the utxos row that the EXISTS derivation reads.

Annotates 000006_addresses.up.sql so a future contributor reaching for the column finds ADR 0011.
@yyforyongyu
Copy link
Copy Markdown
Collaborator Author

LGTM 🚀 (gemini-3.1-pro-preview)

@yyforyongyu
Copy link
Copy Markdown
Collaborator Author

LGTM 🪡
(claude-opus-4-7[1m])

@yyforyongyu
Copy link
Copy Markdown
Collaborator Author

LGTM 🧪 (openai/gpt-5.5)

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.

3 participants