Skip to content

rpc: rename CompatMode::V0_37 to V1#11

Closed
mzabaluev wants to merge 3 commits intomainfrom
mikhail/rpc-v1-rename-mode
Closed

rpc: rename CompatMode::V0_37 to V1#11
mzabaluev wants to merge 3 commits intomainfrom
mikhail/rpc-v1-rename-mode

Conversation

@mzabaluev
Copy link
Contributor

@mzabaluev mzabaluev commented Jan 25, 2024

Last thing to do for cometbft/cometbft#1834

The dialect used since 0.37 has been continued as 1.0.

The endpoints should have the /v1 path prefix in 1.0, but the current API does not lend itself to adding this to the request URL path automatically. Update the docs to mention the need to explicitly encode the version path in the URL used by the client.
The CometBFT Go client API does not do that either, so there will be consistency at least.

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Added entry in .changelog/

The dialect used since 0.37 has been continued as 1.0.
The endpoints should have the "/v1" prefix in 1.0, but the current API
does not facilitate adding this to the request URL path automatically.
The CometBFT Go client API does not do that either, so there is consistency.
Also added notes on RPC versioning to the documentation of client APIs.
@mzabaluev mzabaluev marked this pull request as ready for review January 25, 2024 11:00
@mzabaluev mzabaluev requested a review from romac January 25, 2024 11:00
@romac
Copy link
Contributor

romac commented Jan 25, 2024

Given that the RPC data format changed in v0.37, it feels a bit weird to me to name the corresponding combat mode v1 instead of leaving it at v0.37.

I fear it might be confusing for users when they have to specify a compat mode for a node running CometBFT 0.37, as I suspect they would intuitively want to specify v0.34 as they are running a version greater than v0.34 and lesser than v1.

I would suggest two things:
a) Leave things as-is and keep the V0_34 and V0_37 variants
b) Revert the change, add a new V1 variant, and update the compat check to treat V0_37 and V1 the same.

I am partial to option (a) as it seems the cleaner to me, as it expresses exactly the boundaries at which backward incompatible changes were made to the RPC data format, but I could see a case for (b) in case we want to account for the /v1/ prefix in the RPC URL, though it seems that it's not trivial to handle automatically, right @mzabaluev?

@mzabaluev
Copy link
Contributor Author

I agree @romac, there is no need for the third variant and the "starting from" semantics are less confusing.
In the future, the compatibility mode should go away as we discontinue support for 0.34.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants