Skip to content

rpc: don't lock consensus state #3161

@ebuchman

Description

@ebuchman

RPC calls which grab the ConsensusState may deadlock if the consensus is halted (#1772) and will deadlock if called from an ABCI app during execution (#3148 (comment)).

This includes: /validators, /consensus_params, /status, /dump_consensus_state, /consensus_state,

While HTTP calls from ABCI apps during execution isn't generally encouraged (due to non-determinism), in many cases it's to avoid redundant storage of information (like historical validator sets) in the application. This data is deterministic itself, and if Tendermint can't deterministically respond, there's probably a critical failure in the node anyways. Unless or until we support queries from the App to Tendermint over ABCI directly, it seems beneficial to enable ABCI apps to access this information.

For /validators, /consensus_params, /status, we only need the latest height, so we can easily use blockStore.Height() + 1 instead of locking the consensusState to make them available to applications.

The *consensus_state methods are non-deterministic, so they're off limits to applications in block execution. We could consider making them independent of the main consensus lock by memoizing the relevant JSON every time there's a change to the consensus state, but that's probably not worth it at this point.

Thus, we should:

  • Specify in the RPC and ABCI docs which RPC methods are safe during block execution
  • Make /validators, /consensus_params, /status safe during block execution

Replaces #3151

Metadata

Metadata

Assignees

Labels

C:consensusComponent: ConsensusC:rpcComponent: JSON RPC, gRPCgood first issueContributions Welcome!!

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions