wallet: deprecate the legacy account RPCs (+ migratelabels)#3099
Open
PrestackI wants to merge 4 commits into
Open
wallet: deprecate the legacy account RPCs (+ migratelabels)#3099PrestackI wants to merge 4 commits into
PrestackI wants to merge 4 commits into
Conversation
Promote mapAddressBook's value from a bare std::string to a first-class
CAddressBookData{name, purpose}, mirroring Bitcoin 0.17. The single string
previously served simultaneously as the address-book label and the legacy
account grouping key; this separates the label half so it can survive
independently of the account system that is slated for retirement (gridcoin-community#3086).
- Add CAddressBookData (name + "unknown"-defaulted purpose). No implicit
std::string conversion and no unit serialization: the two fields persist
as separate "name"/"purpose" walletdb records assembled at load, and every
consumer must read .name explicitly so the disentanglement surfaces at
compile time.
- Add SetAddressBookPurpose and WritePurpose/ErasePurpose; ReadKeyValue loads
the new "purpose" record alongside "name". Old wallets (name only) default
purpose to "unknown" with no migration write, preserving downgrade safety.
- DelAddressBookName erases both records on one CWalletDB with no short-circuit.
- NotifyAddressBookChanged carries the purpose (6-arg); emits, the Qt free
function, and the bind placeholders update in lockstep. The GUI slot stays
4-arg for now (purpose accepted but not yet surfaced).
- Migrate all value-read sites to .name across wallet, rpc and qt.
- Add addressbook_tests: struct defaults, name/purpose round-trip, legacy
name-only compat, del-erases-both, and the new signal arity.
Wallet-local, no consensus impact.
Add the Bitcoin 0.17 label RPC surface that Gridcoin skipped, built on the
CAddressBookData storage from the previous commit. These give first-class,
account-free access to address-book labels:
- setlabel(address, label): sets the label and tags purpose "receive" for
owned addresses, "send" otherwise.
- getaddressesbylabel(label): returns the Bitcoin-shaped object keyed by
address with a {purpose} value; throws RPC_WALLET_INVALID_LABEL_NAME when no
address carries the label.
- listlabels(purpose?): sorted, de-duplicated label names, optionally filtered
by purpose, excluding the implicit empty default label.
Adds a LabelFromValue helper (mirrors AccountFromValue), registers all three in
server.h (function + helpman externs) and the server.cpp dispatch table, wires
them into the rpchelpman help-render coverage, and extends addressbook_tests
with RPC-level round-trip, unknown-label, and purpose-filter cases.
Wallet-local, no consensus impact.
Phase C of retiring the accounts subsystem (gridcoin-community#3086): now that labels are first-class (CAddressBookData) and the label RPCs exist, mark the account surface deprecated and gate the parts that have no label replacement. - Bridge RPCs keep working, with a one-time deprecation log warning: getaccount returns the label, getaddressesbyaccount lists addresses, and setaccount now also tags purpose like setlabel. setaccount's account-only "unused current key" fixup (no label analogue) runs only under -enableaccounts. - Account-only RPCs are gated: sendfrom (spends an account-ledger balance) and the named-fromaccount path of sendmany take the full accounting gate (-enableaccounts + staking=0); getaccountaddress takes a lighter gate (-enableaccounts only -- it touches no ledger, so it must not force staking off). `sendmany "" {...}` and `getbalance "*"` stay ungated. - The on-disk acentry/acc ledger is preserved read-only: nothing is deleted, and it stays usable under -enableaccounts; record removal is left to the later removal phase. accounting_tests is therefore unaffected. Adds addressbook_tests cases for the bridge RPCs working flag-free and the account-only RPCs throwing without -enableaccounts. Wallet-local, no consensus impact.
During the accounts deprecation window, give users a one-time maintenance
command for the labels transition. Address-book *names* need no conversion —
an account name already is the label (shared field) — but entries that predate
the label system load with purpose "unknown", so `listlabels "receive"` won't
find them until re-set.
`migratelabels` walks the address book and backfills purpose on every
"unknown" entry: owned addresses become "receive", others "send". It returns
{examined, updated}, leaves entries that already have a purpose untouched, and
is safe to run repeatedly.
Registered in server.h/server.cpp and the rpchelpman help-render coverage;
addressbook_tests gains a backfill + idempotency case.
Wallet-local, no consensus impact.
PrestackI
force-pushed
the
wallet/deprecate-accounts
branch
from
e6581c5 to
19db0be
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 3 of 3 in retiring the legacy accounts system (#3086). Stacked on #3098 → #3097.
What this adds
Now that labels are first-class (#3097) and the label RPCs exist (#3098), mark the account
surface deprecated and gate the parts with no label replacement.
getaccountreturnsthe label,
getaddressesbyaccountkeeps its flat-array shape,setaccountnow also tagspurpose like
setlabel.setaccount's account-only "unused current key" fixup (no labelanalogue) runs only under
-enableaccounts.-enableaccountsmechanism:sendfromand thenamed-
fromaccountpath ofsendmanytake the full accounting gate (-enableaccounts+staking=0);getaccountaddresstakes a lighter gate (-enableaccountsonly — it touchesno ledger, so it must not force staking off).
sendmany ""andgetbalance "*"stayungated (those sentinels are baseline functionality, not account usage).
acentry/accrecords andCAccountingEntrymachinery are untouched and still usable under
-enableaccounts; nothing is deleted.accounting_testsstays green. Record removal is left to the later removal phase.migratelabels— one-time maintenance RPC for the deprecation window: backfills purpose(
receive/send) on address-book entries that predate labels (loaded asunknown).Names need no conversion — an account name already is the label — so this only fills the
missing purpose. Returns
{examined, updated}, idempotent.addressbook_testsgains cases for the bridge RPCs working flag-free, the account-only RPCsthrowing without
-enableaccounts, and themigratelabelsbackfill/idempotency.Not included — removal
Removal (deleting the RPCs, the ledger machinery,
-enableaccounts) is deliberately left forafter a deprecation window — see the discussion on #3086. Open questions there: window
length, and the fate of existing
acentry/accrecords on disk.Wallet-local, no consensus impact. Refs #3086.