Skip to content

qt: Add option to (not) spend unconfirmed change#3676

Merged
laanwj merged 1 commit intobitcoin:masterfrom
laanwj:2014_02_gui_nospendzeroconfchange
Feb 16, 2014
Merged

qt: Add option to (not) spend unconfirmed change#3676
laanwj merged 1 commit intobitcoin:masterfrom
laanwj:2014_02_gui_nospendzeroconfchange

Conversation

@laanwj
Copy link
Copy Markdown
Member

@laanwj laanwj commented Feb 15, 2014

  • Add a wallet tab to options dialog
  • Move fee setting to wallet tab
  • Add new setting to set -spendzeroconfchange from UI

spend-unconfirmed-gui

Implementation is straightforward, messages are up for discussion as usual.

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/341b993ca1fe6a6c247a4ed129d39fb4936668fe for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@christophebiocca
Copy link
Copy Markdown

Nit about the wording:

If you disable the spending of unconfirmed change, your transaction cannot be completed until your previous transactions have at least one confirmation. This also affects how your balance is computed.

Isn't that a little pessimistic? I mean in practice, a wallet will have multiple outputs to use, only a few of which will be used by any single transaction. How about:

If you disable the spending of unconfirmed change, the change from a transaction cannot be used until that transaction has at least one confirmation. This also affects how your balance is computed.

Of course, the problem with that a user may not understand the concept of change outputs, since that is hidden from them.

@int03h
Copy link
Copy Markdown

int03h commented Feb 15, 2014

Think of this in the real world.

Spending something you don’t have is obviously not possible. So why not simply say the following:

ADVANCED:

[ ] Allow spending of unconfirmed changed.

(leaving the flag off by default).

You are not going to explain this adequately in 5 lines or less – especially to nubs .. so why bother ? Let them that wish to use it, RTFM some and then its caveat emptor if they flip it on.

@dgenr8
Copy link
Copy Markdown
Contributor

dgenr8 commented Feb 16, 2014

This looks to be the first mention of "change" period. A rational user will think it relates to change returned by recipient for amount paid over some agreed-on price, which further confuses, since no such prices are known by bitcoin-qt.

The question is what should the message say, given the abstractions user is working with? That is a tough one.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, why this is needed, we didn't have to use this before right?
E.g. settings.value("fUseUPnP").toBool()...

Shouldn't this be SoftSetBoolArg()?

@Diapolo
Copy link
Copy Markdown

Diapolo commented Feb 16, 2014

If this also affects, how we compute the wallet balance, perhaps name the option Treat unconfirmed change as confirmed (experts only). And append (experts only), because it is similar complex for normal users like coin control IMHO and you need some knowledge of what you are doing.

To go further, we should also rework the descriptive string for the command line option, because this misses also that using that options or disabling it will affect how balance is computed. IMHO it could be the same string with (experts only) left out.

I also like @christophebiocca's suggestion for a less dramatic wording. As unused change won't block a transaction, it just won't allow using the change in new transactions, right?

@KF-R
Copy link
Copy Markdown

KF-R commented Feb 16, 2014

That wording makes a lot of sense (Diapolo).

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Feb 16, 2014

Will switch to @christophebiocca's wording and SetSoftBoolArg and add (experts only) like for coin control.

- Add a wallet tab to options dialog
- Move fee setting to wallet tab
- Add new setting to set -nospendzeroconfchange from UI
laanwj added a commit that referenced this pull request Feb 16, 2014
29d4507 qt: Add option to (not) spend unconfirmed change (Wladimir J. van der Laan)
@laanwj laanwj merged commit 29d4507 into bitcoin:master Feb 16, 2014
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You added 2 spaces here ^^.

@Diapolo
Copy link
Copy Markdown

Diapolo commented Feb 16, 2014

Why did you decide agains Spend unconfirmed change (experts only)? IMO Spend unconfirmed change (experts only) is not the whole truth (even more true for just the command line option), as it changes how we compute the balance...

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Feb 16, 2014

@Diapolo Don't change the name of the command line option because people are relying on it in production. Feel free to send pulls for improvements to the UI message though...

Also: it affects coin selection primarily. The reason it also affects the balance computation is because otherwise you cannot trust that the (confirmed) balance is really how much you can spend.

@Diapolo
Copy link
Copy Markdown

Diapolo commented Feb 16, 2014

@laanwj Don't get it, if it affects balance computation (the reason why doesn't even matter, it's just the fact), then it has to be mentioned in the help message. The reason to not change the command line message is also unclear still... it is in an unreleased branch (no official client released with it), why can't we just use Treat unconfirmed change as confirmed?

I know you will tell me we have bigger problems to solve, but this is something I fell is important enought ;).

@sipa
Copy link
Copy Markdown
Member

sipa commented Feb 16, 2014

Maybe unconfirmed balance should just be "balance" (it's the amount of money you're reasonably sure about you have, assuming it doesn't count -1 confirmed coins), and confirmed balance should be "spendable balance". The latter can then be affected by the setting of whether to consider unconfirmed change as spendable.

@int03h
Copy link
Copy Markdown

int03h commented Feb 16, 2014

Armory uses the words : Maximum Funds / Unconfirmed Fund and Spendable and shows 3 balances. One of the many reasons I use Armory. (this is not an advert for Armory - just a suggestion )

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Feb 16, 2014

@sipa That would make sense indeed. "Confirmed balance" has always been "Spendable balance". But "Unconfirmed balance" would then be "Unspendable balance"?

@int03h Bitcoin-qt also shows 3 (or even 4, if you have immature transactions) balances: Confirmed, Unconfirmed, Immature and Total.

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Feb 16, 2014

@Diapolo I'm doubtful about that because "Treat unconfirmed change as confirmed" doesn't really tell much.
It creates a categorical confusion much like #3662 was aimed at solving (in the API we now call transactions that count toward spendable balance "Trusted" instead).
Saying that it is spendable/unspendable at least tells you what you can do with it.
But then the names of the balances need to be changed too.

@sipa
Copy link
Copy Markdown
Member

sipa commented Feb 16, 2014

Yeah, if you're treating unconfirmed change as confirmed, the meaning of the word confirmed becomes blurry.

To answer the question "how much money will I (likely) (eventually) have", the current "unconfirmed balance" (excluding conflicted/noninmempool transactions) seems the correct measurement. I guess that should also include immature coins. Maybe it could be called "Eventual balance" ?

@laanwj
Copy link
Copy Markdown
Member Author

laanwj commented Feb 16, 2014

Well currently it looks like this

balances

Instead of the three groups I suppose we could also make a tally like this (rows only shown if the balance is >0):

Spendable                       XXX BTC
Spendable after 1 block         XXX BTC (=unconfirmed balance)
Spendable after XXX blocks      XXX BTC (=immature balances)
....
------------------------------------------------------
Eventual total                  XXX BTC

Edit: eh no that's no good either, one of course cannot be sure that the unconfirmed balance will confirm in 1 block...

laanwj added a commit to laanwj/bitcoin that referenced this pull request Feb 19, 2014
The word 'Spendable' more precisely says what the balance actually means.

Avoids the confirmed/unconfirmed confusion that can be caused by bitcoin#3676.
@laanwj laanwj deleted the 2014_02_gui_nospendzeroconfchange branch April 9, 2014 14:10
MathyV pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request Oct 27, 2014
The word 'Spendable' more precisely says what the balance actually means.

Avoids the confirmed/unconfirmed confusion that can be caused by bitcoin#3676.
@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.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants