Implement JSON-RPC interface instead of CLI.

[domob1812]

Replace the connection to Bitcoin Core via bitcoin-cli by a connection through its JSON-RPC interface. This is cleaner, as it does not require to convert everything to/from strings. It is also able to handle error codes.

See #137.

I have tested that it works successfully for a sendpayment.py order. More testing would probably be good, though.

[fivepiece]

(moving this comment to #137 )

[chris-belcher]

Read the code, looks pretty good.

I will try to test soon. Eventually run a long-running yield generator with it.

[CohibAA]

ACK - FYI, no problems here during testing of wallet-tool, ob-watcher, sendpayment, patientsendpayment (see 147), and yield-generator. I did not test tumbler, yet.

[chris-belcher]

Tumbler will almost certainly work if the others do.

Did you try getting a coinjoin as well with yield generator?

I have testnet coins if you need them

[CohibAA]

I've received successful orders as a maker using yield-generator with this commit, but I have seen this now several times in the console (doesn't show in logs):

[2015/07/11 xx:xx:xx] rpc: getrawtransaction ['redacted (transaction is valid and confirmed on blockchain']
----------------------------------------
Exception happened during processing of request from ('127.0.0.1', 43359)
Traceback (most recent call last):
  File "/usr/lib/python2.7/SocketServer.py", line 295, in _handle_request_noblock
    self.process_request(request, client_address)
  File "/usr/lib/python2.7/SocketServer.py", line 321, in process_request
    self.finish_request(request, client_address)
  File "/usr/lib/python2.7/SocketServer.py", line 334, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/home/redacted/joinmarket/lib/blockchaininterface.py", line 283, in __init__
    SimpleHTTPServer.SimpleHTTPRequestHandler.__init__(self, request, client_address, base_server)
  File "/usr/lib/python2.7/SocketServer.py", line 649, in __init__
self.handle()
  File "/usr/lib/python2.7/BaseHTTPServer.py", line 340, in handle
self.handle_one_request()
  File "/usr/lib/python2.7/BaseHTTPServer.py", line 328, in handle_one_request
method()
  File "/home/redactedjoinmarket/lib/blockchaininterface.py", line 290, in do_HEAD
txd = btc.deserialize(self.btcinterface.rpc('getrawtransaction', [txid]))
  File "/home/redacted/joinmarket/lib/bitcoin/transaction.py", line 76, in deserialize
obj["version"] = read_as_int(4)
  File "/home/redacted/joinmarket/lib/bitcoin/transaction.py", line 57, in read_as_int
return decode(tx[pos[0]-bytez:pos[0]][::-1], 256)
  File "/home/redacted/joinmarket/lib/bitcoin/py2specials.py", line 89, in decode
result += code_string.find(string[0])
UnicodeDecodeError: 'ascii' codec can't decode byte 0x80 in position 128: ordinal not in range(128)
----------------------------------------

This seems to result in the balances for available mix levels not being updated on the orderbook until another fill request is made.

[chris-belcher]

Good find. That's definitely a bug with this.
Looks like getrawtransaction returns something that causes btc.deserialize() to crash. Looks like an encoding issue.

[CohibAA]

Yes, the jsonrpc call returns something like:

{"result":"RAWTRANSACTIONVALUEHERE","error":null,"id":"getrawtransactiontest"}

while the command line simply returns the bare string (result value from above). I'm sure it's probably a relatively easy fix, but I don't know exactly how to do it myself for certain.

[domob1812]

@CohibAA Thanks for testing! This result of the JSON-RPC call is expected, that's how JSON-RPC works. It should be handled correctly by my code (picking out the result part). Not sure where the error you are seeing comes from. Seems to be related to character encodings (possibly). I can not imagine how, though, as the string is hex encoded.

Is there a chance you can send me the full log without censored tx privately? E. g., by email with GPG or private message on Bitcointalk?

[CohibAA]

@domob1812 sure, whats your id on bct, send me a pm? me@bct

[domob1812]

Sent you a PM; I'm "domob" there. I've now also started my own yield-generator with the patch applied, maybe I can see the problem live for myself.

[domob1812]

Was the decoding error fixed by ad749d7? I've rebased my patch now.

I've been running a yield generator for some time now with JSON-RPC, and it seems to work fine. The only thing I see from time to time as the "utxo already spent" issue ad749d7. This may occur more often now due to the longer confirmation times in the Bitcoin network recently.

[chris-belcher]

I'm going to start running my yield generator with this.

[chris-belcher]

I had to edit some stuff to make it work for me. I think we might be using different python versions that handle encoding slightly differently. I made it check whether the string is unicode and cast it back to an ascii string.
Also fixed the conflicts between some other commits done since then.

