labels are associated with addresses (rather than addresses being associated with labels, as was the case with accounts). The getlabeladdress does not make sense in this model, so remove it.
getaccountaddress is still supported for one release as the accounts API is deprecated.
Builds on top of #12953. Please remove that PR first.
In my opinion, the getlabeladdress should not have been introduced. However, it was included in #12892 to remove controversy and ensure that PR could be merged.
There are two main reasons for removing this RPC method:
getlabeladdress does not make sense in this model and blurs the distinction that "labels are associated with addresses, instead of addresses associated with labels".getlabeladdress requires keeping the whole CAccount administration in place. Part of the reason for the account to label refactor is simplify the wallet mess and to get rid of that.Many contributors have already concept ACK'ed removing this method (@laanwj at #12892 (comment), @PierreRochard at #12892 (comment), @promag at #12892#pullrequestreview-110855143, @jonasschnelli at #12892 (review))
Main opposition to removing this method came from @ryanofsky and @luke-jr . See #7729 (comment) onwards for a full discussion.
Main opposition to removing this method came from @ryanofsky and @luke-jr
No opposition here, I just didn't think this should be removed or changed in the middle of a PR that otherwise was only adding functionality.
But I actually don't know what "labels are associated with addresses rather than addresses being associated with labels" is supposed to mean. This sounds like a zen koan. It may be more accurate to say "Only one label can be associated with an address, but many addresses can be associated with one label. Labels (unlike accounts) do not have a default receiving address."
It could also be good to update release notes with a workaround for the deprecated getaccountadddress, though I couldn't think of anything very straightforward. Could suggest combining getnewaddress(label), listreceivedbylabel, and getaddressesbylabel for a replacement depending on the use-case.
I actually don't know what "labels are associated with addresses rather than addresses being associated with labels".
The idea is that an address HAS labels (currently an address can have only one label, but it should be possible to extend that in future). Conversely an account HAS addresses, ie the address belongs to the account.
In a model where accounts have addresses, it makes sense for an account to have a 'default receiving address'. It doesn't make sense for a label to have a 'default receiving address', since a label is just a description that can be attached to one or more addresses.
I don't think that's too mystical. The relationship of addresses owning labels is simple, and the existence of a 'label address' confuses that simple relationship.
It could also be good to update release notes with a workaround for the deprecated getaccountadddress, though I couldn't think of anything very straightforward.
There are two suggested workarounds here: #7729 (comment) and here: #7729 (comment).
I'm happy to add those to the release notes if you think that's a good idea.
Purged GitHub to get rid of the merged commits without rebase:
<details><summary></summary>
git push git@github.com:jnewbery/bitcoin.git a3d7d6ad9ff3b40904fae0b8d97b46126e810f3b:remove_getlabeladdress && git push git@github.com:jnewbery/bitcoin.git 8d22e03929f1c6fdcf8669fb01246b7099d861f2:remove_getlabeladdress -f
</details>
91 | @@ -92,17 +92,25 @@ def _run_subtest(self, accounts_api, node): 92 | # recognize the label/address associations. 93 | labels = [Label(name, accounts_api) for name in ("a", "b", "c", "d", "e")] 94 | for label in labels: 95 | - label.add_receive_address(node.getlabeladdress(label=label.name, force=True)) 96 | + if accounts_api: 97 | + address = node.getaccountaddress(label.name) 98 | + else: 99 | + address = node.getnewaddress()
address = node.getnewaddress(label.name)?
(or is the point here to test setlabel?)
No, yours is better. setlabel is tested further down.
Fixed and pushed the new version.
Concept ACK.
labels are associated with addresses (rather than addresses being
associated with labels, as was the case with accounts). The
getlabeladdress does not make sense in this model, so remove it.
getaccountaddress is still supported for one release as the accounts
API is deprecated.
rebased
Request that this get a V17.0 tag. it's not essential that it goes in for the next release, but it'd be preferable to not change the API twice in two consecutive releases if possible.
Concept ACK
utACK 67e0e04140b3dfac12d628cee391d40b5fac5cfa
Milestone
0.17.0