diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectionRecord.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectionRecord.java index 3415d3f820c..54ce0c7c991 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectionRecord.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectionRecord.java @@ -28,15 +28,18 @@ class LocalSlashingProtectionRecord { private final Path slashingProtectedPath; // In the same way as the MAP in LocalSlashingProtector, signingRecord gets maintained over time private ValidatorSigningRecord signingRecord; + private final boolean isNew; private final ReentrantLock lock; LocalSlashingProtectionRecord( final Path slashingProtectedPath, final ValidatorSigningRecord signingRecord, + final boolean isNew, final ReentrantLock lock) { this.slashingProtectedPath = slashingProtectedPath; this.signingRecord = signingRecord; + this.isNew = isNew; this.lock = lock; } @@ -57,6 +60,10 @@ ValidatorSigningRecord getSigningRecord() { return signingRecord; } + boolean isNew() { + return isNew; + } + boolean writeSigningRecord( final SyncDataAccessor dataAccessor, final Optional maybeRecord) throws IOException { diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtector.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtector.java index 4837c247506..d9ef2774661 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtector.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtector.java @@ -31,11 +31,15 @@ public class LocalSlashingProtector implements SlashingProtector { private final Map slashingProtectionPath = new HashMap<>(); private final SyncDataAccessor dataAccessor; private final Path slashingProtectionBaseDir; + private final boolean slashingProtectionStrictModeEnabled; public LocalSlashingProtector( - final SyncDataAccessor dataAccessor, final Path slashingProtectionBaseDir) { + final SyncDataAccessor dataAccessor, + final Path slashingProtectionBaseDir, + final boolean slashingProtectionStrictModeEnabled) { this.dataAccessor = dataAccessor; this.slashingProtectionBaseDir = slashingProtectionBaseDir; + this.slashingProtectionStrictModeEnabled = slashingProtectionStrictModeEnabled; } @Override @@ -43,9 +47,13 @@ public synchronized SafeFuture maySignBlock( final BLSPublicKey validator, final Bytes32 genesisValidatorsRoot, final UInt64 slot) { return SafeFuture.of( () -> { - final ValidatorSigningRecord signingRecord = - loadOrCreateSigningRecord(validator, genesisValidatorsRoot); - return handleResult(validator, signingRecord.maySignBlock(genesisValidatorsRoot, slot)); + final Optional signingRecord = + loadSigningRecord(validator, genesisValidatorsRoot); + if (signingRecord.isEmpty()) { + return false; + } + return handleResult( + validator, signingRecord.get().maySignBlock(genesisValidatorsRoot, slot)); }); } @@ -57,11 +65,14 @@ public synchronized SafeFuture maySignAttestation( final UInt64 targetEpoch) { return SafeFuture.of( () -> { - final ValidatorSigningRecord signingRecord = - loadOrCreateSigningRecord(validator, genesisValidatorsRoot); + final Optional signingRecord = + loadSigningRecord(validator, genesisValidatorsRoot); + if (signingRecord.isEmpty()) { + return false; + } return handleResult( validator, - signingRecord.maySignAttestation(genesisValidatorsRoot, sourceEpoch, targetEpoch)); + signingRecord.get().maySignAttestation(genesisValidatorsRoot, sourceEpoch, targetEpoch)); }); } @@ -88,16 +99,19 @@ public Optional getSigningRecord(final BLSPublicKey vali return loaded; } - private ValidatorSigningRecord loadOrCreateSigningRecord( + private Optional loadSigningRecord( final BLSPublicKey validator, final Bytes32 genesisValidatorsRoot) throws IOException { final Optional record = getSigningRecord(validator); - return record.orElseGet( - () -> { - final ValidatorSigningRecord newRecord = - ValidatorSigningRecord.emptySigningRecord(genesisValidatorsRoot); - signingRecords.put(validator, newRecord); - return newRecord; - }); + if (record.isPresent()) { + return record; + } + if (slashingProtectionStrictModeEnabled) { + return Optional.empty(); + } + final ValidatorSigningRecord newRecord = + ValidatorSigningRecord.emptySigningRecord(genesisValidatorsRoot); + signingRecords.put(validator, newRecord); + return Optional.of(newRecord); } private void writeSigningRecord(final BLSPublicKey validator, final ValidatorSigningRecord record) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccess.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccess.java index 5de8d6afae6..2c8fc017e02 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccess.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccess.java @@ -35,11 +35,15 @@ public class LocalSlashingProtectorConcurrentAccess implements SlashingProtector new ConcurrentHashMap<>(); private final SyncDataAccessor dataAccessor; private final Path slashingProtectionBaseDir; + private final boolean slashingProtectionStrictModeEnabled; public LocalSlashingProtectorConcurrentAccess( - final SyncDataAccessor dataAccessor, final Path slashingProtectionBaseDir) { + final SyncDataAccessor dataAccessor, + final Path slashingProtectionBaseDir, + final boolean slashingProtectionStrictModeEnabled) { this.dataAccessor = dataAccessor; this.slashingProtectionBaseDir = slashingProtectionBaseDir; + this.slashingProtectionStrictModeEnabled = slashingProtectionStrictModeEnabled; } @Override @@ -47,14 +51,18 @@ public SafeFuture maySignBlock( final BLSPublicKey validator, final Bytes32 genesisValidatorsRoot, final UInt64 slot) { return SafeFuture.of( () -> { - final LocalSlashingProtectionRecord record = - getOrCreateSigningRecord(validator, genesisValidatorsRoot); - record.lock(); + final Optional record = + getSigningRecordForSigning(validator, genesisValidatorsRoot); + if (record.isEmpty()) { + return false; + } + final LocalSlashingProtectionRecord protectedRecord = record.get(); + protectedRecord.lock(); try { - return record.writeSigningRecord( - dataAccessor, record.maySignBlock(genesisValidatorsRoot, slot)); + return protectedRecord.writeSigningRecord( + dataAccessor, protectedRecord.maySignBlock(genesisValidatorsRoot, slot)); } finally { - record.unlock(); + protectedRecord.unlock(); } }); } @@ -67,15 +75,19 @@ public SafeFuture maySignAttestation( final UInt64 targetEpoch) { return SafeFuture.of( () -> { - final LocalSlashingProtectionRecord record = - getOrCreateSigningRecord(validator, genesisValidatorsRoot); - record.lock(); + final Optional record = + getSigningRecordForSigning(validator, genesisValidatorsRoot); + if (record.isEmpty()) { + return false; + } + final LocalSlashingProtectionRecord protectedRecord = record.get(); + protectedRecord.lock(); try { - return record.writeSigningRecord( + return protectedRecord.writeSigningRecord( dataAccessor, - record.maySignAttestation(genesisValidatorsRoot, sourceEpoch, targetEpoch)); + protectedRecord.maySignAttestation(genesisValidatorsRoot, sourceEpoch, targetEpoch)); } finally { - record.unlock(); + protectedRecord.unlock(); } }); } @@ -98,9 +110,14 @@ public Optional getSigningRecord(final BLSPublicKey vali } @VisibleForTesting - LocalSlashingProtectionRecord getOrCreateSigningRecord( + Optional getSigningRecordForSigning( final BLSPublicKey validator, final Bytes32 genesisValidatorsRoot) { - return records.computeIfAbsent(validator, __ -> addRecord(validator, genesisValidatorsRoot)); + final LocalSlashingProtectionRecord record = + records.computeIfAbsent(validator, __ -> addRecord(validator, genesisValidatorsRoot)); + if (slashingProtectionStrictModeEnabled && record.isNew()) { + return Optional.empty(); + } + return Optional.of(record); } private LocalSlashingProtectionRecord addRecord( @@ -113,6 +130,7 @@ private LocalSlashingProtectionRecord addRecord( return new LocalSlashingProtectionRecord( slashingProtectedPath, maybeRecord.orElse(ValidatorSigningRecord.emptySigningRecord(genesisValidatorsRoot)), + maybeRecord.isEmpty(), new ReentrantLock()); } diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccessTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccessTest.java index b18a06562c4..b7e166d860d 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccessTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccessTest.java @@ -33,7 +33,7 @@ public class LocalSlashingProtectorConcurrentAccessTest extends LocalSlashingPro private static final Logger LOG = LogManager.getLogger(); private final LocalSlashingProtectorConcurrentAccess slashingProtectionStorage = - new LocalSlashingProtectorConcurrentAccess(dataWriter, baseDir); + new LocalSlashingProtectorConcurrentAccess(dataWriter, baseDir, false); private final AsyncRunnerFactory asyncRunnerFactory = AsyncRunnerFactory.createDefault(new MetricTrackingExecutorFactory(new StubMetricsSystem())); @@ -54,8 +54,9 @@ void cannotAccessSameValidatorConcurrently() asyncRunner.runAsync( () -> { final LocalSlashingProtectionRecord record = - slashingProtectionStorage.getOrCreateSigningRecord( - validator, GENESIS_VALIDATORS_ROOT); + slashingProtectionStorage + .getSigningRecordForSigning(validator, GENESIS_VALIDATORS_ROOT) + .orElseThrow(); try { record.lock(); LOG.debug("LOCKED firstSigner"); @@ -72,7 +73,9 @@ void cannotAccessSameValidatorConcurrently() } }); final LocalSlashingProtectionRecord snoopRecord = - slashingProtectionStorage.getOrCreateSigningRecord(validator, GENESIS_VALIDATORS_ROOT); + slashingProtectionStorage + .getSigningRecordForSigning(validator, GENESIS_VALIDATORS_ROOT) + .orElseThrow(); while (!snoopRecord.getLock().isLocked()) { Thread.sleep(10); } @@ -84,8 +87,9 @@ void cannotAccessSameValidatorConcurrently() asyncRunner.runAsync( () -> { final LocalSlashingProtectionRecord record = - slashingProtectionStorage.getOrCreateSigningRecord( - validator, GENESIS_VALIDATORS_ROOT); + slashingProtectionStorage + .getSigningRecordForSigning(validator, GENESIS_VALIDATORS_ROOT) + .orElseThrow(); try { record.lock(); LOG.debug("LOCKED secondSigner"); @@ -121,8 +125,9 @@ void canAccessDifferentValidatorConcurrently() asyncRunner.runAsync( () -> { final LocalSlashingProtectionRecord record = - slashingProtectionStorage.getOrCreateSigningRecord( - validator, GENESIS_VALIDATORS_ROOT); + slashingProtectionStorage + .getSigningRecordForSigning(validator, GENESIS_VALIDATORS_ROOT) + .orElseThrow(); try { record.lock(); LOG.debug("LOCKED firstSigner"); @@ -144,8 +149,10 @@ void canAccessDifferentValidatorConcurrently() () -> { threadAcquired.countDown(); final LocalSlashingProtectionRecord record = - slashingProtectionStorage.getOrCreateSigningRecord( - dataStructureUtil.randomPublicKey(), GENESIS_VALIDATORS_ROOT); + slashingProtectionStorage + .getSigningRecordForSigning( + dataStructureUtil.randomPublicKey(), GENESIS_VALIDATORS_ROOT) + .orElseThrow(); try { record.lock(); LOG.debug("LOCKED secondSigner"); diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorTest.java index 6b44a1fbb9a..01d75ba220e 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorTest.java @@ -52,7 +52,7 @@ class LocalSlashingProtectorTest { baseDir.resolve(validator.toBytesCompressed().toUnprefixedHexString() + ".yml"); private final LocalSlashingProtector slashingProtectionStorage = - new LocalSlashingProtector(dataWriter, baseDir); + new LocalSlashingProtector(dataWriter, baseDir, false); @ParameterizedTest(name = "maySignBlock({0})") @MethodSource("blockCases") @@ -215,4 +215,55 @@ private void assertBlockSigningDisallowed( verify(dataWriter, never()).syncedWrite(any(), any()); } + + @ParameterizedTest(name = "maySignBlockWithStrictMode({0})") + @MethodSource("blockCases") + void maySignBlockWithStrictMode( + @SuppressWarnings("unused") final String name, + final Optional lastSignedRecord, + final UInt64 slot, + final boolean allowed) + throws Exception { + final LocalSlashingProtector strictSlashingProtector = + new LocalSlashingProtector(dataWriter, baseDir, true); + when(dataWriter.read(signingRecordPath)) + .thenReturn(lastSignedRecord.map(this::blockTestSigningRecord)); + + if (lastSignedRecord.isEmpty()) { + assertThat(strictSlashingProtector.maySignBlock(validator, GENESIS_VALIDATORS_ROOT, slot)) + .isCompletedWithValue(false); + verify(dataWriter, never()).syncedWrite(any(), any()); + } else { + assertThat(strictSlashingProtector.maySignBlock(validator, GENESIS_VALIDATORS_ROOT, slot)) + .isCompletedWithValue(allowed); + } + } + + @ParameterizedTest(name = "maySignAttestationWithStrictMode({0})") + @MethodSource("attestationCases") + void maySignAttestationWithStrictMode( + @SuppressWarnings("unused") final String name, + final Optional lastSignedRecord, + final UInt64 sourceEpoch, + final UInt64 targetEpoch, + final boolean allowed) + throws Exception { + final LocalSlashingProtector strictSlashingProtector = + new LocalSlashingProtector(dataWriter, baseDir, true); + when(dataWriter.read(signingRecordPath)) + .thenReturn(lastSignedRecord.map(ValidatorSigningRecord::toBytes)); + + if (lastSignedRecord.isEmpty()) { + assertThat( + strictSlashingProtector.maySignAttestation( + validator, GENESIS_VALIDATORS_ROOT, sourceEpoch, targetEpoch)) + .isCompletedWithValue(false); + verify(dataWriter, never()).syncedWrite(any(), any()); + } else { + assertThat( + strictSlashingProtector.maySignAttestation( + validator, GENESIS_VALIDATORS_ROOT, sourceEpoch, targetEpoch)) + .isCompletedWithValue(allowed); + } + } } diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java index 878896c6c77..2e2589c10d7 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java @@ -17,6 +17,7 @@ import static tech.pegasys.teku.validator.api.ValidatorConfig.DEFAULT_DOPPELGANGER_DETECTION_ENABLED; import static tech.pegasys.teku.validator.api.ValidatorConfig.DEFAULT_SHUTDOWN_WHEN_VALIDATOR_SLASHED_ENABLED; import static tech.pegasys.teku.validator.api.ValidatorConfig.DEFAULT_VALIDATOR_IS_LOCAL_SLASHING_PROTECTION_SYNCHRONIZED_ENABLED; +import static tech.pegasys.teku.validator.api.ValidatorConfig.DEFAULT_VALIDATOR_SLASHING_PROTECTION_STRICT_MODE_ENABLED; import java.nio.file.Path; import java.util.Optional; @@ -191,6 +192,17 @@ public class ValidatorOptions { fallbackValue = "true") private boolean shutdownWhenValidatorSlashed = DEFAULT_SHUTDOWN_WHEN_VALIDATOR_SLASHED_ENABLED; + @Option( + names = {"--slashing-protection-strict-mode-enabled"}, + paramLabel = "", + description = + "If enabled, Teku will refuse to sign if slashing protection data for a validator is not found.", + showDefaultValue = CommandLine.Help.Visibility.ALWAYS, + arity = "0..1", + fallbackValue = "true") + private boolean slashingProtectionStrictModeEnabled = + DEFAULT_VALIDATOR_SLASHING_PROTECTION_STRICT_MODE_ENABLED; + public void configure(final TekuConfiguration.Builder builder) { builder.validator( config -> @@ -210,6 +222,7 @@ public void configure(final TekuConfiguration.Builder builder) { .executorThreads(executorThreads) .exitWhenNoValidatorKeysEnabled(exitWhenNoValidatorKeysEnabled) .shutdownWhenValidatorSlashedEnabled(shutdownWhenValidatorSlashed) + .slashingProtectionStrictModeEnabled(slashingProtectionStrictModeEnabled) .executorMaxQueueSize(executorMaxQueueSize) .beaconApiExecutorThreads(beaconApiExecutorThreads) .beaconApiReadinessExecutorThreads(beaconApiReadinessExecutorThreads)); diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java index 5bc1d428809..464fcc2c367 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java @@ -62,6 +62,7 @@ public class ValidatorConfig { public static final int MAXIMUM_VALIDATOR_EXTERNAL_SIGNER_CONCURRENT_REQUEST_LIMIT = 1024; public static final boolean DEFAULT_VALIDATOR_KEYSTORE_LOCKING_ENABLED = true; public static final boolean DEFAULT_VALIDATOR_EXTERNAL_SIGNER_SLASHING_PROTECTION_ENABLED = true; + public static final boolean DEFAULT_VALIDATOR_SLASHING_PROTECTION_STRICT_MODE_ENABLED = false; public static final boolean DEFAULT_GENERATE_EARLY_ATTESTATIONS = true; public static final Optional DEFAULT_GRAFFITI = Optional.empty(); public static final ClientGraffitiAppendFormat DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT = @@ -117,6 +118,7 @@ public class ValidatorConfig { private final OptionalInt beaconApiReadinessExecutorThreads; private final boolean isLocalSlashingProtectionSynchronizedModeEnabled; + private final boolean slashingProtectionStrictModeEnabled; private final boolean dvtSelectionsEndpointEnabled; private final boolean attestationsV2ApisEnabled; @@ -164,6 +166,7 @@ private ValidatorConfig( final OptionalInt beaconApiReadinessExecutorThreads, final Optional sentryNodeConfigurationFile, final boolean isLocalSlashingProtectionSynchronizedModeEnabled, + final boolean slashingProtectionStrictModeEnabled, final boolean dvtSelectionsEndpointEnabled, final boolean attestationsV2ApisEnabled, final UInt64 builderMinBid, @@ -212,6 +215,7 @@ private ValidatorConfig( this.sentryNodeConfigurationFile = sentryNodeConfigurationFile; this.isLocalSlashingProtectionSynchronizedModeEnabled = isLocalSlashingProtectionSynchronizedModeEnabled; + this.slashingProtectionStrictModeEnabled = slashingProtectionStrictModeEnabled; this.dvtSelectionsEndpointEnabled = dvtSelectionsEndpointEnabled; this.attestationsV2ApisEnabled = attestationsV2ApisEnabled; this.builderMinBid = builderMinBid; @@ -391,6 +395,10 @@ public boolean isLocalSlashingProtectionSynchronizedModeEnabled() { return isLocalSlashingProtectionSynchronizedModeEnabled; } + public boolean isSlashingProtectionStrictModeEnabled() { + return slashingProtectionStrictModeEnabled; + } + public boolean isDvtSelectionsEndpointEnabled() { return dvtSelectionsEndpointEnabled; } @@ -463,6 +471,8 @@ public static final class Builder { private int executorThreads = DEFAULT_VALIDATOR_EXECUTOR_THREADS; private boolean isLocalSlashingProtectionSynchronizedModeEnabled = DEFAULT_VALIDATOR_IS_LOCAL_SLASHING_PROTECTION_SYNCHRONIZED_ENABLED; + private boolean slashingProtectionStrictModeEnabled = + DEFAULT_VALIDATOR_SLASHING_PROTECTION_STRICT_MODE_ENABLED; private boolean dvtSelectionsEndpointEnabled = DEFAULT_OBOL_DVT_SELECTIONS_ENDPOINT_ENABLED; private boolean attestationsV2ApisEnabled = DEFAULT_ATTESTATIONS_V2_APIS_ENABLED; private UInt64 builderMinBid = DEFAULT_BUILDER_MIN_BID; @@ -731,6 +741,12 @@ public Builder isLocalSlashingProtectionSynchronizedModeEnabled( return this; } + public Builder slashingProtectionStrictModeEnabled( + final boolean slashingProtectionStrictModeEnabled) { + this.slashingProtectionStrictModeEnabled = slashingProtectionStrictModeEnabled; + return this; + } + public Builder obolDvtSelectionsEndpointEnabled(final boolean dvtSelectionsEndpointEnabled) { this.dvtSelectionsEndpointEnabled = dvtSelectionsEndpointEnabled; return this; @@ -800,6 +816,7 @@ public ValidatorConfig build() { beaconApiReadinessExecutorThreads, sentryNodeConfigurationFile, isLocalSlashingProtectionSynchronizedModeEnabled, + slashingProtectionStrictModeEnabled, dvtSelectionsEndpointEnabled, attestationsV2ApisEnabled, builderMinBid, diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 8de926b1007..c075370b77a 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -432,9 +432,13 @@ private static ValidatorLoader createValidatorLoader( final SlashingProtector slashingProtector = config.getValidatorConfig().isLocalSlashingProtectionSynchronizedModeEnabled() ? new LocalSlashingProtector( - SyncDataAccessor.create(slashingProtectionPath), slashingProtectionPath) + SyncDataAccessor.create(slashingProtectionPath), + slashingProtectionPath, + config.getValidatorConfig().isSlashingProtectionStrictModeEnabled()) : new LocalSlashingProtectorConcurrentAccess( - SyncDataAccessor.create(slashingProtectionPath), slashingProtectionPath); + SyncDataAccessor.create(slashingProtectionPath), + slashingProtectionPath, + config.getValidatorConfig().isSlashingProtectionStrictModeEnabled()); final SlashingProtectionLogger slashingProtectionLogger = new SlashingProtectionLogger( slashingProtector, config.getSpec(), asyncRunner, VALIDATOR_LOGGER);