From 64942657a11a51796292bdeb6cd6564abe75c095 Mon Sep 17 00:00:00 2001 From: courtneyeh Date: Mon, 6 May 2024 08:00:44 +1000 Subject: [PATCH] Retry loading of external validators if failed --- .../infrastructure/logging/StatusLogger.java | 4 + .../cli/subcommand/VoluntaryExitCommand.java | 4 +- .../client/ValidatorClientService.java | 3 +- .../client/loader/ExternalUrlKeyReader.java | 84 +++++++++++ .../client/loader/PublicKeyLoader.java | 31 ++-- .../loader/ExternalUrlKeyReaderTest.java | 139 ++++++++++++++++++ .../client/loader/PublicKeyLoaderTest.java | 25 ++-- 7 files changed, 266 insertions(+), 24 deletions(-) create mode 100644 validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReader.java create mode 100644 validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReaderTest.java diff --git a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java index e48c3bee166..424947b736c 100644 --- a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java +++ b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java @@ -260,6 +260,10 @@ public void reconstructedHistoricalBlocks( totalToRecord); } + public void failedToLoadPublicKeysFromUrl(final String url) { + log.error("Failed to load public keys from URL: {}", url); + } + public void failedToStartValidatorClient(final String message) { log.fatal( "An error was encountered during validator client service start up. Error: {}", message); diff --git a/teku/src/main/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommand.java b/teku/src/main/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommand.java index d392e391135..5ebfec1429b 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommand.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommand.java @@ -423,7 +423,9 @@ private void initialise() { new RejectingSlashingProtector(), slashingProtectionLogger, new PublicKeyLoader( - externalSignerHttpClientFactory, validatorConfig.getValidatorExternalSignerUrl()), + externalSignerHttpClientFactory, + validatorConfig.getValidatorExternalSignerUrl(), + asyncRunner), asyncRunner, metricsSystem, dataDirLayout, 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 6da9ed43902..eaf6ef67fc7 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 @@ -430,7 +430,8 @@ private static ValidatorLoader createValidatorLoader( slashingProtectionLogger, new PublicKeyLoader( externalSignerHttpClientFactory, - config.getValidatorConfig().getValidatorExternalSignerUrl()), + config.getValidatorConfig().getValidatorExternalSignerUrl(), + asyncRunner), asyncRunner, services.getMetricsSystem(), config.getValidatorRestApiConfig().isRestApiEnabled() diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReader.java new file mode 100644 index 00000000000..ecaec67994b --- /dev/null +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReader.java @@ -0,0 +1,84 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * 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.teku.validator.client.loader; + +import static tech.pegasys.teku.infrastructure.logging.StatusLogger.STATUS_LOG; + +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.time.Duration; +import java.util.Arrays; +import java.util.stream.Stream; +import org.apache.tuweni.bytes.Bytes; +import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.infrastructure.async.AsyncRunner; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; + +public class ExternalUrlKeyReader { + private static final Duration FIXED_DELAY = Duration.ofSeconds(5); + + private final String url; + private final ObjectMapper mapper; + private final AsyncRunner asyncRunner; + + public ExternalUrlKeyReader( + final String url, final ObjectMapper mapper, final AsyncRunner asyncRunner) { + this.url = url; + this.mapper = mapper; + this.asyncRunner = asyncRunner; + } + + public Stream readKeys() { + final String[] keys = + readUrl() + .exceptionallyCompose( + ex -> { + if (ex instanceof MalformedURLException) { + return SafeFuture.failedFuture( + new InvalidConfigurationException( + "Failed to load public keys from invalid URL: " + url, ex)); + } + return retry(); + }) + .join(); + return Arrays.stream(keys).map(key -> BLSPublicKey.fromSSZBytes(Bytes.fromHexString(key))); + } + + public SafeFuture readUrl() { + try { + return SafeFuture.completedFuture(mapper.readValue(new URL(url), String[].class)); + } catch (IOException e) { + return SafeFuture.failedFuture(e); + } + } + + public SafeFuture retry() { + return asyncRunner + .runWithRetry( + () -> { + STATUS_LOG.failedToLoadPublicKeysFromUrl(url); + return readUrl(); + }, + FIXED_DELAY, + 59) + .exceptionallyCompose( + ex -> + SafeFuture.failedFuture( + new InvalidConfigurationException( + "Failed to load public keys from URL: " + url, ex))); + } +} diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoader.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoader.java index c9fe2f193fc..7ed66b8bc81 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoader.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoader.java @@ -27,11 +27,13 @@ import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.tuweni.bytes.Bytes; import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.infrastructure.async.AsyncRunner; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; import tech.pegasys.teku.infrastructure.http.HttpStatusCodes; @@ -42,6 +44,7 @@ public class PublicKeyLoader { final ObjectMapper objectMapper; final Supplier externalSignerHttpClientFactory; final URL externalSignerUrl; + final AsyncRunner asyncRunner; @VisibleForTesting PublicKeyLoader() { @@ -50,24 +53,35 @@ public class PublicKeyLoader { () -> { throw new UnsupportedOperationException(); }, + null, null); } public PublicKeyLoader( - final Supplier externalSignerHttpClientFactory, final URL externalSignerUrl) { - this(new ObjectMapper(), externalSignerHttpClientFactory, externalSignerUrl); + final Supplier externalSignerHttpClientFactory, + final URL externalSignerUrl, + final AsyncRunner asyncRunner) { + this(new ObjectMapper(), externalSignerHttpClientFactory, externalSignerUrl, asyncRunner); } public PublicKeyLoader( final ObjectMapper objectMapper, final Supplier externalSignerHttpClientFactory, - final URL externalSignerUrl) { + final URL externalSignerUrl, + final AsyncRunner asyncRunner) { this.objectMapper = objectMapper; this.externalSignerHttpClientFactory = externalSignerHttpClientFactory; this.externalSignerUrl = externalSignerUrl; + this.asyncRunner = asyncRunner; } public List getPublicKeys(final List sources) { + return getPublicKeys( + sources, (url) -> new ExternalUrlKeyReader(url, objectMapper, asyncRunner)); + } + + public List getPublicKeys( + final List sources, final Function urlReader) { if (sources == null || sources.isEmpty()) { return Collections.emptyList(); } @@ -81,7 +95,7 @@ public List getPublicKeys(final List sources) { return readKeysFromExternalSigner(); } if (key.contains(":")) { - return readKeysFromUrl(key); + return urlReader.apply(key).readKeys(); } return Stream.of(BLSPublicKey.fromSSZBytes(Bytes.fromHexString(key))); @@ -95,15 +109,6 @@ public List getPublicKeys(final List sources) { } } - private Stream readKeysFromUrl(final String url) { - try { - final String[] keys = objectMapper.readValue(new URL(url), String[].class); - return Arrays.stream(keys).map(key -> BLSPublicKey.fromSSZBytes(Bytes.fromHexString(key))); - } catch (IOException ex) { - throw new InvalidConfigurationException("Failed to load public keys from URL " + url, ex); - } - } - private Stream readKeysFromExternalSigner() { try { final URI uri = externalSignerUrl.toURI().resolve(EXTERNAL_SIGNER_PUBKEYS_ENDPOINT); diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReaderTest.java new file mode 100644 index 00000000000..36204032716 --- /dev/null +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReaderTest.java @@ -0,0 +1,139 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * 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.teku.validator.client.loader; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import java.net.ConnectException; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.UnknownHostException; +import java.time.Duration; +import java.util.concurrent.CompletionException; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Test; +import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.async.StubAsyncRunner; +import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; +import tech.pegasys.teku.infrastructure.time.StubTimeProvider; +import tech.pegasys.teku.spec.TestSpecFactory; +import tech.pegasys.teku.spec.util.DataStructureUtil; + +class ExternalUrlKeyReaderTest { + private static final Duration DELAY = Duration.ofSeconds(5); + private static final String VALID_URL = "http://test:0000/api/v1/eth2/publicKeys"; + private final ObjectMapper mapper = mock(ObjectMapper.class); + + private final StubTimeProvider timeProvider = StubTimeProvider.withTimeInMillis(0); + private final StubAsyncRunner asyncRunner = new StubAsyncRunner(timeProvider); + + private final DataStructureUtil dataStructureUtil = + new DataStructureUtil(TestSpecFactory.createDefault()); + + final BLSPublicKey publicKey1 = dataStructureUtil.randomPublicKey(); + final BLSPublicKey publicKey2 = dataStructureUtil.randomPublicKey(); + final String[] expectedKeys = new String[] {publicKey1.toHexString(), publicKey2.toHexString()}; + + @Test + void readKeys_validUrlReturnsValidKeys() throws IOException { + when(mapper.readValue(any(URL.class), eq(String[].class))).thenReturn(expectedKeys); + final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner); + + assertThat(reader.readKeys()).contains(publicKey1, publicKey2); + verify(mapper).readValue(any(URL.class), eq(String[].class)); + } + + @Test + void readKeys_validUrlReturnsEmptyKeys() throws IOException { + when(mapper.readValue(any(URL.class), eq(String[].class))).thenReturn(new String[] {}); + final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner); + + assertThat(reader.readKeys()).isEmpty(); + verify(mapper).readValue(any(URL.class), eq(String[].class)); + } + + @Test + void readKeys_validUrlReturnsInvalidKeys() throws IOException { + when(mapper.readValue(any(URL.class), eq(String[].class))) + .thenReturn(new String[] {"invalid", "keys"}); + final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner); + + assertThatThrownBy(() -> reader.readKeys().collect(Collectors.toSet())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid odd-length hex binary representation"); + verify(mapper).readValue(any(URL.class), eq(String[].class)); + } + + @Test + void readKeys_malformedUrlString() { + final String invalidUrl = "invalid:url"; + final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(invalidUrl, mapper, asyncRunner); + + assertThatThrownBy(reader::readKeys) + .isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(InvalidConfigurationException.class) + .hasMessageContaining("Failed to load public keys from invalid URL: " + invalidUrl) + .hasRootCauseInstanceOf(MalformedURLException.class); + verifyNoInteractions(mapper); + } + + @Test + void readKeysWithRetry_unreachableUrlRetryUntilReachable() throws IOException { + final UnknownHostException exception = new UnknownHostException("Unknown host"); + when(mapper.readValue(any(URL.class), eq(String[].class))) + .thenThrow(exception, exception, exception) + .thenReturn(expectedKeys); + final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner); + + final SafeFuture keys = reader.retry(); + for (int i = 0; i < 3; i++) { + assertThat(keys).isNotCompleted(); + timeProvider.advanceTimeBy(DELAY); + asyncRunner.executeQueuedActions(); + } + assertThat(keys).isCompletedWithValue(expectedKeys); + verify(mapper, times(4)).readValue(any(URL.class), eq(String[].class)); + } + + @Test + void readKeysWithRetry_unreachableUrlRetryUntilMaxRetries() throws IOException { + final IOException exception = new ConnectException("Connection refused"); + when(mapper.readValue(any(URL.class), eq(String[].class))).thenThrow(exception); + final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner); + + final SafeFuture keys = reader.retry(); + for (int i = 0; i < 59; i++) { + assertThat(keys).isNotCompleted(); + timeProvider.advanceTimeBy(DELAY); + asyncRunner.executeQueuedActions(); + } + assertThat(keys).isCompletedExceptionally(); + assertThatThrownBy(keys::join) + .isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(InvalidConfigurationException.class) + .hasRootCause(exception); + verify(mapper, times(60)).readValue(any(URL.class), eq(String[].class)); + } +} diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoaderTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoaderTest.java index fd750f94c6e..5faa172db66 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoaderTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoaderTest.java @@ -24,18 +24,20 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; -import java.net.UnknownHostException; import java.net.http.HttpClient; import java.net.http.HttpResponse; import java.net.http.HttpResponse.BodyHandler; import java.util.List; +import java.util.concurrent.CompletionException; import java.util.function.Supplier; import org.apache.tuweni.bytes.Bytes48; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentMatchers; import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.infrastructure.async.StubAsyncRunner; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; +import tech.pegasys.teku.infrastructure.time.StubTimeProvider; import tech.pegasys.teku.spec.TestSpecFactory; import tech.pegasys.teku.spec.util.DataStructureUtil; import tech.pegasys.teku.validator.client.loader.PublicKeyLoader.ExternalSignerException; @@ -57,13 +59,16 @@ public class PublicKeyLoaderTest { private final ObjectMapper mapper = mock(ObjectMapper.class); private final HttpClient httpClient = mock(HttpClient.class); + private final StubTimeProvider timeProvider = StubTimeProvider.withTimeInMillis(0); + private final StubAsyncRunner asyncRunner = new StubAsyncRunner(timeProvider); + @SuppressWarnings("unchecked") private final HttpResponse externalSignerHttpResponse = mock(HttpResponse.class); private final Supplier externalSignerHttpClientSupplier = () -> httpClient; private final PublicKeyLoader loader = - new PublicKeyLoader(mapper, externalSignerHttpClientSupplier, externalSignerUrl); + new PublicKeyLoader(mapper, externalSignerHttpClientSupplier, externalSignerUrl, asyncRunner); public PublicKeyLoaderTest() throws MalformedURLException {} @@ -121,16 +126,18 @@ void shouldHandleEmptyResponseFromUrl() throws IOException { } @Test - void shouldThrowInvalidConfigurationExceptionWhenUrlFailsToLoad() throws Exception { - final UnknownHostException exception = new UnknownHostException("Unknown host"); - when(mapper.readValue(new URL(urlSource), String[].class)).thenThrow(exception); - assertThatThrownBy(() -> loader.getPublicKeys(List.of(urlSource))) - .isInstanceOf(InvalidConfigurationException.class) - .hasRootCause(exception); + void shouldThrowExceptionWhenUrlFailsToLoad() { + final InvalidConfigurationException cause = new InvalidConfigurationException("Unknown host"); + final CompletionException exception = new CompletionException(cause); + final ExternalUrlKeyReader reader = mock(ExternalUrlKeyReader.class); + when(reader.readKeys()).thenThrow(exception); + + assertThatThrownBy(() -> loader.getPublicKeys(List.of(urlSource), (key) -> reader)) + .isEqualTo(exception); } @Test - void shouldThrowInvalidConfigurationExceptionWhenExternalSignerReturnsNon200() throws Exception { + void shouldThrowInvalidConfigurationExceptionWhenExternalSignerReturnsNon200() { when(externalSignerHttpResponse.statusCode()).thenReturn(400); assertThatThrownBy(() -> loader.getPublicKeys(List.of(EXTERNAL_SIGNER_SOURCE_ID))) .isInstanceOf(InvalidConfigurationException.class)