beacon-APIs icon indicating copy to clipboard operation
beacon-APIs copied to clipboard

API conversions and CI

Open djrtwo opened this issue 4 years ago • 7 comments

It seems desirable for standard conversions to exist between formats (YAML, protosbufs, SSZ, etc), regardless of whichever format this API repo uses to spec things.

Base on conversations from the recent eth2 call, I propose the following

  1. Better understand the constraints put on each format if we retain easy convertibility (e.g. does maintaining OpenAPI -> protobuf eliminate some useful type or feature we would otherwise be able to use?)
  2. Do some experimentation on any existing tools (especially in going from OpenAPI to protobuf and protobuf to OpenAPI) to see if this a simple and clean task

If the above investigations prove fruitful, then (following @paulhauner's suggestion) we should integrate autoconversions in the to CI to ensure that the API remains usable across the formats we explicitly want to support.

djrtwo avatar Apr 09 '20 17:04 djrtwo

One specific item mentioned in the last call was to try a one-off conversion of the Prysm protobuf to Swagger/OpenAPI to see what the output looks like

djrtwo avatar Apr 09 '20 18:04 djrtwo

The output of Prysm's generated OpenAPI can be found here: https://api.prylabs.net (raw json)

prestonvanloon avatar Apr 09 '20 18:04 prestonvanloon

I think it's easier to implement additional beacon apis manually with existing types (that contain validations and descriptions). I am in process of refactoring current openapi schema so routes/route groups are split into smaller files(less conflicts and it's easier to maintain smaller files).

Reusing types also enable us to switch from hex to base64 by simply changing validation at base types.

I find it cool to have ci generate multiple artifacts like protobufs and postman definitions and we can attach them to api releases.

mpetrunic avatar Apr 09 '20 19:04 mpetrunic

YAML, protosbufs, SSZ

Regarding YAML, Lighthouse supports --header "Accept: application/yaml" which gives a YAML encoding of whatever struct we would have encoded with JSON. Our mapping between the two is just whatever serde does, which may or may not be contentious. However, I do know that it matches the test format encoding (at least in the places I've checked).

Regarding SSZ, Lighthouse presently supports --header "Accept: application/ssz" which just sends back the SSZ encoding of whatever struct we would have encoded with JSON. However, this is not perfect for the following reasons:

  • No String type in SSZ: string could easily be utf-8 List[uint8, 2**32], however we've avoided this for the time being. For now, any endpoint that has a string doesn't support SSZ.
  • All lists in SSZ need to be fixed or have a maximum length: we could say "any JS array maps to a List[N, 2**32]" however this wouldn't be true for all objects since some fields (e.g., block.body.attestations) have smaller max lengths.

I haven't looked into mapping to protobuf any more than I've documented in https://github.com/ethereum/eth2.0-pm/issues/141#issuecomment-611335972. I couldn't speak on how onerous this might be.

So in terms of supporting YAML and SSZ via the REST API as alternative content-types to JSON, I don't think it's going to be particularly onerous or limiting from an implementation perspective. I think the challenge might be with documenting them. I need to look into OpenAPI a bit more, but I expect we can add YAML encoding to the spec for free. I'd expect defining the SSZ encoding would be more onerous. Perhaps @mpetrunic has thought about this.

There's also the question about whether actually we want these encodings or not. I don't see a whole lot of value in YAML; I only added since it was a useful tool for the 2019 interop, however complying with eth2-testnet means that genesis states are now SSZ (not YAML). I could take it or leave it with YAML.

SSZ might come in handy for a few reasons:

  1. It's more compact (no schema, bytes not ASCII-hex, etc) and it is perfectly designed for all the structs in the spec (what a coincidence!)
  2. SSZ is designed to support a bunch of fancy Merkle stuff; if someone is producing partials or whatever then perhaps grabbing the SSZ first avoids an annoying JSON->SSZ conversion.
  3. Clients (hopefully) already have a hardened SSZ decoder. It might be advantageous from a security perspective to do VC<>BN comms via SSZ to remove the JSON parser from the attack surface.

I'd be interested to hear about (2) from those researching phase 1+; does it seem like a common use case to want to manipulate SSZ from the BN API?

paulhauner avatar Apr 09 '20 23:04 paulhauner

So in terms of supporting YAML and SSZ via the REST API as alternative content-types to JSON, I don't think it's going to be particularly onerous or limiting from an implementation perspective. I think the challenge might be with documenting them. I need to look into OpenAPI a bit more, but I expect we can add YAML encoding to the spec for free. I'd expect defining the SSZ encoding would be more onerous. Perhaps @mpetrunic has thought about this.

We thought about that and it shouldn't be much of and issue to document them. For example, currently we have:

  responses:
    200:
      description: Success response
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/Attestation'

we can modify it like this for api endpoints that makes sense to return ssz

  responses:
    200:
      description: Success response
      content:
        application/json:
          schema:
            $ref: '../../beacon-node-oapi.yaml#/components/schemas/Attestation'
        application/ssz:
          schema:
            type: string
            description: "Base64 encoded output of SSZ serialized [Attestation](link to md file with object description) object"
            example: "YXNkYXNkYXNk"

Which looks like this in swagger: image

We would probably need to add md files containing "ssz description" for custom types that are not in spec repo so we can link them.

mpetrunic avatar Apr 10 '20 07:04 mpetrunic

I suppose if desired, we can also maintain application/x-protobuf responses and link to types defined by prysmatic.

That said, I'm not certain if mixing protos in here makes much sense. Any thoughts on that?

djrtwo avatar Apr 14 '20 02:04 djrtwo

Played around with generating a description of a gRPC service from openapi schema. Here is summary of that: https://hackmd.io/@mpetrunic/BJMAwwusI Seems like going with gnostic (attempt 2) is better option but maybe @prestonvanloon could check it out also.

mpetrunic avatar May 24 '20 22:05 mpetrunic

API format is stable now

dapplion avatar Dec 08 '23 12:12 dapplion