informalsystems / tendermint-rs

Client libraries for Tendermint/CometBFT in Rust!
Apache License 2.0
609 stars 224 forks source link

[rpc] module concerns #337

Open liamsi opened 4 years ago

liamsi commented 4 years ago

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: https://github.com/tendermint/tendermint/issues/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 (https://github.com/informalsystems/tendermint-rs/issues/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: https://github.com/informalsystems/tendermint-rs/issues/7#issuecomment-645487039

Drawbacks

related

brapse commented 4 years ago

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. 🙏

liamsi commented 4 years ago

referencing this: https://github.com/informalsystems/tendermint-rs/issues/289#issuecomment-645867633 (@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.

ebuchman commented 3 years ago

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