Float formatting differs from bitcoin core

[arnuschky]

Bitcoin core formats all floats as %.8f in JSON, while btcd omits trailing zeros.

This also leads to btcd returning an integer (1) when bitcoin core returns a float (1.0). Differing results based on this could be considered a bug in the serializer, but if your striving for compatibility, this should maybe be fixed. For example, I am using python-bitcoinrpc. It converts all floats automatically to Decimal objects, but misses the ones btcd returns as integers.

Note: @davecgh mentioned that there's only a single number type in JSON, so technically there's no difference

[davecgh]

Thanks for opening this for discussion.

[jrick]

Might need something similar to this to control how the numbers are formatted.

http://play.golang.org/p/1blGf1w_Rc

[luke-jr]

IMO it shouldn't be btcd's job to workaround bugs in other software. bitcoind doesn't guarantee any particular formatting either.

[jrick]

I generally agree. Modifying all of our structures to use json.Number (a string type) is a huge amount of effort for very little gain, and any small gain could be broken a day later with a change from core. It would be better to modify the client code to hint as to what kind of types to unmarshal json values as (I don't know if this is possible with the python client).

The real issue is that json has so many different valid encodings of the same thing, and there's no standard specification of how to go from marshalled json -> language types. A simple byte comparison can't be used to test equivalence of marshalled json, and the python code is not wrong per se to decode the json number to different concrete types.

[davecgh]

I'm leaning towards closing this. I'm certainly not against changing things for the sake of compatibility when the risk is low, but as the others have noted, changing this would require changing every instance of a float to a string type which entails its own level of risk since invalid values could now unmarshal and would have to be manually checked at every call site versus allowing the parser to do it. Also, changing this would cause a trickle effect into not only btcd, but also btcwallet, the RPC client, btcsim, and everything else using the structs.

All of this would introduce a considerable amount of risk in regards to introducing an error. Given this and the fact that it's really the caller's responsibility to handle JSON numeric conversions properly (some languages have typed numbers, some don't), I don't feel like this is something we should attempt to change.

[jrick]

This is also an option: http://play.golang.org/p/lqde4Ub-aZ

Would need explicit casting everywhere it's used.

[davecgh]

That's certainly a better approach than using json.Number which would have the problem I discussed above where the errors would have to be checked at every call site versus allowing the parser to do it.

At least with your proposed approach, only casts would be needed and the compiler would catch those.

[jrick]

The following bogus cast wouldn't be caught:

amt := btcutil.Amount(123.456e8)
famt := btcjson.FloatAmount(amt)

but the existing code has the same potential issue converting from btcutil.Amount to float64.