Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Keep current JSONRPC implementation or use library #92

Closed
njgheorghita opened this issue Sep 3, 2021 · 3 comments
Closed

Keep current JSONRPC implementation or use library #92

njgheorghita opened this issue Sep 3, 2021 · 3 comments

Comments

@njgheorghita
Copy link
Collaborator

@ogenev

My concern about the current json-rpc implementation, is that we may need to drop it soon 😄. I'm currently playing with paritytech's json-rpc library and I'm going to implement it in the testing framework branch. I guess next week we will have more clarity if we want to switch to this library or not. (looks promising so far, better error handling, logging and of course less boilerplate code).

@njgheorghita
Copy link
Collaborator Author

njgheorghita commented Sep 3, 2021

@ogenev I just created this issue to track this question.

On one hand I love using a well-written library, on the other hand, it seems to me like the jsonrpc handling is fairly straightforward logic to write and we'll probably want to the ability to fine-tune it somewhat granularly (which some rust libraries make difficult in my experience).

So I'd be curious to hear your thoughts on the library after playing around with it. What are the benefits of using it? What are some potential drawbacks? etc.

@ogenev
Copy link
Member

ogenev commented Sep 5, 2021

I time boxed this issue to 2 days and spend some time trying to prototype implementation of parity's json-rpc library for the testing framework and see if it can be useful for our needs. Keep in mind, that my Rust experience is minimal and i'm learning on demand.

I think that the biggest bottleneck for this library is it's async/await support which is almost non existent, although json-rpc derive crate looks cool. Async support requires from us to return a future from the rpc handlers and await for it in the client code, which is not helpful if we want to use external json-rpc clients (netcat, web3). We also need to add additional boilerplate code to support background async tasks as proposed in this issue.

It also looks like that the library is not under active development and the core crate feels kinda outdated.

I think we should stick with our current json-rpc implementation.

@njgheorghita
Copy link
Collaborator Author

@ogenev Cool, sounds like a thorough review to me, thanks for digging into it. In my (also limited) rust experience, there are some crates that are phenomenal, and some that end up causing more frustration than writing your own implementation if they don't cater to your particular need. Seems safe to close this issue for now

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants