Skip to content

Commit

Permalink
rpc: Version the RPC APIs (cometbft#1412)
Browse files Browse the repository at this point in the history
* Starting to add versions

* Adding a test with API version

* Updates the openapi documentation

* Updating the openapi.yml file.

* Placating the linter

* Improves the text in openapi.yml and updates the RPC spec.

* Fix lint error

* Renames the methods in the client code.

* Updating inspect to support the v1 API

* Port the fix for 1428

* Simplifying the code for handling 2 versions of the API

* updating the http client routes

* Fixing routes

* Adding the changelog

* Reverting changes to route and instead add different root handlers

* Reverting changes to route and instead add different root handlers

* Updating UPDATING.md

* Apply suggestions from code review

Co-authored-by: Thane Thomson <[email protected]>

* Fixing e2e

* Updating some tests

* Testing the use of the prefix along with the websocket keywork

* update commands to use /v1/websockets

* update tests to use v1/websocket

* update loadtest

* Debug docker vm adjustments

* fixing test

* fix typo

* fix lint

* fix test

* move startComet to inside the test loop

* handling different roots

* rpc: Fix v1 path handling

Signed-off-by: Thane Thomson <[email protected]>

* fixing typo

* revert timeout change

* rpc: Hard-code /websocket endpoint in client

Signed-off-by: Thane Thomson <[email protected]>

* Add upgrading entry regarding hard-coding of /websocket endpoint

Signed-off-by: Thane Thomson <[email protected]>

* Reverting debugging change

* Add changelog entry for hard-coded websocket endpoint path

Signed-off-by: Thane Thomson <[email protected]>

* Updating tests

---------

Signed-off-by: Thane Thomson <[email protected]>
Co-authored-by: Thane Thomson <[email protected]>
  • Loading branch information
lasarojc and thanethomson authored Oct 31, 2023
1 parent 0ddc508 commit d93ba1a
Show file tree
Hide file tree
Showing 27 changed files with 421 additions and 242 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- `[rpc/client]` Hard-code the `/websocket` endpoint path such that it is
no longer configurable, removing the related client constructor parameter
([\#1412](https://github.com/cometbft/cometbft/pull/1412))
3 changes: 3 additions & 0 deletions .changelog/unreleased/improvements/1412-rpc-versioning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- `[rpc]` The RPC API is now versioned, with all existing endpoints accessible
via `/v1/*` as well as `/*`
([\#1412](https://github.com/cometbft/cometbft/pull/1412))
26 changes: 23 additions & 3 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,20 @@ The minimum Go version has been bumped to [v1.21][go121].
* The `Mempool` interface was modified on the following methods. Note that this
interface is meant for internal use only, so you should be aware of these
changes only if you happen to call these methods directly.
- `CheckTx`'s signature changed from
* `CheckTx`'s signature changed from
`CheckTx(tx types.Tx, cb func(*abci.ResponseCheckTx), txInfo TxInfo) error`
to `CheckTx(tx types.Tx) (abcicli.ReqRes, error)`.
- The method used to take a callback function `cb` to be applied to the ABCI
* The method used to take a callback function `cb` to be applied to the ABCI
`CheckTx` response. Now `CheckTx` returns the ABCI response of type
`abcicli.ReqRes`, on which the callback must be applied manually. For
example:

```golang
reqRes, err := CheckTx(tx)
cb(reqRes.Response.GetCheckTx())
```
- The second parameter was `txInfo`, which essentially contained information

* The second parameter was `txInfo`, which essentially contained information
about the sender of the transaction. Now that information is stored in the
mempool reactor instead of the data structure, so it is no longer needed in
this method.
Expand All @@ -41,6 +43,24 @@ The minimum Go version has been bumped to [v1.21][go121].
* Removed the `replay` and `replay-console` subcommands
([\#1170](https://github.com/cometbft/cometbft/pull/1170))

### RPC API

* The RPC API is now versioned.
Although invoking methods without specifying the version is still supported for now,
support will be dropped in future releases and users are urged to use the versioned
approach.
For example, instead of `curl localhost:26657/block?height=5`, use
`curl localhost:26657/v1/block?height=5`.

* The `/websocket` endpoint path is no longer configurable in the client or
server. Creating an RPC client now takes the form:

```golang
// The WebSocket endpoint in the following example is assumed to be available
// at http://localhost:26657/v1/websocket
rpcClient, err := client.New("http://localhost:26657/v1")
```

## v0.38.0

This release introduces state machine-breaking changes, as well as substantial changes
Expand Down
4 changes: 2 additions & 2 deletions cmd/cometbft/commands/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ func init() {
DebugCmd.PersistentFlags().StringVar(
&nodeRPCAddr,
flagNodeRPCAddr,
"tcp://localhost:26657",
"the CometBFT node's RPC address (<host>:<port>)",
"tcp://localhost:26657/v1",
"the CometBFT node's RPC address (<host>:<port>) and version",
)

DebugCmd.AddCommand(killCmd)
Expand Down
2 changes: 1 addition & 1 deletion cmd/cometbft/commands/debug/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func dumpCmdHandler(_ *cobra.Command, args []string) error {
}
}

rpc, err := rpchttp.New(nodeRPCAddr, "/websocket")
rpc, err := rpchttp.New(nodeRPCAddr)
if err != nil {
return fmt.Errorf("failed to create new http client: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/cometbft/commands/debug/kill.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func killCmdHandler(_ *cobra.Command, args []string) error {
return errors.New("invalid output file")
}

rpc, err := rpchttp.New(nodeRPCAddr, "/websocket")
rpc, err := rpchttp.New(nodeRPCAddr)
if err != nil {
return fmt.Errorf("failed to create new http client: %w", err)
}
Expand Down
20 changes: 10 additions & 10 deletions inspect/inspect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestBlock(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)
resultBlock, err := cli.Block(context.Background(), &testHeight)
require.NoError(t, err)
Expand Down Expand Up @@ -143,7 +143,7 @@ func TestTxSearch(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)

page := 1
Expand Down Expand Up @@ -191,7 +191,7 @@ func TestTx(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)

res, err := cli.Tx(context.Background(), testHash, false)
Expand Down Expand Up @@ -240,7 +240,7 @@ func TestConsensusParams(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)
params, err := cli.ConsensusParams(context.Background(), &testHeight)
require.NoError(t, err)
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestBlockResults(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)
res, err := cli.BlockResults(context.Background(), &testHeight)
require.NoError(t, err)
Expand Down Expand Up @@ -337,7 +337,7 @@ func TestCommit(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)
res, err := cli.Commit(context.Background(), &testHeight)
require.NoError(t, err)
Expand Down Expand Up @@ -390,7 +390,7 @@ func TestBlockByHash(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)
res, err := cli.BlockByHash(context.Background(), testHash)
require.NoError(t, err)
Expand Down Expand Up @@ -442,7 +442,7 @@ func TestBlockchain(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)
res, err := cli.BlockchainInfo(context.Background(), 0, 100)
require.NoError(t, err)
Expand Down Expand Up @@ -494,7 +494,7 @@ func TestValidators(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)

testPage := 1
Expand Down Expand Up @@ -554,7 +554,7 @@ func TestBlockSearch(t *testing.T) {
// Determine more deterministic method for prompting a context switch
startedWG.Wait()
requireConnect(t, rpcConfig.ListenAddress, 20)
cli, err := httpclient.New(rpcConfig.ListenAddress, "/websocket")
cli, err := httpclient.New(rpcConfig.ListenAddress + "/v1")
require.NoError(t, err)

testPage := 1
Expand Down
1 change: 1 addition & 0 deletions inspect/rpc/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ func Handler(rpcConfig *config.RPCConfig, routes core.RoutesMap, logger log.Logg
server.ReadLimit(rpcConfig.MaxBodyBytes))
wm.SetLogger(wmLogger)
mux.HandleFunc("/websocket", wm.WebsocketHandler)
mux.HandleFunc("/v1/websocket", wm.WebsocketHandler)

server.RegisterRPCFuncs(mux, routes, logger)
var rootHandler http.Handler = mux
Expand Down
2 changes: 1 addition & 1 deletion light/provider/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func New(chainID, remote string) (provider.Provider, error) {
remote = "http://" + remote
}

httpClient, err := rpchttp.NewWithTimeout(remote, "/websocket", timeout)
httpClient, err := rpchttp.NewWithTimeout(remote, timeout)
if err != nil {
return nil, err
}
Expand Down
121 changes: 65 additions & 56 deletions light/provider/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,60 +34,69 @@ func TestNewProvider(t *testing.T) {
}

func TestProvider(t *testing.T) {
app := kvstore.NewInMemoryApplication()
app.RetainBlocks = 10
node := rpctest.StartCometBFT(app)

cfg := rpctest.GetConfig()
defer os.RemoveAll(cfg.RootDir)
rpcAddr := cfg.RPC.ListenAddress
genDoc, err := types.GenesisDocFromFile(cfg.GenesisFile())
require.NoError(t, err)
chainID := genDoc.ChainID

c, err := rpchttp.New(rpcAddr, "/websocket")
require.Nil(t, err)

p := lighthttp.NewWithClient(chainID, c)
require.NoError(t, err)
require.NotNil(t, p)

// let it produce some blocks
err = rpcclient.WaitForHeight(c, 10, nil)
require.NoError(t, err)

// let's get the highest block
lb, err := p.LightBlock(context.Background(), 0)
require.NoError(t, err)
require.NotNil(t, lb)
assert.True(t, lb.Height < 1000)

// let's check this is valid somehow
assert.Nil(t, lb.ValidateBasic(chainID))

// historical queries now work :)
lower := lb.Height - 3
lb, err = p.LightBlock(context.Background(), lower)
require.NoError(t, err)
assert.Equal(t, lower, lb.Height)

// fetching missing heights (both future and pruned) should return appropriate errors
lb, err = p.LightBlock(context.Background(), 1000)
require.Error(t, err)
require.Nil(t, lb)
assert.Equal(t, provider.ErrHeightTooHigh, err)

_, err = p.LightBlock(context.Background(), 1)
require.Error(t, err)
require.Nil(t, lb)
assert.Equal(t, provider.ErrLightBlockNotFound, err)

// stop the full node and check that a no response error is returned
rpctest.StopCometBFT(node)
time.Sleep(10 * time.Second)
lb, err = p.LightBlock(context.Background(), lower+2)
// we should see a connection refused
require.Error(t, err)
require.Contains(t, err.Error(), "connection refused")
require.Nil(t, lb)
for _, path := range []string{"", "/", "/v1", "/v1/"} {
app := kvstore.NewInMemoryApplication()
app.RetainBlocks = 10
node := rpctest.StartCometBFT(app, rpctest.RecreateConfig)

cfg := rpctest.GetConfig()
defer os.RemoveAll(cfg.RootDir)
rpcAddr := cfg.RPC.ListenAddress
genDoc, err := types.GenesisDocFromFile(cfg.GenesisFile())
require.NoError(t, err)
chainID := genDoc.ChainID

c, err := rpchttp.New(rpcAddr + path)
require.Nil(t, err)

p := lighthttp.NewWithClient(chainID, c)
require.NoError(t, err)
require.NotNil(t, p)

// let it produce some blocks
err = rpcclient.WaitForHeight(c, 10, nil)
require.NoError(t, err)

// let's get the highest block
lb, err := p.LightBlock(context.Background(), 0)
require.NoError(t, err)
require.NotNil(t, lb)
assert.True(t, lb.Height < 1000)
assert.True(t, lb.Height >= 10)

// let's check this is valid somehow
assert.Nil(t, lb.ValidateBasic(chainID))

// historical queries now work :)
lb, err = p.LightBlock(context.Background(), 0)
require.NoError(t, err)
require.NotNil(t, lb)
lower := lb.Height - 3
lb, err = p.LightBlock(context.Background(), lower)
require.NoError(t, err)
assert.Equal(t, lower, lb.Height)

// fetching missing heights (both future and pruned) should return appropriate errors
lb, err = p.LightBlock(context.Background(), 0)
require.NoError(t, err)
require.NotNil(t, lb)
lb, err = p.LightBlock(context.Background(), lb.Height+1000)
require.Error(t, err)
require.Nil(t, lb)
assert.Equal(t, provider.ErrHeightTooHigh, err)

_, err = p.LightBlock(context.Background(), 1)
require.Error(t, err)
require.Nil(t, lb)
assert.Equal(t, provider.ErrLightBlockNotFound, err)

// stop the full node and check that a no response error is returned
rpctest.StopCometBFT(node)
time.Sleep(10 * time.Second)
lb, err = p.LightBlock(context.Background(), lower+2)
// we should see a connection refused
require.Error(t, err)
require.Contains(t, err.Error(), "connection refused")
require.Nil(t, lb)
}
}
3 changes: 2 additions & 1 deletion light/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func NewProxy(
logger log.Logger,
opts ...lrpc.Option,
) (*Proxy, error) {
rpcClient, err := rpchttp.NewWithTimeout(providerAddr, "/websocket", uint(config.WriteTimeout.Seconds()))
rpcClient, err := rpchttp.NewWithTimeout(providerAddr, uint(config.WriteTimeout.Seconds()))
if err != nil {
return nil, fmt.Errorf("failed to create http client for %s: %w", providerAddr, err)
}
Expand Down Expand Up @@ -104,6 +104,7 @@ func (p *Proxy) listen() (net.Listener, *http.ServeMux, error) {
)
wm.SetLogger(wmLogger)
mux.HandleFunc("/websocket", wm.WebsocketHandler)
mux.HandleFunc("/v1/websocket", wm.WebsocketHandler)

// 3) Start a client.
if !p.Client.IsRunning() {
Expand Down
1 change: 1 addition & 0 deletions node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,7 @@ func (n *Node) startRPC() ([]net.Listener, error) {
)
wm.SetLogger(wmLogger)
mux.HandleFunc("/websocket", wm.WebsocketHandler)
mux.HandleFunc("/v1/websocket", wm.WebsocketHandler)
rpcserver.RegisterRPCFuncs(mux, routes, rpcLogger)
listener, err := rpcserver.Listen(
listenAddr,
Expand Down
4 changes: 2 additions & 2 deletions rpc/client/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func ExampleHTTP_simple() {

// Create our RPC client
rpcAddr := rpctest.GetConfig().RPC.ListenAddress
c, err := rpchttp.New(rpcAddr, "/websocket")
c, err := rpchttp.New(rpcAddr)
if err != nil {
log.Fatal(err) //nolint:gocritic
}
Expand Down Expand Up @@ -72,7 +72,7 @@ func ExampleHTTP_batching() {

// Create our RPC client
rpcAddr := rpctest.GetConfig().RPC.ListenAddress
c, err := rpchttp.New(rpcAddr, "/websocket")
c, err := rpchttp.New(rpcAddr)
if err != nil {
log.Fatal(err)
}
Expand Down
Loading

0 comments on commit d93ba1a

Please sign in to comment.