rpc: simplify getaddressinfo labels, deprecate previous behavior #17578

jonatack · 2019-11-24T17:23:40Z

This PR builds on #17283 (now merged) and is followed by #17585.

It modifies the value returned by rpc getaddressinfo labels to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label name and address purpose key/value pairs.

before

  "labels": [
    {
      "name": "DOUBLE SPEND",
      "purpose": "receive"
    }

after

  "labels": [
    "DOUBLE SPEND"
  ]

The deprecated behavior can be re-enabled by starting bitcoind with -deprecatedrpc=labelspurpose.

For context, see:

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

Next steps: deprecate the rpc getaddressinfo label field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those.

DrahtBot · 2019-11-24T23:54:11Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#17809 (rpc: Auto-format RPCResult by MarcoFalke)
#17804 (doc: Misc RPC help fixes by MarcoFalke)
#17585 (rpc: deprecate getaddressinfo label by jonatack)
#17037 (Testschains: Many regtests with different genesis and default datadir by jtimon)

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.

jnewbery

Concept ACK. A few comments inline.

src/wallet/rpcwallet.cpp

test/functional/test_runner.py

jnewbery · 2019-11-26T15:04:08Z

I also think you need to squash pc: simplify getaddressinfo labels, deprecate previous behavior and test: update getaddressinfo labels tests commits. I think the intermediate commit with the RPC changes but not the test changes breaks git bisect.

33f5fc3 test: add rpc getaddressinfo labels test coverage (Jon Atack) 0f3539a test: add listlabels test in wallet_labels.py (Jon Atack) 1388de8 rpc: add getaddressinfo code documentation (Jon Atack) 2ee0cb3 rpc: update getaddressinfo RPCExamples to bech32 (Jon Atack) 8d1ed0c rpc: clarify label vs labels in getaddressinfo RPCHelpman (Jon Atack) 5a0ed85 rpc: improve getaddressinfo RPCHelpman content (Jon Atack) 70cda34 rpc: improve getaddressinfo RPCHelpman formatting (Jon Atack) Pull request description: This PR is a continuation of the work in #12892. Main motivations: - There is currently no test coverage for the getaddressinfo `labels` response. Coverage here is a prerequisite before deprecating the `label` response or adding multiple labels per address. - `bitcoin-cli help getaddressinfo` returns a few content errors, difficult-to-read formatting, and no explanation why it returns both `label` and `labels` and how they relate, which can be confusing for application developers. Changes by order of commits: - [x] improve/fix getaddressinfo RPCHelpman layout formatting - [x] improve/fix getaddressinfo RPCHelpman content - [x] clarify the `label` and `labels` fields in getaddressinfo RPCHelpman - [x] update getaddressinfo RPCExamples addresses to bech32 - [x] add getaddressinfo code docs - [x] add a `listlabels` test assertion in wallet_labels.py - [x] add missing getaddressinfo `labels` test coverage and improve the existing `label` tests Here are gists of the CLI help output: [`bitcoin-cli help getaddressinfo` before this PR](https://gist.github.com/jonatack/022af5221a85c069780359a22643c810) [`bitcoin-cli help getaddressinfo` after this PR](https://gist.github.com/jonatack/4ee5f6abc62a3d99269570206a5f90ba) It seems we ought to begin a deprecation process for the getaddressinfo `label` field? If yes, I have a follow-up ready. _--> EDIT: Deprecation follow-ups #17578 and #17585 now build on this PR._ ACKs for top commit: fjahr: Re-ACK 33f5fc3 jnewbery: ACK 33f5fc3. Tree-SHA512: a001aa863090ec2566a31059477945b1c303ebeb430b33472f8b150e420fa5742fc33bca9d95571746395b607f43f6078dd5b53e238ac1f3fc648b51c8f79a07

jnewbery · 2019-11-26T16:34:04Z

rebase please!

… code docs 33f5fc3 test: add rpc getaddressinfo labels test coverage (Jon Atack) 0f3539a test: add listlabels test in wallet_labels.py (Jon Atack) 1388de8 rpc: add getaddressinfo code documentation (Jon Atack) 2ee0cb3 rpc: update getaddressinfo RPCExamples to bech32 (Jon Atack) 8d1ed0c rpc: clarify label vs labels in getaddressinfo RPCHelpman (Jon Atack) 5a0ed85 rpc: improve getaddressinfo RPCHelpman content (Jon Atack) 70cda34 rpc: improve getaddressinfo RPCHelpman formatting (Jon Atack) Pull request description: This PR is a continuation of the work in bitcoin#12892. Main motivations: - There is currently no test coverage for the getaddressinfo `labels` response. Coverage here is a prerequisite before deprecating the `label` response or adding multiple labels per address. - `bitcoin-cli help getaddressinfo` returns a few content errors, difficult-to-read formatting, and no explanation why it returns both `label` and `labels` and how they relate, which can be confusing for application developers. Changes by order of commits: - [x] improve/fix getaddressinfo RPCHelpman layout formatting - [x] improve/fix getaddressinfo RPCHelpman content - [x] clarify the `label` and `labels` fields in getaddressinfo RPCHelpman - [x] update getaddressinfo RPCExamples addresses to bech32 - [x] add getaddressinfo code docs - [x] add a `listlabels` test assertion in wallet_labels.py - [x] add missing getaddressinfo `labels` test coverage and improve the existing `label` tests Here are gists of the CLI help output: [`bitcoin-cli help getaddressinfo` before this PR](https://gist.github.com/jonatack/022af5221a85c069780359a22643c810) [`bitcoin-cli help getaddressinfo` after this PR](https://gist.github.com/jonatack/4ee5f6abc62a3d99269570206a5f90ba) It seems we ought to begin a deprecation process for the getaddressinfo `label` field? If yes, I have a follow-up ready. _--> EDIT: Deprecation follow-ups bitcoin#17578 and bitcoin#17585 now build on this PR._ ACKs for top commit: fjahr: Re-ACK 33f5fc3 jnewbery: ACK 33f5fc3. Tree-SHA512: a001aa863090ec2566a31059477945b1c303ebeb430b33472f8b150e420fa5742fc33bca9d95571746395b607f43f6078dd5b53e238ac1f3fc648b51c8f79a07

jonatack · 2019-12-02T09:23:03Z

I also think you need to squash pc: simplify getaddressinfo labels, deprecate previous behavior and test: update getaddressinfo labels tests commits. I think the intermediate commit with the RPC changes but not the test changes breaks git bisect.

Done.

Thanks @jnewbery for reviewing! I took all your all suggestions except merging the deprecation test into rpc_deprecated.py (per my comment above), added a commit incorporating your review feedback from #17283, and rebased. I also beefed up the deprecation test a bit and moved the release note to its own file rather than modifying release-notes.md directly.

jnewbery

utACK 583be1e

One style nit in the test. Feel free to take or ignore.

test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py

jonatack · 2019-12-06T12:33:56Z

Thanks for reviewing @jnewbery. Addressed the style nit and rebased.

jnewbery · 2019-12-11T20:16:58Z

ACK 2e06da5

jonatack · 2019-12-13T13:25:05Z

Rebased.

jnewbery · 2019-12-13T14:08:43Z

ACK 55dfbc8

Only change is rebasing on master.

instagibbs · 2019-12-13T15:38:36Z

What's the best argument in favor of the "purpose" field? Aside from it being "strange" I didn't see any discussion really on the links? I'd like to know why it was there before removing it.

jnewbery · 2020-01-03T19:03:40Z

reACK 8925df8

promag

I thought the only change in tests should be to enable labelspurpose? Then these test changes would be done when the deprecated behavior is removed.

jonatack · 2020-01-03T19:21:13Z

@promag The labelspurpose deprecation itself is tested in 8bb405b. That test can be deleted after the 0.21 release. I think it's good to test both sides of the change in this PR, but particularly the new default state.

promag

Code review ACK 8925df8.

doc/release-notes-17578.md

meshcollider

Code review ACK 8925df8

src/wallet/rpcwallet.cpp

… behavior 8925df8 doc: update release notes (Jon Atack) 8bb405b test: getaddressinfo labels purpose deprecation test (Jon Atack) 60aba1f rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack) 7851f14 rpc: incorporate review feedback from PR 17283 (Jon Atack) Pull request description: This PR builds on #17283 (now merged) and is followed by #17585. It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs. before ``` "labels": [ { "name": "DOUBLE SPEND", "purpose": "receive" } ``` after ``` "labels": [ "DOUBLE SPEND" ] ``` The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`. For context, see: - #17283 (comment) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output. Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those. ACKs for top commit: jnewbery: reACK 8925df8 promag: Code review ACK 8925df8. meshcollider: Code review ACK 8925df8 Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438

