chris-belcher / electrum-personal-server

Maximally lightweight electrum server for a single user
MIT License
599 stars 109 forks source link

Batching? #186

Open Overtorment opened 4 years ago

Overtorment commented 4 years ago

I noticed there is no support for json-rpc batching:

$ echo '[{"id": 1, "method": "blockchain.scripthash.get_balance", "params": ["716decbe1660861c3d93906cb1d98ee68b154fd4d23aed9783859c1271b52a9c"]}, {"id": 2, "method": "blockchain.scripthash.get_balance", "params": ["9f23070df9a696196571f1be061059dea4076d72ee6b321aff3b749967c6f5b7"]}]' | timeout 2 openssl s_client -quiet  -connect 69.64.32.45:50666  2>/dev/null
$ 

in EPS logs: ERROR:2020-04-01 14:47:03,607: IOError: OSError('Bad client query, no "method"',)

while electrumx has it:

$ echo '[{"id": 1, "method": "blockchain.scripthash.get_balance", "params": ["716decbe1660861c3d93906cb1d98ee68b154fd4d23aed9783859c1271b52a9c"]}, {"id": 2, "method": "blockchain.scripthash.get_balance", "params": ["9f23070df9a696196571f1be061059dea4076d72ee6b321aff3b749967c6f5b7"]}]' | timeout 2 openssl s_client -quiet  -connect electrum1.bluewallet.io:443 2>/dev/null
[{"jsonrpc": "2.0", "result": {"confirmed": 51432, "unconfirmed": 0}, "id": 1}, {"jsonrpc": "2.0", "result": {"confirmed": 1217111, "unconfirmed": 0}, "id": 2}]

any plans to add it? bluewallet relies on batching a lot

Overtorment commented 4 years ago

spec: https://www.jsonrpc.org/specification#batch

chris-belcher commented 4 years ago

Thanks for the issue. I've got no plans right now, but would accept PRs. I'll probably do some work on EPS again in 2-3 months so might implement it then, since batching possibly looks quick to implement.

Since it looks like you're playing around with LN on EPS, if you haven't already read issue #174. EPS can't support lightning now unless Bluewallet does something funky. We'll probably end up using Option 4. from the issue to make EPS work with Electrum's LN implementation.

ncoelho commented 4 years ago

Thanks for the reply @chris-belcher

Just to give you some context, this is not related to Lightning. EPS/Electrs/ElectrumX are starting to come as service installed by default by the "node in the box" startups (nodl, mynode, etc). And we started to notice many users complaining that their connection to Bluewallet/EPS was failing (we promote full node usage heavily), but working for ElectrumX users. So we decided to look deeper into it and stumble upon this issue.

Gonna link to the issues we got on our repo (many others on twitter/telegram) just give a sense of "demand" :D

https://github.com/BlueWallet/BlueWallet/issues/936 https://github.com/BlueWallet/BlueWallet/issues/861 https://github.com/BlueWallet/BlueWallet/issues/813 https://github.com/BlueWallet/BlueWallet/issues/876 https://github.com/BlueWallet/BlueWallet/issues/579

Overtorment commented 4 years ago

@chris-belcher theoretically, what files should I look at if I wanted to implement batching and send a PR..?

chris-belcher commented 4 years ago

@Overtorment This is probably the most relevant part: https://github.com/chris-belcher/electrum-personal-server/blob/master/electrumpersonalserver/server/common.py#L115-L142 It is the code which reads data from the client, parses it into json and passes that json to the handle_query() method. Probably the needed fix is to use something like if isinstance(query, list): to check if the query is batched. The callback method send_reply_fun() needs to be edited to store replies into a list which is an output buffer, and at the end of each loop iteration that output buffer can be sent to the client. That should be enough to support batching I think.

Overtorment commented 4 years ago

I prepared some workarounds meanwhile
https://github.com/BlueWallet/BlueWallet/pull/940