rpc: Avoid unnecessary parsing roundtrip in number formatting, fix locale issue #6456

1. 
   laanwj commented at 7:25 am on July 18, 2015: member

   Three weakly related fixes to number handling after the univalue switch. These came to the surface while troubleshooting #6443.

   Make ValueFromAmount always return 8 decimals

   This is the format that was always returned to JSON clients. The difference was not noticed before, because VREAL values are post-processed by univalue.

   By implementing the functionality directly it breaks the dependency of rpcserver on utilmoneystr. FormatMoney is now only used for debugging purposes.

   To test, port over the formatting tests from util_tests.cpp to rpc_tests.cpp.

   univalue: Avoid unnecessary roundtrip through double for numbers

   JSON makes no distinction between numbers and reals, and our code doesn’t need to do so either.

   This removes VREAL, as well as its specific post-processing in UniValue::write. Non-monetary amounts do not need to be forcibly formatted with 8 decimals, so the extra roundtrip was unnecessary (and potentially loses precision).

   util: use locale-independent parsing in ParseDouble

   Use locale-indepent C++ based parsing instead of C’s strtod, which checks for different input based on the user’s locale. Fixes #6443.

2. rpc: Make ValueFromAmount always return 8 decimals

   This is the format that was always returned to JSON clients.
   The difference was not noticed before, because VREAL values
   are post-processed by univalue.
   
   By implementing the functionality directly it breaks the dependency
   of rpcserver on utilmoneystr. FormatMoney is now only used for debugging
   purposes.
   
   To test, port over the formatting tests from util_tests.cpp to
   rpc_tests.cpp.

   e061e2778d
3. univalue: Avoid unnecessary roundtrip through double for numbers

   JSON makes no distinction between numbers and reals, and our code
   doesn't need to do so either.
   
   This removes VREAL, as well as its specific post-processing in
   `UniValue::write`. Non-monetary amounts do not need to be forcibly
   formatted with 8 decimals, so the extra roundtrip was unnecessary
   (and potentially loses precision).

   7650449a67
4. util: use locale-independent parsing in ParseDouble

   Use locale-indepent C++ based parsing instead of C's strtod,
   which checks for different input based on the user's locale.
   Fixes #6443.

   ec249d4a1d
5. laanwj added the label RPC on Jul 18, 2015
6. laanwj renamed this:
   rpc: Avoid unnecessary roundtrips in number parsing, fix locale issue
   rpc: Avoid unnecessary parsing roundtrip in number formatting, fix locale issue
   on Jul 18, 2015
7. laanwj added the label Bug on Jul 18, 2015
8. 
   dexX7 commented at 9:28 am on July 18, 2015: contributor

   I can confirm this resolves #6443, thanks!

   As far as I can see the output is as expected, except for the difficulty, returned by "getinfo", which is shown in e notation ("difficulty": 4.656542373906925e-10 vs. 0.00000000, as returned by Core 11.0). Edit: I wouldn’t say this is bad though, as it offers a greater precision.

9. 
   laanwj commented at 8:52 am on July 20, 2015: member

   as returned by Core 11.0). Edit: I wouldn’t say this is bad though, as it offers a greater precision

   Exactly - the fixed 8-digit formatting is for clarity in monetary amounts. Earlier it was applied to all numbers because JSON spirit had no other way. But now we have full control over how values are rendered, and other floating point quantities such as bits, difficulty can be represented in full precision.

10. 
    jonasschnelli commented at 8:55 am on July 20, 2015: contributor

    Tested ACK.

    If i understand this right, then there is a slightly API change. Every double/float (non-monetary) comes now with a precision of 16 instead of 8 (because of https://github.com/bitcoin/bitcoin/pull/6456/files#diff-0f1b401041a14398229cf7e31b6db7eeR89). Because we are using oss << std::setprecision(16) << val it will rewrite things like 0.00000000046565423739069247 into scientific notation. That is what @dexX7 has detected.

11. 
    laanwj commented at 3:08 pm on July 20, 2015: member

    Right. All numbers were ‘cut’ to %.8f notation, so it would have reported 0.00000000046565423739069247 as 0.00000000 instead of going to scientific notation. E.g. a difficulty smaller than a “satoshi” was reported as 0. This makes no sense at all for non-monetary numbers. This is all within the limits of number representation in the JSON protocol.
12. 
    jgarzik commented at 7:00 pm on July 23, 2015: contributor

    ACK. Happy to see VREAL go away.
13. laanwj merged this on Jul 24, 2015
14. laanwj closed this on Jul 24, 2015

    ---

15. laanwj referenced this in commit bfd807ff32 on Jul 24, 2015
16. zkbot referenced this in commit eaaa5f625f on Feb 10, 2017
17. zkbot referenced this in commit 9af55822fb on Feb 15, 2017
18. zkbot referenced this in commit a7cf698873 on Mar 4, 2017
19. MarcoFalke locked this on Sep 8, 2021

Contributors
laanwj dexX7 jonasschnelli jgarzik

Labels
Bug RPC/REST/ZMQ