review suggestions

Consensys · Feb 15, 2024 · 0cc227f · 0cc227f
1 parent 76a7520
commit 0cc227f
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,9 +2,11 @@
 
 ## Next version
 
+### New Features and Improvements
+- Improve Key Manager API import operation to use parallel processing instead of serial processing.
+
 ### Bugs fixed
 - Ensure that Web3Signer stops the http server when a sigterm is received
-- Improve Key Manager API import operation to use parallel processing instead of serial processing.
 
 ## 24.1.1
 

diff --git a/...asys/web3signer/core/service/http/handlers/keymanager/imports/ImportKeystoresHandler.java b/...asys/web3signer/core/service/http/handlers/keymanager/imports/ImportKeystoresHandler.java
@@ -103,26 +103,26 @@ public void handle(final RoutingContext context) {
       return;
     }
 
-    // load "active" keys
-    final Set<String> activePubKeys =
+    // "active" keys which are already loaded by Web3Signer before this import call.
+    final Set<String> existingPubKeys =
         artifactSignerProvider.availableIdentifiers().stream()
             .map(IdentifierUtils::normaliseIdentifier)
             .collect(Collectors.toSet());
 
     // map incoming keystores either as duplicate or to be imported
     final List<ImportKeystoreData> importKeystoreDataList =
-        getKeystoreDataToProcess(parsedBody, activePubKeys);
+        getKeystoreDataToProcess(parsedBody, existingPubKeys);
 
     // Step 3: import slashing protection data for all to-be-IMPORTED keys
-    final List<String> importedPubKeys = getToBeImportedPubKeys(importKeystoreDataList);
+    final List<String> pubKeysToBeImported = getPubKeysToBeImported(importKeystoreDataList);
 
     if (slashingProtection.isPresent()
         && !StringUtils.isEmpty(parsedBody.getSlashingProtection())) {
       try {
         final InputStream slashingProtectionData =
             new ByteArrayInputStream(
                 parsedBody.getSlashingProtection().getBytes(StandardCharsets.UTF_8));
-        slashingProtection.get().importDataWithFilter(slashingProtectionData, importedPubKeys);
+        slashingProtection.get().importDataWithFilter(slashingProtectionData, pubKeysToBeImported);
       } catch (final Exception e) {
         // since we haven't written any keys to the file system, we don't need to clean up
         context.fail(BAD_REQUEST, e);
@@ -132,33 +132,28 @@ public void handle(final RoutingContext context) {
 
     // must return status 200 from here onward ...
 
-    // step 4: add validators to be imported in parallel stream
+    // step 4: add validators to be imported
     importValidators(importKeystoreDataList);
 
     // final step, send sorted results ...
     try {
-      final List<ImportKeystoreResult> results = getKeystoreResults(importKeystoreDataList);
+      final List<ImportKeystoreResult> results = getImportKeystoreResults(importKeystoreDataList);
       context
           .response()
           .putHeader(CONTENT_TYPE, JSON_UTF_8)
           .setStatusCode(SUCCESS)
           .end(objectMapper.writeValueAsString(new ImportKeystoresResponse(results)));
     } catch (final Exception e) {
       // critical bug, clean out imported keystores files ...
-      removeSignersAndCleanupImportedKeystoreFiles(importedPubKeys);
+      removeSignersAndCleanupImportedKeystoreFiles(pubKeysToBeImported);
       context.fail(SERVER_ERROR, e);
     }
   }
 
-  /**
-   * Import validators in parallel stream
-   *
-   * @param importKeystoreDataList List of keystore data to import
-   */
   private void importValidators(final List<ImportKeystoreData> importKeystoreDataList) {
-    final List<ImportKeystoreData> toBeImported =
-        importKeystoreDataList.stream().filter(ImportKeystoresHandler::imported).toList();
-    toBeImported.parallelStream()
+    importKeystoreDataList.stream()
+        .filter(ImportKeystoresHandler::imported)
+        .parallel()
         .forEach(
             data -> {
               try {
@@ -176,13 +171,7 @@ private void importValidators(final List<ImportKeystoreData> importKeystoreDataL
     removeSignersAndCleanupImportedKeystoreFiles(getFailedValidators(importKeystoreDataList));
   }
 
-  /**
-   * Get the results of the keystore import
-   *
-   * @param importKeystoreDataList Import Keystore Data
-   * @return Import Keystore Results in sorted order
-   */
-  private static List<ImportKeystoreResult> getKeystoreResults(
+  private static List<ImportKeystoreResult> getImportKeystoreResults(
       final List<ImportKeystoreData> importKeystoreDataList) {
     return importKeystoreDataList.stream()
         .sorted()
@@ -217,8 +206,8 @@ private List<ImportKeystoreData> getKeystoreDataToProcess(
         .toList();
   }
 
-  private static List<String> getToBeImportedPubKeys(
-      List<ImportKeystoreData> importKeystoreDataList) {
+  private static List<String> getPubKeysToBeImported(
+      final List<ImportKeystoreData> importKeystoreDataList) {
     return importKeystoreDataList.stream()
         .filter(ImportKeystoresHandler::imported)
         .map(ImportKeystoreData::pubKey)