Author

Topic: [PULL] Full-precision display/entry for bitcoin amounts (Read 2327 times)

legendary
Activity: 2576
Merit: 1186
(yes, you'll have to-- horrors! -- use that evil, not 100%-pure-open-source github)
Since the GitHub terms of service require me to agree to fund a legal defense of them if they get sued, this is not an economically viable option. tcatm did it, though.
legendary
Activity: 1652
Merit: 2216
Chief Scientist
That seems like an internationalization bug. What was wrong with using the correct localization?

I was tempted to do more than fix the rounding problem...
... but then sanity overwhelmed me.

If you'd like to start a discussion of whether or not it is a good idea for one-thousand German bitcoin amounts to be displayed as "1.000,00", be my guest.  I think there was such a discussion in the past, but I didn't pay attention to it.  I think it would be nice if an amount like "1.001 BTC" (or even "1.001 tonal bitcoins") was unambiguous.

RE: subcent throwaway change:  turn it into a proper PULL request (yes, you'll have to-- horrors! -- use that evil, not 100%-pure-open-source github) and it'll happen faster.
legendary
Activity: 2576
Merit: 1186
I modified the patch to format numbers the way they were formatted before:  always , for the thousands separator, and . for the decimal point (instead of letting sprintf try to do the right thing).
That seems like an internationalization bug. What was wrong with using the correct localization? It would be nice to have BitCoin clients "just work" when LC_NUMERIC=tonal is fixed.

Also, especially with this change, it's even more important to merge my bugfix for unnecessary subcent waste/throwaway. That's in the gitorious bitcoin master branch ( git pull git://gitorious.org/bitcoin/bitcoin.git master )
sr. member
Activity: 337
Merit: 263
That fixes the problem.
legendary
Activity: 1652
Merit: 2216
Chief Scientist
It was LC_NUMERIC, I bet... (I'd just set LANG and unset the rest and assumed they'd all get picked up; internationalizing C++ applications is something I know very little about).

I modified the patch to format numbers the way they were formatted before:  always , for the thousands separator, and . for the decimal point (instead of letting sprintf try to do the right thing).
sr. member
Activity: 337
Merit: 263


Code:
LANG=de_DE.utf8
LC_CTYPE="de_DE.utf8"
LC_NUMERIC="de_DE.utf8"
LC_TIME="de_DE.utf8"
LC_COLLATE="de_DE.utf8"
LC_MONETARY="de_DE.utf8"
LC_MESSAGES="de_DE.utf8"
LC_PAPER="de_DE.utf8"
LC_NAME="de_DE.utf8"
LC_ADDRESS="de_DE.utf8"
LC_TELEPHONE="de_DE.utf8"
LC_MEASUREMENT="de_DE.utf8"
LC_IDENTIFICATION="de_DE.utf8"
LC_ALL=
legendary
Activity: 1652
Merit: 2216
Chief Scientist
This patch introduces a bug on de_DE locale when sending using the wxGUI. Dialogbox with "Error in amount"

Can anybody else reproduce this?  It works for me:
sr. member
Activity: 337
Merit: 263
This patch introduces a bug on de_DE locale when sending using the wxGUI. Dialogbox with "Error in amount"
legendary
Activity: 1652
Merit: 2216
Chief Scientist
Does this still work correctly for 0.1 BTC, or does it expose bitcoind to the problem with representing this amount in binary (which would make it truncated as 0.09999999 BTC)? This fix should at the very least be sure to round at 8 decimal places, if not reading the digits directly into an int64.

Converting from a double-precision float from the JSON library to an int64 bitcoin is:
Code:
int64 nAmount = roundint64(dAmount * COIN);
... which will always do the right thing (COIN is 100000000).

int64 to JSON string there are no code changes.

GUI string to int64 is a direct conversion, no intermediate double precision.

And int64 to GUI string is:
Code:
strprintf("%.08f", double(amount)/double(COIN))
... which also always does the right thing (printf of a floating point number rounds, and there is enough precision in a double the rounding will always be correct).

0.1 bitcoins will always become exactly 10000000 base units internally, and 10000000 base units will always be shown as exactly 0.10 (in the GUI) or 0.10000000 (in JSON).

administrator
Activity: 5166
Merit: 12850
Does this still work correctly for 0.1 BTC, or does it expose bitcoind to the problem with representing this amount in binary (which would make it truncated as 0.09999999 BTC)? This fix should at the very least be sure to round at 8 decimal places, if not reading the digits directly into an int64.

I agree that this needs to be addressed. I've been converting everything to strings and then moving the decimal within the string to perform the "/COIN" division. This has worked fine with BBE.
legendary
Activity: 2576
Merit: 1186
Does this still work correctly for 0.1 BTC, or does it expose bitcoind to the problem with representing this amount in binary (which would make it truncated as 0.09999999 BTC)? This fix should at the very least be sure to round at 8 decimal places, if not reading the digits directly into an int64.
legendary
Activity: 1652
Merit: 2216
Chief Scientist
I just did some testing (on Linux) and the GUI seems to handle full-precision values quite nicely.
legendary
Activity: 1652
Merit: 2216
Chief Scientist
https://github.com/bitcoin/bitcoin/pull/79

This modifies FormatMoney to display full-precision values (with trailing zeroes trimmed correctly-- e.g. 0 is 0.00 but 0.00010000 displays as 0.0001).

And ParseMoney allows entry of full-precision values.

And JSON's AmountFromValue doesn't round to two places, so you can send/move full-precision values.

I haven't tested this with the GUI bitcoin yet, it will probably require UI layout tweaks.
Jump to: