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

[Bellatrix]: Use block_header instead of block for block signing requests (BLOCK_V2) #547

Merged
merged 24 commits into from
May 10, 2022

Conversation

usmansaleem
Copy link
Contributor

@usmansaleem usmansaleem commented Apr 21, 2022

PR Description

For Bellatrix and onward forks, use block_header instead of block for block signing request (BLOCK_V2). block_header contains body_root instead of complete beacon block body which significantly reduces the traffic between beacon client (such as Teku) and Web3Signer for block signing request.

Either block_header or block can be used to calculate signing root by the Web3Signer. For backward compatibility with PHASE0 and ALTAIR forks, only block is required; for Bellatrix and later forks, only block_header is required.

Fixed Issue(s)

See #437

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.

@usmansaleem usmansaleem marked this pull request as ready for review April 27, 2022 23:52
@james-prysm
Copy link

Is this PR just for backward compatibility?

@usmansaleem usmansaleem changed the title Bellatrix fork: Use block_header instead of block for block signing requests (BLOCK_V2) [Bellatrix]: Use block_header instead of block for block signing requests (BLOCK_V2) May 9, 2022
Copy link
Contributor

@jframe jframe left a comment

Choose a reason for hiding this comment

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

LGTM

@usmansaleem
Copy link
Contributor Author

@james-prysm This PR is to use optimized block signing request starting from Bellatrix fork. See #437 and #486 for more details as well. Feel free to reach out to the team on Discord.

Copy link
Contributor

@siladu siladu 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 a few questions really :)

@jframe jframe merged commit 8fe4798 into Consensys:master May 10, 2022
@jframe
Copy link
Contributor

jframe commented May 10, 2022

@james-prysm This PR is partly to optimise the block signing so less data is needed to be passed in when signing block. It also means we can support the Bellatrix fork as we don't need to make additional changes to include ExecutionPayload with this change as long as are using the block_header with the body root in the request.

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

4 participants