Page MenuHomePhabricator

Merge #12892: [wallet] [rpc] introduce 'label' API for wallet
AcceptedPublic

Authored by nakihito on Mon, Aug 26, 20:38.

Details

Reviewers
deadalnix
Fabien
jasonbcox
markblundeberg
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

41ba061 [docs] Add release notes for wallet 'label' API. (John Newbery)
189e0ef [wallet] [rpc] introduce 'label' API for wallet (Wladimir J. van der Laan)

Pull request description:

Add label API to wallet RPC.

This is one step towards #3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.

These initially mirror the account functions, with the following differences:

- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
  associated with addresses, instead of addresses associated with labels. (unlike
  with accounts.)
- Labels have no balance
  - No balances in `listlabels`
  - `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
  Being unable to delete them is a common annoyance (see #1231).
  Currently only by reassigning all addresses using `setlabel`, but an explicit
  call `deletelabel` which assigns all address to the default label may make
  sense.

Tree-SHA512: 45cc313c68ad529ce3a15c02181d2ab0083a7e14fe824e2cde34972713fecce512e3d4b9aa46db5355f2baa857c44b234d4fe9709225bc23c7ebbc0e03febbf5

Backport of Core PR12892
https://github.com/bitcoin/bitcoin/pull/12892/

Depends on D3308

Test Plan
make check
test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR12892
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7293
Build 12631: Bitcoin ABC Teamcity Staging
Build 12630: arc lint + arc unit

Event Timeline

nakihito created this revision.Mon, Aug 26, 20:38
Owners added a reviewer: Restricted Owners Package.Mon, Aug 26, 20:38
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Aug 26, 20:38
Fabien requested changes to this revision.Tue, Aug 27, 10:13
Fabien added inline comments.
src/wallet/rpcwallet.cpp
375 ↗(On Diff #10965)

Nit: : => ., or remove uppercase If => if

4489 ↗(On Diff #10965)

Move the block between walletpassphrase and generate.

test/functional/timing.json
403 ↗(On Diff #10965)

This is a duplicate (see 4 lines above).
You don't have to edit this file when updating a test unless there is a massive timing change from the previous version, which is obviously not the case here.

This revision now requires changes to proceed.Tue, Aug 27, 10:13
nakihito added inline comments.Tue, Aug 27, 17:55
src/wallet/rpcwallet.cpp
4489 ↗(On Diff #10965)

So it should look like:

{ "wallet",             "walletpassphrase",             walletpassphrase,             {"passphrase","timeout"} },
/** Account functions (deprecated) */
{ "wallet",             "getaccountaddress",            getlabeladdress,              {"account"} },
...
{ "wallet",             "setlabel",                     setlabel,                     {"address","label"} },
{ "generating",         "generate",                     generate,                     {"nblocks","maxtries"} },
};

?

nakihito updated this revision to Diff 11037.Thu, Aug 29, 20:52

Addressed nits and reverted timing.json change.

Fabien requested changes to this revision.Fri, Aug 30, 12:23

Please rebase on top of https://reviews.bitcoinabc.org/D3977

src/wallet/rpcwallet.cpp
4489 ↗(On Diff #10965)

Almost, look at what core does, you are missing the empty lines as a separator.

This revision now requires changes to proceed.Fri, Aug 30, 12:23
nakihito updated this revision to Diff 11048.Fri, Aug 30, 17:51

Rebased and moved label and account function blocks inline with Core's placement.

jasonbcox accepted this revision.Wed, Sep 4, 18:10
Fabien accepted this revision.Thu, Sep 12, 20:01
This revision is now accepted and ready to land.Thu, Sep 12, 20:01