API versioning; represent and accept amounts as strings #431

This is a two-patch patch against 0.3.24 (since master has a lot of churn in rpc.cpp and I don't want to do all the work merging until I know that this patch has community support).

The first commit adds RPC versioning support. As things need to be changed, older clients still need to be supported in some way. This has been done by taking a numeric token from the URL and using that to determine the version. So if the API is accessed at http://localhost:8332/ then it will use version 0. If http://localhost:8332/bitcoin.v1 then it will use version 1. And of course, http://localhost:8332/bitcoin.v2 would be version 2. For the purposes of versioning, I am declaring versions 0 and 1 identical, to avoid confusion.

The second commit does two things:

1. Instead of requiring a JSON number, BTC amounts may now be sent as strings.
2. If the service is accessed using version 2 or higher, BTC amounts will be returned as strings instead of numbers.

This is an important change, because not all JSON libraries store numbers in a format with 16 decimal digits of precision. I've tested, and when I set bitcoind's paytxfee setting to 10000000.00000001, Python is unable to handle this many significant digits; it represents the number as 10000000.000000009. While not off by much, it does seem to indicate that representing amounts as pure numbers in JSON data may cause some clients to have accuracy issues.

By taking the amounts directly from int64s to strings, the client is free to convert the values to whatever high-precision format it wants (like the Decimal type in Python).

However, while many languages will convert strings to numbers automatically, many will not, so this change won't be backwards-compatible; hence the new API versioning feature.

Here are the effects of this branch, using Python:

>>> bitcoindv0 = jsonrpclib.Server("http://chris:testing@localhost:8332/")
>>> bitcoindv1 = jsonrpclib.Server("http://chris:testing@localhost:8332/bitcoin.v1")
>>> bitcoindv2 = jsonrpclib.Server("http://chris:testing@localhost:8332/bitcoin.v2")
>>> bitcoindv0.getinfo()[u'balance'];
0.10000000000000001
>>> bitcoindv1.getinfo()[u'balance'];
0.10000000000000001
>>> bitcoindv2.getinfo()[u'balance'];
u'0.10000000'

If merged, I believe that f5f97fa would close #247

This was discussed to death a few months ago.

10000000.000000009 will be correctly rounded to 10000000.00000001 by bitcoin, so there is no issue.

Going the other way, bitcoin always gives precise values.

If you want all of your json strings to look pretty, use the latest version of simplejson that fully supports Decimal JSON values; e.g.

import decimal import simplejson simplejson.dumps(decimal.Decimal('10000000.000000009'), use_decimal=True) '10000000.000000009'

I definitely agree that if we ever make a backward-incompatible upgrade to the RPC protocol, amounts encoded as strings should be part of it. However, as each new protocol version introduces legacy for many versions to come, we should be careful about introducing new ones.

Maybe the time is indeed ripe for a new RPC version. In that case, i would prefer to do a larger overhaul, including renaming the calls to be more consistent.

I like the idea of putting version information in the URL, though.

Instead of converting the value to a string by yourself, you should use boost::lexical_cast as I'm doing here,

https://gitorious.org/intersango/bitcoind/commit/665294c486f1eba1bcd84025a42fa5e2886a89a9/diffs

10000000.000000009 will be correctly rounded to 10000000.00000001 by bitcoin, so there is no issue.

There is no information in the JSON specification regarding how numbers should be stored; a JSON implementation is free to use single-precision floating point, and that would not be a bug with that implementation. I repeat, we should not be conveying financial information using an encoded numeric format that does not require implementations to use any specific amount of precision.

(Fixed conversions to double still occurring.)

If you can find a reasonably-widely-used JSON implementation that cannot use double-precision floating point please send me a pointer. I'm strongly against adding 100 more lines of code because "there might theoretically be a bug on hardware or software that we haven't actually ever encountered and may or may not exist."

See #452

I think some amount of API versioning is fair, even outside this decimal-vs-float discussion.

The pull has become unmergeable (without conflicts), and will be closed in 15 days from this message if action is not taken.

To prevent closure, kindly rebase the pull to merge cleanly with the current codebase. If a time extension is needed, please respond to this comment or contact QA@BitcoinTesting.org.

Amounts shouldn't be strings anyway, they should be Numbers.

Their type is numeric yes. If the devs are unwilling to incorporate an integral (satoshi-based) API then a less intrusive change would be to represent the numeric data as strings. In other words, in the order of best to worst:

1. Convey BTC amounts as integers, where the unit 1 is 0.00000001 BTC.
2. Convey BTC amounts as strings representing fractional numbers.
3. Convey BTC amounts as fractional numbers in a data type that is poorly-defined and imprecise enough to cause rounding errors.

3 is what we have now. Using strings is still better than what we have now, though I would strongly urge the BTC devs to adopt 1.

Do you really want to have this argument AGAIN?

Responding to your 1/2/3:

1. JSON does not have an integer type. It just has Number.
2. JSON Numbers ARE strings as they are sent across the wire.
3. The JSON Number type on EVERY IMPLEMENTATION OF JSON THAT ANYBODY CAN FIND is precise enough to represent every single possible bitcoin amount without rounding errors.