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

REST server fixes and improvements. #5422

Merged
merged 9 commits into from
Sep 27, 2023
Merged

REST server fixes and improvements. #5422

merged 9 commits into from
Sep 27, 2023

Conversation

cheatfate
Copy link
Contributor

  • Refactor serialization code to use Opt[T] instead of Option[T].
  • Adding finalized flag to REST API calls
    • GET /eth/v1/beacon/states/{state_id}/root
    • GET /eth/v1/beacon/states/{state_id}/fork
    • GET /eth/v1/beacon/states/{state_id}/finality_checkpoints
    • GET /eth/v1/beacon/states/{state_id}/validator
    • GET /eth/v1/beacon/states/{state_id}/validators/{validator_id}
    • GET /eth/v1/beacon/states/{state_id}/validator_balances
    • GET /eth/v1/beacon/states/{state_id}/committees
    • GET /eth/v1/beacon/states/{state_id}/sync_committees
    • GET /eth/v1/beacon/states/{state_id}/randao
    • GET /eth/v1/beacon/headers
    • GET /eth/v1/beacon/headers/{block_id}
    • GET /eth/v2/beacon/blocks/{block_id}
    • GET /eth/v1/beacon/blocks/{block_id}/root
    • GET /eth/v1/beacon/blocks/{block_id}/attestations
  • Fix API calls to return proper HTTP error code
    • GET /eth/v1/beacon/states/{state_id}/validators - HTTP 414
    • POST /eth/v1/beacon/blinded_blocks - HTTP 415, HTTP 400 (FOO)
    • POST /eth/v1/beacon/blocks - HTTP 415
    • POST /eth/v2/beacon/blocks - HTTP 415
  • Improve error message for some HTTP REST error messages.

@github-actions
Copy link

github-actions bot commented Sep 12, 2023

Unit Test Results

         9 files  ±0    1 089 suites  ±0   39m 1s ⏱️ - 2m 52s
  3 886 tests ±0    3 589 ✔️ ±0  297 💤 ±0  0 ±0 
15 955 runs  ±0  15 632 ✔️ ±0  323 💤 ±0  0 ±0 

Results for commit 55f996f. ± Comparison against base commit 710f267.

♻️ This comment has been updated with latest results.

except CatchableError:
return err("Unexpected deserialization error")
return err(RestErrorMessage.init(Http400, UnableDecodeError,
[version, exc.formatMsg("<data>")]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so in general, this is where we would include a short version of the data itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we can, but it will not be very helpful, we put escaped JSON into JSON, and so exc.formatMsg() will not provide proper offsets in the escaped JSON.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't really matter - the point is more that the raw text usually contains an error message that the server has generated and therefore can be searched for when resolving problems .. it is a lot more useful than the json parsing error in general.

@arnetheduck arnetheduck merged commit 4fb95d0 into unstable Sep 27, 2023
11 checks passed
@arnetheduck arnetheduck deleted the rest-finalized branch September 27, 2023 14:45
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

Successfully merging this pull request may close these issues.

None yet

3 participants