Kixunil / btc-rpc-proxy

Finer-grained permission management for bitcoind.
MIT License
63 stars 18 forks source link

Major Overhaul #6

Closed dr-bonez closed 3 years ago

dr-bonez commented 4 years ago

So this is a big one. It was also a bit heavy handed with the removal of configure_me since I don't know it as well, but I tried to make it easy to sub back in. All that needs to be done on that front is fill in the create_env::cfg_me module.

With this overhaul we get:

This has all been naively tested, but make sure to do a heavier battery of tests before putting this in a mission critical place.

Kixunil commented 3 years ago

One more thing you might want to know: I plan to add systemd support via my new crate systemd_socket after merging this. Shouldn't change much for you, but let me know if there's some issue I need to address. The crate is not published yet, I'm going to publish it when I test it with a real-world application.

dr-bonez commented 3 years ago

Ok, I think I've addressed everything.

Also, we had an issue with some customers blowing through their data caps during lnd/c-lightning IBD, due to the fact that we reach out to all peers concurrently for the same block. I have added a config option that can limit this concurrency. resolves #7

As a thought for later, perhaps 0.3.0, I have been thinking that there may be a lot of different features that the proxy may want to provide besides this dynamic block fetching, and perhaps the best way to do it would be with a plugin system a-la c-lightning. This way, plugins could register to intercept rpc calls, and provide additional services. In this scenario, this block fetching would be re-implemented as a plugin which is registered to intercept the getblock rpc command.

Kixunil commented 3 years ago

I was thinking very simmilar things long before. :) I would definitely want to also provide txindex via electrs (I added the required method to electrs recently) but I think such change is too small for running yet another service.

However, what I'd really love to see is simulated wallet. This could be a separate daemon. Yet, a simpler implementation may be that the multiple wallets feature of bitcoind is used and the proxy merely makes sure the caller doesn't attempt to access other wallets (and if it doesn't specify which wallet, the proxy forces the right one). Another interesting possibility is to redirect wallet calls to NBXplorer, so that the same wallet can be used for PayJoin and Eclair.

Kixunil commented 3 years ago

I attempted to use it with btcpayserver on regtest and I'm getting this error:

INFO public called getblockchaininfo: ERROR -1 EOF while parsing a value at line 1 column 0

getblockhash seems to work. I also got this message, but only once: getblockchaininfo: ERROR -1 expected value at line 1 column 1

Kixunil commented 3 years ago

Also #![type_length_limit="1717928"] is still missing

dr-bonez commented 3 years ago

EOF while parsing a value

This seems to indicate that bitcoin core is returning a response that isn't valid for JSONRPC. I have noticed this on 401s, but I'm sure other things can cause this. I can make the error message better.

dr-bonez commented 3 years ago

Also #![type_length_limit="1717928"] is still missing

wasn't missing, just isn't high enough :) it seems to depend on rustc version, so maybe I'll give it some margin.

Kixunil commented 3 years ago

This seems to indicate that bitcoin core is returning a response that isn't valid for JSONRPC. I have noticed this on 401s, but I'm sure other things can cause this. I can make the error message better.

Yeah, it was due to the cookie. Would be nice to improve it a bit.

just isn't high enough :)

Hmm, interesting, giving it some margin is a good idea.

Kixunil commented 3 years ago

I'm going to test again tomorrow. Any thoughts on solving rescan?

dr-bonez commented 3 years ago

unfortunately I think rescan is too internal to bitcoind to mock out individually. We would basically have to rebuild the wallet api from scratch to handle this.

Kixunil commented 3 years ago

Damn, that sucks. Cc @openoms

Kixunil commented 3 years ago

Oh, I think I know how it could be achieved:

Either use electrs or scan the chain one block at a time to find transactions related to the addresses from the wallet. Each time a transaction is found call importprunedfunds to notify bitcoind of its existence.

I guess it'd be painfully slow without electrs and eat ton of bandwidth, so requiring electrs for starters would make more sense IMO. Alternatively try to use block filters, but I don't know if block filters can work with pruning.

Kixunil commented 3 years ago

I still get errors when attempting to connect. I believe most likely because cookie is not sent base64 encoded.

Dump from Wireshark (don't worry about cookie, it's a disposable regtest node)

POST / HTTP/1.1
authorization: Basic __cookie__:958d85f51f2851e42a9edd57a1a633418a146643ed1b796d6f6d9c0ad8bfe548
host: 127.0.0.1:18442
content-length: 60

{"id":"1606826395898","method":"getnetworkinfo","params":[]}HTTP/1.1 401 Unauthorized
WWW-Authenticate: Basic realm="jsonrpc"
Date: Tue, 01 Dec 2020 12:39:56 GMT
Content-Length: 0
Content-Type: text/html; charset=ISO-8859-1

POST / HTTP/1.1
authorization: Basic __cookie__:958d85f51f2851e42a9edd57a1a633418a146643ed1b796d6f6d9c0ad8bfe548
host: 127.0.0.1:18442
content-length: 60

{"id":"1606826425876","method":"getnetworkinfo","params":[]}HTTP/1.1 401 Unauthorized
WWW-Authenticate: Basic realm="jsonrpc"
Date: Tue, 01 Dec 2020 12:40:26 GMT
Content-Length: 0
Content-Type: text/html; charset=ISO-8859-1

The content matches the cookie file, but the encoding is not base64. Stream sent from the client to the proxy:

POST / HTTP/1.1
host: 127.0.0.1:18443
authorization: Basic cHVibGljOnB1YmxpYw==
content-length: 60
Connection: close

{"id":"1606826395898","method":"getnetworkinfo","params":[]}HTTP/1.1 401 Unauthorized
www-authenticate: Basic realm="jsonrpc"
date: Tue, 01 Dec 2020 12:39:56 GMT
content-type: text/html; charset=ISO-8859-1
content-length: 0
Kixunil commented 3 years ago

Seems to work, merging and will test it in experimental for a while. I think I'll make a few more changes to the proxy before activating this feature in my repository. Do you want to get pinged about them?

dr-bonez commented 3 years ago

yes please