…revious behavior 8925df8 doc: update release notes (Jon Atack) 8bb405b test: getaddressinfo labels purpose deprecation test (Jon Atack) 60aba1f rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack) 7851f14 rpc: incorporate review feedback from PR 17283 (Jon Atack) Pull request description: This PR builds on bitcoin#17283 (now merged) and is followed by bitcoin#17585. It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs. before ``` "labels": [ { "name": "DOUBLE SPEND", "purpose": "receive" } ``` after ``` "labels": [ "DOUBLE SPEND" ] ``` The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`. For context, see: - bitcoin#17283 (comment) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output. Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in bitcoin#17585) and add support for multiple labels per address. This PR will unblock those. ACKs for top commit: jnewbery: reACK 8925df8 promag: Code review ACK 8925df8. meshcollider: Code review ACK 8925df8 Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438

- bitcoin#17578 (comment) - bitcoin#17578 (comment) - bitcoin#17578 (comment)

42ec499 doc: developer notes guideline on RPCExamples addresses (Jon Atack) Pull request description: to make explicit the use of invalid addresses for user safety and to encourage the use of bech32 addresses by default. See #17578 (comment) and #17578 (comment). Fix a typo to appease the linter. ACKs for top commit: promag: ACK 42ec499, no strong opinion as whether this belongs to developer notes or not but why not. fjahr: ACK 42ec499 michaelfolkson: ACK 42ec499 Tree-SHA512: 64f90e227d256aa194c4fd48435440bdc233a51213dd4a6ac5b05d04263f729c6b4bb5f3afd3b87719b20cb1b159d5a9673d58a11b72823a4a6a16e8a26ae10e

