Looks great!

Looks great!

Glad to hear you think so! Are there any other endpoints that you'd like to see exposed here @romac?

This looks great for us, I think it would cover our use-case and replace our current proxy service.

It could also be helpful to have access to the /genesis and /net_info endpoints, which we use to implement an auto-configurator (pd testnet join, which pulls config data from a running node and uses it to build a skeleton set of configs), but this is less important to us than the RPCs required for processing transactions.

Glad to hear you think so! Are there any other endpoints that you'd like to see exposed here @romac?

Here's the full list of RPC endpoints we use from Hermes: https://github.com/informalsystems/hermes/blob/master/docs/spec/relayer/Data-Requirements.md

Aside from the first two endpoints which are strictly required for relaying (/health & /consensus_params), the gRPC list is missing these three endpoints:

• /header
• /tx_search
• /block_search

While it would be nice to get rid of JSON-RPC altogether, since we'll have to maintain multi-version support for some time we could initially do without and keep hitting the JSON-RPC endpoints for those, and wait until they are added at a later time.

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@hdevalence I've added a NetworkService from which you can query the genesis data and currently connected peers - does that suffice? Do you need the listener info that the /net_info provides?

@romac I've updated the endpoints - will this suffice for the relayer?

@romac I've updated the endpoints - will this suffice for the relayer?

I might be missing something but I believe the /latest_commit, /commit, and /validators endpoints are missing. Those are required by the light client to fetch and verify light blocks.

Aside from that, looks great! 🙏

@thanethomson what about privval support? (previously tracked as tendermint/tendermint#9256)

If that's in the cards, I have some feedback on the design of that API based on some recent experiences (namely it would be nice if there were an error which KMS systems can use to communicate the last signed h/r/s back to CometBFT in the event of double signing, so CometBFT could potentially update its round counter in response)

what about privval support?

That could definitely be on the cards @tony-iqlusion. We do have a separate tracking issue for that in #476, but we'd be very interested to hear what your ideal API/interaction model would look like.

Really all I'd like is a documented standard way of communicating a double signing error from sign_vote and sign_proposal and including metadata that CometBFT can parse and use to inform future signing requests. Something like the following:

• On double signing error, return 409 Conflict to indicate a signature at the requested height already exists
• The error should include metadata for last_height in h/r/s format, which reflects the last height a signature was computed for

If last_height reflects the current block height, but CometBFT's round counter is lower than the KMS's, then CometBFT can advance the round counter to one more than the one included in last_height.

Right now CometBFT does not handle privval errors very well. This was pretty painful with the last Neutron halt, where the chain halted for several hours and validators needed to resync. When the halt height was reached syncing, it took an additional hour for validators using TMKMS to reach the correct round to participate in consensus. It seems settings like timeout_prevote(_delta) and timeout_precommit(_delta) are not properly honored in this case (which perhaps I should report as a separate bug).

Edit: see also RemoteSignerError

@thanethomson what's the state of this work? Should we take over here?

@thanethomson what's the state of this work? Should we take over here?

Yes please @melekes!