diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f27906c0..1e3e1d490 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## Next Release + +### Breaking Changes +- The behavior of reload API endpoint has been modified due to issue [#1018][issue_1018] implemented by PR [#1054][pr_1054]. +The reload API call will remove stale keys therefore they will not be return in public_keys endpoint neither will be +able to perform any signing requests. +- `--Xworker-pool-size` deprecated cli option has been removed. Use `--vertx-worker-pool-size` instead. + +[issue_1018]: https://github.com/Consensys/web3signer/issues/1018 +[pr_1054]: https://github.com/Consensys/web3signer/pull/1054 + +### Features Added +- Remove stale keys during reload API call. [#1018][issue_1018] [#1054][pr_1054] + +--- ## 24.12.0 ### Breaking Changes diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/publickeys/KeyIdentifiersAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/publickeys/KeyIdentifiersAcceptanceTest.java index 843fb91c3..1aa01d6d6 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/publickeys/KeyIdentifiersAcceptanceTest.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/publickeys/KeyIdentifiersAcceptanceTest.java @@ -147,7 +147,7 @@ public void healthCheckReportsKeysLoadedAfterReloadInEth2Mode() { @ParameterizedTest @EnumSource(value = KeyType.class) - public void alreadyLoadedPublicKeysAreNotRemovedAfterReload(final KeyType keyType) { + public void publicKeysAreRemovedAfterReloadDefault(final KeyType keyType) { final String[] prvKeys = privateKeys(keyType); final String[] keys = createKeys(keyType, true, prvKeys); @@ -161,7 +161,15 @@ public void alreadyLoadedPublicKeysAreNotRemovedAfterReload(final KeyType keyTyp // reload API call signer.callReload().then().statusCode(200); - validateApiResponse(signer.callApiPublicKeys(keyType), containsInAnyOrder(keys)); + // reload is async ... assert that the key is removed + Awaitility.await() + .atMost(5, SECONDS) + .untilAsserted( + () -> { + final List publicKeysList = + signer.callApiPublicKeys(keyType).jsonPath().getList("."); + assertThat(publicKeysList).containsOnly(keys[0]); + }); } @ParameterizedTest diff --git a/commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java b/commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java index 70dbbe8f3..915d5ca32 100644 --- a/commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java +++ b/commandline/src/main/java/tech/pegasys/web3signer/commandline/Web3SignerBaseCommand.java @@ -213,10 +213,6 @@ public class Web3SignerBaseCommand implements BaseConfig, Runnable { paramLabel = INTEGER_FORMAT_HELP) private Integer vertxWorkerPoolSize = null; - @Deprecated(forRemoval = true) - @Option(names = "--Xworker-pool-size", hidden = true) - private Integer deprecatedWorkerPoolSize = null; - @CommandLine.Mixin private PicoCliTlsServerOptions picoCliTlsServerOptions; @Override @@ -323,19 +319,10 @@ public boolean keystoreParallelProcessingEnabled() { @Override public int getVertxWorkerPoolSize() { - // both values are not allowed on cli, they will be verified in validateArgs() ... - if (vertxWorkerPoolSize != null && deprecatedWorkerPoolSize != null) { - return -1; - } - if (vertxWorkerPoolSize != null) { return vertxWorkerPoolSize; } - if (deprecatedWorkerPoolSize != null) { - return deprecatedWorkerPoolSize; - } - return VERTX_WORKER_POOL_SIZE_DEFAULT; } @@ -386,12 +373,6 @@ public void validateArgs() { "--metrics-enabled option and --metrics-push-enabled option can't be used at the same " + "time. Please refer to CLI reference for more details about this constraint."); } - - if (vertxWorkerPoolSize != null && deprecatedWorkerPoolSize != null) { - throw new CommandLine.MutuallyExclusiveArgsException( - spec.commandLine(), - "--vertx-worker-pool-size option and --Xworker-pool-size option can't be used at the same time."); - } } public static class Web3signerMetricCategoryConverter extends MetricCategoryConverter { diff --git a/commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java b/commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java index 23bb35904..f0127abc3 100644 --- a/commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java +++ b/commandline/src/test/java/tech/pegasys/web3signer/commandline/CommandlineParserTest.java @@ -559,21 +559,6 @@ void awsWithoutModeDefaultsToSpecified() { .contains("v1", "v2", "v3"); } - @Test - void vertxWorkerPoolSizeWithWorkerPoolSizeFailsToParse() { - String cmdline = validBaseCommandOptions(); - cmdline += - "--vertx-worker-pool-size=30 --Xworker-pool-size=40 eth2 --slashing-protection-enabled=false"; - - parser.registerSubCommands(new MockEth2SubCommand()); - final int result = parser.parseCommandLine(cmdline.split(" ")); - - assertThat(result).isNotZero(); - assertThat(commandError.toString()) - .contains( - "Error parsing parameters: --vertx-worker-pool-size option and --Xworker-pool-size option can't be used at the same time."); - } - @Test void vertxWorkerPoolSizeDefaultParsesSuccessfully() { String cmdline = validBaseCommandOptions(); @@ -587,19 +572,6 @@ void vertxWorkerPoolSizeDefaultParsesSuccessfully() { assertThat(mockEth2SubCommand.getConfig().getVertxWorkerPoolSize()).isEqualTo(20); } - @Test - void vertxWorkerPoolSizeDeprecatedParsesSuccessfully() { - String cmdline = validBaseCommandOptions(); - cmdline += "--Xworker-pool-size=40 eth2 --slashing-protection-enabled=false"; - - MockEth2SubCommand mockEth2SubCommand = new MockEth2SubCommand(); - parser.registerSubCommands(mockEth2SubCommand); - final int result = parser.parseCommandLine(cmdline.split(" ")); - - assertThat(result).isZero(); - assertThat(mockEth2SubCommand.getConfig().getVertxWorkerPoolSize()).isEqualTo(40); - } - @Test void vertxWorkerPoolSizeParsesSuccessfully() { String cmdline = validBaseCommandOptions(); @@ -688,7 +660,9 @@ public void run() {} @Override protected List createArtifactSignerProvider( final Vertx vertx, final MetricsSystem metricsSystem) { - return List.of(new DefaultArtifactSignerProvider(Collections::emptyList, Optional.empty())); + return List.of( + new DefaultArtifactSignerProvider( + Collections::emptyList, Optional.empty(), Optional.empty())); } @Override diff --git a/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java b/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java index 4a470d229..e92111ee3 100644 --- a/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java +++ b/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java @@ -116,6 +116,7 @@ protected List createArtifactSignerProvider( .getValues()); return signers; }, + Optional.empty(), Optional.empty()); // uses eth1 address as identifier @@ -148,13 +149,12 @@ private MappedResults loadSignersFromKeyConfigFiles( awsKmsSignerFactory, true); - return new SignerLoader(baseConfig.keystoreParallelProcessingEnabled()) - .load( - baseConfig.getKeyConfigPath(), - "yaml", - new YamlSignerParser( - List.of(ethSecpArtifactSignerFactory), - YamlMapperFactory.createYamlMapper(baseConfig.getKeyStoreConfigFileMaxSize()))); + return SignerLoader.load( + baseConfig.getKeyConfigPath(), + new YamlSignerParser( + List.of(ethSecpArtifactSignerFactory), + YamlMapperFactory.createYamlMapper(baseConfig.getKeyStoreConfigFileMaxSize())), + baseConfig.keystoreParallelProcessingEnabled()); } } diff --git a/core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java b/core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java index eebd89ba7..210526fd7 100644 --- a/core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java +++ b/core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java @@ -58,16 +58,18 @@ import tech.pegasys.web3signer.signing.config.metadata.yubihsm.YubiHsmOpaqueDataProvider; import tech.pegasys.web3signer.slashingprotection.DbHealthCheck; import tech.pegasys.web3signer.slashingprotection.DbPrunerRunner; +import tech.pegasys.web3signer.slashingprotection.PostLoadingValidatorsProcessor; import tech.pegasys.web3signer.slashingprotection.SlashingProtectionContext; import tech.pegasys.web3signer.slashingprotection.SlashingProtectionContextFactory; import tech.pegasys.web3signer.slashingprotection.SlashingProtectionParameters; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; +import java.util.function.Supplier; import io.vertx.core.Vertx; import io.vertx.core.json.JsonObject; @@ -155,30 +157,25 @@ protected List createArtifactSignerProvider( final Vertx vertx, final MetricsSystem metricsSystem) { return List.of( new DefaultArtifactSignerProvider( - () -> { - try (final AzureKeyVaultFactory azureKeyVaultFactory = new AzureKeyVaultFactory()) { - final List signers = new ArrayList<>(); - signers.addAll( - loadSignersFromKeyConfigFiles(vertx, azureKeyVaultFactory, metricsSystem) - .getValues()); - signers.addAll(bulkLoadSigners(azureKeyVaultFactory).getValues()); + createArtifactSignerSupplier(vertx, metricsSystem), + slashingProtectionContext.map(PostLoadingValidatorsProcessor::new), + Optional.of(commitBoostApiParameters))); + } - final List validators = - signers.stream() - .map(ArtifactSigner::getIdentifier) - .map(Bytes::fromHexString) - .collect(Collectors.toList()); - if (validators.isEmpty()) { - LOG.warn("No BLS keys loaded. Check that the key store has BLS key config files"); - } else { - slashingProtectionContext.ifPresent( - context -> context.getRegisteredValidators().registerValidators(validators)); - } + private Supplier> createArtifactSignerSupplier( + final Vertx vertx, final MetricsSystem metricsSystem) { + return () -> { + try (final AzureKeyVaultFactory azureKeyVaultFactory = new AzureKeyVaultFactory()) { + final List signers = new ArrayList<>(); + // load keys from key config files + signers.addAll( + loadSignersFromKeyConfigFiles(vertx, azureKeyVaultFactory, metricsSystem).getValues()); + // bulk load keys + signers.addAll(bulkLoadSigners(azureKeyVaultFactory).getValues()); - return signers; - } - }, - Optional.of(commitBoostApiParameters))); + return signers; + } + }; } private MappedResults loadSignersFromKeyConfigFiles( @@ -204,14 +201,12 @@ private MappedResults loadSignersFromKeyConfigFiles( azureKeyVaultFactory); final MappedResults results = - new SignerLoader(baseConfig.keystoreParallelProcessingEnabled()) - .load( - baseConfig.getKeyConfigPath(), - "yaml", - new YamlSignerParser( - List.of(artifactSignerFactory), - YamlMapperFactory.createYamlMapper( - baseConfig.getKeyStoreConfigFileMaxSize()))); + SignerLoader.load( + baseConfig.getKeyConfigPath(), + new YamlSignerParser( + List.of(artifactSignerFactory), + YamlMapperFactory.createYamlMapper(baseConfig.getKeyStoreConfigFileMaxSize())), + baseConfig.keystoreParallelProcessingEnabled()); registerSignerLoadingHealthCheck(KEYS_CHECK_CONFIG_FILE_LOADING, results); return results; @@ -302,9 +297,7 @@ private void registerSignerLoadingHealthCheck( @Override public void run() { super.run(); - if (pruningEnabled && slashingProtectionContext.isPresent()) { - scheduleAndExecuteInitialDbPruning(); - } + scheduleAndExecuteInitialDbPruning(); slashingProtectionContext.ifPresent(this::scheduleDbHealthCheck); } @@ -326,6 +319,10 @@ private void scheduleDbHealthCheck(final SlashingProtectionContext protectionCon } private void scheduleAndExecuteInitialDbPruning() { + if (!pruningEnabled || slashingProtectionContext.isEmpty()) { + return; + } + final DbPrunerRunner dbPrunerRunner = new DbPrunerRunner( slashingProtectionParameters, diff --git a/core/src/main/java/tech/pegasys/web3signer/core/service/http/handlers/ReloadHandler.java b/core/src/main/java/tech/pegasys/web3signer/core/service/http/handlers/ReloadHandler.java index 4c7277bdf..c41d82394 100644 --- a/core/src/main/java/tech/pegasys/web3signer/core/service/http/handlers/ReloadHandler.java +++ b/core/src/main/java/tech/pegasys/web3signer/core/service/http/handlers/ReloadHandler.java @@ -22,27 +22,26 @@ import io.vertx.ext.web.RoutingContext; public class ReloadHandler implements Handler { - List orderedArtifactSignerProviders; + private final List orderedArtifactSignerProviders; - public ReloadHandler(List orderedArtifactSignerProviders) { + public ReloadHandler(final List orderedArtifactSignerProviders) { this.orderedArtifactSignerProviders = orderedArtifactSignerProviders; } @Override - public void handle(RoutingContext routingContext) { + public void handle(final RoutingContext routingContext) { Executors.newSingleThreadExecutor() .submit( () -> - orderedArtifactSignerProviders.stream() - .forEachOrdered( - signer -> { - try { - signer.load().get(); - } catch (InterruptedException | ExecutionException e) { - throw new RuntimeException(e); - } - })); + orderedArtifactSignerProviders.forEach( + signer -> { + try { + signer.load().get(); + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException(e); + } + })); routingContext.response().setStatusCode(200).end(); } } diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/BlsArtifactSigner.java b/signing/src/main/java/tech/pegasys/web3signer/signing/BlsArtifactSigner.java index c7a04b131..594eba2f2 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/BlsArtifactSigner.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/BlsArtifactSigner.java @@ -20,6 +20,7 @@ import java.util.Objects; import java.util.Optional; +import com.google.common.base.MoreObjects; import org.apache.tuweni.bytes.Bytes; public class BlsArtifactSigner implements ArtifactSigner { @@ -77,4 +78,12 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(keyPair); } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("keyPair", keyPair.getPublicKey()) + .add("origin", origin) + .toString(); + } } diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/config/ConfigFileContent.java b/signing/src/main/java/tech/pegasys/web3signer/signing/config/ConfigFileContent.java deleted file mode 100644 index 86e3c6959..000000000 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/config/ConfigFileContent.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright 2023 ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package tech.pegasys.web3signer.signing.config; - -import java.nio.file.Path; -import java.util.Collections; -import java.util.Map; - -class ConfigFileContent { - private final Map contentMap; - private final int errorCount; - - public ConfigFileContent(final Map contentMap, final int errorCount) { - this.contentMap = contentMap; - this.errorCount = errorCount; - } - - static ConfigFileContent withSingleErrorCount() { - return new ConfigFileContent(Collections.emptyMap(), 1); - } - - public Map getContentMap() { - return contentMap; - } - - public int getErrorCount() { - return errorCount; - } -} diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProvider.java b/signing/src/main/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProvider.java index bfa2893b5..beb580fdd 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProvider.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProvider.java @@ -25,15 +25,17 @@ import java.io.File; import java.nio.file.Path; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.function.BiFunction; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -41,19 +43,34 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +/** + * Default implementation of {@link ArtifactSignerProvider} that loads signers and proxy signers. + * This class is designed to provide concurrent access to signers and proxy signers, ensuring thread + * safety and efficient read operations. The {@code load()} method is called infrequently, typically + * at startup or via a reload API call, while the getter methods are called frequently. + * + *

The class uses a {@link ConcurrentHashMap} for storing signers and proxy signers to allow + * efficient concurrent read access. It also uses a single-threaded executor to ensure that the + * {@code load()} method and other write operations are executed sequentially. + */ public class DefaultArtifactSignerProvider implements ArtifactSignerProvider { private static final Logger LOG = LogManager.getLogger(); + private final Supplier> artifactSignerCollectionSupplier; - private final Map signers = new HashMap<>(); - private final Map> proxySigners = new HashMap<>(); - private final ExecutorService executorService = Executors.newSingleThreadExecutor(); + private final Optional>> postLoadingCallback; private final Optional commitBoostKeystoresParameters; + private final ConcurrentMap signers = new ConcurrentHashMap<>(); + private final ConcurrentMap> proxySigners = new ConcurrentHashMap<>(); + private final ExecutorService executorService = Executors.newSingleThreadExecutor(); + public DefaultArtifactSignerProvider( final Supplier> artifactSignerCollectionSupplier, + final Optional>> postLoadingCallback, final Optional commitBoostKeystoresParameters) { this.artifactSignerCollectionSupplier = artifactSignerCollectionSupplier; + this.postLoadingCallback = postLoadingCallback; this.commitBoostKeystoresParameters = commitBoostKeystoresParameters; } @@ -63,41 +80,58 @@ public Future load() { () -> { LOG.debug("Signer keys pre-loaded in memory {}", signers.size()); - artifactSignerCollectionSupplier.get().stream() - .collect( - Collectors.toMap( - ArtifactSigner::getIdentifier, - Function.identity(), - (signer1, signer2) -> { - LOG.warn( - "Duplicate keys were found while loading. {}", Function.identity()); - return signer1; - })) - .forEach(signers::putIfAbsent); - - // for each loaded signer, load commit boost proxy signers (if any) + // Load new signers into a temporary map - this is time-consuming logic + final Map newSigners = + artifactSignerCollectionSupplier.get().stream() + .collect( + Collectors.toMap( + ArtifactSigner::getIdentifier, + Function.identity(), + (signer1, signer2) -> { + LOG.warn( + "Duplicate keys were found while loading. {}", Function.identity()); + return signer1; + })); + + // Collect all stale keys that are no longer valid + final Set staleKeys = new HashSet<>(signers.keySet()); + staleKeys.removeAll(newSigners.keySet()); + + // Update the signers map with new signers + signers.putAll(newSigners); + + // remove stale keys from signers map + staleKeys.forEach(signers::remove); + + // Callback to perform further actions specific to eth1/eth2 mode (if any) + postLoadingCallback.ifPresent(callback -> callback.accept(signers.keySet())); + + // For each loaded signer, load commit boost proxy signers (if any) commitBoostKeystoresParameters .filter(KeystoresParameters::isEnabled) .ifPresent( - keystoreParameter -> - signers - .keySet() - .forEach( - consensusPubKey -> { - LOG.trace( - "Loading proxy signers for signer '{}' ...", consensusPubKey); - loadProxySigners( - keystoreParameter, - consensusPubKey, - SECP256K1.name(), - SecpV3KeystoresBulkLoader::loadECDSAProxyKeystores); - - loadProxySigners( - keystoreParameter, - consensusPubKey, - BLS.name(), - BlsKeystoreBulkLoader::loadKeystoresUsingPasswordFile); - })); + keystoreParameter -> { + signers + .keySet() + .forEach( + consensusPubKey -> { + LOG.trace( + "Loading proxy signers for signer '{}' ...", consensusPubKey); + loadProxySigners( + keystoreParameter, + consensusPubKey, + SECP256K1.name(), + SecpV3KeystoresBulkLoader::loadECDSAProxyKeystores); + + loadProxySigners( + keystoreParameter, + consensusPubKey, + BLS.name(), + BlsKeystoreBulkLoader::loadKeystoresUsingPasswordFile); + }); + // Remove stale proxy signers + staleKeys.forEach(proxySigners::remove); + }); LOG.info("Total signers (keys) currently loaded in memory: {}", signers.size()); return null; @@ -130,7 +164,7 @@ public Set availableIdentifiers() { @Override public Map> getProxyIdentifiers(final String consensusPubKey) { final Set artifactSigners = - proxySigners.computeIfAbsent(consensusPubKey, k -> Set.of()); + proxySigners.computeIfAbsent(consensusPubKey, k -> ConcurrentHashMap.newKeySet()); return artifactSigners.stream() .collect( Collectors.groupingBy( @@ -164,7 +198,9 @@ public Future addProxySigner( final ArtifactSigner proxySigner, final String consensusPubKey) { return executorService.submit( () -> { - proxySigners.computeIfAbsent(consensusPubKey, k -> new HashSet<>()).add(proxySigner); + proxySigners + .computeIfAbsent(consensusPubKey, k -> ConcurrentHashMap.newKeySet()) + .add(proxySigner); LOG.info( "Loaded new proxy signer {} for consensus public key '{}'", proxySigner.getIdentifier(), @@ -205,7 +241,9 @@ private void loadProxySigners( final MappedResults signersResult = loaderFunction.apply(proxyDir, keystoreParameter.getKeystoresPasswordFile()); final Collection signers = signersResult.getValues(); - proxySigners.computeIfAbsent(consensusPubKey, k -> new HashSet<>()).addAll(signers); + proxySigners + .computeIfAbsent(consensusPubKey, k -> ConcurrentHashMap.newKeySet()) + .addAll(signers); } } } diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java b/signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java index e3f53378a..984b68aa2 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java @@ -15,6 +15,7 @@ import tech.pegasys.web3signer.keystorage.common.MappedResults; import tech.pegasys.web3signer.signing.ArtifactSigner; import tech.pegasys.web3signer.signing.config.metadata.SigningMetadata; +import tech.pegasys.web3signer.signing.config.metadata.SigningMetadataException; import tech.pegasys.web3signer.signing.config.metadata.parser.SignerParser; import java.io.IOException; @@ -26,13 +27,14 @@ import java.time.Instant; import java.util.AbstractMap.SimpleEntry; import java.util.Collection; -import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; @@ -45,216 +47,204 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +/** + * The SignerLoaded loads the metadata files and converts them to ArtifactSigners. This class keeps + * track of the metadata files and ArtifactSigners that have been read and only reads them again if + * they have been modified. It also removes the cached ArtifactSigners if the metadata file has been + * removed. + */ public class SignerLoader { private static final Logger LOG = LogManager.getLogger(); private static final long FILES_PROCESSED_TO_REPORT = 10; - // enable or disable parallel streams to convert and load private keys from metadata files - private final boolean useParallelStreams; - - private static final Map metadataConfigFilesPathCache = new HashMap<>(); - - public SignerLoader(final boolean useParallelStreams) { - this.useParallelStreams = useParallelStreams; - } - - public SignerLoader() { - this(true); - } - - public MappedResults load( - final Path configsDirectory, final String fileExtension, final SignerParser signerParser) { - final Instant start = Instant.now(); + private static final Set CONFIG_FILE_EXTENSIONS = Set.of("yaml", "yml"); + + record CachedArtifactSigners( + Path metadataFile, FileTime lastModifiedTime, Set artifactSigners) {} + + private static final Map cachedArtifactSigners = + new ConcurrentHashMap<>(); + + /** + * Load ArtifactSigners for new or modified metadata files. Return cached ArtifactSigners if + * metadata files have not been modified. Remove cached ArtifactSigner if metadata file has been + * removed. + * + * @param configsDirectory Location of the metadata files + * @param signerParser An implementation of SignerParser to parse the metadata files + * @return A MappedResults of ArtifactSigners and error count + */ + public static MappedResults load( + final Path configsDirectory, + final SignerParser signerParser, + final boolean useParallelStreams) { LOG.info("Loading signer configuration metadata files from {}", configsDirectory); + final Instant loadStartTime = Instant.now(); + // get all metadata file paths from the config directory. + final Map filteredPaths; + try { + filteredPaths = getMetadataConfigFiles(configsDirectory); + } catch (final IOException e) { + LOG.error("Unable to access the supplied key directory", e); + return MappedResults.errorResult(); + } - final ConfigFileContent configFileContent = - getNewOrModifiedConfigFilesContents(configsDirectory, fileExtension); - - LOG.info( - "Signer configuration metadata files read in memory {} in {}", - configFileContent.getContentMap().size(), - calculateTimeTaken(start).orElse("unknown duration")); - - final Instant conversionStartInstant = Instant.now(); - // Step 1: convert yaml file content to list of SigningMetadata - final MappedResults signingMetadataResults = - convertConfigFileContent(configFileContent.getContentMap(), signerParser); - - LOG.debug( - "Signing configuration metadata files converted to signing metadata {}", - signingMetadataResults.getValues().size()); - - // Step 2: Convert SigningMetadata to ArtifactSigners. This involves connecting to remote - // Hashicorp vault, AWS, Azure or decrypting local keystore files. - final MappedResults artifactSigners = - mapMetadataToArtifactSigner(signingMetadataResults.getValues(), signerParser); - - // merge error counts of config file parsing errors ... - artifactSigners.mergeErrorCount(signingMetadataResults.getErrorCount()); - artifactSigners.mergeErrorCount(configFileContent.getErrorCount()); + // remove cached metadata file mappings that have been deleted or not loaded + var cachedArtifactSignerSize = cachedArtifactSigners.size(); + cachedArtifactSigners.keySet().removeIf(path -> !filteredPaths.containsKey(path)); + if (cachedArtifactSignerSize != cachedArtifactSigners.size()) { + LOG.info( + "Removed {} cached metadata files that has not been loaded.", + cachedArtifactSignerSize - cachedArtifactSigners.size()); + } + // reload the metadata files that are either new or modified LOG.info( - "Total Artifact Signer loaded via configuration files: {}\nError count {}\nTime Taken: {}.", - artifactSigners.getValues().size(), - artifactSigners.getErrorCount(), - calculateTimeTaken(conversionStartInstant).orElse("unknown duration")); - - return artifactSigners; - } + "Loading and converting SigningMetadata to ArtifactSigner using {} streams ...", + useParallelStreams ? "parallel" : "sequential"); + final Stream> pathStream = + useParallelStreams + ? filteredPaths.entrySet().parallelStream() + : filteredPaths.entrySet().stream(); - private MappedResults convertConfigFileContent( - final Map contentMap, final SignerParser signerParser) { + final AtomicLong configFilesHandled = new AtomicLong(0); final AtomicInteger errorCount = new AtomicInteger(0); - final List signingMetadataList = - contentMap.entrySet().parallelStream() + final Map> loadedArtSigners = + pathStream + .filter(SignerLoader::isModifiedOrNew) .flatMap( entry -> { + if (configFilesHandled.incrementAndGet() % FILES_PROCESSED_TO_REPORT == 0) { + LOG.info("{} signing metadata processed", configFilesHandled.get()); + } try { - return signerParser.readSigningMetadata(entry.getValue()).stream(); - } catch (final Exception e) { - renderException(e, entry.getKey().toString()); + return Stream.of( + new SimpleEntry<>( + entry.getKey(), + Files.readString(entry.getKey(), StandardCharsets.UTF_8))); + } catch (IOException e) { + LOG.error("Error reading metadata config file: {}", entry.getKey(), e); errorCount.incrementAndGet(); return Stream.empty(); } }) - .collect(Collectors.toList()); - return MappedResults.newInstance(signingMetadataList, errorCount.get()); - } - - @VisibleForTesting - static Optional calculateTimeTaken(final Instant start) { - final Instant now = Instant.now(); - final long timeTaken = Duration.between(start, now).toMillis(); - if (timeTaken < 0) { - LOG.warn("System Clock returned time in past. Start: {}, Now: {}.", start, now); - return Optional.empty(); - } - return Optional.of(DurationFormatUtils.formatDurationHMS(timeTaken)); - } - - private ConfigFileContent getNewOrModifiedConfigFilesContents( - final Path configsDirectory, final String fileExtension) { - // Step 1, read Paths in config directory without reading the file content since Files.list does - // not use parallel stream - final List filteredPaths; - try (final Stream fileStream = Files.list(configsDirectory)) { - filteredPaths = - fileStream - .filter(path -> matchesFileExtension(fileExtension, path)) - .filter(this::isNewOrModifiedMetadataFile) - .collect(Collectors.toList()); - } catch (final IOException e) { - LOG.error("Unable to access the supplied key directory", e); - return ConfigFileContent.withSingleErrorCount(); - } - - final AtomicInteger errorCount = new AtomicInteger(0); - // step 2, read file contents in parallel stream - final Map configFileMap = - filteredPaths.parallelStream() - .map( - path -> { + .flatMap( + entry -> { + try { + final List signingMetadata = + signerParser.readSigningMetadata(entry.getValue()); + return Stream.of(new SimpleEntry<>(entry.getKey(), signingMetadata)); + } catch (final SigningMetadataException e) { + LOG.error( + "Error parsing metadata file {} to signing metadata: {}", + entry.getKey(), + ExceptionUtils.getRootCauseMessage(e)); + errorCount.incrementAndGet(); + return Stream.empty(); + } + }) + .flatMap( + entry -> { try { - return getMetadataFileContent(path); - } catch (final IOException e) { + final Set artifactSigners = + new HashSet<>(signerParser.parse(entry.getValue())); + return Stream.of(new SimpleEntry<>(entry.getKey(), artifactSigners)); + } catch (final SigningMetadataException e) { + LOG.error( + "Error converting signing metadata to Artifact Signer: {}", + ExceptionUtils.getRootCauseMessage(e)); errorCount.incrementAndGet(); - LOG.error("Error reading config file: {}", path, e); - return null; + return Stream.empty(); } }) - .filter(Objects::nonNull) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - return new ConfigFileContent(configFileMap, errorCount.get()); - } + // merge the new loaded ArtifactSigners with the cached ones + loadedArtSigners.forEach( + (path, artifactSigners) -> { + cachedArtifactSigners.put( + path, + new CachedArtifactSigners( + path, filteredPaths.get(path).lastModifiedTime, artifactSigners)); + }); + + // return all ArtifactSigners from the cache. If same ArtifactSigner is loaded from multiple + // paths, only one will + // be returned. + final Collection allArtifactSigners = + cachedArtifactSigners.values().stream() + .flatMap(cachedArtifactSigners -> cachedArtifactSigners.artifactSigners.stream()) + .collect(Collectors.toSet()); - private boolean isNewOrModifiedMetadataFile(final Path path) { - // only read file if is not previously read or has been since modified - try { - final FileTime lastModifiedTime = Files.getLastModifiedTime(path); - if (metadataConfigFilesPathCache.containsKey(path)) { - if (metadataConfigFilesPathCache.get(path).compareTo(lastModifiedTime) == 0) { - return false; - } - } + LOG.info( + "Total Artifact Signers loaded via configuration files: {}\nTotal Paths cached: {}, Error count: {}\nTime Taken: {}.", + allArtifactSigners.size(), + cachedArtifactSigners.size(), + errorCount.get(), + calculateTimeTaken(loadStartTime).orElse("unknown duration")); - // keep the path and last modified time in local cache - metadataConfigFilesPathCache.put(path, lastModifiedTime); - return true; - } catch (final IOException e) { - LOG.error("Error reading config file: {}", path, e); - return false; - } + return MappedResults.newInstance(allArtifactSigners, errorCount.get()); } - private SimpleEntry getMetadataFileContent(final Path path) throws IOException { - return new SimpleEntry<>(path, Files.readString(path, StandardCharsets.UTF_8)); + @VisibleForTesting + static void clearCache() { + cachedArtifactSigners.clear(); } - private MappedResults mapMetadataToArtifactSigner( - final Collection signingMetadataCollection, - final SignerParser signerParser) { - - if (signingMetadataCollection.isEmpty()) { - return MappedResults.newSetInstance(); - } - - LOG.info( - "Converting signing metadata to Artifact Signer using {} streams ...", - useParallelStreams ? "parallel" : "sequential"); - - try { - if (useParallelStreams) { - return mapToArtifactSigner(signingMetadataCollection.parallelStream(), signerParser); - } else { - return mapToArtifactSigner(signingMetadataCollection.stream(), signerParser); - } - } catch (final Exception e) { - LOG.error("Unexpected error in processing configuration files: {}", e.getMessage(), e); - return MappedResults.errorResult(); + /** + * Load Metadata config file paths and their timestamps. + * + * @param configsDirectory Path to the directory containing the metadata files + * @return A map of metadata file paths and their last modified timestamps + * @throws IOException If there is an error reading the config directory. + */ + private static Map getMetadataConfigFiles( + final Path configsDirectory) throws IOException { + try (final Stream fileStream = Files.list(configsDirectory)) { + return fileStream + .filter(SignerLoader::validFileExtension) + .map( + path -> { + try { + return new SimpleEntry<>( + path, + new CachedArtifactSigners(path, Files.getLastModifiedTime(path), Set.of())); + } catch (final IOException e) { + // very unlikely to happen as Files.list is already successful. + LOG.error("Error getting last modified time for config file: {}", path, e); + return null; + } + }) + .filter(Objects::nonNull) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } } - private MappedResults mapToArtifactSigner( - final Stream signingMetadataStream, final SignerParser signerParser) { - final AtomicLong configFilesHandled = new AtomicLong(); - final AtomicInteger errorCount = new AtomicInteger(0); + private static boolean isModifiedOrNew(final Map.Entry entry) { + var metadataFilePath = entry.getKey(); + var metadataFile = entry.getValue(); + return !cachedArtifactSigners.containsKey(metadataFilePath) + || cachedArtifactSigners + .get(metadataFilePath) + .lastModifiedTime + .compareTo(metadataFile.lastModifiedTime) + != 0; + } - final Set artifactSigners = - signingMetadataStream - .flatMap( - metadataContent -> { - final long filesProcessed = configFilesHandled.incrementAndGet(); - if (filesProcessed % FILES_PROCESSED_TO_REPORT == 0) { - LOG.info("{} signing metadata processed", filesProcessed); - } - try { - return signerParser.parse(List.of(metadataContent)).stream(); - } catch (final Exception e) { - LOG.error( - "Error converting signing metadata to Artifact Signer: {}", e.getMessage()); - LOG.debug(ExceptionUtils.getStackTrace(e)); - errorCount.incrementAndGet(); - return Stream.empty(); - } - }) - .collect(Collectors.toSet()); - LOG.debug("Signing metadata mapped to Artifact Signer: {}", artifactSigners.size()); - return MappedResults.newInstance(artifactSigners, errorCount.get()); + @VisibleForTesting + static Optional calculateTimeTaken(final Instant start) { + final Instant now = Instant.now(); + final long timeTaken = Duration.between(start, now).toMillis(); + if (timeTaken < 0) { + LOG.warn("System Clock returned time in past. Start: {}, Now: {}.", start, now); + return Optional.empty(); + } + return Optional.of(DurationFormatUtils.formatDurationHMS(timeTaken)); } - private boolean matchesFileExtension(final String validFileExtension, final Path filename) { + private static boolean validFileExtension(final Path filename) { final boolean isHidden = filename.toFile().isHidden(); final String extension = FilenameUtils.getExtension(filename.toString()); - return !isHidden - && extension.toLowerCase(Locale.ROOT).endsWith(validFileExtension.toLowerCase(Locale.ROOT)); - } - - private void renderException(final Throwable t, final String filename) { - LOG.error( - "Error parsing signing metadata file {}: {}", - filename, - ExceptionUtils.getRootCauseMessage(t)); - LOG.debug(ExceptionUtils.getStackTrace(t)); + return !isHidden && CONFIG_FILE_EXTENSIONS.contains(extension.toLowerCase(Locale.ROOT)); } } diff --git a/signing/src/test/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProviderTest.java b/signing/src/test/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProviderTest.java index 962db1102..37e593f35 100644 --- a/signing/src/test/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProviderTest.java +++ b/signing/src/test/java/tech/pegasys/web3signer/signing/config/DefaultArtifactSignerProviderTest.java @@ -67,7 +67,9 @@ void signerReturnedForMatchingIdentifier() { final ArtifactSigner mockSigner = mock(ArtifactSigner.class); when(mockSigner.getIdentifier()).thenReturn(PUBLIC_KEY1); - signerProvider = new DefaultArtifactSignerProvider(() -> List.of(mockSigner), Optional.empty()); + signerProvider = + new DefaultArtifactSignerProvider( + () -> List.of(mockSigner), Optional.empty(), Optional.empty()); assertThatCode(() -> signerProvider.load().get()).doesNotThrowAnyException(); final Optional signer = signerProvider.getSigner(PUBLIC_KEY1); @@ -84,7 +86,7 @@ void signerProviderOnlyHasSingleEntryIfPassedInListHasMultipleMatchingSigners() signerProvider = new DefaultArtifactSignerProvider( - () -> List.of(mockSigner1, mockSigner2), Optional.empty()); + () -> List.of(mockSigner1, mockSigner2), Optional.empty(), Optional.empty()); assertThatCode(() -> signerProvider.load().get()).doesNotThrowAnyException(); assertThat(signerProvider.availableIdentifiers()).hasSize(1); @@ -100,7 +102,7 @@ void signerProviderCanMapInTwoSigners() { signerProvider = new DefaultArtifactSignerProvider( - () -> List.of(mockSigner1, mockSigner2), Optional.empty()); + () -> List.of(mockSigner1, mockSigner2), Optional.empty(), Optional.empty()); assertThatCode(() -> signerProvider.load().get()).doesNotThrowAnyException(); assertThat(signerProvider.availableIdentifiers()).hasSize(2); assertThat(signerProvider.availableIdentifiers()).containsOnly(PUBLIC_KEY1, PUBLIC_KEY2); @@ -129,7 +131,9 @@ void proxySignersAreLoadedCorrectly() throws IOException { signerProvider = new DefaultArtifactSignerProvider( - () -> List.of(mockSigner1, mockSigner2), Optional.of(commitBoostParameters)); + () -> List.of(mockSigner1, mockSigner2), + Optional.empty(), + Optional.of(commitBoostParameters)); // methods under test assertThatCode(() -> signerProvider.load().get()).doesNotThrowAnyException(); @@ -167,7 +171,9 @@ void emptyProxySignersAreLoadedSuccessfully() { signerProvider = new DefaultArtifactSignerProvider( - () -> List.of(mockSigner1, mockSigner2), Optional.of(commitBoostParameters)); + () -> List.of(mockSigner1, mockSigner2), + Optional.empty(), + Optional.of(commitBoostParameters)); // methods under test assertThatCode(() -> signerProvider.load().get()).doesNotThrowAnyException(); diff --git a/signing/src/test/java/tech/pegasys/web3signer/signing/config/SignerLoaderTest.java b/signing/src/test/java/tech/pegasys/web3signer/signing/config/SignerLoaderTest.java index 521ef5e5a..1c48cfc09 100644 --- a/signing/src/test/java/tech/pegasys/web3signer/signing/config/SignerLoaderTest.java +++ b/signing/src/test/java/tech/pegasys/web3signer/signing/config/SignerLoaderTest.java @@ -62,7 +62,6 @@ @ExtendWith(MockitoExtension.class) class SignerLoaderTest { private static final YAMLMapper YAML_MAPPER = YamlMapperFactory.createYamlMapper(); - private static final String FILE_EXTENSION = "yaml"; @TempDir Path configsDirectory; @Mock private MetricsSystem metricsSystem; @Mock private HashicorpConnectionFactory hashicorpConnectionFactory; @@ -77,7 +76,6 @@ class SignerLoaderTest { private static final BLSKeyPair blsKeyPair2 = BLSTestUtil.randomKeyPair(2); private static final BLSKeyPair blsKeyPair3 = BLSTestUtil.randomKeyPair(3); - private SignerLoader signerLoader; private SignerParser signerParser; @BeforeEach @@ -108,7 +106,8 @@ public void setup() { signerParser = new YamlSignerParser( List.of(blsArtifactSignerFactory), YamlMapperFactory.createYamlMapper()); - signerLoader = new SignerLoader(); + + SignerLoader.clearCache(); } @ParameterizedTest(name = "{index} - Signer created for file name {0}") @@ -117,8 +116,9 @@ void signerReturnedForValidMetadataFile(final String fileName) throws IOExceptio final String privateKeyHex = blsKeyPair1.getSecretKey().toBytes().toHexString(); createFileInConfigsDirectory(fileName, privateKeyHex); + SignerLoader.load(configsDirectory, signerParser, true); final Collection signerList = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser).getValues(); + SignerLoader.load(configsDirectory, signerParser, true).getValues(); assertThat(signerList.size()).isOne(); assertThat(signerList.stream().findFirst().orElseThrow().getIdentifier()) @@ -130,7 +130,9 @@ static Stream validMetadataFileNameProvider() { configFileName(blsKeyPair1), "prefix_" + configFileName(blsKeyPair1), "test.yaml", - "test.YAML"); + "test.YAML", + "test.yml", + "test.YML"); } @Test @@ -140,7 +142,7 @@ void wrongFileExtensionReturnsEmptySigner() throws IOException { createFileInConfigsDirectory(filename, privateKeyHex); final MappedResults result = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); + SignerLoader.load(configsDirectory, signerParser, true); assertThat(result.getValues()).isEmpty(); assertThat(result.getErrorCount()).isZero(); @@ -151,7 +153,7 @@ void failedParserReturnsEmptySigner() throws IOException { createFileInConfigsDirectory(configFileName(blsKeyPair1), "NOT_A_VALID_KEY"); final MappedResults result = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); + SignerLoader.load(configsDirectory, signerParser, true); assertThat(result.getValues()).isEmpty(); assertThat(result.getErrorCount()).isOne(); @@ -161,7 +163,7 @@ void failedParserReturnsEmptySigner() throws IOException { void failedWithDirectoryErrorReturnEmptySigner() throws IOException { final Path missingConfigDir = configsDirectory.resolve("idontexist"); final MappedResults result = - signerLoader.load(missingConfigDir, FILE_EXTENSION, signerParser); + SignerLoader.load(missingConfigDir, signerParser, true); assertThat(result.getValues()).isEmpty(); assertThat(result.getErrorCount()).isOne(); @@ -176,7 +178,7 @@ void multipleMatchesForSameIdentifierReturnsSameSigners() throws IOException { createFileInConfigsDirectory(filename2, privateKeyHex); final MappedResults result = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); + SignerLoader.load(configsDirectory, signerParser, true); assertThat(result.getValues()).hasSize(1); assertThat(result.getErrorCount()).isZero(); @@ -187,7 +189,7 @@ void signerIdentifiersNotReturnedInvalidMetadataFile() throws IOException { createEmptyFileInConfigsDirectory(configFileName(blsKeyPair1)); createEmptyFileInConfigsDirectory(configFileName(blsKeyPair2)); final MappedResults result = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); + SignerLoader.load(configsDirectory, signerParser, true); assertThat(result.getValues()).isEmpty(); assertThat(result.getErrorCount()).isEqualTo(2); @@ -203,7 +205,7 @@ void signerIdentifiersNotReturnedForHiddenFiles() throws IOException { createFileInConfigsDirectory(configFileName(blsKeyPair2), privateKeyHex2); final MappedResults result = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); + SignerLoader.load(configsDirectory, signerParser, true); assertThat(result.getValues()).hasSize(1); assertThat(result.getValues().stream().findFirst().orElseThrow().getIdentifier()) @@ -221,8 +223,7 @@ void signerIdentifiersReturnedForAllValidMetadataFilesInDirectory() throws IOExc createFileInConfigsDirectory(configFileName(blsKeyPair2), privateKeyHex2); createFileInConfigsDirectory(configFileName(blsKeyPair3), privateKeyHex3); - MappedResults result = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); + MappedResults result = SignerLoader.load(configsDirectory, signerParser, true); assertThat(result.getValues()).hasSize(3); assertThat( @@ -236,7 +237,7 @@ void signerIdentifiersReturnedForAllValidMetadataFilesInDirectory() throws IOExc } @Test - void callingLoadTwiceDoesNotReloadUnmodifiedConfigFiles() throws IOException { + void callingLoadTwiceReturnedSameNumberOfArtifacts() throws IOException { final String privateKeyHex1 = blsKeyPair1.getSecretKey().toBytes().toHexString(); final String privateKeyHex2 = blsKeyPair2.getSecretKey().toBytes().toHexString(); final String privateKeyHex3 = blsKeyPair3.getSecretKey().toBytes().toHexString(); @@ -246,7 +247,7 @@ void callingLoadTwiceDoesNotReloadUnmodifiedConfigFiles() throws IOException { createFileInConfigsDirectory(configFileName(blsKeyPair3), privateKeyHex3); final MappedResults result = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); + SignerLoader.load(configsDirectory, signerParser, true); assertThat(result.getValues()).hasSize(3); assertThat( @@ -259,12 +260,12 @@ void callingLoadTwiceDoesNotReloadUnmodifiedConfigFiles() throws IOException { blsKeyPair3.getPublicKey().toHexString()); final MappedResults reloadedResult = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); - assertThat(reloadedResult.getValues()).isEmpty(); + SignerLoader.load(configsDirectory, signerParser, true); + assertThat(reloadedResult.getValues()).hasSize(3); } @Test - void callingLoadTwiceOnlyLoadSignersFromModifiedConfigFiles() throws IOException { + void callingLoadTwiceWithRemovedFiles() throws IOException { final String privateKeyHex1 = blsKeyPair1.getSecretKey().toBytes().toHexString(); final String privateKeyHex2 = blsKeyPair2.getSecretKey().toBytes().toHexString(); final String privateKeyHex3 = blsKeyPair3.getSecretKey().toBytes().toHexString(); @@ -274,7 +275,7 @@ void callingLoadTwiceOnlyLoadSignersFromModifiedConfigFiles() throws IOException createFileInConfigsDirectory(configFileName(blsKeyPair3), privateKeyHex3); final MappedResults result = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser); + SignerLoader.load(configsDirectory, signerParser, true); assertThat(result.getValues()).hasSize(3); assertThat( @@ -286,14 +287,18 @@ void callingLoadTwiceOnlyLoadSignersFromModifiedConfigFiles() throws IOException blsKeyPair2.getPublicKey().toHexString(), blsKeyPair3.getPublicKey().toHexString()); - // recreate file - which would change the last modified time - createFileInConfigsDirectory(configFileName(blsKeyPair3), privateKeyHex3); + // remove file + Files.delete(configsDirectory.resolve(configFileName(blsKeyPair3))); final Collection reloadedArtifactSigner = - signerLoader.load(configsDirectory, FILE_EXTENSION, signerParser).getValues(); - assertThat(reloadedArtifactSigner).hasSize(1); - assertThat(reloadedArtifactSigner.stream().findFirst().get().getIdentifier()) - .isEqualTo(blsKeyPair3.getPublicKey().toHexString()); + SignerLoader.load(configsDirectory, signerParser, true).getValues(); + assertThat(reloadedArtifactSigner).hasSize(2); + assertThat( + reloadedArtifactSigner.stream() + .map(ArtifactSigner::getIdentifier) + .collect(Collectors.toList())) + .containsOnly( + blsKeyPair1.getPublicKey().toHexString(), blsKeyPair2.getPublicKey().toHexString()); } @Test @@ -325,7 +330,7 @@ private Path createFileInConfigsDirectory(final String fileName, final String pr } private static String configFileName(final BLSKeyPair blsKeyPair) { - return String.format("%s.%s", blsKeyPair.getPublicKey().toHexString(), FILE_EXTENSION); + return blsKeyPair.getPublicKey().toHexString() + ".yaml"; } private void createEmptyFileInConfigsDirectory(final String filename) throws IOException { diff --git a/slashing-protection/src/main/java/tech/pegasys/web3signer/slashingprotection/PostLoadingValidatorsProcessor.java b/slashing-protection/src/main/java/tech/pegasys/web3signer/slashingprotection/PostLoadingValidatorsProcessor.java new file mode 100644 index 000000000..5d168031c --- /dev/null +++ b/slashing-protection/src/main/java/tech/pegasys/web3signer/slashingprotection/PostLoadingValidatorsProcessor.java @@ -0,0 +1,37 @@ +/* + * Copyright 2025 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.web3signer.slashingprotection; + +import java.util.List; +import java.util.Set; +import java.util.function.Consumer; + +import org.apache.tuweni.bytes.Bytes; + +/** Process new validators by registering them with slashing database. */ +public record PostLoadingValidatorsProcessor(SlashingProtectionContext slashingProtectionContext) + implements Consumer> { + @Override + public void accept(final Set newValidators) { + registerNewValidators(newValidators); + } + + private void registerNewValidators(final Set newValidators) { + if (newValidators == null || newValidators.isEmpty()) { + return; + } + + final List validatorsList = newValidators.stream().map(Bytes::fromHexString).toList(); + slashingProtectionContext.getRegisteredValidators().registerValidators(validatorsList); + } +}