…dresses 42ec499 doc: developer notes guideline on RPCExamples addresses (Jon Atack) Pull request description: to make explicit the use of invalid addresses for user safety and to encourage the use of bech32 addresses by default. See bitcoin#17578 (comment) and bitcoin#17578 (comment). Fix a typo to appease the linter. ACKs for top commit: promag: ACK 42ec499, no strong opinion as whether this belongs to developer notes or not but why not. fjahr: ACK 42ec499 michaelfolkson: ACK 42ec499 Tree-SHA512: 64f90e227d256aa194c4fd48435440bdc233a51213dd4a6ac5b05d04263f729c6b4bb5f3afd3b87719b20cb1b159d5a9673d58a11b72823a4a6a16e8a26ae10e

d3bc184 doc: update release notes with getaddressinfo label deprecation (Jon Atack) 72af93f test: getaddressinfo label deprecation test (Jon Atack) d48875f rpc: deprecate getaddressinfo label field (Jon Atack) dc0cabe test: remove getaddressinfo label tests (Jon Atack) c7654af doc: address pr17578 review feedback (Jon Atack) Pull request description: 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. ACKs for top commit: jnewbery: ACK d3bc184 laanwj: ACK d3bc184 meshcollider: utACK d3bc184 Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d

d3bc184 doc: update release notes with getaddressinfo label deprecation (Jon Atack) 72af93f test: getaddressinfo label deprecation test (Jon Atack) d48875f rpc: deprecate getaddressinfo label field (Jon Atack) dc0cabe test: remove getaddressinfo label tests (Jon Atack) c7654af doc: address pr17578 review feedback (Jon Atack) Pull request description: This PR builds on bitcoin#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 bitcoin#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. ACKs for top commit: jnewbery: ACK d3bc184 laanwj: ACK d3bc184 meshcollider: utACK d3bc184 Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d

- bitcoin#17578 (comment) - bitcoin#17578 (comment) - bitcoin#17578 (comment)

