Skip to content

Commit 3fc0230

Browse files
courtneyehcourtneyeh
committed
Disable retry for VoluntaryExitCommand
1 parent 7837c7c commit 3fc0230

File tree

6 files changed

+115
-54
lines changed

6 files changed

+115
-54
lines changed

‎teku/src/main/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommand.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,7 @@ private void initialise() {
423423
new RejectingSlashingProtector(),
424424
slashingProtectionLogger,
425425
new PublicKeyLoader(
426-
externalSignerHttpClientFactory,
427-
validatorConfig.getValidatorExternalSignerUrl(),
428-
asyncRunner),
426+
externalSignerHttpClientFactory, validatorConfig.getValidatorExternalSignerUrl()),
429427
asyncRunner,
430428
metricsSystem,
431429
dataDirLayout,

‎validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ private static ValidatorLoader createValidatorLoader(
431431
new PublicKeyLoader(
432432
externalSignerHttpClientFactory,
433433
config.getValidatorConfig().getValidatorExternalSignerUrl(),
434-
asyncRunner),
434+
Optional.of(asyncRunner)),
435435
asyncRunner,
436436
services.getMetricsSystem(),
437437
config.getValidatorRestApiConfig().isRestApiEnabled()

‎validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReader.java

+20-10
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.net.URL;
2323
import java.time.Duration;
2424
import java.util.Arrays;
25+
import java.util.Optional;
2526
import java.util.stream.Stream;
2627
import org.apache.tuweni.bytes.Bytes;
2728
import tech.pegasys.teku.bls.BLSPublicKey;
@@ -35,29 +36,38 @@ public class ExternalUrlKeyReader {
3536

3637
private final URL url;
3738
private final ObjectMapper mapper;
38-
private final AsyncRunner asyncRunner;
39+
private final Optional<AsyncRunner> asyncRunner;
3940

41+
@VisibleForTesting
4042
public ExternalUrlKeyReader(
4143
final String urlString, final ObjectMapper mapper, final AsyncRunner asyncRunner) {
44+
this(urlString, mapper, Optional.of(asyncRunner));
45+
}
46+
47+
public ExternalUrlKeyReader(
48+
final String urlString, final ObjectMapper mapper, final Optional<AsyncRunner> asyncRunner) {
4249
this.url = getUrl(urlString);
4350
this.mapper = mapper;
4451
this.asyncRunner = asyncRunner;
4552
}
4653

4754
public Stream<BLSPublicKey> readKeys() {
48-
final String[] keys = getKeysWithRetry().join();
55+
final String[] keys =
56+
asyncRunner
57+
.map(this::getKeysWithRetry)
58+
.orElseGet(this::readUrl)
59+
.exceptionallyCompose(
60+
ex ->
61+
SafeFuture.failedFuture(
62+
new InvalidConfigurationException(
63+
"Failed to load public keys from URL: " + url, ex)))
64+
.join();
4965
return Arrays.stream(keys).map(key -> BLSPublicKey.fromSSZBytes(Bytes.fromHexString(key)));
5066
}
5167

5268
@VisibleForTesting
53-
SafeFuture<String[]> getKeysWithRetry() {
54-
return asyncRunner
55-
.runWithRetry(this::readUrl, FIXED_DELAY, MAX_RETRIES)
56-
.exceptionallyCompose(
57-
ex ->
58-
SafeFuture.failedFuture(
59-
new InvalidConfigurationException(
60-
"Failed to load public keys from URL: " + url, ex)));
69+
SafeFuture<String[]> getKeysWithRetry(final AsyncRunner asyncRunner) {
70+
return asyncRunner.runWithRetry(this::readUrl, FIXED_DELAY, MAX_RETRIES);
6171
}
6272

6373
private SafeFuture<String[]> readUrl() {

‎validator/client/src/main/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoader.java

+10-4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.Arrays;
2727
import java.util.Collections;
2828
import java.util.List;
29+
import java.util.Optional;
2930
import java.util.Set;
3031
import java.util.function.Function;
3132
import java.util.function.Supplier;
@@ -44,7 +45,7 @@ public class PublicKeyLoader {
4445
final ObjectMapper objectMapper;
4546
final Supplier<HttpClient> externalSignerHttpClientFactory;
4647
final URL externalSignerUrl;
47-
final AsyncRunner asyncRunner;
48+
final Optional<AsyncRunner> asyncRunner;
4849

4950
@VisibleForTesting
5051
PublicKeyLoader() {
@@ -54,21 +55,26 @@ public class PublicKeyLoader {
5455
throw new UnsupportedOperationException();
5556
},
5657
null,
57-
null);
58+
Optional.empty());
59+
}
60+
61+
public PublicKeyLoader(
62+
final Supplier<HttpClient> externalSignerHttpClientFactory, final URL externalSignerUrl) {
63+
this(externalSignerHttpClientFactory, externalSignerUrl, Optional.empty());
5864
}
5965

6066
public PublicKeyLoader(
6167
final Supplier<HttpClient> externalSignerHttpClientFactory,
6268
final URL externalSignerUrl,
63-
final AsyncRunner asyncRunner) {
69+
final Optional<AsyncRunner> asyncRunner) {
6470
this(new ObjectMapper(), externalSignerHttpClientFactory, externalSignerUrl, asyncRunner);
6571
}
6672

6773
public PublicKeyLoader(
6874
final ObjectMapper objectMapper,
6975
final Supplier<HttpClient> externalSignerHttpClientFactory,
7076
final URL externalSignerUrl,
71-
final AsyncRunner asyncRunner) {
77+
final Optional<AsyncRunner> asyncRunner) {
7278
this.objectMapper = objectMapper;
7379
this.externalSignerHttpClientFactory = externalSignerHttpClientFactory;
7480
this.externalSignerUrl = externalSignerUrl;

‎validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReaderTest.java

+80-30
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.net.URL;
3131
import java.net.UnknownHostException;
3232
import java.time.Duration;
33+
import java.util.Optional;
3334
import java.util.concurrent.CompletionException;
3435
import java.util.stream.Collectors;
3536
import org.junit.jupiter.api.Test;
@@ -58,44 +59,81 @@ class ExternalUrlKeyReaderTest {
5859
private final String[] expectedKeys =
5960
new String[] {publicKey1.toHexString(), publicKey2.toHexString()};
6061

62+
private ExternalUrlKeyReader reader;
63+
64+
@Test
65+
void malformedUrlString() {
66+
final String invalidUrl = "invalid:url";
67+
assertThatThrownBy(() -> new ExternalUrlKeyReader(invalidUrl, mapper, Optional.empty()))
68+
.isInstanceOf(InvalidConfigurationException.class)
69+
.hasMessageContaining("Failed to load public keys from invalid URL: " + invalidUrl)
70+
.hasRootCauseInstanceOf(MalformedURLException.class);
71+
verifyNoInteractions(mapper);
72+
}
73+
6174
@Test
62-
void readKeys_validUrlReturnsValidKeys() throws IOException {
63-
when(mapper.readValue(any(URL.class), eq(String[].class))).thenReturn(expectedKeys);
64-
final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
75+
void readKeys_retryDisabled_validUrlReturnsValidKeys() throws IOException {
76+
initialise(expectedKeys, false);
6577

6678
assertThat(reader.readKeys()).contains(publicKey1, publicKey2);
67-
verify(mapper).readValue(any(URL.class), eq(String[].class));
79+
verifyReadCall();
6880
}
6981

7082
@Test
71-
void readKeys_validUrlReturnsEmptyKeys() throws IOException {
72-
when(mapper.readValue(any(URL.class), eq(String[].class))).thenReturn(new String[] {});
73-
final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
83+
void readKeys_retryDisabled_validUrlReturnsEmptyKeys() throws IOException {
84+
initialise(new String[] {}, false);
7485

7586
assertThat(reader.readKeys()).isEmpty();
76-
verify(mapper).readValue(any(URL.class), eq(String[].class));
87+
verifyReadCall();
7788
}
7889

7990
@Test
80-
void readKeys_validUrlReturnsInvalidKeys() throws IOException {
81-
when(mapper.readValue(any(URL.class), eq(String[].class)))
82-
.thenReturn(new String[] {"invalid", "keys"});
83-
final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
91+
void readKeys_retryDisabled_validUrlReturnsInvalidKeys() throws IOException {
92+
initialise(new String[] {"invalid", "keys"}, false);
8493

8594
assertThatThrownBy(() -> reader.readKeys().collect(Collectors.toSet()))
8695
.isInstanceOf(IllegalArgumentException.class)
8796
.hasMessage("Invalid odd-length hex binary representation");
88-
verify(mapper).readValue(any(URL.class), eq(String[].class));
97+
verifyReadCall();
8998
}
9099

91100
@Test
92-
void readKeys_malformedUrlString() {
93-
final String invalidUrl = "invalid:url";
94-
assertThatThrownBy(() -> new ExternalUrlKeyReader(invalidUrl, mapper, asyncRunner))
95-
.isInstanceOf(InvalidConfigurationException.class)
96-
.hasMessageContaining("Failed to load public keys from invalid URL: " + invalidUrl)
97-
.hasRootCauseInstanceOf(MalformedURLException.class);
98-
verifyNoInteractions(mapper);
101+
void readKeys_retryDisabled_unreachableUrlFails() throws IOException {
102+
final UnknownHostException exception = new UnknownHostException("Unknown host");
103+
when(mapper.readValue(any(URL.class), eq(String[].class))).thenThrow(exception);
104+
reader = new ExternalUrlKeyReader(VALID_URL, mapper, Optional.empty());
105+
106+
assertThatThrownBy(reader::readKeys)
107+
.isInstanceOf(CompletionException.class)
108+
.hasCauseInstanceOf(InvalidConfigurationException.class)
109+
.hasRootCause(exception);
110+
verifyReadCall();
111+
}
112+
113+
@Test
114+
void readKeys_retryEnabled_validUrlReturnsValidKeys() throws IOException {
115+
initialise(expectedKeys, true);
116+
117+
assertThat(reader.readKeys()).contains(publicKey1, publicKey2);
118+
verifyReadCall();
119+
}
120+
121+
@Test
122+
void readKeys_retryEnabled_validUrlReturnsEmptyKeys() throws IOException {
123+
initialise(new String[] {}, true);
124+
125+
assertThat(reader.readKeys()).isEmpty();
126+
verifyReadCall();
127+
}
128+
129+
@Test
130+
void readKeys_retryEnabled_validUrlReturnsInvalidKeys() throws IOException {
131+
initialise(new String[] {"invalid", "keys"}, true);
132+
133+
assertThatThrownBy(() -> reader.readKeys().collect(Collectors.toSet()))
134+
.isInstanceOf(IllegalArgumentException.class)
135+
.hasMessage("Invalid odd-length hex binary representation");
136+
verifyReadCall();
99137
}
100138

101139
@Test
@@ -104,35 +142,47 @@ void readKeysWithRetry_unreachableUrlRetryUntilReachable() throws IOException {
104142
when(mapper.readValue(any(URL.class), eq(String[].class)))
105143
.thenThrow(exception, exception, exception)
106144
.thenReturn(expectedKeys);
107-
final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
145+
reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
108146

109-
final SafeFuture<String[]> keys = reader.getKeysWithRetry();
147+
final SafeFuture<String[]> keys = reader.getKeysWithRetry(asyncRunner);
110148
for (int i = 0; i < 3; i++) {
111149
assertThat(keys).isNotCompleted();
112150
timeProvider.advanceTimeBy(DELAY);
113151
asyncRunner.executeQueuedActions();
114152
}
115153
assertThat(keys).isCompletedWithValue(expectedKeys);
116-
verify(mapper, times(4)).readValue(any(URL.class), eq(String[].class));
154+
verifyReadCalls(4);
117155
}
118156

119157
@Test
120158
void readKeysWithRetry_unreachableUrlRetryUntilMaxRetries() throws IOException {
121159
final IOException exception = new ConnectException("Connection refused");
122160
when(mapper.readValue(any(URL.class), eq(String[].class))).thenThrow(exception);
123-
final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
161+
reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
124162

125-
final SafeFuture<String[]> keys = reader.getKeysWithRetry();
163+
final SafeFuture<String[]> keys = reader.getKeysWithRetry(asyncRunner);
126164
for (int i = 0; i < MAX_RETRIES; i++) {
127165
assertThat(keys).isNotCompleted();
128166
timeProvider.advanceTimeBy(DELAY);
129167
asyncRunner.executeQueuedActions();
130168
}
131169
assertThat(keys).isCompletedExceptionally();
132-
assertThatThrownBy(keys::join)
133-
.isInstanceOf(CompletionException.class)
134-
.hasCauseInstanceOf(InvalidConfigurationException.class)
135-
.hasRootCause(exception);
136-
verify(mapper, times(MAX_RETRIES + 1)).readValue(any(URL.class), eq(String[].class));
170+
assertThatThrownBy(keys::join).isInstanceOf(CompletionException.class).hasRootCause(exception);
171+
verifyReadCalls(MAX_RETRIES + 1);
172+
}
173+
174+
private void initialise(final String[] keys, final boolean retryEnabled) throws IOException {
175+
when(mapper.readValue(any(URL.class), eq(String[].class))).thenReturn(keys);
176+
reader =
177+
new ExternalUrlKeyReader(
178+
VALID_URL, mapper, Optional.ofNullable(retryEnabled ? asyncRunner : null));
179+
}
180+
181+
private void verifyReadCall() throws IOException {
182+
verifyReadCalls(1);
183+
}
184+
185+
private void verifyReadCalls(final int times) throws IOException {
186+
verify(mapper, times(times)).readValue(any(URL.class), eq(String[].class));
137187
}
138188
}

‎validator/client/src/test/java/tech/pegasys/teku/validator/client/loader/PublicKeyLoaderTest.java

+3-6
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,15 @@
2828
import java.net.http.HttpResponse;
2929
import java.net.http.HttpResponse.BodyHandler;
3030
import java.util.List;
31+
import java.util.Optional;
3132
import java.util.concurrent.CompletionException;
3233
import java.util.function.Supplier;
3334
import org.apache.tuweni.bytes.Bytes48;
3435
import org.junit.jupiter.api.BeforeEach;
3536
import org.junit.jupiter.api.Test;
3637
import org.mockito.ArgumentMatchers;
3738
import tech.pegasys.teku.bls.BLSPublicKey;
38-
import tech.pegasys.teku.infrastructure.async.StubAsyncRunner;
3939
import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException;
40-
import tech.pegasys.teku.infrastructure.time.StubTimeProvider;
4140
import tech.pegasys.teku.spec.TestSpecFactory;
4241
import tech.pegasys.teku.spec.util.DataStructureUtil;
4342
import tech.pegasys.teku.validator.client.loader.PublicKeyLoader.ExternalSignerException;
@@ -59,16 +58,14 @@ public class PublicKeyLoaderTest {
5958
private final ObjectMapper mapper = mock(ObjectMapper.class);
6059
private final HttpClient httpClient = mock(HttpClient.class);
6160

62-
private final StubTimeProvider timeProvider = StubTimeProvider.withTimeInMillis(0);
63-
private final StubAsyncRunner asyncRunner = new StubAsyncRunner(timeProvider);
64-
6561
@SuppressWarnings("unchecked")
6662
private final HttpResponse<String> externalSignerHttpResponse = mock(HttpResponse.class);
6763

6864
private final Supplier<HttpClient> externalSignerHttpClientSupplier = () -> httpClient;
6965

7066
private final PublicKeyLoader loader =
71-
new PublicKeyLoader(mapper, externalSignerHttpClientSupplier, externalSignerUrl, asyncRunner);
67+
new PublicKeyLoader(
68+
mapper, externalSignerHttpClientSupplier, externalSignerUrl, Optional.empty());
7269

7370
public PublicKeyLoaderTest() throws MalformedURLException {}
7471

0 commit comments

Comments
 (0)