Skip to content

Add slashing protection strict mode for external signers #11182

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

Open
ayaanoncrypto wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add slashing protection strict mode for external signers #11182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions ...m/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectionRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -57,6 +60,10 @@ ValidatorSigningRecord getSigningRecord() {
return signingRecord;
}

boolean isNew() {
return isNew;
}

boolean writeSigningRecord(
final SyncDataAccessor dataAccessor, final Optional<ValidatorSigningRecord> maybeRecord)
throws IOException {
Expand Down
44 changes: 29 additions & 15 deletions ethereum/spec/src/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,29 @@ public class LocalSlashingProtector implements SlashingProtector {
private final Map<BLSPublicKey, Path> 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
public synchronized SafeFuture<Boolean> 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<ValidatorSigningRecord> signingRecord =
loadSigningRecord(validator, genesisValidatorsRoot);
if (signingRecord.isEmpty()) {
return false;
}
return handleResult(
validator, signingRecord.get().maySignBlock(genesisValidatorsRoot, slot));
});
}

Expand All @@ -57,11 +65,14 @@ public synchronized SafeFuture<Boolean> maySignAttestation(
final UInt64 targetEpoch) {
return SafeFuture.of(
() -> {
final ValidatorSigningRecord signingRecord =
loadOrCreateSigningRecord(validator, genesisValidatorsRoot);
final Optional<ValidatorSigningRecord> signingRecord =
loadSigningRecord(validator, genesisValidatorsRoot);
if (signingRecord.isEmpty()) {
return false;
}
return handleResult(
validator,
signingRecord.maySignAttestation(genesisValidatorsRoot, sourceEpoch, targetEpoch));
signingRecord.get().maySignAttestation(genesisValidatorsRoot, sourceEpoch, targetEpoch));
});
}

Expand All @@ -88,16 +99,19 @@ public Optional<ValidatorSigningRecord> getSigningRecord(final BLSPublicKey vali
return loaded;
}

private ValidatorSigningRecord loadOrCreateSigningRecord(
private Optional<ValidatorSigningRecord> loadSigningRecord(
final BLSPublicKey validator, final Bytes32 genesisValidatorsRoot) throws IOException {
final Optional<ValidatorSigningRecord> 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)
Expand Down
48 changes: 33 additions & 15 deletions ...c/main/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,34 @@ 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
public SafeFuture<Boolean> maySignBlock(
final BLSPublicKey validator, final Bytes32 genesisValidatorsRoot, final UInt64 slot) {
return SafeFuture.of(
() -> {
final LocalSlashingProtectionRecord record =
getOrCreateSigningRecord(validator, genesisValidatorsRoot);
record.lock();
final Optional<LocalSlashingProtectionRecord> 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();
}
});
}
Expand All @@ -67,15 +75,19 @@ public SafeFuture<Boolean> maySignAttestation(
final UInt64 targetEpoch) {
return SafeFuture.of(
() -> {
final LocalSlashingProtectionRecord record =
getOrCreateSigningRecord(validator, genesisValidatorsRoot);
record.lock();
final Optional<LocalSlashingProtectionRecord> 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();
}
});
}
Expand All @@ -98,9 +110,14 @@ public Optional<ValidatorSigningRecord> getSigningRecord(final BLSPublicKey vali
}

@VisibleForTesting
LocalSlashingProtectionRecord getOrCreateSigningRecord(
Optional<LocalSlashingProtectionRecord> 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();
}

@cursor cursor Bot Aug 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Strict mode caches missing records permanently

Medium Severity

When strict mode is enabled, getSigningRecordForSigning inserts a record via computeIfAbsent before checking isNew. Because isNew is immutable, a first miss is remembered for the process lifetime, so later slashing-protection files are ignored and signing stays refused until restart.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 75767ed. Configure here.

return Optional.of(record);
}

private LocalSlashingProtectionRecord addRecord(
Expand All @@ -113,6 +130,7 @@ private LocalSlashingProtectionRecord addRecord(
return new LocalSlashingProtectionRecord(
slashingProtectedPath,
maybeRecord.orElse(ValidatorSigningRecord.emptySigningRecord(genesisValidatorsRoot)),
maybeRecord.isEmpty(),
new ReentrantLock());
}

Expand Down
27 changes: 17 additions & 10 deletions ...st/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorConcurrentAccessTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand All @@ -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");
Expand All @@ -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);
}
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand Down
53 changes: 52 additions & 1 deletion ...reum/spec/src/test/java/tech/pegasys/teku/spec/signatures/LocalSlashingProtectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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<UInt64> 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<ValidatorSigningRecord> 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);
}
}
}
13 changes: 13 additions & 0 deletions teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 = "<BOOLEAN>",
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 ->
Expand All @@ -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));
Expand Down
Loading
Loading