rpc: deprecate getaddressinfo label #17585

pull jonatack wants to merge 5 commits into bitcoin:master from jonatack:deprecate-getaddressinfo-label changing 10 files +75 −33
  1. jonatack commented at 1:56 AM on November 25, 2019: contributor

    This PR builds on #17578 (now merged) and deprecates the rpc getaddressinfo label field. The deprecated behavior can be re-enabled by starting bitcoind with -deprecatedrpc=label.

    See http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 and #17283 (comment) for more context.

    Reviewers: This PR may be tested manually by building, then running bitcoind with and without the -deprecatedrpc=label flag while verifying the rpc getaddressinfo output and help text.

    Next step: add support for multiple labels.

  2. jonatack cross-referenced this on Nov 25, 2019 from issue rpc: improve getaddressinfo test coverage, help, code docs by jonatack
  3. fanquake added the label RPC/REST/ZMQ on Nov 25, 2019
  4. jonatack force-pushed on Nov 25, 2019
  5. DrahtBot commented at 4:16 AM on November 25, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17958 (rpc: query general daemon information via RPC by brakmic)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. jonatack cross-referenced this on Nov 25, 2019 from issue rpc: simplify getaddressinfo labels, deprecate previous behavior by jonatack
  7. laanwj commented at 3:48 PM on November 25, 2019: member

    I'm not sure about this. It wasn't too long ago that we deprecated "account" here. Now we're deprecating "label" too. For most purposes (like identifying invoices), assigning one label per address seems to be enough. If someone uses multiple labels per address then yes, a single "label" won't do. But I'm not sure it's worth the hassle for all current clients.

  8. jonatack commented at 4:17 PM on November 25, 2019: contributor

    Thanks. It seems to me that the current state of both label and labels being returned might be confusing. Based on the previous two wallet meeting discussions, at the moment I imagine one of these options going forward:

    1. deprecate label, and add multiple label capability to labels (seemed to be the most popular option)

    2. keep label, and deprecate labels

    3. keep label, and change labels to tags as a separate feature from the label

  9. laanwj commented at 11:44 AM on November 26, 2019: member

    Maybe add 'label' only if there's a single label. Clients that don't use multiple-label-per-address functionality will be unaffected, while there's no need to guess what to return if there's multiple when you are using this.

  10. jnewbery commented at 3:11 PM on November 26, 2019: member

    Concept ACK. Returning the same information in two places in the same RPC call is redundant and confusing (especially when we add support for multiple labels per address, as I believe has always been the plan).

    change labels to tags as a separate feature from the label

    Please no!

    Maybe add 'label' only if there's a single label.

    I think it adds unnecessary complexity to clients to have the structure of the return object change based on values.

    A few comments on the approach (same comments as for #17578):

    • squash the first two commits (RPC changes and test changes) to avoid breaking git bisect
    • add more text to the deprecated RPC help text (what the user should do to retain the old behaviour and when the old behaviour will be removed entirely)
    • move the deprecation test to rpc_deprecated.py
  11. laanwj referenced this in commit d8a66626d6 on Nov 26, 2019
  12. laanwj commented at 12:11 PM on November 27, 2019: member

    I think it adds unnecessary complexity to clients to have the structure of the return object change based on values.

    Could return it as "" then. That already happens if there's no label so clients can cope with it.

    I would be the last to complain about breaking backwards compatibility for good reason, but I really think we're going overboard here with RPC deprecation changes every release, but okay, I seem to be the only one with that opinion.

  13. laanwj commented at 12:28 PM on November 27, 2019: member

    whoops :blush: I think I'm confused here. joinmarket, for example, already switched to using labels in 2018

    commit b52bc06acfead65c2b88ee4f64af44bc223656e8
Author: chris-belcher <chris-belcher@users.noreply.github.com>
Date:   Thu Sep 6 18:03:17 2018 +0100

    Switch over to using labels instead of accounts
    
    Bitcoin Core 0.17 deprecates the accounts feature and replaces it with
    labels. The RPC functions using accounts still work for use with older
    version of Core.

    I thought it was something recently introduced, but it was added in 189e0ef33ec66f03abf85cfd4d0ede1a0c5c02d3 in the initial introduction of the labels API in 2016. I'm not exactly sure why a single "label" ever got added.

    • 0.16: did not have getaddressinfo at all, but validateaddress, with a account field (not label)
    • 189e0ef33ec66f03abf85cfd4d0ede1a0c5c02d3: labels was added
    • 109e05dcd163b8ddb7f4b3550db6b9ab833b2c04 : label was added
    • 0.17: getaddressinfo was introduced, with both label and labels already there

    My confusion came from that I thought there was an intermediate release with only label (but it's even the other way around, labels was first!). Anyhow, no problem here. Concept ACK.

  14. sidhujag referenced this in commit 44df8dbe56 on Nov 27, 2019
  15. DrahtBot added the label Needs rebase on Dec 13, 2019
  16. jonatack force-pushed on Dec 13, 2019
  17. DrahtBot removed the label Needs rebase on Dec 13, 2019
  18. meshcollider referenced this in commit 7ea3b85ecf on Jan 7, 2020
  19. DrahtBot added the label Needs rebase on Jan 7, 2020
  20. sidhujag referenced this in commit 5c7fa94863 on Jan 8, 2020
  21. doc: address pr17578 review feedback
    - https://github.com/bitcoin/bitcoin/pull/17578#discussion_r363975411
- https://github.com/bitcoin/bitcoin/pull/17578#discussion_r363969721
- https://github.com/bitcoin/bitcoin/pull/17578#discussion_r362703553
    c7654af6f8
  22. test: remove getaddressinfo label tests dc0cabeda4
  23. rpc: deprecate getaddressinfo label field d48875fa20
  24. test: getaddressinfo label deprecation test 72af93f364
  25. jonatack force-pushed on Jan 9, 2020
  26. jonatack commented at 5:18 PM on January 9, 2020: contributor

    Rebased and addressed the remaining nits in #17578.

  27. DrahtBot removed the label Needs rebase on Jan 9, 2020
  28. in doc/release-notes-17585.md:1 in 77b340e77a outdated 
       0 | @@ -0,0 +1,6 @@
   1 | +Deprecated or removed RPCs
    jnewbery commented at 4:46 PM on January 10, 2020:

    This release note could be added to release-notes-17578.md and restructured to something like:

    - The `getaddressinfo` return object has been changed:
  - the `label` field has been deprecated and will be removed...
  - the `labels` field now returns an array...

    I think that will be a bit easier to parse in the final release notes.

    The only reason we have separate release note files for different PRs is to avoid merge conflicts. That's not a problem here because 17578 has already been merged, so it's fine to just add to that file.

    jonatack commented at 12:38 PM on January 11, 2020:

    Thanks! Good idea; done.

  29. jnewbery commented at 5:00 PM on January 10, 2020: member

    Code review ACK 77b340e77aa6faa03feedb04f18bb9a7f7e9e8a3

    One minor doc nit, which you can ignore.

  30. jonatack force-pushed on Jan 11, 2020
  31. doc: update release notes with getaddressinfo label deprecation d3bc184081
  32. jonatack force-pushed on Jan 11, 2020
  33. jonatack commented at 12:47 PM on January 11, 2020: contributor

    Thanks for reviewing and the info/suggestion, @jnewbery! Updated the last release note commit.

  34. jnewbery commented at 10:14 PM on January 13, 2020: member

    ACK d3bc18408146e91b3836f72360ff6fa2420b6887

  35. laanwj commented at 12:29 PM on January 29, 2020: member

    ACK d3bc18408146e91b3836f72360ff6fa2420b6887

  36. meshcollider commented at 8:32 AM on February 2, 2020: contributor

    utACK d3bc18408146e91b3836f72360ff6fa2420b6887

  37. pull[bot] referenced this in commit 6d0e532ae0 on Feb 2, 2020
  38. meshcollider merged this on Feb 2, 2020
  39. meshcollider closed this on Feb 2, 2020
  40. jonatack deleted the branch on Feb 2, 2020
  41. sidhujag referenced this in commit 97c86196db on Feb 3, 2020
  42. jonatack cross-referenced this on Feb 10, 2020 from issue Newsletters: add 84 (2020-02-12) by harding
  43. MarkLTZ cross-referenced this on Apr 4, 2020 from issue Bitcoin PR tracking by MarkLTZ
  44. jonatack cross-referenced this on Jun 7, 2020 from issue rpc: remove deprecated getaddressinfo fields by jonatack
  45. meshcollider referenced this in commit 02b26ba1c1 on Jun 21, 2020
  46. sidhujag referenced this in commit 0039aa5c7a on Jul 7, 2020
  47. nopara73 cross-referenced this on Jul 22, 2020 from issue Update to Bitcoin Core 0.20.0 by nopara73
  48. sidhujag referenced this in commit c81cfe7f9e on Nov 10, 2020
  49. sidhujag referenced this in commit ae30b1a10d on Nov 10, 2020
  50. sidhujag referenced this in commit 86863f0148 on Nov 10, 2020
  51. deadalnix referenced this in commit 0eed7c672c on Dec 18, 2020
  52. Fabcien referenced this in commit 9623f4e186 on Dec 18, 2020
  53. Fabcien referenced this in commit 5dadedebd9 on Dec 19, 2020
  54. deadalnix referenced this in commit 58c2ace496 on Dec 19, 2020
  55. deadalnix referenced this in commit 43a25ae628 on Dec 19, 2020
  56. bitcoin locked this on Feb 15, 2022
Contributors
jonatackDrahtBotlaanwjjnewberymeshcollider
Labels
RPC/REST/ZMQ
Linked (view graph)
#17283 rpc: improve getaddressinfo test coverage, help, code docs#17578 rpc: simplify getaddressinfo labels, deprecate previous behavior#19200 rpc: remove deprecated getaddressinfo fields
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:54 UTC