[v0.21.x-branch] Backport #11065: walletrpc: add XCreateAccount for wallet-derived accounts - #11086
Conversation
Declares the RPC and its messages, and regenerates the stubs. The implementation follows in the next commits. The account's address type selects the BIP-0043 key scope it is created under, which is permanent and also fixes the address type of its change outputs, so the proto spells out both that pairing and the fact that a seed-only recovery does not rediscover funds held in such an account. The X prefix follows XImportMissionControl and the XAddLocalChanAliases family: it marks the API as experimental, so it may change or be removed without the usual deprecation period. It comes off once a seed-only recovery can find these accounts. (cherry picked from commit 9a6a3a7)
Adds the wallet-side operation the RPC will call, implemented by BtcWallet through btcwallet's Wallet.NextAccount. Unlike ImportAccount, which registers a watch-only account from an extended public key and whose inputs the wallet can therefore never sign, the account created here is derived from the wallet's master key and is fully spendable. NextAccount is reached through a local interface assertion rather than by widening btcwallet's base.Interface, so lnd does not have to carry a forked btcwallet: a replace directive here would not propagate to modules that depend on lnd and each of them would have to duplicate it. Duplicate names are rejected across every key scope, not just the requested one, because coin selection resolves a custom account through lookupFirstCustomAccount, which returns whichever scope matches first; the same name under two scopes would make later funding calls ambiguous. The wallet's own reserved names are refused too. RPCKeyRing refuses the operation outright. It embeds the WalletController interface, so it would otherwise promote this implementation and fail deep inside waddrmgr with a bare "watching-only wallet"; creating the account on the remote signer and importing its extended public key is the supported path. The same lock is taken by ImportAccount, which does the identical check-then-act against the same namespace and would otherwise let a concurrent pair create one name under two key scopes from the other side of the invariant. Note one deliberate contract change: keyScopeForAccountAddr now reports a wrong-scope lookup as an untyped error naming the scope the account does live in, rather than passing through waddrmgr's typed ErrAccountNotFound. Nothing outside this package inspects that code, and the bare "not found" it replaces is the reason this edge has had to be worked around several times downstream. (cherry picked from commit 634b104)
Maps the requested address type onto the key scope the account is created in, mirroring ListAccounts, and defaults an unset type to taproot. NESTED_WITNESS_PUBKEY_HASH is rejected rather than served. An account derived by the wallet stores no address schema of its own, so BIP-0049Plus always behaves as the hybrid scheme; honouring the strict request is impossible here and silently substituting the hybrid one would return an account whose change outputs are not what the caller asked for. ImportAccount can honour the distinction because it passes an address schema through. Creation is additionally gated on an explicit acknowledgement, following AbandonChannel: a dev build passes, and a release build requires i_know_what_i_am_doing. Funds held in a created account are not rediscovered by a seed-only restore, so a caller has to state that it accepts that before one is made. The X prefix alone does not carry that, and a dev-build-only gate would not either: release images are what real deployments run, so it would have put the RPC out of reach of exactly the nodes that need the isolation. The acknowledgement keeps it unreachable by accident while leaving it usable by an operator who has read what they are signing up for. (cherry picked from commit 9a41438)
|
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-11065-to-v0.21.x-branch
git worktree add --checkout .worktree/backport-11065-to-v0.21.x-branch backport-11065-to-v0.21.x-branch
cd .worktree/backport-11065-to-v0.21.x-branch
git reset --hard HEAD^
git cherry-pick -x 9664abd4f4c95d8e128853e0364e3b4e293fda68
git push --force-with-lease |
5ceee36 to
adadeda
Compare
🔴 PR Severity: CRITICAL
🔴 Critical (4 files)
🟠 High (8 files)
🟡 Medium (1 file)
🟢 Low (7 files)
AnalysisThis PR adds a new Because this adds a new account-derivation code path to the wallet layer (affecting key/address derivation), it warrants careful review from someone familiar with To override, add a |
Exercises the property the RPC exists for and that a unit test cannot reach: an account derived from the wallet's master key is not watch-only, its funds are reported against it rather than the default account, and the wallet can sign a spend from it. An imported account gets as far as funding a PSBT, since that needs only public data, and fails at finalize; publishing the signed transaction and asserting it confirms is what separates the two. Also covers the requests lnd refuses: a duplicate name in any key scope, the wallet's reserved names, an empty name, and the strict nested-witness type, which a wallet-derived account cannot honour. (cherry picked from commit b9d2157)
The address type is optional and defaults to taproot, matching the RPC, because the choice selects the account's key scope and is permanent for its lifetime. (cherry picked from commit c658190)
adadeda to
34eda61
Compare
XCreateAccount is being backported to the v0.21.x branch in #11086, so its entries belong with the release that first ships it rather than with 0.22.0. This matches how the other changes backported to 0.21.3 (#11035, #11075, #10869) are documented: their entries live only in the 0.21.3 notes, even though their code is on master and will also ship in 0.22.0. The entry text is moved verbatim. Elle Mouton is added to the 0.21.3 contributor list.
(cherry picked from commit 9664abd)
34eda61 to
23fd4f7
Compare
ziggie1984
left a comment
There was a problem hiding this comment.
LGTM (merge conflict was the wrong Release Notes which got fixed also on master with: #11091)
Backport of #11065
Motivation
lnd can already confine coin selection, change, balance, address derivation and
signing to a named wallet account —
FundPsbt,FinalizePsbt,ListUnspent,NextAddr,WalletBalanceandListTransactionsall take one. What is missingis a way to create such an account.
The only account-creating RPC today is
ImportAccount, which registers awatch-only account from an extended public key. The wallet stores no account
private key for it, so
waddrmgrcan only derive public keys for its addressesand
SignPsbt/FinalizePsbtsilently skip its inputs. That makes it unusablefor partitioning one wallet into isolated pockets of funds, which is what two
applications sharing a single lnd node need in order not to spend each other's
coins.
btcwallet already supports this via
Wallet.NextAccount, which derives a newaccount from the wallet's master key. lnd simply never exposed it.
Changes
Split so each commit stands alone:
walletrpc: define the XCreateAccount RPC— proto, REST annotation, regen.lnwallet: add CreateAccount to the WalletController interface— theBtcWalletimplementation, the two mocks, and an explicit refusal inRPCKeyRing.walletrpc: implement the XCreateAccount RPC— the handler.itest: cover XCreateAccount end to end.lncli: add wallet accounts create command.docs: add release notes for XCreateAccount.Notes for reviewers
Duplicates are rejected across all key scopes, not just the requested one.
Coin selection resolves a custom account through
lookupFirstCustomAccount,which returns whichever scope matches first, so the same name under two scopes
would make later funding calls ambiguous.
NESTED_WITNESS_PUBKEY_HASHis rejected. A wallet-derived account storesno address schema, so BIP-0049Plus always behaves as the hybrid scheme.
Accepting the strict type would hand back an account whose change outputs are
not what was asked for.
ImportAccountcan honour the distinction because itpasses an
addrSchemathrough;NextAccountcannot.Remote signing is refused explicitly.
RPCKeyRingembeds theWalletControllerinterface, so without an override it would promoteBtcWallet's implementation and fail deep insidewaddrmgrwith a bare"watching-only wallet". Creating the account on the signer and importing its
xpub here is the supported path.
Two caveats are documented in the proto and are worth a second opinion:
a later
NextAddrmust ask for the same address type or the account willappear not to exist.
account: lnd's recovery scan only derives addresses for account 0. Ordinary
rescans are unaffected.
BtcWallet.CreateAccountreachesNextAccountthrough a local interfaceassertion rather than widening btcwallet's
base.Interface, so lnd does nothave to carry a forked btcwallet — a
replacehere would not propagate tomodules that depend on lnd, and each of them would have to duplicate it. Happy
to do the btcwallet PR instead; the assertion is marked for removal if so.
Testing
Unit tests cover the guards, key-scope forwarding, the unsupported-wallet path
and error wrapping.
make rpc-checkand the REST-annotation check pass.There are also two itests, with the matching
HarnessRPChelpers. The firstcovers the property a unit test cannot reach: create →
NewAddress→ fund →FundPsbt→FinalizePsbt→ publish, asserting the balance lands on the newaccount and not the default one, and that the spend confirms. Funding a PSBT
works for a watch-only account too, since it needs only public data — it is
finalizing and confirming that separates a wallet-derived account from an
imported one. The second covers the refusals: duplicate names across key
scopes, reserved names, an empty name, and the strict nested-witness type.
🤖 Generated with Claude Code