Handle API get and validator get graffiti differently

courtneyeh · Apr 22, 2024 · 9693c3e · 9693c3e
1 parent 00536fb
commit 9693c3e
Show file tree

Hide file tree

Showing 10 changed files with 114 additions and 41 deletions.
diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/GraffitiManagementException.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/GraffitiManagementException.java
@@ -13,9 +13,7 @@
 
 package tech.pegasys.teku.validator.api;
 
-import java.io.IOException;
-
-public class GraffitiManagementException extends IOException {
+public class GraffitiManagementException extends Exception {
 
   public GraffitiManagementException(final String message) {
     super(message);

diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/GraffitiManager.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/GraffitiManager.java
@@ -74,7 +74,8 @@ public void deleteGraffiti(final BLSPublicKey publicKey) throws GraffitiManageme
     }
   }
 
-  public Optional<Bytes32> getGraffiti(final BLSPublicKey publicKey) {
+  public Optional<Bytes32> getGraffiti(final BLSPublicKey publicKey)
+      throws GraffitiManagementException {
     final Path filePath = directory.resolve(resolveFileName(publicKey));
     if (!filePath.toFile().exists()) {
       return Optional.empty();
@@ -83,8 +84,9 @@ public Optional<Bytes32> getGraffiti(final BLSPublicKey publicKey) {
     try {
       return Optional.of(GraffitiParser.loadFromFile(filePath));
     } catch (GraffitiLoaderException | IllegalArgumentException e) {
-      LOG.error("Unable to read graffiti from storage.", e);
-      return Optional.empty();
+      LOG.error("Loading graffiti from graffiti storage failed.", e);
+      throw new GraffitiManagementException(
+          "Unable to retrieve stored graffiti for validator " + publicKey, e);
     }
   }
 

diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/UpdatableGraffitiProvider.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/UpdatableGraffitiProvider.java
@@ -14,21 +14,34 @@
 package tech.pegasys.teku.validator.api;
 
 import java.util.Optional;
-import java.util.function.Supplier;
 import org.apache.tuweni.bytes.Bytes32;
+import tech.pegasys.teku.infrastructure.async.ExceptionThrowingSupplier;
 
 public class UpdatableGraffitiProvider implements GraffitiProvider {
-  private final Supplier<Optional<Bytes32>> storageProvider;
+  private final ExceptionThrowingSupplier<Optional<Bytes32>> storageProvider;
   private final GraffitiProvider defaultProvider;
 
   public UpdatableGraffitiProvider(
-      final Supplier<Optional<Bytes32>> storageProvider, final GraffitiProvider defaultProvider) {
+      final ExceptionThrowingSupplier<Optional<Bytes32>> storageProvider,
+      final GraffitiProvider defaultProvider) {
     this.storageProvider = storageProvider;
     this.defaultProvider = defaultProvider;
   }
 
   @Override
   public Optional<Bytes32> get() {
+    return getFromStorage().or(defaultProvider::get);
+  }
+
+  private Optional<Bytes32> getFromStorage() {
+    try {
+      return storageProvider.get();
+    } catch (Throwable e) {
+      return Optional.empty();
+    }
+  }
+
+  public Optional<Bytes32> getGraffitiWithThrowable() throws Throwable {
     return storageProvider.get().or(defaultProvider::get);
   }
 }
diff --git a/validator/api/src/test/java/tech/pegasys/teku/validator/api/GraffitiManagerTest.java b/validator/api/src/test/java/tech/pegasys/teku/validator/api/GraffitiManagerTest.java
@@ -56,7 +56,7 @@ void shouldThrowExceptionWhenUnableToCreateManagementDirectory(@TempDir final Pa
 
   @Test
   void setGraffiti_shouldSetGraffitiWhenFileNotExist(@TempDir final Path tempDir)
-      throws IOException {
+      throws GraffitiManagementException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     manager = new GraffitiManager(dataDirLayout);
     assertThat(getGraffitiManagementDir().toFile().exists()).isTrue();
@@ -66,7 +66,8 @@ void setGraffiti_shouldSetGraffitiWhenFileNotExist(@TempDir final Path tempDir)
   }
 
   @Test
-  void setGraffiti_shouldSetGraffitiWhenFileExist(@TempDir final Path tempDir) throws IOException {
+  void setGraffiti_shouldSetGraffitiWhenFileExist(@TempDir final Path tempDir)
+      throws IOException, GraffitiManagementException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     manager = new GraffitiManager(dataDirLayout);
     assertThat(getGraffitiManagementDir().resolve(getFileName(publicKey)).toFile().createNewFile())
@@ -117,7 +118,7 @@ void deleteGraffiti_shouldSucceedWhenNoGraffitiToDelete(@TempDir final Path temp
 
   @Test
   void deleteGraffiti_shouldDeleteGraffitiWhenFileExist(@TempDir final Path tempDir)
-      throws IOException {
+      throws IOException, GraffitiManagementException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     manager = new GraffitiManager(dataDirLayout);
     assertThat(getGraffitiManagementDir().resolve(getFileName(publicKey)).toFile().createNewFile())
@@ -145,7 +146,7 @@ void deleteGraffiti_shouldReturnErrorMessageWhenUnableToDeleteFile(@TempDir fina
 
   @Test
   void shouldSetAndDeleteGraffitiWhenManagementPreexisting(@TempDir final Path tempDir)
-      throws IOException {
+      throws GraffitiManagementException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     final Path managementDir = getGraffitiManagementDir();
     assertThat(managementDir.toFile().mkdirs()).isTrue();
@@ -174,7 +175,8 @@ private void checkNoGraffitiFile(final BLSPublicKey publicKey) {
   }
 
   @Test
-  void getGraffiti_shouldGetGraffitiFromStorage(@TempDir final Path tempDir) throws IOException {
+  void getGraffiti_shouldGetGraffitiFromStorage(@TempDir final Path tempDir)
+      throws IOException, GraffitiManagementException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     manager = new GraffitiManager(dataDirLayout);
     final Path filePath = getGraffitiManagementDir().resolve(getFileName(publicKey));
@@ -185,41 +187,62 @@ void getGraffiti_shouldGetGraffitiFromStorage(@TempDir final Path tempDir) throw
   }
 
   @Test
-  void getGraffiti_shouldReturnEmptyWhenFileNotExist(@TempDir final Path tempDir) {
+  void getGraffiti_shouldReturnEmptyWhenFileNotExist(@TempDir final Path tempDir)
+      throws GraffitiManagementException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     manager = new GraffitiManager(dataDirLayout);
 
     assertThat(manager.getGraffiti(publicKey)).isEmpty();
   }
 
   @Test
-  void getGraffiti_shouldReturnEmptyWhenFileTooBig(@TempDir final Path tempDir) throws IOException {
+  void getGraffiti_shouldThrowExceptionWhenGraffitiOver32Bytes(@TempDir final Path tempDir)
+      throws IOException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     manager = new GraffitiManager(dataDirLayout);
 
     final String invalidGraffiti = "This graffiti is a bit too long!!";
     final Path filePath = getGraffitiManagementDir().resolve(getFileName(publicKey));
     Files.writeString(filePath, invalidGraffiti);
 
-    assertThat(manager.getGraffiti(publicKey)).isEmpty();
+    assertThatThrownBy(() -> manager.getGraffiti(publicKey))
+        .isInstanceOf(GraffitiManagementException.class)
+        .hasMessage("Unable to retrieve stored graffiti for validator " + publicKey);
+  }
+
+  @Test
+  void getGraffiti_shouldThrowExceptionWhenFileOver40Bytes(@TempDir final Path tempDir)
+      throws IOException {
+    dataDirLayout = new SimpleDataDirLayout(tempDir);
+    manager = new GraffitiManager(dataDirLayout);
+
+    final String invalidGraffiti = "This graffiti is a bit too long to get from file!!";
+    final Path filePath = getGraffitiManagementDir().resolve(getFileName(publicKey));
+    Files.writeString(filePath, invalidGraffiti);
+
+    assertThatThrownBy(() -> manager.getGraffiti(publicKey))
+        .isInstanceOf(GraffitiManagementException.class)
+        .hasMessage("Unable to retrieve stored graffiti for validator " + publicKey);
   }
 
   @Test
   @DisabledOnOs(OS.WINDOWS) // Can't set permissions on Windows
-  void getGraffiti_shouldReturnEmptyWhenNotReadableFile(@TempDir final Path tempDir)
+  void getGraffiti_shouldThrowExceptionWhenNotReadableFile(@TempDir final Path tempDir)
       throws IOException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     manager = new GraffitiManager(dataDirLayout);
     final Path filePath = getGraffitiManagementDir().resolve(getFileName(publicKey));
     Files.writeString(filePath, graffiti);
     assertThat(filePath.toFile().setReadable(false)).isTrue();
 
-    assertThat(manager.getGraffiti(publicKey)).isEmpty();
+    assertThatThrownBy(() -> manager.getGraffiti(publicKey))
+        .isInstanceOf(GraffitiManagementException.class)
+        .hasMessage("Unable to retrieve stored graffiti for validator " + publicKey);
   }
 
   @Test
   void getGraffiti_shouldReturnEmptyBytesWhenFileEmpty(@TempDir final Path tempDir)
-      throws IOException {
+      throws IOException, GraffitiManagementException {
     dataDirLayout = new SimpleDataDirLayout(tempDir);
     manager = new GraffitiManager(dataDirLayout);
     final Path filePath = getGraffitiManagementDir().resolve(getFileName(publicKey));

diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/Validator.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/Validator.java
@@ -13,7 +13,6 @@
 
 package tech.pegasys.teku.validator.client;
 
-import com.google.common.annotations.VisibleForTesting;
 import java.util.Optional;
 import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.bls.BLSPublicKey;
@@ -58,7 +57,6 @@ public boolean isReadOnly() {
     return readOnly;
   }
 
-  @VisibleForTesting
   public GraffitiProvider getGraffitiProvider() {
     return graffitiProvider;
   }

diff --git a/...dator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/...dator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java
@@ -23,14 +23,14 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Random;
-import java.util.function.Function;
 import java.util.function.Supplier;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes32;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
 import tech.pegasys.teku.bls.BLSPublicKey;
 import tech.pegasys.teku.infrastructure.async.AsyncRunner;
+import tech.pegasys.teku.infrastructure.async.ExceptionThrowingFunction;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
 import tech.pegasys.teku.infrastructure.events.EventChannels;
 import tech.pegasys.teku.infrastructure.exceptions.ExceptionUtil;
@@ -163,12 +163,17 @@ public static ValidatorClientService create(
             validatorApiConfig.isRestApiEnabled()
                 ? new GraffitiManager(services.getDataDirLayout())
                 : null);
+    final ExceptionThrowingFunction<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider =
+        (publicKey) -> {
+          if (graffitiManager.isPresent()) {
+            final GraffitiManager manager = graffitiManager.get();
+            return manager.getGraffiti(publicKey);
+          }
+          return Optional.empty();
+        };
+
     final ValidatorLoader validatorLoader =
-        createValidatorLoader(
-            services,
-            config,
-            asyncRunner,
-            (publicKey) -> graffitiManager.flatMap(manager -> manager.getGraffiti(publicKey)));
+        createValidatorLoader(services, config, asyncRunner, updatableGraffitiProvider);
     final ValidatorStatusProvider validatorStatusProvider =
         new OwnedValidatorStatusProvider(
             services.getMetricsSystem(),
@@ -409,7 +414,7 @@ private static ValidatorLoader createValidatorLoader(
       final ServiceConfig services,
       final ValidatorClientConfiguration config,
       final AsyncRunner asyncRunner,
-      final Function<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider) {
+      final ExceptionThrowingFunction<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider) {
     final Path slashingProtectionPath = getSlashingProtectionPath(services.getDataDirLayout());
     final SlashingProtector slashingProtector =
         config.getValidatorConfig().isLocalSlashingProtectionSynchronizedModeEnabled()

diff --git a/...src/main/java/tech/pegasys/teku/validator/client/loader/MultithreadedValidatorLoader.java b/...src/main/java/tech/pegasys/teku/validator/client/loader/MultithreadedValidatorLoader.java
@@ -24,9 +24,9 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.function.Function;
 import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.bls.BLSPublicKey;
+import tech.pegasys.teku.infrastructure.async.ExceptionThrowingFunction;
 import tech.pegasys.teku.service.serviceutils.layout.DataDirLayout;
 import tech.pegasys.teku.spec.signatures.DeletableSigner;
 import tech.pegasys.teku.validator.api.GraffitiProvider;
@@ -49,7 +49,7 @@ public static void loadValidators(
       final OwnedValidators ownedValidators,
       final Map<BLSPublicKey, ValidatorProvider> providers,
       final GraffitiProvider defaultGraffitiProvider,
-      final Function<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider,
+      final ExceptionThrowingFunction<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider,
       final Optional<DataDirLayout> maybeDataDirLayout) {
     final int totalValidatorCount = providers.size();
     STATUS_LOG.loadingValidators(totalValidatorCount);

diff --git a/...dator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ValidatorLoader.java b/...dator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ValidatorLoader.java
@@ -20,7 +20,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.function.Function;
 import java.util.function.Supplier;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -31,6 +30,7 @@
 import tech.pegasys.teku.bls.keystore.model.KeyStoreData;
 import tech.pegasys.teku.data.SlashingProtectionImporter;
 import tech.pegasys.teku.infrastructure.async.AsyncRunner;
+import tech.pegasys.teku.infrastructure.async.ExceptionThrowingFunction;
 import tech.pegasys.teku.service.serviceutils.layout.DataDirLayout;
 import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.signatures.DeletableSigner;
@@ -55,7 +55,8 @@ public class ValidatorLoader {
   private final Optional<ValidatorSource> mutableExternalValidatorSource;
   private final OwnedValidators ownedValidators = new OwnedValidators();
   private final GraffitiProvider defaultGraffitiProvider;
-  private final Function<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider;
+  private final ExceptionThrowingFunction<BLSPublicKey, Optional<Bytes32>>
+      updatableGraffitiProvider;
   private final Optional<DataDirLayout> maybeDataDirLayout;
   private final SlashingProtectionLogger slashingProtectionLogger;
 
@@ -64,7 +65,7 @@ private ValidatorLoader(
       final Optional<ValidatorSource> mutableLocalValidatorSource,
       final Optional<ValidatorSource> mutableExternalValidatorSource,
       final GraffitiProvider defaultGraffitiProvider,
-      final Function<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider,
+      final ExceptionThrowingFunction<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider,
       final Optional<DataDirLayout> maybeDataDirLayout,
       final SlashingProtectionLogger slashingProtectionLogger) {
     this.validatorSources = validatorSources;
@@ -241,7 +242,7 @@ public static ValidatorLoader create(
       final AsyncRunner asyncRunner,
       final MetricsSystem metricsSystem,
       final Optional<DataDirLayout> maybeMutableDir,
-      final Function<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider) {
+      final ExceptionThrowingFunction<BLSPublicKey, Optional<Bytes32>> updatableGraffitiProvider) {
     final ValidatorSourceFactory validatorSources =
         new ValidatorSourceFactory(
             spec,

diff --git a/...tor/client/src/main/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffiti.java b/...tor/client/src/main/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffiti.java
@@ -14,6 +14,7 @@
 package tech.pegasys.teku.validator.client.restapi.apis;
 
 import static tech.pegasys.teku.ethereum.json.types.SharedApiTypes.PUBKEY_API_TYPE;
+import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_INTERNAL_SERVER_ERROR;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_NOT_FOUND;
 import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_OK;
 import static tech.pegasys.teku.validator.client.restapi.ValidatorRestApi.TAG_GRAFFITI;
@@ -33,6 +34,7 @@
 import tech.pegasys.teku.infrastructure.restapi.endpoints.RestApiEndpoint;
 import tech.pegasys.teku.infrastructure.restapi.endpoints.RestApiRequest;
 import tech.pegasys.teku.validator.api.Bytes32Parser;
+import tech.pegasys.teku.validator.api.UpdatableGraffitiProvider;
 import tech.pegasys.teku.validator.client.KeyManager;
 import tech.pegasys.teku.validator.client.Validator;
 
@@ -88,7 +90,13 @@ public void handleRequest(final RestApiRequest request) throws JsonProcessingExc
       return;
     }
 
-    request.respondOk(new GraffitiResponse(publicKey, maybeValidator.get().getGraffiti()));
+    try {
+      final UpdatableGraffitiProvider provider =
+          (UpdatableGraffitiProvider) maybeValidator.get().getGraffitiProvider();
+      request.respondOk(new GraffitiResponse(publicKey, provider.getGraffitiWithThrowable()));
+    } catch (Throwable e) {
+      request.respondError(SC_INTERNAL_SERVER_ERROR, e.getMessage());
+    }
   }
 
   private static String processGraffitiString(final Bytes32 graffiti) {