Skip to content

Make it possible to disable BLS verification for everything except Deposits (attempt 2) #7065

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.

Sign up for GitHub

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

Merged
rolfyone merged 8 commits into from
Apr 24, 2023
Merged

Make it possible to disable BLS verification for everything except Deposits (attempt 2) #7065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8b3c8aa
Make it possible to disable BLS verification for everything except De…
Nashatyrev Aug 24, 2022
35ae094
BLS.aggregateVerify/fastAggregateVerify() should return false if an i…
Nashatyrev Apr 21, 2023
ad207a8
Spotless
Nashatyrev Apr 21, 2023
7a33ac6
Adjust ProcessSyncAggregateBenchmark
Nashatyrev Apr 21, 2023
06d1715
Adjust GossipValidationHelperTest
Nashatyrev Apr 21, 2023
afe1e2e
Merge remote-tracking branch 'Consensys/master' into feature/alter-de…
Nashatyrev Apr 21, 2023
72dad53
Merge branch 'master' into feature/alter-deposit-bls-verifier-1
rolfyone Apr 23, 2023
89229de
Merge branch 'master' into feature/alter-deposit-bls-verifier-1
rolfyone Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import tech.pegasys.teku.bls.BLSSignature;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
import tech.pegasys.teku.infrastructure.ssz.SszList;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
Expand Down Expand Up @@ -94,12 +95,13 @@ class BlockFactoryTest {

@BeforeAll
public static void initSession() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void resetSession() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

@Test
Expand Down
3 changes: 2 additions & 1 deletion eth-benchmark-tests/src/jmh/java/tech/pegasys/teku/benchmarks/EpochTransitionBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import tech.pegasys.teku.benchmarks.gen.BlsKeyPairIO;
import tech.pegasys.teku.benchmarks.util.CustomRunner;
import tech.pegasys.teku.bls.BLSKeyPair;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.async.eventthread.InlineEventThread;
import tech.pegasys.teku.infrastructure.ssz.collections.SszMutableUInt64List;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
Expand Down Expand Up @@ -88,7 +89,7 @@ public class EpochTransitionBenchmark {

@Setup(Level.Trial)
public void init() throws Exception {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;

spec = TestSpecFactory.createMainnetAltair();
String blocksFile =
Expand Down
2 changes: 1 addition & 1 deletion ...chmark-tests/src/jmh/java/tech/pegasys/teku/benchmarks/ProcessSyncAggregateBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public class ProcessSyncAggregateBenchmark {
@Setup(Level.Trial)
public void init() throws Exception {
spec = TestSpecFactory.createMainnetAltair();
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;

String blocksFile =
"/blocks/blocks_epoch_"
Expand Down
5 changes: 3 additions & 2 deletions eth-benchmark-tests/src/jmh/java/tech/pegasys/teku/benchmarks/ProfilingRun.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import tech.pegasys.teku.benchmarks.gen.BlsKeyPairIO;
import tech.pegasys.teku.bls.BLSKeyPair;
import tech.pegasys.teku.bls.BLSPublicKey;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.bls.BLSTestUtil;
import tech.pegasys.teku.infrastructure.async.eventthread.InlineEventThread;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
Expand Down Expand Up @@ -63,7 +64,7 @@ public class ProfilingRun {
@Test
public void importBlocks() throws Exception {

AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;

int validatorsCount = 32 * 1024;
int iterationBlockLimit = 1024;
Expand Down Expand Up @@ -157,7 +158,7 @@ public static void main(String[] args) throws Exception {
@Test
public void importBlocksMemProfiling() throws Exception {

AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;

int validatorsCount = 32 * 1024;

Expand Down
3 changes: 2 additions & 1 deletion eth-benchmark-tests/src/jmh/java/tech/pegasys/teku/benchmarks/TransitionBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import tech.pegasys.teku.benchmarks.gen.BlockIO;
import tech.pegasys.teku.benchmarks.gen.BlsKeyPairIO;
import tech.pegasys.teku.bls.BLSKeyPair;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.async.eventthread.InlineEventThread;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
import tech.pegasys.teku.spec.Spec;
Expand Down Expand Up @@ -72,7 +73,7 @@ public abstract class TransitionBenchmark {
@Setup(Level.Trial)
public void init() throws Exception {
spec = TestSpecFactory.createMainnetAltair();
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;

String blocksFile =
"/blocks/blocks_epoch_"
Expand Down
3 changes: 2 additions & 1 deletion eth-benchmark-tests/src/jmh/java/tech/pegasys/teku/benchmarks/gen/Generator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.junit.jupiter.api.Test;
import tech.pegasys.teku.benchmarks.gen.BlockIO.Writer;
import tech.pegasys.teku.bls.BLSKeyPair;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.bls.BLSTestUtil;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
import tech.pegasys.teku.spec.Spec;
Expand All @@ -49,7 +50,7 @@ public class Generator {
public void generateBlocks() throws Exception {
final Spec spec = TestSpecFactory.createMainnetAltair();

AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;

System.out.println("Generating keypairs...");
int validatorsCount = 400000;
Expand Down
6 changes: 4 additions & 2 deletions ...rc/test/java/tech/pegasys/teku/ethereum/executionlayer/BuilderCircuitBreakerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
import tech.pegasys.teku.spec.Spec;
import tech.pegasys.teku.spec.TestSpecFactory;
Expand All @@ -43,12 +44,13 @@ public class BuilderCircuitBreakerImplTest {

@BeforeAll
public static void disableDepositBlsVerification() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void enableDepositBlsVerification() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

@BeforeEach
Expand Down
19 changes: 11 additions & 8 deletions .../spec/src/main/java/tech/pegasys/teku/spec/logic/common/block/AbstractBlockProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static tech.pegasys.teku.spec.config.SpecConfig.FAR_FUTURE_EPOCH;

import com.google.common.annotations.VisibleForTesting;
import it.unimi.dsi.fastutil.ints.IntList;
import it.unimi.dsi.fastutil.objects.Object2IntMap;
import java.util.ArrayList;
Expand All @@ -28,7 +29,6 @@
import org.apache.logging.log4j.Logger;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import tech.pegasys.teku.bls.BLS;
import tech.pegasys.teku.bls.BLSPublicKey;
import tech.pegasys.teku.bls.BLSSignature;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
Expand Down Expand Up @@ -81,11 +81,16 @@
import tech.pegasys.teku.spec.logic.versions.deneb.block.KzgCommitmentsProcessor;

public abstract class AbstractBlockProcessor implements BlockProcessor {

@VisibleForTesting
public static final BLSSignatureVerifier DEFAULT_DEPOSIT_SIGNATURE_VERIFIER =
BLSSignatureVerifier.SIMPLE;
/**
* For debug/test purposes only enables/disables {@link DepositData} BLS signature verification
* Setting to <code>false</code> significantly speeds up state initialization
*/
public static boolean blsVerifyDeposit = true;
@VisibleForTesting
public static BLSSignatureVerifier depositSignatureVerifier = DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
Copy link
Contributor

@rolfyone rolfyone Apr 24, 2023

Choose a reason for hiding this comment

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

are we planning on differentiating? why not just reference the default_deposit_signature_verifier?

Copy link
Contributor Author

@Nashatyrev Nashatyrev Apr 24, 2023

Choose a reason for hiding this comment

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

DEFAULT_DEPOSIT_SIGNATURE_VERIFIER was intended as a default value for resetting the depositSignatureVerifier field after testing with disabled verifier. But you are right, it's probably an over-complication for the production code.


private static final Logger LOG = LogManager.getLogger();

Expand Down Expand Up @@ -667,8 +672,7 @@ public void processDeposits(MutableBeaconState state, SszList<? extends Deposit>
throws BlockProcessingException {
safelyProcess(
() -> {
final boolean depositSignaturesAreAllGood =
!blsVerifyDeposit || batchVerifyDepositSignatures(deposits);
final boolean depositSignaturesAreAllGood = batchVerifyDepositSignatures(deposits);
for (Deposit deposit : deposits) {
processDeposit(state, deposit, depositSignaturesAreAllGood);
}
Expand All @@ -687,7 +691,7 @@ private boolean batchVerifyDepositSignatures(SszList<? extends Deposit> deposits
signatures.add(deposit.getData().getSignature());
}
// Overwhelmingly often we expect all the deposit signatures to be good
return BLS.batchVerify(publicKeys, messages, signatures);
return depositSignatureVerifier.verify(publicKeys, messages, signatures);
} catch (final BlsException e) {
return false;
}
Expand Down Expand Up @@ -767,9 +771,8 @@ private void handleInvalidDeposit(

private boolean depositSignatureIsValid(final Deposit deposit, BLSPublicKey pubkey) {
try {
return !blsVerifyDeposit
|| BLS.verify(
pubkey, computeDepositSigningRoot(deposit, pubkey), deposit.getData().getSignature());
return depositSignatureVerifier.verify(
pubkey, computeDepositSigningRoot(deposit, pubkey), deposit.getData().getSignature());
} catch (final BlsException e) {
return false;
}
Expand Down
5 changes: 3 additions & 2 deletions ...reum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/block/BlockProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ public abstract class BlockProcessorTest {
protected abstract Spec createSpec();

@Test
void ensureVerifyDepositDefaultsToTrue() {
assertThat(AbstractBlockProcessor.blsVerifyDeposit).isTrue();
void ensureDepositSignatureVerifierHasDefaultValue() {
assertThat(AbstractBlockProcessor.depositSignatureVerifier)
.isSameAs(AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER);
}

@Test
Expand Down
6 changes: 4 additions & 2 deletions ...tetransition/src/test/java/tech/pegasys/teku/statetransition/block/BlockImporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.mockito.ArgumentCaptor;
import tech.pegasys.teku.bls.BLSKeyGenerator;
import tech.pegasys.teku.bls.BLSKeyPair;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.bls.BLSTestUtil;
import tech.pegasys.teku.ethereum.execution.types.Eth1Address;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
Expand Down Expand Up @@ -116,12 +117,13 @@ public class BlockImporterTest {

@BeforeAll
public static void init() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void dispose() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

@BeforeEach
Expand Down
6 changes: 4 additions & 2 deletions ...atetransition/src/test/java/tech/pegasys/teku/statetransition/block/BlockManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
import tech.pegasys.teku.infrastructure.async.eventthread.InlineEventThread;
import tech.pegasys.teku.infrastructure.collections.LimitedMap;
Expand Down Expand Up @@ -143,12 +144,13 @@ public class BlockManagerTest {

@BeforeAll
public static void initSession() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void resetSession() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

@BeforeEach
Expand Down
6 changes: 4 additions & 2 deletions ...on/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifierTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.invocation.InvocationOnMock;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.ethereum.execution.types.Eth1Address;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
import tech.pegasys.teku.infrastructure.async.SafeFutureAssert;
Expand Down Expand Up @@ -93,12 +94,13 @@ class ForkChoiceNotifierTest {

@BeforeAll
public static void initSession() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void resetSession() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

@BeforeEach
Expand Down
6 changes: 4 additions & 2 deletions ...test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoicePayloadExecutorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
import tech.pegasys.teku.spec.Spec;
import tech.pegasys.teku.spec.TestSpecFactory;
Expand Down Expand Up @@ -59,12 +60,13 @@ class ForkChoicePayloadExecutorTest {

@BeforeAll
public static void initSession() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void resetSession() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

@BeforeEach
Expand Down
6 changes: 4 additions & 2 deletions .../java/tech/pegasys/teku/statetransition/forkchoice/MergeTransitionBlockValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
import tech.pegasys.teku.infrastructure.exceptions.FatalServiceFailureException;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
Expand Down Expand Up @@ -52,12 +53,13 @@ class MergeTransitionBlockValidatorTest {

@BeforeAll
public static void initSession() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void resetSession() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

/**
Expand Down
6 changes: 4 additions & 2 deletions ...c/test/java/tech/pegasys/teku/statetransition/forkchoice/TerminalPowBlockMonitorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import tech.pegasys.infrastructure.logging.LogCaptor;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
import tech.pegasys.teku.infrastructure.async.StubAsyncRunner;
import tech.pegasys.teku.infrastructure.logging.EventLogger;
Expand Down Expand Up @@ -77,12 +78,13 @@ public class TerminalPowBlockMonitorTest {

@BeforeAll
public static void initSession() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void resetSession() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

private void setUpTerminalBlockHashConfig() {
Expand Down
5 changes: 3 additions & 2 deletions .../java/tech/pegasys/teku/statetransition/validation/AggregateAttestationValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,13 @@ class AggregateAttestationValidatorTest {

@BeforeAll
public static void init() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void reset() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

@BeforeEach
Expand Down
6 changes: 4 additions & 2 deletions .../test/java/tech/pegasys/teku/statetransition/validation/AttestationStateSelectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import tech.pegasys.teku.bls.BLSSignatureVerifier;
import tech.pegasys.teku.infrastructure.async.SafeFuture;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
import tech.pegasys.teku.spec.Spec;
Expand Down Expand Up @@ -52,12 +53,13 @@ class AttestationStateSelectorTest {

@BeforeAll
public static void initSession() {
AbstractBlockProcessor.blsVerifyDeposit = false;
AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP;
}

@AfterAll
public static void resetSession() {
AbstractBlockProcessor.blsVerifyDeposit = true;
AbstractBlockProcessor.depositSignatureVerifier =
AbstractBlockProcessor.DEFAULT_DEPOSIT_SIGNATURE_VERIFIER;
}

@BeforeEach
Expand Down
Loading