Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[rpc] module concerns #337

Open
liamsi opened this issue Jun 17, 2020 · 3 comments
Open

[rpc] module concerns #337

liamsi opened this issue Jun 17, 2020 · 3 comments
Labels
enhancement New feature or request rpc

Comments

@liamsi
Copy link
Member

liamsi commented Jun 17, 2020

Motivation

As we are introducing an rpc service (aka light-node #219), now is a good time to look at different options regarding the rpc client (and soon server) lib.

The current approach seems to be to write all types and logic from scratch.

This has the major benefit that we don't introduce any dependencies, and, that we can manually guarantee compatibility with the go implementation (which might not always adhere to standards, e.g. see: tendermint/tendermint#2949).

The major drawback here is that we need to implement details that are not directly tied to tendermint core types or business logic. e.g., we need to make the jsonrpc wrapper type is properly tested (#298), or that we care about control messages in the web-socket implementation (#311).

Actionable Items

Reconsider using a library for most of the rpc server (and partly client) logic.

For that a brief writeup (adr) on pros and cons on libraries leaving a documentation trace why a particular choice was made would be ideal.

Currently paritiy's crates look most promising here:

But they might soon be replaced by an asny/awaitified variant: https://github.com/paritytech/jsonrpsee

Also, consider making the rpc module a separate crate, see: #7 (comment)

Drawbacks

related

@liamsi liamsi changed the title [rpc] Overhaul rpc module [rpc] rpc module concerns Jun 17, 2020
@romac romac added the rpc label Jun 17, 2020
@xla xla added the enhancement New feature or request label Jun 17, 2020
@brapse
Copy link
Contributor

brapse commented Jun 18, 2020

Just to relay the conclusion from out of band conversations, I think this makes sense. Going with jsonrpc to provide consistent documentation is a win. Let's continue to be mindful of dependencies and the complexities that come with them. 🙏

@xla xla self-assigned this Jun 18, 2020
@xla xla changed the title [rpc] rpc module concerns [rpc] module concerns Jun 18, 2020
@liamsi
Copy link
Member Author

liamsi commented Jun 18, 2020

referencing this: #289 (comment) (@xla)

[...] introduce fine-grained feature flags in the rpc crate which on-demand enable the client, event_listener and such.

Makes most sense to me.

xla added a commit that referenced this issue Jun 18, 2020
Guard client code paths To avoid the transient dependency on net related
crates for users who are primarily interested in the core types.

Follow-up #339
Ref #337
liamsi pushed a commit that referenced this issue Jun 18, 2020
Guard client code paths To avoid the transient dependency on net related
crates for users who are primarily interested in the core types.

Follow-up #339
Ref #337
@ebuchman
Copy link
Member

Given all the recent work with the rpc crate, is this issue still relevant? cc @thanethomson

@xla xla removed their assignment Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rpc
Projects
None yet
Development

No branches or pull requests

5 participants