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

Various bug fixes in Eth API #9649

Merged
merged 10 commits into from
Sep 23, 2021
Merged

Various bug fixes in Eth API #9649

merged 10 commits into from
Sep 23, 2021

Conversation

rkapka
Copy link
Contributor

@rkapka rkapka commented Sep 22, 2021

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

  • Fixes wrong block structure for /eth/v2/validator/blocks
  • Changes status code to NotFound when block returned from core libraries is nil
  • Make /internal grpc-gateway paths work with events and SSZ endpoints

Which issues(s) does this PR fix?

Fixes #9647
Fixes #9636
Fixes #9652

@rkapka rkapka added Ready For Review A pull request ready for code review API Api related tasks labels Sep 22, 2021
@rkapka rkapka requested a review from a team as a code owner September 22, 2021 15:06
@rkapka rkapka requested review from prestonvanloon, terencechain and nisdas and removed request for a team September 22, 2021 15:06
@codecov
Copy link

codecov bot commented Sep 22, 2021

Codecov Report

Merging #9649 (5b96c9f) into develop (b9ad81d) will decrease coverage by 0.01%.
The diff coverage is 62.50%.

❗ Current head 5b96c9f differs from pull request most recent head 2dd5ae4. Consider uploading reports for the commit 2dd5ae4 to get more accurate results

@@             Coverage Diff             @@
##           develop    #9649      +/-   ##
===========================================
- Coverage    61.93%   61.92%   -0.02%     
===========================================
  Files          564      564              
  Lines        44646    44645       -1     
===========================================
- Hits         27653    27647       -6     
+ Misses       12513    12512       -1     
- Partials      4480     4486       +6     

@rkapka rkapka changed the title Fix block structure and status code bugs in Eth API Various bug fixes in Eth API Sep 22, 2021
Data: &beaconBlockContainerJson{
Message: respContainer.Data.Phase0Block,
},
Data: respContainer.Data.Phase0Block,
Copy link
Member

Choose a reason for hiding this comment

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

Does this need a regression test? Im surprised there's 0 test change to reflect on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I improved block serialization tests.

Version string `json:"version"`
Data *beaconBlockAltairContainerJson `json:"data"`
Version string `json:"version"`
Data *beaconBlockAltairJson `json:"data"`
Copy link
Contributor

Choose a reason for hiding this comment

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

presumably this is addressing Fixes wrong block structure for /eth/v2/validator/blocks?

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, this is correct. We don't need an additional wrapper, we should use beaconBlockAltairJson directly.

@prylabs-bulldozer prylabs-bulldozer bot merged commit 36b1f32 into develop Sep 23, 2021
@delete-merged-branch delete-merged-branch bot deleted the some-api-fixes branch September 23, 2021 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Api related tasks Ready For Review A pull request ready for code review
Projects
None yet
3 participants