bc01f7a doc: release note for rpc getaddressinfo removals (Jon Atack) 90e9893 rpc: getaddressinfo RPCResult fixup (Jon Atack) a8507c9 rpc: remove deprecated getaddressinfo `labels: purpose` (Jon Atack) 645a865 rpc: remove deprecated getaddressinfo `label` field (Jon Atack) Pull request description: These were deprecated in #17578 and #17585, with expected 0.21 removal notified in the 0.20 release notes. ``` - The `getaddressinfo` RPC has had its `label` field deprecated (re-enable for this release using the configuration parameter `-deprecatedrpc=label`). The `labels` field is altered from returning JSON objects to returning a JSON array of label names (re-enable previous behavior for this release using the configuration parameter `-deprecatedrpc=labelspurpose`). Backwards compatibility using the deprecated configuration parameters is expected to be dropped in the 0.21 release. (#17585, #17578) ``` ACKs for top commit: Sjors: utACK bc01f7a adamjonas: utACK bc01f7a meshcollider: utACK bc01f7a Tree-SHA512: ae1af381e32c4c3bde8b061a56382838513a9a82c88767843cdeae3a2ab8aa7d8c2e66e106d2b31ea07d74bb80c191a2f842c9aaecc7c5438ad9a9bc66d1b251

bc01f7a doc: release note for rpc getaddressinfo removals (Jon Atack) 90e9893 rpc: getaddressinfo RPCResult fixup (Jon Atack) a8507c9 rpc: remove deprecated getaddressinfo `labels: purpose` (Jon Atack) 645a865 rpc: remove deprecated getaddressinfo `label` field (Jon Atack) Pull request description: These were deprecated in bitcoin#17578 and bitcoin#17585, with expected 0.21 removal notified in the 0.20 release notes. ``` - The `getaddressinfo` RPC has had its `label` field deprecated (re-enable for this release using the configuration parameter `-deprecatedrpc=label`). The `labels` field is altered from returning JSON objects to returning a JSON array of label names (re-enable previous behavior for this release using the configuration parameter `-deprecatedrpc=labelspurpose`). Backwards compatibility using the deprecated configuration parameters is expected to be dropped in the 0.21 release. (bitcoin#17585, bitcoin#17578) ``` ACKs for top commit: Sjors: utACK bc01f7a adamjonas: utACK bc01f7a meshcollider: utACK bc01f7a Tree-SHA512: ae1af381e32c4c3bde8b061a56382838513a9a82c88767843cdeae3a2ab8aa7d8c2e66e106d2b31ea07d74bb80c191a2f842c9aaecc7c5438ad9a9bc66d1b251

