Skip to content

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented Dec 30, 2014

No description provided.

@petertodd
Copy link
Contributor

utACK

@laanwj
Copy link
Member

laanwj commented Jan 3, 2015

ut ACK

@fanquake
Copy link
Member

fanquake commented Jan 3, 2015

utACK

@jtimon
Copy link
Contributor

jtimon commented Jan 3, 2015

ACK

@sipa
Copy link
Member

sipa commented Jan 4, 2015

I have an alternative suggestion here, namely to 'degrade' accounts back into labels. That means marking all RPC methods that operate on the concept of an "account balance" as deprecated, but not the ability to list transactions received with a particular label etc.

Pro: address labels already exist in the GUI and we probably want to keep them there, so making the RPC functionality analogous should be easy, and it doesn't have the potential confusion risk that accounts have.
Cons: If we're going to break functionality, maybe it should be broken entirely.

@jgarzik
Copy link
Contributor

jgarzik commented Jan 4, 2015

@sipa That was my suggestion months ago in IRC: Adding one or more "tags" to each address is still useful.

@luke-jr
Copy link
Member Author

luke-jr commented Jan 4, 2015

@sipa That may not be as simple as you think - GUI labels and accounts overlap only for receive transactions. Send transaction labels and send account are entirely unrelated right now. :/

@laanwj
Copy link
Member

laanwj commented Jan 5, 2015

I like sipa's idea. The labels feature is useful, and aiming to expose it eventually in both RPC and the GUI would make things symmetrical, which is what many users expect.

On the other hand doing that without confusing current users of accounts even more will be difficult. Hence I like first communicating a complete deprecation of the account system as in this pull.

Then introduce a new API for labels. This also makes it clear that the label system is separately useful and not just 'degraded accounts'. The usual warning apply - "don't use this at the same time as the account system, or it will get confused".

@laanwj
Copy link
Member

laanwj commented Jan 8, 2015

That means marking all RPC methods that operate on the concept of an "account balance" as deprecated, but not the ability to list transactions received with a particular label etc.

As there would no longer be a concept of 'account address' or 'account balance' the label API could be very minimal, e.g.:

  • getaddressesforlabel [like getaddressesforaccount]
  • listlabels [like listaccounts, but would just return a list of labels, without balances - and would need an argument to filter for sending/receiving labels]
  • setlabel (address, label) [like setaccount, but also allows setting labels for sending addresses]

These would stay the same, and accept a label instead of an account:

  • getnewaddress (label)
  • listtransactions (label,...)

Others, like sendfrom would no longer make sense at all, eg. there is no point in sending from a label.

laanwj pushed a commit that referenced this pull request Jan 8, 2015
@luke-jr
Copy link
Member Author

luke-jr commented Jan 8, 2015

Note: BFGMiner needs some equivalent of getaccountaddress.

It would make sense to let sendfrom (or at least sendmany) set a label for a send, to avoid having to make 2 RPC calls for each send.

@laanwj
Copy link
Member

laanwj commented Jan 8, 2015

What would be the point of getaccountaddress without accounts? Why not use getnewaddress (label)?

Re: sendfrom, fine, but then rename it and get rid of the from and call it sendandlabel or such... But IMO we have too many send functions already :/ In principle we could do with just sendmany.

@luke-jr
Copy link
Member Author

luke-jr commented Jan 8, 2015

getnewaddress always creates a new address. In the case of mining, you want to continue to use the same one you were using last time until you find a block. I believe this goes for P2Pool as well.

sendmany-only sounds fine to me.

@laanwj
Copy link
Member

laanwj commented Jan 8, 2015

Couldn't you use getaddressesforlabel then, and if no results, get a new address? Alternatively, remember the last-used address internally.

@luke-jr
Copy link
Member Author

luke-jr commented Jan 8, 2015

getaddressesforlabel doesn't tell you if addresses are used or not, does it? At this time, there is no internal storage of any sort... hate to add it for this.

