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

Derive OpenAPI type schema from pyspec

Open dapplion opened this issue 1 year ago • 7 comments

Thanks to ethereum/consensus-specs/pull/3506 we have canonical mapping of SSZ -> YAML, so it is possible to derive OpenAPI schemas for all the spec types.

Since the vast majority of API types are spec types auto-deriving the types would reduce future fork development and maintenance. Plus it paves the way to SSZ-ing most API routes.

We can define API-only extra types in SSZ syntax, such as:

// extra_types/bellatrix.md

## Containers

### Registration

From the Builder API specification

```python
class ValidatorRegistration(Container):
    fee_recipient: ExecutionAddress  # Address to receive fees from the block
    gas_limit: uint64  # Preferred gas limit of validator
    timestamp: uint64  # Unix timestamp of registration
    pubkey: BLSPubkey  # BLS public key of validator

class SignedValidatorRegistration(Container):
    message: ValidatorRegistration
    signature: BLSSignature

Current status

I've created a simple python script to translate the pyspec to OpenAPI schemas https://pypi.org/project/pyspec2openapi. Here is a demo integration into the beacon-APIs repo https://github.com/dapplion/eth2.0-APIs/pull/1

The diff is quite large since it merges all types into a single output file.

One can compare the bundled outputs with

git checkout master
swagger-cli bundle ./beacon-node-oapi.yaml -r -t yaml -o ./deploy/beacon-node-oapi-master.yaml
git checkout dapplion/derive-spec-types
swagger-cli bundle ./beacon-node-oapi.yaml -r -t yaml -o ./deploy/beacon-node-oapi-derive.yaml
diff -l -u deploy/beacon-node-oapi-master.yaml deploy/beacon-node-oapi-derive.yaml | colordiff | more -R 

The bundled diff is also quite substantial due to some key characteristics

  • Usage of allOf to define block types. Generated spec does not de-duplicate keys, resulting in less indentations
  BeaconBlock:
    allOf:
      - $ref: '.../BeaconBlockCommon'
      - $ref: '#/Deneb/BeaconBlockBody'
  • Lots of comments per-property and per type not present in the generated specs https://github.com/ethereum/beacon-APIs/blob/4882aa0803b622b75bab286b285599d70b7a2429/types/block.yaml#L24

Next steps

  • [ ] Get buy-in from repo mantainer's, or how could I tune this to align with your prefered DX

If yes, decide how to merge

  • One big diff PR switching directly to generated types, a-la https://github.com/dapplion/eth2.0-APIs/pull/1
  • Big diff PR merging existing existing schemas into a single spec.yaml file, then replace with generated schemas for a clearer diff
  • Modify current master branch to align with generated output (remove block allOf usage, remove comments), then merge a smaller diff PR switching to generated types

dapplion avatar Jan 08 '24 11:01 dapplion

ooh, great stuff! :+1: from me, was hoping it would lead to this.

arnetheduck avatar Jan 09 '24 10:01 arnetheduck

It would be good to maintain the script either in this repo or another repo under ethereum. I feel uneasy about one person maintaining it privately (not from a security perspective, but because of bus factor == 1)

rkapka avatar Jan 12 '24 15:01 rkapka

It would be good to maintain the script either in this repo or another repo under ethereum. I feel uneasy about one person maintaining it privately (not from a security perspective, but because of bus factor == 1)

Sounds good, will look how to integrate it without polluting this repo too much. @rolfyone thoughts?

dapplion avatar Jan 15 '24 04:01 dapplion

Can we have the conversion script in this repo or in consensus-specs and just run it during build or something? or produce something in consensus-specs as part of that build that we then ingest would be ideal maybe?

It looks like we're stripping out a bunch of types, can we just ingest it and ignore the types we dont need or something? I like the direction, agree polluting this repo would be less than ideal

rolfyone avatar Jan 18 '24 00:01 rolfyone

Can we have the conversion script in this repo or in consensus-specs and just run it during build or something? or produce something in consensus-specs as part of that build that we then ingest would be ideal maybe?

I don't think we should pollute the spec tests, since this is purely derivative it should leave in this repo

can we just ingest it and ignore the types we dont need or something?

There may be some types that are un-used but looks like the minority. For that we should either maintain and opt-in list or an exclusion list. Is it worth the effort to have a few less lines?

dapplion avatar Jan 18 '24 04:01 dapplion

If we can just ingest the spec and it works, then the extra objects are pretty un-important, does make it easy when we add endpoints... The only thing would be if people get annoyed at unused objects if they cause issues in code generation maybe?

rolfyone avatar Jan 18 '24 08:01 rolfyone

https://redocly.com/docs/cli/commands/bundle/ offers a --remove-unused-components option that should take care of unused objects. Note that we are currently not using redocly for bundling but swagger-cli. swagger-cli is deprecated and recommends migrating to redocly

jeluard avatar Jan 18 '24 08:01 jeluard