wallet: disentangle address-book labels from accounts (storage)#3097
Open
PrestackI wants to merge 1 commit into
Open
wallet: disentangle address-book labels from accounts (storage)#3097PrestackI wants to merge 1 commit into
PrestackI wants to merge 1 commit into
Conversation
9c12efe to
43d55b4
Compare
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.
43d55b4 to
22b8554
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
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.
First step of retiring the legacy accounts system (#3086), sequenced safely behind a labels disentanglement. Wallet-local, no consensus impact — no fork.
Why
Bitcoin deprecated accounts in 0.17 and removed them in 0.18; crucially 0.17 first introduced the label system so labels could survive the account removal. Gridcoin kept accounts and never took the 0.17 label step. In Gridcoin
mapAddressBookisstd::map<CTxDestination, std::string>— that one string is simultaneously the address-book label (load-bearing: GUI address book,listreceivedbyaddress, transaction display) and the legacy account grouping key. They cannot be cleanly separated until the label half is formalized.What this PR does (Phase A — storage)
Promotes
mapAddressBook's value to a first-classCAddressBookData{name, purpose}, mirroring Bitcoin 0.17, so labels become independent of accounts.CAddressBookDatawithname+"unknown"-defaultedpurpose. No implicitstd::stringconversion and no unit serialization — the two fields persist as separatename/purposewalletdb records assembled at load, and every consumer must read.nameexplicitly (the disentanglement surfaces at compile time).SetAddressBookPurpose+WritePurpose/ErasePurpose;ReadKeyValueloads the newpurposerecord alongsidename. Old wallets (name only) defaultpurposeto"unknown"with no migration write — downgrade-safe.DelAddressBookNameerases both records on oneCWalletDBwith no short-circuit.NotifyAddressBookChangedcarriespurpose(6-arg); emits, the Qt free function, and bind placeholders updated in lockstep. The GUI slot stays 4-arg for now (purpose accepted but not yet surfaced)..nameacross wallet/rpc/qt.Tests
New
addressbook_tests: struct defaults, name/purpose round-trip, legacy name-only compat, del-erases-both, and the new signal arity.accounting_testsis unaffected (it exercises only theCAccountingEntryledger).Sequence
This is PR1 of 3. Phase B (label RPCs
setlabel/getaddressesbylabel/listlabels) and Phase C (deprecate the account surface) will follow as stacked PRs. Marked draft pending that sequencing.Refs #3086.