bdk icon indicating copy to clipboard operation
bdk copied to clipboard

Add Socks5 proxy transport for RPC blockchain

Open rajarshimaitra opened this issue 3 years ago • 10 comments

Description

This attempts to solve https://github.com/bitcoindevkit/bdk/issues/373.

Because bitcoincore_rpc::Client doesn't support socks5 proxy internally, the easiest option for us is to implement our own socks5 transport and make it compatible with bitcoincore_rpc::Client.

The Transport trait is implemented for ProxyTransport to make it compatible with core rpc Client. ProxyTransport is a simple Socks5 wrapper over TcpStream and handles HTTP data like regular connection, but passes it through a given proxy.

I have implemented the basic structure but getting a generic error while trying to connect through local tor proxy running at 127.0.0.1:9050.

Error

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: RpcClient(JsonRpc(Transport(Io(Custom { kind: Other, error: "general SOCKS server failure" }))))', src/main.rs:77:62
stack backtrace:

The sample test code is here https://github.com/rajarshimaitra/bdk-example/blob/main/src/main.rs

I am investigating on the error.

Notes to the reviewers

Making the proxy transport probably warrant a dedicated module for rpc. Once the current approach is finalized, that refactoring can be done on a follow up PR. Suggestions welcome.

Checklists

All Submissions:

  • [x] I've signed all my commits
  • [x] I followed the contribution guidelines
  • [x] I ran cargo fmt and cargo clippy before committing

New Features:

  • [ ] I've added tests for the new feature
  • [ ] I've added docs for the new feature
  • [ ] I've updated CHANGELOG.md

rajarshimaitra avatar Dec 03 '21 13:12 rajarshimaitra

Thanks @afilini for the look.

It seems a bit redundant to reimplement an http client in here: can you first try opening a pr to the jsonrpc library to see if they somehow let you build a Client from an existing TcpStream?

I was looking for ways of how to do this easier in one of our upstream bitcoincorerpc or jsonrpc. But unfortunately the way jsonrpc::Client is written, it doesn't allow putting a TcpStream inside.

https://github.com/apoelstra/rust-jsonrpc/blob/ee78ede9a1eb543e0d64c4658019a6e4f7868d09/src/client.rs#L46-L53

The Client is wrapper over a Transport trait object. I think they did it make the lib flexible. So if we want to put a TcpStream in it, we have to make it Transport, which is essentially what I did in this PR with a Socks5Stream.

So weather we wanna do it in jsonrpc or anywhere, the approach kinda needs to be same, unless we propose to heavily refactor jsonrpc::Client which seems infeasible to me.

So instead of putting the whole thing in the upstream, I think its better to maintain it in our own repo for our need. The upstreams (both bitcoincorerpc and jsonrpc) provides specific APIs (from_jsonrpc and with_transport respectively) to enable this customization. So I think that's also their way of saying, "use your own Transport if you don't wanna use the default"?

rajarshimaitra avatar Dec 08 '21 12:12 rajarshimaitra

Sorry I got confused, I meant the http client: https://github.com/apoelstra/rust-jsonrpc/blob/ee78ede9a1eb543e0d64c4658019a6e4f7868d09/src/simple_http.rs

Looking at it they don't store the TcpStream, so maybe that's not feasible.. but it sucks to have to just repeat the same code twice (also, not sure if there are licensing issues if we just copy their stuff in here).

afilini avatar Dec 09 '21 10:12 afilini

Looking at it they don't store the TcpStream, so maybe that's not feasible.. but it sucks to have to just repeat the same code twice (also, not sure if there are licensing issues if we just copy their stuff in here).

The only thing similar with their simplehttp is the http read write part. And that's generic http stuffs. But ya it sucks to have the same http logic twice.

There is another way I can think of is to propose changing their simplehttp client to include Socks5Stream inside depending on weather proxy options are provided. Then jsonrpc itself will be able provide proxy support. Do you think that would be a better approach?

Also any clue on the generic socks failure error? I can't seem to find any concrete reference in open net, and debugging inside isn't giving any more clue than this vague error message.

I have checked that my local tor proxy at port 9050 is working fine with curl.

rajarshimaitra avatar Dec 10 '21 12:12 rajarshimaitra

Now that #501 is in you can rebase on master to fix the CI issue.

notmandatory avatar Dec 16 '21 18:12 notmandatory

Now that https://github.com/bitcoindevkit/bdk/pull/501 is in you can rebase on master to fix the CI issue.

Yeah but I am still not sure if this is the best way to go for this..

rajarshimaitra avatar Apr 04 '22 10:04 rajarshimaitra

No problem, we can leave as draft until you decide how you want to proceed.

notmandatory avatar Apr 05 '22 00:04 notmandatory

My 2 cents on this is I don't think there is a compelling use-case for it. For mobile devices I think the preferred method will be to connect to public or home hosted bitcoind+electrs servers, or use compact block filters. But I'm not completely opposed to it if you have other uses in mind and/or find others who would use it.

notmandatory avatar Jun 07 '22 19:06 notmandatory

One use case I can have in mind, is a personal mobile wallet connected to a home Umbrel node.. Umbrel can't be exposed outside home network without Tor and electrum server overhead just for personal usage doesn't make sense.. Bitcoin RPC is enough to operate a wallet that only handles someone's personal data.. Electrums are useful when they wanna expose blockchain service to public.. And then also users need trusted reputed 3rd party Electrum servers and that server have user transaction data available, so that creates potential privacy leak.. (Tor doesn't help there, because the endpoint itself is the point of failure)

Many of the home use cases with vanilla core (Umbrel or something else) with mobile wallet integration will require Tor support for our RPC blcockhain.. They ideally will need Tor in the mobile too.. But not having Tor support for RPC restricts them to

  • Either connect only in local home network.
  • Or delpoy electrum server too and then route to it via Tor. (In this case the wallet devs falls back on running their own electrum service, because its too much too ask to wallet users).

I personally don't find the electrum way friendly especially for home use cases.. Its an extra overhead (another 400 gb of storage) for home node runners with no added benefits..

I forgot which one but Specter/Sparrow wallet allows to connect to a home RPC over Tor.. That makes it very useful to have it operated from anywhere in the world.. And it doesn't need electrum too..

rajarshimaitra avatar Jun 07 '22 19:06 rajarshimaitra

According to the electrs features doc the index overhead should only be about 10%, you may be thinking of esplora which does use a lot more disk space and is harder to run on a home single board computer. And if someone does want to run only a simple bitcoind node and access it remotely this can also be done with BDK via compact block filters since we do support a single peer. I don't recall if we support the TOR proxy for those connections, if not I think that'd be more useful since it would also allow you to connect to a (single) public node with a little better privacy than the electrum protocol.

All that said, if you really think this is something folks will use then I'm not against it on any technical grounds, my primary concern is that it won't be used and doesn't look like a simple thing to add.

EDIT: The compact_filters Peer looks like it does already support connecting via a TOR proxy.

notmandatory avatar Jun 07 '22 20:06 notmandatory

Yes, the Compact Filter way seems like another possibility, even though isn't meant for this purpose. Users also have to configure their node to allow compact filters since by default its not active for core. But then, it would open compact filter service of the node to public too, and that will cost them good bandwidth (recently pointed out by Murch in a core review, as there are not much CBF nodes out there, the burden of existing node is more, hopefully that will improve in future). But most home node setup packages comes with CBF disabled.

Yes I agree, its not a easy thing to get into and also not dependent on BDK in any way. Tor support for RPC has to be done in lower level crates, and I am not sure if it will ever get done there too. Its not difficult to implement, but its difficult to organize PR and reviews in all those crates.

But pushing the ball on that front in https://github.com/apoelstra/rust-jsonrpc/issues/57

Lets see if it can get somewhere.

I really think it will be very useful. Otherwise the RpcBlockhain has very few limited application (only when the wallet and node are in same local network). With some upcoming changes in core, our RPC sync logic can be made more powerful and faster. But it would be sad if we can't use it in its full form in all home applications, for just this one missing piece.

rajarshimaitra avatar Jun 08 '22 08:06 rajarshimaitra

@rajarshimaitra here's an example of a socks proxy server, and is even in rust, you may be able to use to help with integration testing this feature without involving TOR. You'd have to spin up a socks5-server instance that proxies back to your localhost bitcoind. At least you should be able to verify the socks5 proxy protocol and auth is working (as long as socks5-server is correctly implementing the protocol from the server side).

https://crates.io/crates/socks5-server ~~https://github.com/shadowsocks/shadowsocks-rust~~ nevermind this one.. has non-standard client/server encryption

notmandatory avatar Nov 16 '22 14:11 notmandatory

Thanks @notmandatory , I think this PR can be closed if you wanna clean up the PR list.. We are not going with this approach.. And I will try to test the socks5 proxy manually with your suggestion..

Update:

It worked :tada: .. https://github.com/rust-bitcoin/rust-bitcoincore-rpc/pull/249#issuecomment-1320834261

Thanks for the suggestion.. Opened the upstream PR for review.. :)

rajarshimaitra avatar Nov 17 '22 14:11 rajarshimaitra

Glad to hear the socks5 proxy suggestion helped. Closing this PR in favor of your rust-bitcoincore-rpc PR.

notmandatory avatar Nov 19 '22 20:11 notmandatory