Bellatrix fork + External Signer: Send block_header for block sign request #5354
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Create BeaconBlockHeader from BeaconBlock - Update ExternalSigner for Bellatrix - Update SigningRootUtil to sign BeaconBlockHeader - Update BlockRequestBody to send block_header
ajsutton
approved these changes
Apr 20, 2022
| return spec.computeSigningRoot(block, getDomainForSignBlock(block.getSlot(), forkInfo)); | ||
| } | ||
|
|
||
| public Bytes signingRootForSignBlock( |
There was a problem hiding this comment.
Maybe call this signingRootForSignBlockHeader?
| + block.getSlot() | ||
| + " as it may violate a slashing condition"; | ||
| String.format( | ||
| "External signed refused to sign block at slot %s as it may violate a slashing condition", |
There was a problem hiding this comment.
Suggested change
| "External signed refused to sign block at slot %s as it may violate a slashing condition", | |
| "External signer refused to sign block at slot %s as it may violate a slashing condition", |
| case ALTAIR: | ||
| metadata.put( | ||
| "beacon_block", | ||
| new BlockRequestBody(spec.atSlot(block.getSlot()).getMilestone(), beaconBlock, null)); |
There was a problem hiding this comment.
It probably makes sense to have two separate constructors for BlockRequestBody, one that takes just a beacon block and one that takes just a header. May still need the version that takes both for JSON deserialisation but would be nice to clean this up to not have to specify null.
There was a problem hiding this comment.
Done, introduced overloaded constructor. Also updated unit tests to assert correct fields are populated.
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Description
For Bellatrix fork, send
block_headerinstead ofblockfor external signer block signing request (BLOCK_V2).block_headercontainsbody_rootinstead of complete beacon blockbodywhich significantly reduces the traffic between Teku and external signer for block signing request.Either
block_headerorblockcan be used to calculate signing root by the external signer. For backward compatibility with PHASE0 and ALTAIR forks, onlyblockis sent; for Bellatrix and later forks, onlyblock_headeris sent.Fixed Issue(s)
Consensys/web3signer#437
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog