Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP REST API: Debug and Relay APIs #727

Closed
3 tasks done
jm-clius opened this issue Sep 30, 2021 · 25 comments
Closed
3 tasks done

HTTP REST API: Debug and Relay APIs #727

jm-clius opened this issue Sep 30, 2021 · 25 comments
Assignees
Milestone

Comments

@jm-clius
Copy link
Contributor

jm-clius commented Sep 30, 2021

Background

nim-waku provides a JSON-RPC API for non-Nim clients. The original plan was to have this API be REST-like so that it could eventually map to an HTTP REST API, once a secure web server was implemented in Nim.

With the introduction of nim-presto, this now seems possible. Consequently nimbus-eth2 has added a REST API. nim-waku can follow suit.

Acceptance Criteria

In future we also want to add Store, Filter, Admin and Private API functionality for parity with the existing JSON-RPC API. This worked is tracked here: #1076

@LNSD
Copy link
Contributor

LNSD commented May 26, 2022

From my initial conversation with @jm-clius, one of the requirements for the HTTP RESTful API is to be as consistent as we can with nimbus HTTP REST RPC API. So I did a little analysis on the nimbus beacon_chain. These are some highlights of the nimbus implementation that we should follow in nwaku:

  • Nimbus beacon_node exposes the "control interface" in two forms: RPC and REST
  • REST server request handlers are installed in the nim-presto server via the installRestHandlers methods. Handlers are grouped under a common namespace (e.g. debug, beacon or node).
  • API versioning strategy is in the path (e.g. /eth/v1/debug/beacon/states/{state_id}) and each resource can have a different version. Within a group of resources, each resource is versioned independently.

@LNSD
Copy link
Contributor

LNSD commented May 26, 2022

HTTP RESTful is a widespread way to expose service APIs over HTTP. There is a standardized way of specifying a RESTful API named OpenAPI.

@LNSD
Copy link
Contributor

LNSD commented May 26, 2022

While doing a first review of the current JSON-RPC API, seems straightforward the mapping between the RPC methods and the new REST API resources. There are some small details to discuss (e.g. keep the /waku/v2/ common path).

For example, the specification of the debug namespace RPC call using the OpenAPI spec format would be:

openapi: 3.1.0
info:
  title: Waku REST API - Debug
  version: 1.0.0

paths:
  /debug/info:  # <-- Path: namespace + resource
    get:
      summary: Get node info
      description: Retrieve information about a Waku v2 node
      tags:
        - Debug
      responses:
        '200':
          description: Information about a Waku v2 node
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/WakuInfo'
        # TODO Specify the error format for 4XX and 5XX error codes

The idea is to have one OpenAPI spec file per "namespace" (e.g. debug, store, filter, etc.). And by using speccy's resolve command, merge the spec files into a single file to generate the client or the spec documentation.

@LNSD
Copy link
Contributor

LNSD commented May 26, 2022

As one requirement is consistency with the nimbus codebase. So the nim HTTP REST library will be nim-presto. This library is relatively new and no OpenAPI client/server generator exists.

Indeed seems that the OpenAPI generator does not support nim server code generation (see the openAPI server generators list). There is just a "beta" code generator for nim client-side code.

For these reasons, we cannot benefit from this tool to generate the HTTP REST server handlers/stubs. This will have to be done manually. In the future, depending on the number of times the API spec is modified, might be interesting to implement a custom generator that allows us to automate the code boilerplate generation.

@jakubgs
Copy link
Contributor

jakubgs commented May 26, 2022

Consequently nimbus-eth2 has added a REST API.

@jm-clius It's much better than that, nimbus-eth2 has deprecated it's JSON-RPC API, because JSON-RPC is fucking garbage for interacting with as a human being or sysadmin(which arguably might not be as human):

@jakubgs
Copy link
Contributor

jakubgs commented May 26, 2022

As for what I'd want from a good REST API from point of view of a sysadmin, here are some things that come to mind:

  • Node information: version, commit, open ports, public addresses, enabled/disabled protocols
  • Peer management: listing, adding, removing, and possibly even blacklisting peers
  • Limits management: adjusting limits like max peers, store capacity, and other thresholds
  • Housekeeping: triggering garbage collection, flushing db operations, reloading config files(though we don't have any yet)

Of course some of those things are more important than others.

@jm-clius
Copy link
Contributor Author

Thanks for this @LNSD!

In general I agree with your approach and think it's a good idea to have the OpenAPI specification - this would make the API much more accessible for nwaku users and allow them to generate client code for whichever environment they're working in (or at least have access to readable, standardised docs).

e.g. keep the /waku/v2/ common path

This was something we added for the JSON-RPC API, but I think it's unlikely that we'll host APIs for different versions of waku on the same server in future and it makes the resource paths quite cumbersome. I'll feel comfortable removing this /waku/v2 from the common path of all resources. @oskarth wdyt? This was a conversation we had when starting work on the JSON-RPC API, but back then we may have imagined hosting v1 and v2 resources on the same host (e.g. a bridge). The v1 API, however, is used as is (without versioning) on the bridge and I don't foresee extending/changing it.

@LNSD, even though path versioning isn't a strictly idiomatic RESTful design, shall we keep the versioning for each sub-API, e.g. /debug/v1/info, or is there an easy alternative? This may just help us extend/change/break things with little impact in future.

For these reasons, we cannot benefit from this tool to generate the HTTP REST server handlers/stubs. This will have to be done manually.

Indeed. Manual code, unfortunately, which also means we'll have to maintain the OpenAPI spec to be sure it always matches what we implemented. A generator will be useful in future - hopefully more people have this need and have started working on this already. :)

@jm-clius
Copy link
Contributor Author

Good wishlist from @jakubgs!

Although the JSON-RPC API provides a good minimal set of methods to implement, we can (and must) certainly extend with new useful methods (as we go along). Perhaps also worth taking a look at other Waku v2 APIs (e.g. the bindings API) and see if there are methods worth implementing that does not yet exist for nwaku.

@LNSD
Copy link
Contributor

LNSD commented May 27, 2022

@jm-clius and I have agreed on doing first a prototype porting the Debug API from RPC to REST format in order to discover all the unknown issues.

@LNSD
Copy link
Contributor

LNSD commented May 27, 2022

Diving more into the nimbus beacon_node REST API, I have realized that JSON Serialization (e.g. request unmarshaling, response marshaling) is not done using NIM's std JSON library. It is done by manually serializing things using the JSONWriter object (from status-im/nim-json-serialization) and by marshaling/unmarshalling data structures "manually" (see: eth2_rest_serialization.nim)

Based on what I can read in the nim-json-serialization repo's README:

Flexible JSON serialization not relying on run-time type information.

I guess that the reason behind this decision is to gain in performance. Maybe @Menduist can shed some more light on this decision.

In my opinion, if performance is a concern in nwaku's node REST API, then we should go with the JSONWriter approach. Otherwise, the std/json option should be enough. What do you think @jm-clius?

@Menduist
Copy link
Contributor

The std json library is notoriously unoptimized and hard to use properly

The nim-*-ser repos instead rely on automatic marshaling and efficient parsing / dumping

They're sometime a bit of pain to use, since they rely on a lot of "magic" (generics, templates, macro), but the code generally end up cleaner and faster

Your call though, for a quick prototype std json is good enough :)

@jm-clius
Copy link
Contributor Author

Mmm. I agree that for our first end-to-end POC using std/json would be good enough - we've been using std/json on the JSON-RPC API without issue. I don't foresee that we'd need a high performant REST API in the short term, but we'll probably make things easier for ourselves down the line if we follow nimbus in using nim-json-serialization earlier rather than later.

I may not have a good grasp of how much extra (initial) effort this would be? If it's merely a lot of copying and pasting I'd say go ahead, but if we think that it will be weeks of extra work I'd rather stay with the simpler std/json option.

@arnetheduck
Copy link
Contributor

Nimbus beacon_node exposes the "control interface" in two forms: RPC and REST

RPC was removed recently (not that it matters greatly for this issue but ..)

There is a standardized way of specifying a RESTful API named OpenAPI.

The API that nimbus-eth2 implements is standardised / documented here: https://github.com/ethereum/beacon-APIs (including yaml specs from which in theory code could be generated)

std/json

Summary of performance and implementation bugs: status-im/nimbus-eth2#2430 (we, and most of the Nim community, stays as far away from this module as possible :))

Serialization issues tend to be tricky to fix "afterwards" because you need to concurrently upgrade producers and consumers - eth2_rest_serialization looks messy but the mess is there so that we don't "accidentally" break the serialized output and make it incompatible.

@arnetheduck
Copy link
Contributor

API versioning

in eth2, the key point was to version each request separately - this has worked well so far - the other important point to specify is an upgrade policy: what changes are allowed to each endpoint which don't require a version upgrade - adding "optional" fields tends to be the common ground for most people.,

@LNSD
Copy link
Contributor

LNSD commented Jun 2, 2022

I have been busy with the JSON serialization part. I am still learning Nim 😁

I have implemented the serialization/deserialization part of the POC using the nim-json-serialization library as nimbus does and based on the facts/comments that @arnetheduck and @Menduist have provided above.

I have opened a Draft PR to get feedback about this. See: #988

@LNSD
Copy link
Contributor

LNSD commented Jun 28, 2022

In order to start testing the REST API endpoints, the integration work between the REST HTTP server and the Waku node has been advanced. Please check the following PR for details: #1022

@jm-clius jm-clius changed the title Add HTTP REST API HTTP REST API: Debug and Relay APIs Aug 15, 2022
@jm-clius
Copy link
Contributor Author

Basic Debug, Relay functionality added and being released in v0.11. Remaining work for subsequent releases tracked here: #1076

@jakubgs
Copy link
Contributor

jakubgs commented Aug 16, 2022

Are there any docs on the available API calls?

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

No branches or pull requests

5 participants