Skip to content

Make it possible to disable BLS verification for everything except Deposits#6116

Merged
Nashatyrev merged 5 commits intoConsensys:masterfrom
Nashatyrev:feature/alter-deposit-bls-verifier
Aug 24, 2022
Merged

Make it possible to disable BLS verification for everything except Deposits#6116
Nashatyrev merged 5 commits intoConsensys:masterfrom
Nashatyrev:feature/alter-deposit-bls-verifier

Conversation

@Nashatyrev
Copy link
Contributor

PR Description

When doing experiments it is sometimes desirable to disable BLS verification globally for performance reasons. However verifying Deposit signature is kind of special case: it is legal for a valid block to contain Deposit with invalid signature. Not being able to handle invalid deposit signature leads to consensus break.

This PR is still a bit hacky way to handle this: replace AbstractBlockProcessor.blsVerifyDeposit static flag with BLSSignatureVerifier reference to be able to set the verifier to the working instance even if the BLS verification is globally disabled with BLSConstants.disableBLSVerification().

BTW this change reminded me that there is still pending BLS refactoring to make BLS code cleaner (also includes this issue #2716). Will try to address this in a separate PR later

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.

…posit-bls-verifier

# Conflicts:
#	storage/src/test/java/tech/pegasys/teku/storage/client/RecentChainDataTest.java
@Nashatyrev Nashatyrev force-pushed the feature/alter-deposit-bls-verifier branch from b3821c6 to 4b3399a Compare August 23, 2022 19:17
Copy link
Contributor

@rolfyone rolfyone left a comment

Choose a reason for hiding this comment

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

LGTM

@Nashatyrev Nashatyrev enabled auto-merge (squash) August 24, 2022 06:14
@Nashatyrev Nashatyrev merged commit 35eb475 into Consensys:master Aug 24, 2022
ajsutton added a commit to ajsutton/teku that referenced this pull request Sep 1, 2022
ajsutton added a commit to ajsutton/teku that referenced this pull request Sep 1, 2022
ajsutton added a commit that referenced this pull request Sep 1, 2022
Nashatyrev added a commit to Nashatyrev/teku that referenced this pull request Jan 9, 2023
Nashatyrev added a commit to Nashatyrev/teku that referenced this pull request Apr 21, 2023
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