From e061e2778d592826970483e0844308c4e9a12626 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sat, 18 Jul 2015 07:42:23 +0200 Subject: [PATCH 1/3] 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. --- src/rpcserver.cpp | 8 ++++++-- src/test/rpc_tests.cpp | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index bcad06a0c..c5402e0df 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -11,7 +11,6 @@ #include "sync.h" #include "ui_interface.h" #include "util.h" -#include "utilmoneystr.h" #include "utilstrencodings.h" #ifdef ENABLE_WALLET #include "wallet/wallet.h" @@ -133,7 +132,12 @@ CAmount AmountFromValue(const UniValue& value) UniValue ValueFromAmount(const CAmount& amount) { - return UniValue(UniValue::VREAL, FormatMoney(amount)); + bool sign = amount < 0; + int64_t n_abs = (sign ? -amount : amount); + int64_t quotient = n_abs / COIN; + int64_t remainder = n_abs % COIN; + return UniValue(UniValue::VNUM, + strprintf("%s%d.%08d", sign ? "-" : "", quotient, remainder)); } uint256 ParseHashV(const UniValue& v, string strName) diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 25599beaf..7946b0285 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -120,6 +120,29 @@ BOOST_AUTO_TEST_CASE(rpc_format_monetary_values) BOOST_CHECK(ValueFromAmount(100000000LL).write() == "1.00000000"); BOOST_CHECK(ValueFromAmount(2099999999999990LL).write() == "20999999.99999990"); BOOST_CHECK(ValueFromAmount(2099999999999999LL).write() == "20999999.99999999"); + + BOOST_CHECK_EQUAL(ValueFromAmount(0).write(), "0.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount((COIN/10000)*123456789).write(), "12345.67890000"); + BOOST_CHECK_EQUAL(ValueFromAmount(-COIN).write(), "-1.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(-COIN/10).write(), "-0.10000000"); + + BOOST_CHECK_EQUAL(ValueFromAmount(COIN*100000000).write(), "100000000.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN*10000000).write(), "10000000.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN*1000000).write(), "1000000.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN*100000).write(), "100000.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN*10000).write(), "10000.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN*1000).write(), "1000.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN*100).write(), "100.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN*10).write(), "10.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN).write(), "1.00000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN/10).write(), "0.10000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN/100).write(), "0.01000000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN/1000).write(), "0.00100000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN/10000).write(), "0.00010000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN/100000).write(), "0.00001000"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN/1000000).write(), "0.00000100"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN/10000000).write(), "0.00000010"); + BOOST_CHECK_EQUAL(ValueFromAmount(COIN/100000000).write(), "0.00000001"); } static UniValue ValueFromString(const std::string &str) From 7650449a6777710cf818d41862626164da0cd412 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sat, 18 Jul 2015 07:44:19 +0200 Subject: [PATCH 2/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). --- qa/rpc-tests/rest.py | 3 ++- src/rpcserver.cpp | 2 +- src/test/univalue_tests.cpp | 6 +++--- src/univalue/univalue.cpp | 5 ++--- src/univalue/univalue.h | 3 +-- src/univalue/univalue_write.cpp | 7 ------- 6 files changed, 9 insertions(+), 17 deletions(-) diff --git a/qa/rpc-tests/rest.py b/qa/rpc-tests/rest.py index 1a2d326cc..b0cde7268 100755 --- a/qa/rpc-tests/rest.py +++ b/qa/rpc-tests/rest.py @@ -14,6 +14,7 @@ from struct import * import binascii import json import StringIO +import decimal try: import http.client as httplib @@ -243,7 +244,7 @@ class RESTTest (BitcoinTestFramework): response_header_json = http_get_call(url.hostname, url.port, '/rest/headers/1/'+bb_hash+self.FORMAT_SEPARATOR+"json", "", True) assert_equal(response_header_json.status, 200) response_header_json_str = response_header_json.read() - json_obj = json.loads(response_header_json_str) + json_obj = json.loads(response_header_json_str, parse_float=decimal.Decimal) assert_equal(len(json_obj), 1) #ensure that there is one header in the json response assert_equal(json_obj[0]['hash'], bb_hash) #request/response hash should be the same diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index c5402e0df..201fc5eba 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -120,7 +120,7 @@ void RPCTypeCheckObj(const UniValue& o, CAmount AmountFromValue(const UniValue& value) { - if (!value.isReal() && !value.isNum()) + if (!value.isNum()) throw JSONRPCError(RPC_TYPE_ERROR, "Amount is not a number"); CAmount amount; if (!ParseFixedPoint(value.getValStr(), 8, &amount)) diff --git a/src/test/univalue_tests.cpp b/src/test/univalue_tests.cpp index 16bc8d30f..67cb9b962 100644 --- a/src/test/univalue_tests.cpp +++ b/src/test/univalue_tests.cpp @@ -49,7 +49,7 @@ BOOST_AUTO_TEST_CASE(univalue_constructor) double vd = -7.21; UniValue v7(vd); - BOOST_CHECK(v7.isReal()); + BOOST_CHECK(v7.isNum()); BOOST_CHECK_EQUAL(v7.getValStr(), "-7.21"); string vs("yawn"); @@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(univalue_set) BOOST_CHECK_EQUAL(v.getValStr(), "zum"); BOOST_CHECK(v.setFloat(-1.01)); - BOOST_CHECK(v.isReal()); + BOOST_CHECK(v.isNum()); BOOST_CHECK_EQUAL(v.getValStr(), "-1.01"); BOOST_CHECK(v.setInt((int)1023)); @@ -272,7 +272,7 @@ BOOST_AUTO_TEST_CASE(univalue_object) objTypes["distance"] = UniValue::VNUM; objTypes["time"] = UniValue::VNUM; objTypes["calories"] = UniValue::VNUM; - objTypes["temperature"] = UniValue::VREAL; + objTypes["temperature"] = UniValue::VNUM; objTypes["cat1"] = UniValue::VNUM; objTypes["cat2"] = UniValue::VNUM; BOOST_CHECK(obj.checkObject(objTypes)); diff --git a/src/univalue/univalue.cpp b/src/univalue/univalue.cpp index 6920c44c9..1d49a2cfc 100644 --- a/src/univalue/univalue.cpp +++ b/src/univalue/univalue.cpp @@ -86,7 +86,7 @@ bool UniValue::setFloat(double val) oss << std::setprecision(16) << val; bool ret = setNumStr(oss.str()); - typ = VREAL; + typ = VNUM; return ret; } @@ -210,7 +210,6 @@ const char *uvTypeName(UniValue::VType t) case UniValue::VARR: return "array"; case UniValue::VSTR: return "string"; case UniValue::VNUM: return "number"; - case UniValue::VREAL: return "number"; } // not reached @@ -280,7 +279,7 @@ int64_t UniValue::get_int64() const double UniValue::get_real() const { - if (typ != VREAL && typ != VNUM) + if (typ != VNUM) throw std::runtime_error("JSON value is not a number as expected"); double retval; if (!ParseDouble(getValStr(), &retval)) diff --git a/src/univalue/univalue.h b/src/univalue/univalue.h index 54239741e..4742b56f3 100644 --- a/src/univalue/univalue.h +++ b/src/univalue/univalue.h @@ -16,7 +16,7 @@ class UniValue { public: - enum VType { VNULL, VOBJ, VARR, VSTR, VNUM, VREAL, VBOOL, }; + enum VType { VNULL, VOBJ, VARR, VSTR, VNUM, VBOOL, }; UniValue() { typ = VNULL; } UniValue(UniValue::VType initialType, const std::string& initialStr = "") { @@ -78,7 +78,6 @@ public: bool isBool() const { return (typ == VBOOL); } bool isStr() const { return (typ == VSTR); } bool isNum() const { return (typ == VNUM); } - bool isReal() const { return (typ == VREAL); } bool isArray() const { return (typ == VARR); } bool isObject() const { return (typ == VOBJ); } diff --git a/src/univalue/univalue_write.cpp b/src/univalue/univalue_write.cpp index d360c253b..bce3997af 100644 --- a/src/univalue/univalue_write.cpp +++ b/src/univalue/univalue_write.cpp @@ -61,13 +61,6 @@ string UniValue::write(unsigned int prettyIndent, case VSTR: s += "\"" + json_escape(val) + "\""; break; - case VREAL: - { - std::stringstream ss; - ss << std::showpoint << std::fixed << std::setprecision(8) << get_real(); - s += ss.str(); - } - break; case VNUM: s += val; break; From ec249d4a1d572a17dd010464fe3ead532a8b1b70 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sat, 18 Jul 2015 08:16:21 +0200 Subject: [PATCH 3/3] 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. --- src/utilstrencodings.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 7208ca947..1f7a2cae2 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -464,11 +464,12 @@ bool ParseDouble(const std::string& str, double *out) return false; if (str.size() >= 2 && str[0] == '0' && str[1] == 'x') // No hexadecimal floats allowed return false; - char *endp = NULL; - errno = 0; // strtod will not set errno if valid - double n = strtod(str.c_str(), &endp); - if(out) *out = n; - return endp && *endp == 0 && !errno; + std::istringstream text(str); + text.imbue(std::locale::classic()); + double result; + text >> result; + if(out) *out = result; + return text.eof() && !text.fail(); } std::string FormatParagraph(const std::string& in, size_t width, size_t indent)