Skip to content

walletrpc: add RemoveAccount RPC - #11145

Draft
kaloudis wants to merge 5 commits into
lightningnetwork:masterfrom
kaloudis:remove-account-rpc
Draft

walletrpc: add RemoveAccount RPC#11145
kaloudis wants to merge 5 commits into
lightningnetwork:masterfrom
kaloudis:remove-account-rpc

Conversation

@kaloudis

Copy link
Copy Markdown
Contributor

Change Description

Adds a WalletKit.RemoveAccount RPC (and lncli wallet accounts remove) that
deletes a watch-only account previously registered via ImportAccount, along
with all addresses derived from it. Fixes #8654.

The wallet stops tracking the account's addresses; the funds themselves are
unaffected and re-importing the same extended public key (plus a rescan)
restores tracking. Since removal silently drops the account's balance and
UTXOs from the wallet's view, the lncli command asks for confirmation unless
--yes is given.

Guard rails on the wallet side:

  • The wallet's reserved accounts (default, imported), lnd's internal key
    scope, and accounts owned by the wallet itself (created via
    XCreateAccount) are refused — only watch-only imported accounts can be
    removed.
  • A name that resolves in more than one key scope — possible only on wallets
    that predate the cross-scope uniqueness check — must be disambiguated with
    an explicit address type, which maps to a key scope the same way
    ListAccounts resolves it (the mapping is extracted into a shared helper).
  • The accountMtx introduced for XCreateAccount now also serialises
    removal, since all three account mutations (CreateAccount,
    ImportAccount, RemoveAccount) do a check-then-act against the same
    name-uniqueness invariant across two database transactions.

Upstream dependency

The underlying removal is implemented in btcsuite/btcwallet#1352
(wallet.RemoveAccount, atomically removing the waddrmgr account and its
wtxmgr credits). Until that merges and lnd bumps its btcwallet dependency,
BtcWallet.RemoveAccount asserts for the capability at runtime (the
accountRemover interface, same pattern as accountCreator for
XCreateAccount) and reports the wallet as unsupported. This keeps the PR
compilable and reviewable against the current dependency, but the happy-path
itest fails until the bump — hence draft until #1352 lands and the go.mod
bump commit is added here.

Steps to Test

Unit tests (pass today):

go test ./lnwallet/btcwallet/... ./lnrpc/walletrpc/...

Integration tests:

make itest icase=wallet_remove_account

The rejections case (reserved accounts, default account, unknown account,
wallet-owned account) passes today. The happy path (import Carol's xpub into
Dave's node, fund it, remove with a non-zero balance, assert no trace across
restart, re-import the same xpub) requires the btcwallet dependency; it has
been verified end-to-end against btcsuite/btcwallet#1352 via local replace
directives.

Pull Request Checklist

Testing

Code Style and Documentation

Add a RemoveAccount method that removes a watch-only account previously
registered via ImportAccount, along with all addresses derived from it.
The wallet stops tracking the account's addresses; the funds themselves
are unaffected and re-importing the same extended public key (plus a
rescan) restores tracking.

The wallet's reserved accounts (default, imported), lnd's internal key
scope and accounts owned by the wallet itself (created via
CreateAccount) are refused. A name that resolves in more than one key
scope — possible only on wallets that predate the cross-scope
uniqueness check — must be disambiguated with an explicit scope.

btcwallet's base.Interface does not expose account removal yet, so the
implementation asserts for the capability at runtime, the same way
CreateAccount handles NextAccount. Until the btcwallet dependency
grows a RemoveAccount method, the call reports the wallet as
unsupported instead of failing to compile, which keeps this change
mergeable against the current dependency.
Expose the new WalletController method over the WalletKit service. The
RPC takes the account name plus an optional address type, which is only
needed to pick between several key scopes holding the same name on
wallets that predate the cross-scope uniqueness check, and returns the
properties of the account that was removed.

The address-type-to-key-scope mapping is extracted out of ListAccounts
into a shared helper so both RPCs resolve accounts the same way.
Wraps the new RemoveAccount RPC. Since removing an account silently
drops its balance and UTXOs from the wallet's view, the command asks
for confirmation before proceeding unless --yes is given.
The happy path imports Carol's account xpub into Dave's node, funds it,
removes it while it still holds a balance, and asserts no trace of the
account is left — in the account list, the address list or the wallet
balance — including across a restart. It then re-imports the same xpub,
proving removal returns the wallet to an importable state and that
derivation restarts identically, and pins the documented no-rescan
limitation: the earlier deposit is not rediscovered by the re-import.

The rejections test asserts the guard rails: reserved account names,
an empty or unknown name, and an account the wallet derived from its
own master key are all refused, the latter without damaging the
account.

NOTE: the happy path requires a btcwallet dependency that implements
RemoveAccount; until the upstream change lands and is bumped in go.mod,
the wallet reports removal as unsupported. The rejection paths all fire
before that capability check and pass against the current dependency.
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.

[feature]: Key Import: add ability to delete accounts

1 participant