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

Add BlindBlockUtil for EIP-4844 #6532

Merged
merged 5 commits into from
Dec 2, 2022

Conversation

zilm13
Copy link
Contributor

@zilm13 zilm13 commented Dec 1, 2022

PR Description

Part of #6440:
BlindBlockUtilEip4844

  • SignedBeaconBlockBlinderEip4844
  •  SignedBeaconBlockUnblinderEip4844

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

}

@Override
public SignedBeaconBlock blind(final SignedBeaconBlock signedUnblindedBock) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason we can't just use SignedBeaconBlock.blind method here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice question. Will check why it's not called also in Bellatrix, I've followed

@zilm13 zilm13 self-assigned this Dec 2, 2022
@zilm13
Copy link
Contributor Author

zilm13 commented Dec 2, 2022

As I removed most blind duplicated code I'm not sure we still need 3 classes there, but at least it checks that correct Schema definition is passed in

Copy link
Contributor

@tbenr tbenr left a comment

Choose a reason for hiding this comment

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

LGTM, just two nits

.getOptionalExecutionPayloadHeader()
.orElseThrow()
.isHeaderOfDefaultPayload()) {
// Terminal block not reached, provide default payload
Copy link
Contributor

Choose a reason for hiding this comment

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

youre providing a random block, so you'll never fall in this code path so you can leave only the code in the else


@TestTemplate
void shouldBlindAndUnblindBlock(final SpecContext specContext) {
final SignedBeaconBlock signedBeaconBlock = dataStructureUtil.randomSignedBeaconBlock();
Copy link
Contributor

Choose a reason for hiding this comment

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

CodeQL is actually right :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@zilm13 zilm13 merged commit 5ba3f43 into Consensys:master Dec 2, 2022
@zilm13 zilm13 deleted the eip4844-block-blind-unblind branch December 2, 2022 17:42
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.

3 participants