bitshares / bitshares-core

BitShares Blockchain node and command-line wallet
https://bitshares.github.io/
Other
1.17k stars 647 forks source link

New market API uses doubles #144

Closed vikramrajkumar closed 6 years ago

vikramrajkumar commented 7 years ago

From @theoreticalbts on January 7, 2016 19:22

The new market API implemented as part of https://github.com/cryptonomex/graphene/issues/503 uses doubles. Use of doubles in any financial system is a worst practice, and this situation should be corrected. If https://github.com/bitshares/bitshares-core/issues/65 is finally implemented we should isolate the double's into their own API.

An alternative version is to refactor the JSON code to take a parse_float function pointer, like Python's JSON parser, and then have the floats parsed as some kind of rational or string class that doesn't lose information.

Copied from original issue: cryptonomex/graphene#504

(The description is updated to link to correct issue numbers)

abitmore commented 6 years ago

Since those API's have been heavily used by 3rd-parties, we've missed the best time to change them. If we really want accurate data with similar functionalities, we need new API's without using doubles.

abitmore commented 6 years ago

Just noticed that doubles will be jsonified as strings, so we can still fix the API's.

oxarbitrage commented 6 years ago

we have the easy option to change the doubles to long doubles but that will not be a definitive solution. i was browsing around and found that int can be used to work around the rounding effects of floats and doubles but unsure how to implement yet. i saw the price type in our project but unsure if it will be for our needs either. any input on implementation will be very appreciated. we can replace the doubles but the question will be with what.

abitmore commented 6 years ago

@oxarbitrage the issue is doubles are not accurate even when have high resolution. For example, in https://github.com/bitshares/bitshares-core/pull/455#pullrequestreview-75914447, the test data you wrote, "amount": "1664.17390000000000327" is simply wrong and may confuse users. I'm not sure what's the best solution so far, but I tend to store and calculate with Integers and rationals, and convert them to strings (do rounding in our own code) at the last step.

oxarbitrage commented 6 years ago

closing by https://github.com/bitshares/bitshares-core/pull/594