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 index ce779f870d0..9d51207f2fd 100644 --- 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 index 40950195eb1..ac402bb9766 100644 --- 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 getGraffiti(final BLSPublicKey publicKey) { + public Optional 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 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 index 12281b97329..d70b08d03d1 100644 --- 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> storageProvider; + private final ExceptionThrowingSupplier> storageProvider; private final GraffitiProvider defaultProvider; public UpdatableGraffitiProvider( - final Supplier> storageProvider, final GraffitiProvider defaultProvider) { + final ExceptionThrowingSupplier> storageProvider, + final GraffitiProvider defaultProvider) { this.storageProvider = storageProvider; this.defaultProvider = defaultProvider; } @Override public Optional get() { + return getFromStorage().or(defaultProvider::get); + } + + private Optional getFromStorage() { + try { + return storageProvider.get(); + } catch (Throwable e) { + return Optional.empty(); + } + } + + public Optional 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 index bc823e26277..941d4222b8d 100644 --- 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,7 +187,8 @@ 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); @@ -193,7 +196,8 @@ void getGraffiti_shouldReturnEmptyWhenFileNotExist(@TempDir final Path tempDir) } @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); @@ -201,12 +205,29 @@ void getGraffiti_shouldReturnEmptyWhenFileTooBig(@TempDir final Path tempDir) th 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); @@ -214,12 +235,14 @@ void getGraffiti_shouldReturnEmptyWhenNotReadableFile(@TempDir final Path tempDi 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 index cd5c64ae8df..63252a306c9 100644 --- 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/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 f85529e4855..79d4a468df6 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 @@ -23,7 +23,6 @@ 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; @@ -31,6 +30,7 @@ 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> 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> updatableGraffitiProvider) { + final ExceptionThrowingFunction> updatableGraffitiProvider) { final Path slashingProtectionPath = getSlashingProtectionPath(services.getDataDirLayout()); final SlashingProtector slashingProtector = config.getValidatorConfig().isLocalSlashingProtectionSynchronizedModeEnabled() diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/MultithreadedValidatorLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/MultithreadedValidatorLoader.java index ee8e29e2491..4de6753704f 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/MultithreadedValidatorLoader.java +++ b/validator/client/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 providers, final GraffitiProvider defaultGraffitiProvider, - final Function> updatableGraffitiProvider, + final ExceptionThrowingFunction> updatableGraffitiProvider, final Optional maybeDataDirLayout) { final int totalValidatorCount = providers.size(); STATUS_LOG.loadingValidators(totalValidatorCount); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ValidatorLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ValidatorLoader.java index 90b6f95816d..4576244febb 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ValidatorLoader.java +++ b/validator/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 mutableExternalValidatorSource; private final OwnedValidators ownedValidators = new OwnedValidators(); private final GraffitiProvider defaultGraffitiProvider; - private final Function> updatableGraffitiProvider; + private final ExceptionThrowingFunction> + updatableGraffitiProvider; private final Optional maybeDataDirLayout; private final SlashingProtectionLogger slashingProtectionLogger; @@ -64,7 +65,7 @@ private ValidatorLoader( final Optional mutableLocalValidatorSource, final Optional mutableExternalValidatorSource, final GraffitiProvider defaultGraffitiProvider, - final Function> updatableGraffitiProvider, + final ExceptionThrowingFunction> updatableGraffitiProvider, final Optional maybeDataDirLayout, final SlashingProtectionLogger slashingProtectionLogger) { this.validatorSources = validatorSources; @@ -241,7 +242,7 @@ public static ValidatorLoader create( final AsyncRunner asyncRunner, final MetricsSystem metricsSystem, final Optional maybeMutableDir, - final Function> updatableGraffitiProvider) { + final ExceptionThrowingFunction> updatableGraffitiProvider) { final ValidatorSourceFactory validatorSources = new ValidatorSourceFactory( spec, diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffiti.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffiti.java index d77ef529d49..816354edd47 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffiti.java +++ b/validator/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) { diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffitiTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffitiTest.java index d71ddd74a6d..4e048c687bb 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffitiTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/restapi/apis/GetGraffitiTest.java @@ -17,6 +17,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_BAD_REQUEST; import static tech.pegasys.teku.infrastructure.http.HttpStatusCodes.SC_FORBIDDEN; @@ -39,7 +40,8 @@ import tech.pegasys.teku.spec.TestSpecFactory; import tech.pegasys.teku.spec.util.DataStructureUtil; import tech.pegasys.teku.validator.api.Bytes32Parser; -import tech.pegasys.teku.validator.api.GraffitiProvider; +import tech.pegasys.teku.validator.api.GraffitiManagementException; +import tech.pegasys.teku.validator.api.UpdatableGraffitiProvider; import tech.pegasys.teku.validator.client.OwnedKeyManager; import tech.pegasys.teku.validator.client.Validator; @@ -59,15 +61,36 @@ class GetGraffitiTest { .build(); @Test - void shouldGetGraffiti() throws JsonProcessingException { + void shouldGetGraffiti() throws Throwable { checkGraffiti(Optional.of(bytesGraffiti)); } @Test - void shouldGetEmptyGraffiti() throws JsonProcessingException { + void shouldGetEmptyGraffiti() throws Throwable { checkGraffiti(Optional.empty()); } + @Test + void shouldHandleGraffitiManagementException() throws JsonProcessingException { + final GraffitiManagementException exception = + new GraffitiManagementException("Unable to retrieve graffiti from storage"); + final UpdatableGraffitiProvider provider = + new UpdatableGraffitiProvider( + () -> { + throw exception; + }, + Optional::empty); + + final Validator validator = new Validator(publicKey, NO_OP_SIGNER, provider); + when(keyManager.getValidatorByPublicKey(eq(publicKey))).thenReturn(Optional.of(validator)); + + handler.handleRequest(request); + + assertThat(request.getResponseCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR); + assertThat(request.getResponseBody()) + .isEqualTo(new HttpErrorResponse(SC_INTERNAL_SERVER_ERROR, exception.getMessage())); + } + @Test void shouldHandleValidatorNotFound() throws IOException { when(keyManager.getValidatorByPublicKey(any())).thenReturn(Optional.empty()); @@ -111,8 +134,9 @@ void metadata_shouldHandle500() throws JsonProcessingException { verifyMetadataErrorResponse(handler, SC_INTERNAL_SERVER_ERROR); } - private void checkGraffiti(final Optional graffiti) throws JsonProcessingException { - final GraffitiProvider provider = () -> graffiti; + private void checkGraffiti(final Optional graffiti) throws Throwable { + final UpdatableGraffitiProvider provider = mock(UpdatableGraffitiProvider.class); + when(provider.getGraffitiWithThrowable()).thenReturn(graffiti); final Validator validator = new Validator(publicKey, NO_OP_SIGNER, provider); when(keyManager.getValidatorByPublicKey(eq(publicKey))).thenReturn(Optional.of(validator)); @@ -122,5 +146,6 @@ private void checkGraffiti(final Optional graffiti) throws JsonProcessi new GetGraffiti.GraffitiResponse(publicKey, graffiti); assertThat(request.getResponseCode()).isEqualTo(SC_OK); assertThat(request.getResponseBody()).isEqualTo(expectedResponse); + verify(provider).getGraffitiWithThrowable(); } }