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

fix SSZ response for produceBlindedBlock #4943

Merged
merged 2 commits into from
May 12, 2023
Merged

Conversation

etan-status
Copy link
Contributor

In produceBlindedBlock, we sent the ForkedBlindedBeaconBlock when requested to reply in SSZ format. However, expected result is just the inner ForkyBlindedBeaconBlock together with eth-consensus-version.

Note: We do not use SSZ format in our VC for this endpoint at this time, which explains why we haven't noticed earlier.

In `produceBlindedBlock`, we sent the `ForkedBlindedBeaconBlock` when
requested to reply in SSZ format. However, expected result is just the
inner `ForkyBlindedBeaconBlock` together with `eth-consensus-version`.

Note: We do not use SSZ format in our VC for this endpoint at this time,
which explains why we haven't noticed earlier.
BlockType: ForkedBeaconBlock|ForkedBlindedBeaconBlock](
BlockType: ForkedBeaconBlock](
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Verified that this is not needed anymore, by compiling with this overload

proc writeValue*[
    BlockType: ForkedBlindedBeaconBlock](
    writer: var JsonWriter[RestJson],
    value: BlockType) {.raises: [IOError, Defect].} =

  static: raiseAssert "error"

@github-actions
Copy link

Unit Test Results

         9 files  ±0    1 074 suites  ±0   30m 5s ⏱️ -59s
  3 675 tests ±0    3 396 ✔️ ±0  279 💤 ±0  0 ±0 
15 668 runs  ±0  15 363 ✔️ ±0  305 💤 ±0  0 ±0 

Results for commit 6d20158. ± Comparison against base commit 51418a7.

@tersec tersec merged commit d263f7f into unstable May 12, 2023
@tersec tersec deleted the dev/etan/ba-fixblindedssz branch May 12, 2023 15:40
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.

2 participants