…ous behavior Summary: 8925df86c4df16b1070343fef8e4d238f3cc3bd1 doc: update release notes (Jon Atack) 8bb405bbadf11391ccba7b334b4cfe66dc85b390 test: getaddressinfo labels purpose deprecation test (Jon Atack) 60aba1f2f11529add115d963d05599130288ae28 rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack) 7851f14ccf2bcd1e9b2ad48e5e08881be06d9d21 rpc: incorporate review feedback from [[bitcoin/bitcoin#17283 | PR17283]] (Jon Atack) Pull request description: This PR builds on #17283 (now merged) and is followed by #17585. It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs. before ``` "labels": [ { "name": "DOUBLE SPEND", "purpose": "receive" } ``` after ``` "labels": [ "DOUBLE SPEND" ] ``` The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`. For context, see: - bitcoin/bitcoin#17283 (comment) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output. Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those. --- Depends on D7718 Backport of Core [[bitcoin/bitcoin#17578 | PR17578]] Test Plan: ninja all check check-functional Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7724

- bitcoin/bitcoin#17578 (comment) - bitcoin/bitcoin#17578 (comment) - bitcoin/bitcoin#17578 (comment)

… code docs 33f5fc3 test: add rpc getaddressinfo labels test coverage (Jon Atack) 0f3539a test: add listlabels test in wallet_labels.py (Jon Atack) 1388de8 rpc: add getaddressinfo code documentation (Jon Atack) 2ee0cb3 rpc: update getaddressinfo RPCExamples to bech32 (Jon Atack) 8d1ed0c rpc: clarify label vs labels in getaddressinfo RPCHelpman (Jon Atack) 5a0ed85 rpc: improve getaddressinfo RPCHelpman content (Jon Atack) 70cda34 rpc: improve getaddressinfo RPCHelpman formatting (Jon Atack) Pull request description: This PR is a continuation of the work in bitcoin#12892. Main motivations: - There is currently no test coverage for the getaddressinfo `labels` response. Coverage here is a prerequisite before deprecating the `label` response or adding multiple labels per address. - `bitcoin-cli help getaddressinfo` returns a few content errors, difficult-to-read formatting, and no explanation why it returns both `label` and `labels` and how they relate, which can be confusing for application developers. Changes by order of commits: - [x] improve/fix getaddressinfo RPCHelpman layout formatting - [x] improve/fix getaddressinfo RPCHelpman content - [x] clarify the `label` and `labels` fields in getaddressinfo RPCHelpman - [x] update getaddressinfo RPCExamples addresses to bech32 - [x] add getaddressinfo code docs - [x] add a `listlabels` test assertion in wallet_labels.py - [x] add missing getaddressinfo `labels` test coverage and improve the existing `label` tests Here are gists of the CLI help output: [`bitcoin-cli help getaddressinfo` before this PR](https://gist.github.com/jonatack/022af5221a85c069780359a22643c810) [`bitcoin-cli help getaddressinfo` after this PR](https://gist.github.com/jonatack/4ee5f6abc62a3d99269570206a5f90ba) It seems we ought to begin a deprecation process for the getaddressinfo `label` field? If yes, I have a follow-up ready. _--> EDIT: Deprecation follow-ups bitcoin#17578 and bitcoin#17585 now build on this PR._ ACKs for top commit: fjahr: Re-ACK 33f5fc3 jnewbery: ACK 33f5fc3. Tree-SHA512: a001aa863090ec2566a31059477945b1c303ebeb430b33472f8b150e420fa5742fc33bca9d95571746395b607f43f6078dd5b53e238ac1f3fc648b51c8f79a07

…revious behavior 8925df8 doc: update release notes (Jon Atack) 8bb405b test: getaddressinfo labels purpose deprecation test (Jon Atack) 60aba1f rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack) 7851f14 rpc: incorporate review feedback from PR 17283 (Jon Atack) Pull request description: This PR builds on bitcoin#17283 (now merged) and is followed by bitcoin#17585. It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs. before ``` "labels": [ { "name": "DOUBLE SPEND", "purpose": "receive" } ``` after ``` "labels": [ "DOUBLE SPEND" ] ``` The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`. For context, see: - bitcoin#17283 (comment) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427) - http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output. Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in bitcoin#17585) and add support for multiple labels per address. This PR will unblock those. ACKs for top commit: jnewbery: reACK 8925df8 promag: Code review ACK 8925df8. meshcollider: Code review ACK 8925df8 Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438

…dresses 42ec499 doc: developer notes guideline on RPCExamples addresses (Jon Atack) Pull request description: to make explicit the use of invalid addresses for user safety and to encourage the use of bech32 addresses by default. See bitcoin#17578 (comment) and bitcoin#17578 (comment). Fix a typo to appease the linter. ACKs for top commit: promag: ACK 42ec499, no strong opinion as whether this belongs to developer notes or not but why not. fjahr: ACK 42ec499 michaelfolkson: ACK 42ec499 Tree-SHA512: 64f90e227d256aa194c4fd48435440bdc233a51213dd4a6ac5b05d04263f729c6b4bb5f3afd3b87719b20cb1b159d5a9673d58a11b72823a4a6a16e8a26ae10e

d3bc184 doc: update release notes with getaddressinfo label deprecation (Jon Atack) 72af93f test: getaddressinfo label deprecation test (Jon Atack) d48875f rpc: deprecate getaddressinfo label field (Jon Atack) dc0cabe test: remove getaddressinfo label tests (Jon Atack) c7654af doc: address pr17578 review feedback (Jon Atack) Pull request description: This PR builds on bitcoin#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 bitcoin#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. ACKs for top commit: jnewbery: ACK d3bc184 laanwj: ACK d3bc184 meshcollider: utACK d3bc184 Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d

Summary: Comments and docstring changes only > - bitcoin/bitcoin#17578 (comment) > - bitcoin/bitcoin#17578 (comment) This is a backport of Core [[bitcoin/bitcoin#17585 | PR17585]] [1/5] bitcoin/bitcoin@c7654af Test Plan: proof reading Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8700

- bitcoin/bitcoin#17578 (comment) - bitcoin/bitcoin#17578 (comment) - bitcoin/bitcoin#17578 (comment)

jonatack mentioned this pull request Nov 24, 2019

rpc: improve getaddressinfo test coverage, help, code docs #17283

Merged

7 tasks

DrahtBot added Docs RPC/REST/ZMQ Tests Wallet labels Nov 24, 2019

fanquake removed Docs Tests Wallet labels Nov 24, 2019

jonatack force-pushed the deprecate-getaddress-info-labels-purpose branch 2 times, most recently from fe245f5 to b2e5ca1 Compare November 25, 2019 01:17

jonatack mentioned this pull request Nov 25, 2019

rpc: deprecate getaddressinfo label #17585

Merged

jnewbery reviewed Nov 26, 2019

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

test/functional/test_runner.py Show resolved Hide resolved

jonatack force-pushed the deprecate-getaddress-info-labels-purpose branch from b2e5ca1 to 583be1e Compare December 2, 2019 09:08

jnewbery reviewed Dec 2, 2019

View reviewed changes

test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py Outdated Show resolved Hide resolved

weerayut3 approved these changes Dec 2, 2019

View reviewed changes

jonatack force-pushed the deprecate-getaddress-info-labels-purpose branch from 583be1e to 2e06da5 Compare December 6, 2019 12:31

DrahtBot added the Needs rebase label Dec 13, 2019

jonatack force-pushed the deprecate-getaddress-info-labels-purpose branch from 2e06da5 to 55dfbc8 Compare December 13, 2019 13:23

DrahtBot removed the Needs rebase label Dec 13, 2019

promag reviewed Jan 3, 2020

View reviewed changes

doc/release-notes-17578.md Show resolved Hide resolved

meshcollider reviewed Jan 7, 2020

View reviewed changes

src/wallet/rpcwallet.cpp Show resolved Hide resolved

meshcollider merged commit 8925df8 into bitcoin:master Jan 7, 2020

jonatack deleted the deprecate-getaddress-info-labels-purpose branch January 8, 2020 16:59

jonatack added a commit to jonatack/bitcoin that referenced this pull request Jan 9, 2020

doc: address pr17578 review feedback

c7654af

- bitcoin#17578 (comment) - bitcoin#17578 (comment) - bitcoin#17578 (comment)

jonatack mentioned this pull request Feb 10, 2020

Newsletters: add 84 (2020-02-12) bitcoinops/bitcoinops.github.io#337

Merged

1 task

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Apr 17, 2020

doc: address pr17578 review feedback

bf9aaa7

- bitcoin#17578 (comment) - bitcoin#17578 (comment) - bitcoin#17578 (comment)

jonatack mentioned this pull request Jun 7, 2020

rpc: remove deprecated getaddressinfo fields #19200

Merged

nopara73 mentioned this pull request Jul 22, 2020

Update to Bitcoin Core 0.20.0 MetacoSA/NBitcoin#886

Merged

van-orton pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Oct 30, 2020

doc: address pr17578 review feedback

0daa05e

- bitcoin/bitcoin#17578 (comment) - bitcoin/bitcoin#17578 (comment) - bitcoin/bitcoin#17578 (comment)

backpacker69 pushed a commit to peercoin/peercoin that referenced this pull request Mar 28, 2021

doc: address pr17578 review feedback

da6921f

- bitcoin/bitcoin#17578 (comment) - bitcoin/bitcoin#17578 (comment) - bitcoin/bitcoin#17578 (comment)

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

rpc: simplify getaddressinfo labels, deprecate previous behavior #17578

rpc: simplify getaddressinfo labels, deprecate previous behavior #17578

Uh oh!

Conversation

jonatack commented Nov 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Nov 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

jnewbery left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jnewbery commented Nov 26, 2019

Uh oh!

jnewbery commented Nov 26, 2019

Uh oh!

jonatack commented Dec 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnewbery left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonatack commented Dec 6, 2019

Uh oh!

jnewbery commented Dec 11, 2019

Uh oh!

jonatack commented Dec 13, 2019

Uh oh!

jnewbery commented Dec 13, 2019

Uh oh!

instagibbs commented Dec 13, 2019

Uh oh!

jnewbery commented Jan 3, 2020

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

jonatack commented Jan 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

meshcollider left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jonatack commented Nov 24, 2019 •

edited

Loading

DrahtBot commented Nov 24, 2019 •

edited

Loading

jonatack commented Dec 2, 2019 •

edited

Loading

jonatack commented Jan 3, 2020 •

edited

Loading