Remove wallet 'account' API

jnewbery commented at 7:00 pm on April 11, 2018: member

This issue tracks removing the wallet ‘account’ API. See issue #3816 for the reasons why this is a good idea. Rather than extend that very old and long issue, I’m opening this to track concrete actions to fully remove the ‘account’ API.

Actions are:

Rename account to label where appropriate (#11536)
Introduce ’label’ API for wallet (~~#7729~~ #12892)
(V0.17) Mark ‘account’ API as deprecated. Wallet functional tests are updated to use -deprecatedrpc=accounts to maintain existing behaviour. (~~#11497~~ #12953)
Remove getlabeladdress RPC method. Keeping this method means we need to keep the CAccount internal accounting. (#13060)
Remove ‘account’ API from wallet functional tests. (#13075 and #13138)
(V0.18) Fully remove the ‘account’ API (#13825)

ryanofsky commented at 7:16 pm on April 11, 2018: member

Sounds good, but getlabeladdress is a new method not included in any existing release (I added it in #11536), so I would suggest removing it before there is another release instead of adding and then deprecating it.

jnewbery commented at 7:18 pm on April 11, 2018: member

I would suggest removing it before there is another release instead of adding and then deprecating it.

I’m fully in favour of removing it right now. If you open a PR to do that, I’ll ACK immediately :)

MarcoFalke commented at 7:20 pm on April 11, 2018: member

One person in #3816 mentioned that the calls were not properly deprecated in the documentation. I think they were referring to https://bitcoin.org/en/developer-reference#getaccount et al. I think we should at least file an issue on the bitcoin.org GitHub about the deprecation notice, no?

ryanofsky commented at 7:30 pm on April 11, 2018: member

I’m -0 on removing getlabeladdress, so I wouldn’t open a PR to do this. It seems like it’s providing useful functionality, and the simplification from not having it is pretty minor. I’m just saying if you’re going to remove it, it would be better to do it before adding it instead of adding and immediately deprecating it.

jnewbery commented at 7:38 pm on April 11, 2018: member

if you’re going to remove it, it would be better to do it before adding it…

Yes, better to have removed it already, but I don’t think it being around for a release or two is too much of a disaster either.

fanquake added the label Wallet on Apr 12, 2018

jnewbery commented at 2:44 pm on April 24, 2018: member

#12953 is merged

MarcoFalke referenced this in commit cb9bbf7772 on May 10, 2018

TheCharlatan commented at 8:24 am on June 18, 2018: member

sendmany is the easiest (not necessarily recommended) way currently to batch multiple transactions from the rpc without getting into createrawtransaction. However it is currently not possible to override the label selection. If "*" is selected as a label argument in the function call, an rpc error is thrown (rpcwallet:133). If an empty string "" is passed in, only unlabeled addresses are selected. I do not see a clear reason to not allow "*" as a wildcard argument, to select from all addresses - labeled and unlabeled. Is there a good reason to keep this legacy restriction?

jnewbery commented at 3:55 pm on June 18, 2018: member

If an empty string "" is passed in, only unlabeled addresses are selected.

I’ve double checked sendmany, and the account parameter is not used in coin selection, so I don’t think this is true. However, I think there is a related bug here. In the call to GetLegacyBalance() in sendmany, we use a pointer to strAccount, which is set to "". That means that if there are funds available in non-default accounts, those don’t get counted. We should really pass in a null pointer to indicate not to use accounts, or change the call to GetBalance().

I also see that sendfrom hasn’t been properly deprecated.

I’ll open a PR to fix those.

jnewbery commented at 4:37 pm on June 18, 2018: member

I’ll open a PR to fix those.

Here: #13498

sipa commented at 2:16 am on June 22, 2018: member

Unsure where to post this, but it seems it’s currently impossible to call getbalance with include_watchonly=true without enabling accounts RPC?

jnewbery commented at 1:50 pm on June 22, 2018: member

@sipa - yes that’s currently true. See #13461 for discussion.

jnewbery commented at 2:40 am on July 16, 2018: member

#13461 and #13138 are merged.

All that is left is to fully remove the account API. That can be done in master after 0.17 is branched, probably next week.

jnewbery commented at 6:53 pm on August 30, 2018: member

#13825 is merged!

jnewbery closed this on Aug 30, 2018

cryppty commented at 3:57 pm on November 11, 2019: none

I think majority people used the accounts functionality cause no one wants to pay trx fees to transfer funds between their own addresses in wallet, as well as the long wait. Now that accounts is out, is there an alternative command to send between addresses with 0 fees in your “own” wallet?

MarcoFalke commented at 4:04 pm on November 11, 2019: member

@cryppty Have you seen and read trough issue #3816? This should help you answer your question.

cryppty commented at 4:12 pm on November 11, 2019: none

@MarcoFalke Yes, I have thanks. I see that it is mainly for negative balance etc. But there is no mention there of an alternative. :-( I run a platform where I require instance move of funds between “virtual” accounts, I do run my own ledger but always used accounts as a backup to verify balances. I guess we will just have to exclude BTC from our platform as waiting for lengthy times (some times hours) will not work when using the sendtoaddress plus the high fees for every transaction. I know this is going to sound bad :-) but even banks allow you to transfer between you internal accounts for free :-)

sipa commented at 4:12 pm on November 11, 2019: member

@cryppty There never was a way to send coins between addresses in your own wallet for 0 fees.

cryppty commented at 4:14 pm on November 11, 2019: none

@sipa Yes I am aware of that :-), that is where the accounts came in so handy. There you could move beween accounts with 0 fees. I know that in reality the amounts were still in their respective addresses and the account was just a ledger of movements and balance.

ryanofsky commented at 4:51 pm on November 11, 2019: member

@cryppty the bitcoin wallet still tracks exactly the same information as before. What used to be called an account is now just called a label, and you can still apply labels to addresses.

What was removed was runtime code for “sending from” labels, and code for filtering balances with labels, because it’s error prone to refer to transaction outputs (coins) in the wallet by address or label. Addresses don’t uniquely identify outputs, and not all outputs have addresses, so accounting based on addresses can’t really be sound and is likely to lead to mistakes.

But since the wallet is storing exactly the same information as before, it’s still possible for software using the wallet API to use indirectly use labels ~~for coin selection when creating transactions or~~ computing balances. Instead of referring to transaction outputs indirectly by label it will have to look them up with an api like listtransactions and refer to them precisely.

EDIT (correction above): accounts/labels were never actually used for coin selection when creating transactions. Accounts were just passively stored as part of address and transaction metadata and only used for computing balances.

ryanofsky commented at 1:44 pm on November 12, 2019: member

Small correction above: I misremembered what sending from an account with sendfrom or sendmany actually did before “send from” functionality was removed in #14023 / 0.17.0. It actually did not affect coin selection at all, but just added a mapValue["fromaccount"] string field to outgoing transaction metadata stored in the wallet. Then it would treat transactions with this field as debits when computing account balances.

This means that replicating previous account functionality by just calling different RPC now should actually be simpler than what I suggested #12952 (comment), because it wouldn’t require doing any coin selection when spending, or having to determine whether specific outputs had been spent while computing balances. It would just require associating label names with outgoing transactions, which could be done for example by putting label names in one of the transaction comment fields.

laanwj commented at 1:49 pm on November 12, 2019: member

Correct, accounts never had effect on coin selection or UTXO management. It was always purely a layer on top.

cryppty commented at 2:37 pm on November 12, 2019: none

@ryanofsky Thank you for the detailed explanation. I will look into lables and see if there is a way we can migrate our current RPC accounts interface to labels.

ryanofsky commented at 4:04 pm on November 12, 2019: member

@ryanofsky Thank you for the detailed explanation. I will look into lables and see if there is a way we can migrate our current RPC accounts interface to labels. @cryppty you probably should be careful about trying to replicate old account functionality too closely, because the balances it computed could be misleading. But for reference, if you wanted to port previous RPC client code using accounts without changing functionality, I think you’d basically:

Change previous rpc client code using setaccount or similar RPC methods tagging addresses with account names to instead tag them with label names
Change previous rpc client code sending from an account with sentfrom or sendmany to instead store the “from account” in the transaction comment field
Change previous rpc client code that was calling move to instead store credits and debit adjustments to the two accounts externally
Change previous rpc client code requesting account balances to instead list transactions, count transaction output amounts with matching label names as credits, and count transaction payment amounts with matching comment fields as debits, and compute the sum adding any move amounts as adjustments.
Change previous rpc client code that was calling getaccountaddress to instead look up the most recently generated address with a matching label and use that address if it hasn’t received a payment, otherwise generate a fresh address with the label

AhWon-Jeong commented at 6:21 am on August 13, 2020: none

@cryppty Did you found a way to replace “move”? By any chance, could you let me know if you found it? Please. :)

xdustinface referenced this in commit 2bdbc5b70f on Dec 18, 2020

xdustinface referenced this in commit d43ffc6fba on Dec 18, 2020

xdustinface referenced this in commit 7f9343446a on Dec 18, 2020

xdustinface referenced this in commit 02e4b5b5b5 on Dec 22, 2020

xdustinface referenced this in commit 42a98b0bc8 on Dec 22, 2020

DrahtBot locked this on Feb 15, 2022

Remove wallet ‘account’ API #12952