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

BeaconAPI Spec Conformance #4063

Closed
z3n-chada opened this issue May 25, 2022 · 7 comments · Fixed by #4493
Closed

BeaconAPI Spec Conformance #4063

z3n-chada opened this issue May 25, 2022 · 7 comments · Fixed by #4493
Labels
prio-high Resolve issues as soon as possible.

Comments

@z3n-chada
Copy link

Describe the bug
There are spec conformance issues in the BeaconAPI json-rpc endpoint.

Expected behavior

API Path Inputs Issue
/eth/v1/beacon/states/(state)/root "head" No root key in the data field. No execution_optimistic field
/eth/v1/beacon/states/{state_id}/fork "head" No execution_optimistic field
/eth/v1/beacon/states/{state_id}/finality_checkpoints "head" No execution_optimistic field
/eth/v1/beacon/states/{state_id}/validators "head" No execution_optimistic field
/eth/v1/beacon/states/{state_id}/committees "head" No execution_optimistic field
/eth/v1/beacon/states/{state_id}/sync_committees "head" No execution_optimistic field
/eth/v2/beacon/blocks/{block_id} "head" No execution_optimistic field
/eth/v1/beacon/headers "head" No execution_optimistic field
/eth/v1/node/syncing None No is_optimistic field

Steps to Reproduce
Use the json-rpc endpoint.

Screenshots

Desktop (please complete the following information):

  • OS: N/A
  • Version: N/A
  • Branch: master
  • Commit hash: N/A
@dapplion
Copy link
Contributor

Related to #3682

Tests should download the OpenAPI spec and assert responses follow its format

@dadepo
Copy link
Contributor

dadepo commented Jun 14, 2022

I ended up reaching for Ajv to perform the validation of response object against the schema, but the problem is, the beacon-api spec does not have any of the properties of the response objects as required.

This makes it impossible (or convoluted) to perform a strict validation because a response object that misses any of the properties would still pass the validation check.

Does anyone know if it was a conscious design choice not marking the properties of the response object?

@dapplion
Copy link
Contributor

@dadepo SSZ fromJson already does property validation for all important data structures on the ingress side at runtime

To ensure we conform with the spec on output, just generate testers from the OpenAPI schema and via tests check that we conform to that spec at test runtime

@dadepo
Copy link
Contributor

dadepo commented Jun 14, 2022

just generate testers from the OpenAPI schema

@dapplion do you by chance have any tool in mind that does this? The ones I see are for either generating client libraries from the OpenAPI schema, or this paid tool that offers help with testing.

The only way I found to validate the response is to use a tool like Ajv to check that the response from the API conforms to the OpenAPI spec.

@dapplion
Copy link
Contributor

dapplion commented Jun 14, 2022

just generate testers from the OpenAPI schema

@dapplion do you by chance have any tool in mind that does this? The ones I see are for either generating client libraries from the OpenAPI schema, or this paid tool that offers help with testing.

The only way I found to validate the response is to use a tool like Ajv to check that the response from the API conforms to the OpenAPI spec.

https://www.google.com/search?q=openapi+to+json+schema+converter

https://github.com/openapi-contrib/openapi-schema-to-json-schema

Using generated client libraries may also work but may be more complicated

@dadepo
Copy link
Contributor

dadepo commented Jun 14, 2022

https://github.com/openapi-contrib/openapi-schema-to-json-schema

This does not generate testers though, but converts OpenAPI 3.0 to JSON Schema Draft 4. The output of the conversion, can't also be used directly for comparison in tests. As the tests needs to validate that the json returned by the API is the same as what is specified in the spec.

Using generated client libraries may also work but may be more complicated

I'll check this out but I am not sure how that will work, The client libraries will be generated client code that calls the API, they might not have facility to validate the response. And we do not need client code to call the api as we have that already in lodestar itself.

@dapplion dapplion added the prio-high Resolve issues as soon as possible. label Jun 29, 2022
@dadepo dadepo removed their assignment Sep 9, 2022
@dapplion
Copy link
Contributor

Closing for now, tho note there is still work to do to ensure Lodestar logically conforms to the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio-high Resolve issues as soon as possible.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants