Skip to content
This repository has been archived by the owner on Feb 23, 2022. It is now read-only.

rpc: define spec for RPC #276

Merged
merged 14 commits into from
Apr 27, 2021
Merged

rpc: define spec for RPC #276

merged 14 commits into from
Apr 27, 2021

Conversation

tac0turtle
Copy link
Contributor

@tac0turtle tac0turtle commented Mar 25, 2021

Description

This PR decribes the RPC layer of tendermint. For now this takes what is present on the Go implementation and calls it the canonical implementation. After this is merged we will be able to reduce or expand the spec and anything outside the spec is an implementation detail.

closes #273

this spec does not contain:

  • tx_search
  • block_search
  • dump_consensus_state
  • consensus_state
  • broadcast_tx_commit
  • dial_seeds
  • dial_peers

Many of these routes are implementation specific and my not be shared across implementations.

@tac0turtle tac0turtle self-assigned this Mar 25, 2021
@tac0turtle tac0turtle requested a review from cmwaters March 31, 2021 13:07
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is still a draft. But looking good so far.

A couple of extra points:

  1. I think this could also be a good area to define the standard Tendermint RPC errors
  2. We don't fully comply with the JSON RPC2.0 spec right? Here might be a good place to outline where exactly we diverge

spec/rpc/README.md Outdated Show resolved Hide resolved
spec/rpc/README.md Outdated Show resolved Hide resolved
spec/rpc/README.md Outdated Show resolved Hide resolved
spec/rpc/README.md Outdated Show resolved Hide resolved
@tac0turtle
Copy link
Contributor Author

tac0turtle commented Apr 6, 2021

1. I think this could also be a good area to define the standard Tendermint RPC errors

Agree here, should this be a separate PR?

2. We don't fully comply with the JSON RPC2.0 spec right? Here might be a good place to outline where exactly we diverge

Instead of trying to define what we support of JSON RPC 2.0 and what we don't. I would like to say the spec is JSON RPC 2.0 compliant and Tendermint-Go is not adhering to the spec. This will get the Tendermint team to become spec compliant at the RPC layer. This is still up for debate, but this aligns with the goal of getting the implementation to follow specs instead of veering from the spec in order to make things easier.

@cmwaters
Copy link
Contributor

cmwaters commented Apr 6, 2021

Agree here, should this be a separate PR?

Sure, up to you

@tac0turtle tac0turtle marked this pull request as ready for review April 13, 2021 09:20
spec/rpc/README.md Outdated Show resolved Hide resolved
spec/rpc/README.md Show resolved Hide resolved

#### Parameters

- `limit (integer)` The amount of txs to respond with.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should be paginated or something. In tendermint-go this has a upper bound of 100 i.e. you can only request the top 100 txs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, it would be good to paginate more things.

spec/rpc/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few minor things but otherwise good to go

spec/rpc/README.md Show resolved Hide resolved
spec/rpc/README.md Outdated Show resolved Hide resolved
spec/rpc/README.md Outdated Show resolved Hide resolved
@tac0turtle tac0turtle merged commit 5dfaa54 into master Apr 27, 2021
@tac0turtle tac0turtle deleted the marko-273 branch April 27, 2021 09:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spec: define an RPC spec
2 participants