Explicit casting of Decimal to float for python3.3+

[alecalve]

In python2.*, rounding a Decimal returns a float, however, in python 3, rounding a Decimal returns a Decimal. This leads to infinite loops in EncodeDecimal when decoding a response containing a float in python3 (tested with python 3.4.0).

File "/home/antoine/…/virtualenv/lib/python3.4/site-packages/bitcoinrpc/authproxy.py", line 64, in EncodeDecimal return round(o, 8) RuntimeError: maximum recursion depth exceeded while calling a Python object

[robertsdotpm]

I can confirm the same error still exists in the master branch. I just opened an identical pull before I saw this. Lesson learned: check open pulls first.

[luke-jr]

Duplicate of #27, neither of them actually fixing the problem.

[arnuschky]

Patch works for me, python 3.4.0. Can you be more specific in which way this patch doesn't fix the problem, @luke-jr?

[luke-jr]

See my comment on #27 https://github.com/jgarzik/python-bitcoinrpc/pull/27#issuecomment-89170462

[arnuschky]

Reposting @luke-jr's comment (misposted?):

Hmm, this doesn't seem to actually fix the issue, just avoids the infinite loop. In particular, by casting to a float you're not ensuring the Decimal value gets encoded correctly (with 8 decimal places). I think fixing this properly will require subclassing JSONEncoder - no need to round the value, though, just get it included precisely.

I had a look at it and subclassing JSONEncoder seems to be rather complex. A new Decimal serializer would need to handle all special cases as well (NaN etc). As the float serializer is called in several places, we would need to rewrite large parts of the encoder. Rounding and casting to float seems to be much more feasible and does the job just as well.

Another option would be to switch to simplejson, which handles decimals explicitly. Additionally, it would resolve #27, I think.

[luke-jr]

http://stackoverflow.com/a/1960649/2927053 doesn't look that complicated. The whole point of using Decimal is to avoid the float rounding - which casting to float doesn't do.

[arnuschky]

That patch is ~6 years old and doesn't apply to today's structure. I only have a cursory look at it and thought that it won't be that simple to do it similarly. Glad to be proven wrong, though. :)

Regarding rounding issues: I still don't understand what you mean with included precisely. If I understand correctly, there are two issues: 1) rounding is required rather than truncating to prevent issues as described in https://en.bitcoin.it/wiki/Proper_Money_Handling_%28JSON-RPC%29, and 2) float should not be used as the base type of operations as rounding errors accumulate.

The usage as described in this PR appears to be fine to me.

[ghost]

Without this the library does not work on Python 3. I understand the ideology of @luke-jr, however this PR simply allows the library to continue with the same behavior as it always has. It simply takes into account an incompatible change made in Python. I vote for merging this and if we wanted to rework the entire thing it could be a separate issue.

I have been testing @alecalve's branch on Python 3.4.2 and have had no problems.

[jgarzik]

This is the best fix for right now, until a better fix is written