My yield generator is running with this now, I'll check for that issue of balances not being updated.

Don't worry about trying to copy this, I can include it when I merge (?)

diff --git a/lib/blockchaininterface.py b/lib/blockchaininterface.py
index dd3ae45..e5c5e1e 100644
--- a/lib/blockchaininterface.py
+++ b/lib/blockchaininterface.py
@@ -306,13 +306,12 @@ class NotifyRequestHeader(SimpleHTTPServer.SimpleHTTPRequestHandler):
            if unconfirmfun == None:
                common.debug('txid=' + txid + ' not being listened for')
            else:
-               jsonstr = None #on rare occasions people spend their output without waiting for a confirm
+               txdata = None #on rare occasions people spend their output without waiting for a confirm
                for n in range(len(txd['outs'])):
-                   txout = self.btcinterface.rpc('gettxout', [txid, n, True])
-                   if txout is not None:
+                   txdata = self.btcinterface.rpc('gettxout', [txid, n, True])
+                   if txdata is not None:
                        break
-               assert jsonstr != ''
-               txdata = json.loads(jsonstr)
+               assert txdata != None
                if txdata['confirmations'] == 0:
                    unconfirmfun(txd, txid)
                    #TODO pass the total transfered amount value here somehow
@@ -385,6 +384,9 @@ class BitcoinCoreInterface(BlockchainInterface):
        if method != 'importaddress':
            common.debug('rpc: ' + method + " " + str(args))
        res = self.jsonRpc.call(method, args)
+       if isinstance(res, unicode):
+           res = str(res)
        return res

    def add_watchonly_addresses(self, addr_list, wallet_name):
@@ -410,13 +412,11 @@ class BitcoinCoreInterface(BlockchainInterface):
        if not set(wallet_addr_list).issubset(set(imported_addr_list)):
            self.add_watchonly_addresses(wallet_addr_list, wallet_name)
            return
-       ret = self.rpc('listtransactions', [wallet_name, 1000, 0, True])
-       buf = json.loads(ret)
-       txs = buf
+       txs = self.rpc('listtransactions', [wallet_name, 1000, 0, True])
+       buf = txs
        # If the buffer's full, check for more, until it ain't
        while len(buf) == 1000:
-           txs = self.rpc('listtransactions', [wallet_name, 1000, str(len(txs)), True])
-           buf = json.loads(ret)
+           buf = self.rpc('listtransactions', [wallet_name, 1000, str(len(txs)), True])
            txs += buf
        used_addr_list = [tx['address'] for tx in txs if tx['category'] == 'receive']
        too_few_addr_mix_change = []
@@ -484,6 +484,7 @@ class BitcoinCoreInterface(BlockchainInterface):
                one_addr_imported = True
                break
        if not one_addr_imported:
+           debug('importing the address ' + notifyaddr + ' so that walletnotify will fire')
            self.rpc('importaddress', [notifyaddr, 'joinmarket-notify', False])
        tx_output_set = set([(sv['script'], sv['value']) for sv in txd['outs']])
        self.txnotify_fun.append((tx_output_set, unconfirmfun, confirmfun))

[chris-belcher]

I've been using this the last few days and it seems to work really well. Anyone else apart from @domob1812 tried it?

One issue is people have to update their joinmarket.cfg, but hopefully there's not too many lazy people using JoinMarket right now.

I'm thinking of merging this soon.

[chris-belcher]

I've been thinking, when people upgrade to this it will break their configuration. I don't want that to happen because it discourages regular updating by people. Everyone should have to update very soon because of the bip66 signature fix (I will send an alert about this.) And I wouldn't want people to update then find they have to mess around with configuration before it works again.

So a way around this problem is to make it configurable which kind of json-rpc to use. In the .cfg file add an extra option

blockchain_source = json-rpc
blockchain_source = json-rpc-socket

We'll update the wiki so new people use json-rpc-socket (or another name if you can think of something better than -socket)
Then some time in the future, remove all the bitcoin-cli code and make json-rpc and json-rpc-socket mean the same thing. Before that we'll add a warning that prints for people still using bitcoin-cli

So this PR would require a small rewrite. And ideally another PR created when bitcoin-cli is phased out so it can be readily merged.

[domob1812]

I've implemented the blockchain_source = json-rpc-socket suggestion. I think this makes sense. I've done some basic tests with both json-rpc (CLI as before) and json-rpc-socket (the new JSON-RPC interface), and they seem to work in both cases.

[chris-belcher]

My blockchain is corrupted so I have to redownload. I'll test this.

[CohibAA]

I have not seen the encoding issue since ad749d7.

edit: #141 (comment) popped up again
all is well, was my error

[tailsjoin]

I have been successfully using this with no issues on yield-generator and sendpayment for 3 days.