@laanwj
Copy link
Member

laanwj commented Jan 8, 2015

Well, the concept of a 'label address' just doesn't make sense, and also has no GUI equivalent.

Also there is some weird interaction that prevents the last address from being moved from an account, and that interaction has to do with the 'account address'. To make it work like GUI labels (so you can get rid of them by renaming), that would have to be changed as well. Easiest would be to just get rid of the confusing feature.

@luke-jr
Copy link
Member Author

luke-jr commented Jan 8, 2015

"Get unused address with label" makes sense - albeit only for mining (since bitcoind won't know if it's used-but-not-broadcast-yet). Maybe "getminingaddress" would be appropriate?

@laanwj laanwj merged commit 7b782f5 into bitcoin:master Jan 26, 2015
laanwj added a commit that referenced this pull request Jan 26, 2015
7b782f5 RPCWallet: Notate all account stuff as deprecated (Luke Dashjr)
Fuzzbawls added a commit to Fuzzbawls/PIVX that referenced this pull request Jan 10, 2020
First step in removing the internal accounting system. All RPC command
documentation/help output now notates that account arguments are
deprecated.

Ported from bitcoin#5575, with
additional edits where necessary for cold staking related commands that
also used account arguments.
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Jan 11, 2020
73d331a [RPC] Notate all account stuff as deprecated (Fuzzbawls)

Pull request description:

  First step in removing the internal accounting system. All RPC command
  documentation/help output now notates that account arguments are
  deprecated.

  Ported from bitcoin#5575, with
  additional edits where necessary for cold staking related commands that
  also used account arguments.

ACKs for top commit:
  random-zebra:
    utACK 73d331a
  furszy:
    utACK 73d331a

Tree-SHA512: 6d88f0171b981fd072fcb8b4022f717182e966b9108772a4bca009f60f8e7b6801dbc22c403f956dffd8ef1095b1e8a17cd38d435998618d8f966ef6fdf8f667
Fuzzbawls added a commit to Fuzzbawls/PIVX that referenced this pull request Jan 11, 2020
First step in removing the internal accounting system. All RPC command
documentation/help output now notates that account arguments are
deprecated.

Ported from bitcoin#5575, with
additional edits where necessary for cold staking related commands that
also used account arguments.

Github-Pull: PIVX-Project#1251
Rebased-From: 73d331a
Liquid369 added a commit to dogecash/dogecash-old that referenced this pull request Jan 16, 2020
73d331a [RPC] Notate all account stuff as deprecated (Fuzzbawls)

Pull request description:

  First step in removing the internal accounting system. All RPC command
  documentation/help output now notates that account arguments are
  deprecated.

  Ported from bitcoin/bitcoin#5575, with
  additional edits where necessary for cold staking related commands that
  also used account arguments.

ACKs for top commit:
  random-zebra:
    utACK 73d331a
  furszy:
    utACK 73d331a

Tree-SHA512: 6d88f0171b981fd072fcb8b4022f717182e966b9108772a4bca009f60f8e7b6801dbc22c403f956dffd8ef1095b1e8a17cd38d435998618d8f966ef6fdf8f667
reddink pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request May 27, 2020
Github-Pull: bitcoin#5575
Rebased-From: 1eb1e65
(cherry picked from commit e2677d7)
wqking pushed a commit to wqking-temp/Vitae that referenced this pull request Jun 5, 2020
First step in removing the internal accounting system. All RPC command
documentation/help output now notates that account arguments are
deprecated.

Ported from bitcoin/bitcoin#5575, with
additional edits where necessary for cold staking related commands that
also used account arguments.

Github-Pull: #1251
Rebased-From: 73d331a9dda934b9b31f620c7286fb431d6cd5da

# Conflicts:
#	src/rpcrawtransaction.cpp
#	src/rpcwallet.cpp
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants