Competing json rpc implementations
tcharding opened this issue · 12 comments
There is another crate that also implements json rpc: https://github.com/thomaseizinger/rust-jsonrpc-client
I am no where near an expert on this sort of thing but I spent an hour or so implementing some basic code to hit bitcoind by way of this crate and also @thomaseizinger's crate. As far as I can tell the biggest differences outside of cosmetics are:
- Thomas' crate supports async
- Thomas's crate allows transport backends to be plugged in where as this one implements its own simple transports
- This crate has minimal dependencies (only serde stuff) where as the dependency graph of Thomas' is bigger, albeit still lean until one starts adding transport backends
I'm just flagging this since we are considering bringing this crate under the rust-bitcoin ecosystem.
I think, if @thomaseizinger's crate is (a) maintained, (b) willing to define a fixed MSRV (ideally 1.41, at least when no backends are available), and (c) willing to port our "simple HTTP" backend so that it's usable without reqwest or other stuff, then we could deprecate this crate in favor of that one.
I think, if @thomaseizinger's crate is (a) maintained, (b) willing to define a fixed MSRV (ideally 1.41, at least when no backends are available), and (c) willing to port our "simple HTTP" backend so that it's usable without reqwest or other stuff, then we could deprecate this crate in favor of that one.
a) It is definitely maintained but help is always appreciated.
b) I'll have to check what the current MSRV is but the goal is to keep it as minimal as possible given the dependencies. syn and serde are kind of essential, we might be able to drop url if it causes problems. I am open to make 1.41 work.
c) If we'd want to use a "simple-http" backend, it would be great to release it as another crate and provide support for it similar to all other clients.
Regarding (c), our "simple http" backend is probably not general enough to be in a separate crate with a useful API. It basically just echoes hardcoded headers into a socket.
Regarding (c), our "simple http" backend is probably not general enough to be in a separate crate with a useful API. It basically just echoes hardcoded headers into a socket.
Hmm, I see the issue.
For the client to be async, one needs to choose an async TcpSocket: either tokio, async-std or something else. async-io did some innovation here as well I think.
Meaning, embedding a client within my client crate would require us to make that decision or forward that decision to the user via feature-flags. Neither are particularly clean and in the spirit of pluggable backends.
If we don't call it simple-http, does the API have to be particularly general to be worth publishing to crates.io? We could name it post-async-json for example and thus restrict the API to only what we need: asynchronously write JSON as a HTTP POST onto a stream.
I'm happy to try to extract stuff and publish them, but it's sounding like these libraries have different goals ... in particular this one has no async support, it's really just trying to be a barebones MVP for talking to bitcoin core.
What sort of API are you imagining?
One natural thing to share would be serialization types. I think they are already in a separate crate.
I'm happy to try to extract stuff and publish them, but it's sounding like these libraries have different goals ... in particular this one has no async support, it's really just trying to be a barebones MVP for talking to bitcoin core.
That may be true. Coming back to @tcharding original motivation:
I'm just flagging this since we are considering bringing this crate under the rust-bitcoin ecosystem.
My 2c are that for outsiders, it should be clear when to use which library if there will be multiple.
The other thing is, if there is a production-grade client, is there a need for a more basic one or shouldn't the effort better be spent making the production-grade one better/leaner/fit more usecases etc.
@thomaseizinger this library does exactly what's needed of it, in 1500 lines of straightforward code with no deps except base64 (which we could inline in another 20LOC if we wanted) and serde. If that's what you mean by "production ready" then I agree with you -- but if by "production ready" you mean something that supports async and depends on the whole tokio tree etc., then I don't see how these libraries can converge.
IOW the point of this library is for people who want to talk to bitcoin core over a dead-simple ascii protocol without needing to double their dep tree and being exposed to tons of Rust language churn.
Perhaps "production-grade" was the wrong terminology to use. I've based it off your wording that this library is an MVP and thus being feature-rich is by definition a non-goal.
Supporting async may not necessarily imply "production-grade" but at the same time, blocking IO creates a lot of inconvenience for async-applications.
In any case, I don't think we need to force any convergence if the goals are distinctly different.
Cool, I think we're in agreement. The main reason I describe this project as a "MVP" is because it's mostly unmaintained ... which historically has been because of the total lack of ecosystem support for doing this.
I think though we can probably start moving this library beyond the MVP stage and start tightening it up, without trying to be too feature-ful (or async-supporting, sorry), because we have enough people in the rust-bitcoin community that we can manage ongoing maintanance.