Takes up and supersedes #4738.
This PR keeps the JSON Spirit “backward-compatibility” and passes all tests. Therefore there is a new UniValue number type VREAL to distinct between int and double with the current JSON spirit precision of 8 digits after point.
Because after rfc4627 (and UniValue honors that) some basic types/strings like "null" or "0" and "string" are not valid JSON strings, i had to slightly change some unit tests (will comment below directly in the code).
141@@ -142,8 +142,8 @@ Object CallRPC(const string& strMethod, const Array& params)
142 throw runtime_error("no response from server");
143
144 // Parse reply
145- Value valReply;
146- if (!read_string(strReply, valReply))
147+ Value valReply(UniValue::VSTR);
Value should be replaced with UniValue soon.
149+ else
150+ {
151+ if (!jVal.read(strVal) || (jVal.isNull() && strVal.size() > 0))
152+ if(!jVal.setNumStr(strVal) || jVal.isNull())
153+ throw runtime_error(string("Error parsing JSON:")+strVal);
154+ }
132 static Value ValueFromString(const std::string &str)
133 {
134- Value value;
135- BOOST_CHECK(read_string(str, value));
136+ UniValue value;
137+ BOOST_CHECK(value.setNumStr(str));
Had to change this to setNumStr because read() only parses valid JSON (things like “0.00000001” are not valid JSON).
EDIT: spelling
48@@ -49,7 +49,7 @@ BOOST_AUTO_TEST_CASE(univalue_constructor)
49
50 double vd = -7.21;
51 UniValue v7(vd);
52- BOOST_CHECK(v7.isNum());
53+ BOOST_CHECK(v7.isReal());
Because now there is a distinction between int and real (to make it JSON Spirit compatible), things like -7.21 are now VREAL and no longer VNUM. I decided to make VREAL a standalone type instead of a VNUM subtype.
Edit: spelling
244+ BOOST_CHECK_EQUAL(obj.size(), 0);
245 }
246
247 static const char *json1 =
248-"[1.1,{\"key1\":\"str\",\"key2\":800,\"key3\":{\"name\":\"martian\"}}]";
249+"[1.10000000,{\"key1\":\"str\",\"key2\":800,\"key3\":{\"name\":\"martian\"}}]";
29@@ -26,6 +30,9 @@ class UniValue {
30 UniValue(int64_t val_) {
31 setInt(val_);
32 }
33+ UniValue(bool val_) {
34+ setBool(val_);
35+ }
ut ACK - reviewed the whole thing
Will do some testing later this week.
Thanks for moving this forward!
Thanks for doing this work.
One remark: I see AmountFromValue and ValueFromAmount still make the value pass through a double. My prime gripe with JSON spirit is that monetary values still had to be converted from and to floating point which can cause deviations (see See #3759 and https://bitcoin.stackexchange.com/questions/22716/bitcoind-sendfrom-round-amount-error ).
Ideally a new JSON implementation will avoid this and can interpret and generate Numbers from/to fixed-point or decimal directly. Similar to Python’s
0postdata = json.dumps(list(rpc_call_list), default=EncodeDecimal)
1response = json.loads(responsedata, parse_float=decimal.Decimal)
Agree w/ your gripe @laanwj. However, I think of that as a separate logical step:
Separated thusly it is easier to test the conversion before moving on to more ambitious cleanups.
This is now complete.
Just added another commit (https://github.com/bitcoin/bitcoin/commit/2a3126c336fc0cc2b19f9b89bd48ed3e489a0817) which does completely remove JSON Spirit (also the UniValue wrapper) from the sources and the compile process.
Passes all tests.
If the diff size makes this PR to risky, there would also be the chance to pull this in without https://github.com/bitcoin/bitcoin/commit/2a3126c336fc0cc2b19f9b89bd48ed3e489a0817 or / and separate https://github.com/bitcoin/bitcoin/commit/2a3126c336fc0cc2b19f9b89bd48ed3e489a0817 into another PR.
I think it would be good to remove JSON Spirit in this PR while not breaking the RPC API. After this has been done, we could focus on optimizing the monetary values.
Yes, I agree JSON-spirit should be removed in this PR.
Reviewing & testing right now…
This breaks linearize-hashes.py. Likely, this is caused by JSON-RPC batching no longer working:
0./linearize-hashes.py linearize.cfg
1Traceback (most recent call last):
2 File "./linearize-hashes.py", line 112, in <module>
3 get_block_hashes(settings)
4 File "./linearize-hashes.py", line 68, in get_block_hashes
5 for x,resp_obj in enumerate(reply):
6TypeError: 'NoneType' object is not iterable
(expected output: a list of block hashes)
getrawmempool and getmempoolinfo
- avoid breaking the API because of different number/percision handling
UniValues read() does only read valid json.
- implement find_value() function for UniValue
- replace all Array/Value/Object types with UniValues, remove JSON Spirit to UniValue wrapper
- remove JSON Spirit sources
# Conflicts:
# src/test/rpc_tests.cpp
Strict parsing functions for other numeric types.
- ParseInt64 analogous to ParseInt32, but for 64-bit values.
- ParseDouble for doubles.
- Make all three Parse* functions more strict (e.g. reject whitespace on
the inside)
Also add tests.
I’m getting an error now when running qa/rpc-tests/getblocktemplate_proposals.py:
JSONRPC error: Missing data String key for proposal
Looks like it’s here:
0File "./getblocktemplate_proposals.py", line 86, in assert_template
1 rsp = node.getblocktemplate({'data':template_to_hex(tmpl, txlist),'mode':'proposal'})
