From ae9cfcdf96e585cae65901dadf9d68ff1cd0ca64 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Tue, 4 Feb 2025 11:24:36 +1300 Subject: [PATCH] Add support to builder validator registration using ssz (#9065) --- CHANGELOG.md | 1 + .../rest/RestBuilderClientTest.java | 128 +++++++++++++++++- .../rest/RestBuilderClient.java | 70 ++++++++-- .../rest/RestBuilderClientOptions.java | 31 +++++ .../ExecutionLayerManagerImpl.java | 4 +- 5 files changed, 218 insertions(+), 16 deletions(-) create mode 100644 ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClientOptions.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 087aa5ad238..b929eae6928 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,5 +13,6 @@ - Applied spec change to alter `GOSSIP_MAX_SIZE` to `MAX_PAYLOAD_SIZE`. - `MAX_PAYLOAD_SIZE` is now used instead of `MAX_CHUNK_SIZE`. - Updated 3rd party products to latest versions. + - Add SSZ support to validator registration via Builder API. ### Bug Fixes \ No newline at end of file diff --git a/ethereum/executionclient/src/integration-test/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClientTest.java b/ethereum/executionclient/src/integration-test/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClientTest.java index 16d915ddf03..1e2f467a8a9 100644 --- a/ethereum/executionclient/src/integration-test/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClientTest.java +++ b/ethereum/executionclient/src/integration-test/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClientTest.java @@ -15,6 +15,10 @@ import static com.google.common.base.Preconditions.checkArgument; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; import static tech.pegasys.teku.spec.SpecMilestone.BELLATRIX; import static tech.pegasys.teku.spec.SpecMilestone.CAPELLA; import static tech.pegasys.teku.spec.SpecMilestone.DENEB; @@ -26,11 +30,13 @@ import com.google.common.io.Resources; import java.io.IOException; import java.io.UncheckedIOException; +import java.net.ConnectException; import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Locale; import java.util.Optional; import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; import java.util.function.Consumer; import java.util.regex.Pattern; import okhttp3.OkHttpClient; @@ -45,13 +51,16 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.ethereum.executionclient.BuilderApiMethod; import tech.pegasys.teku.ethereum.executionclient.schema.BuilderApiResponse; +import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.json.JsonUtil; import tech.pegasys.teku.infrastructure.json.exceptions.MissingRequiredFieldException; import tech.pegasys.teku.infrastructure.json.types.DeserializableTypeDefinition; import tech.pegasys.teku.infrastructure.ssz.SszData; import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.infrastructure.ssz.sos.SszDeserializeException; +import tech.pegasys.teku.infrastructure.time.StubTimeProvider; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.SpecMilestone; @@ -98,11 +107,13 @@ class RestBuilderClientTest { private final OkHttpClient okHttpClient = new OkHttpClient.Builder().build(); private final MockWebServer mockWebServer = new MockWebServer(); + private StubTimeProvider timeProvider; private Spec spec; private SpecMilestone milestone; private OkHttpRestClient okHttpRestClient; private SchemaDefinitionsBellatrix schemaDefinitions; + private RestBuilderClientOptions restBuilderClientOptions; private RestBuilderClient restBuilderClient; private String signedValidatorRegistrationsRequest; @@ -117,6 +128,7 @@ class RestBuilderClientTest { @BeforeEach void setUp(final SpecContext specContext) throws IOException { mockWebServer.start(); + timeProvider = StubTimeProvider.withTimeInMillis(UInt64.ONE); spec = specContext.getSpec(); final String endpoint = "http://localhost:" + mockWebServer.getPort(); @@ -152,7 +164,9 @@ void setUp(final SpecContext specContext) throws IOException { "data") .orElseThrow(); - restBuilderClient = new RestBuilderClient(okHttpRestClient, spec, true); + restBuilderClientOptions = RestBuilderClientOptions.DEFAULT; + restBuilderClient = + new RestBuilderClient(restBuilderClientOptions, okHttpRestClient, timeProvider, spec, true); } @AfterEach @@ -207,7 +221,7 @@ void registerValidators_success() { assertThat(response.payload()).isNull(); }); - verifyPostRequestJson("/eth/v1/builder/validators", signedValidatorRegistrationsRequest); + verifyPostRequestSsz("/eth/v1/builder/validators", signedValidatorRegistrations); } @TestTemplate @@ -232,6 +246,8 @@ void registerValidators_failures() { final String unknownValidatorError = "{\"code\":400,\"message\":\"unknown validator\"}"; + // Both ssz and json requests will fail + mockWebServer.enqueue(new MockResponse().setResponseCode(400).setBody(unknownValidatorError)); mockWebServer.enqueue(new MockResponse().setResponseCode(400).setBody(unknownValidatorError)); final SszList signedValidatorRegistrations = @@ -245,8 +261,10 @@ void registerValidators_failures() { assertThat(response.errorMessage()).isEqualTo(unknownValidatorError); }); + verifyPostRequestSsz("/eth/v1/builder/validators", signedValidatorRegistrations); verifyPostRequestJson("/eth/v1/builder/validators", signedValidatorRegistrationsRequest); + // Only json will fail (ssz backoff enabled) mockWebServer.enqueue( new MockResponse().setResponseCode(500).setBody(INTERNAL_SERVER_ERROR_MESSAGE)); @@ -261,10 +279,105 @@ void registerValidators_failures() { verifyPostRequestJson("/eth/v1/builder/validators", signedValidatorRegistrationsRequest); } + @TestTemplate + void registerValidators_retryWithJsonAfterSszFailure() { + final String errorMsg = "{\"code\":415,\"message\":\"unsupported media type\"}"; + mockWebServer.enqueue(new MockResponse().setResponseCode(415).setBody(errorMsg)); + mockWebServer.enqueue( + new MockResponse().setResponseCode(200).setBody(signedBuilderBidResponseJson)); + + final SszList signedValidatorRegistrations = + createSignedValidatorRegistrations(); + + assertThat(restBuilderClient.registerValidators(SLOT, signedValidatorRegistrations)) + .succeedsWithin(WAIT_FOR_CALL_COMPLETION) + .satisfies(response -> assertThat(response.isSuccess()).isTrue()); + + verifyPostRequestSsz("/eth/v1/builder/validators", signedValidatorRegistrations); + verifyPostRequestJson("/eth/v1/builder/validators", signedValidatorRegistrationsRequest); + + assertThat(mockWebServer.getRequestCount()).isEqualTo(2); + } + + @TestTemplate + void registerValidators_trySszAfterBackoffTime() { + final String errorMsg = "{\"code\":400,\"message\":\"bad request\"}"; + mockWebServer.enqueue(new MockResponse().setResponseCode(400).setBody(errorMsg)); + mockWebServer.enqueue( + new MockResponse().setResponseCode(200).setBody(signedBuilderBidResponseJson)); + mockWebServer.enqueue( + new MockResponse().setResponseCode(200).setBody(signedBuilderBidResponseJson)); + + final SszList signedValidatorRegistrations = + createSignedValidatorRegistrations(); + + // First call, will try SSZ and fallback into JSON + assertThat(restBuilderClient.registerValidators(SLOT, signedValidatorRegistrations)) + .succeedsWithin(WAIT_FOR_CALL_COMPLETION) + .satisfies(response -> assertThat(response.isSuccess()).isTrue()); + + verifyPostRequestSsz("/eth/v1/builder/validators", signedValidatorRegistrations); + verifyPostRequestJson("/eth/v1/builder/validators", signedValidatorRegistrationsRequest); + + // After backoff period, will try SSZ again + timeProvider.advanceTimeBy(Duration.ofDays(1)); + assertThat(restBuilderClient.registerValidators(SLOT, signedValidatorRegistrations)) + .succeedsWithin(WAIT_FOR_CALL_COMPLETION) + .satisfies(response -> assertThat(response.isSuccess()).isTrue()); + + verifyPostRequestSsz("/eth/v1/builder/validators", signedValidatorRegistrations); + + assertThat(mockWebServer.getRequestCount()).isEqualTo(3); + } + + @TestTemplate + void registerValidators_shouldNotFallbackWhenTimingOut() { + // Creates a RestBuilderClient that always timeout + restBuilderClient = + new RestBuilderClient( + forcedTimeoutRestBuilderClientOptions(), okHttpRestClient, timeProvider, spec, true); + + final SszList signedValidatorRegistrations = + createSignedValidatorRegistrations(); + + assertThat(restBuilderClient.registerValidators(SLOT, signedValidatorRegistrations)) + .failsWithin(WAIT_FOR_CALL_COMPLETION) + .withThrowableThat() + .withCauseInstanceOf(TimeoutException.class); + + verifyPostRequestSsz("/eth/v1/builder/validators", signedValidatorRegistrations); + // Check that we do not fallback into JSON (only 1 request) + assertThat(mockWebServer.getRequestCount()).isEqualTo(1); + } + + @TestTemplate + void registerValidators_shouldNotFallbackWhenFailingForNonHttpReasons() { + okHttpRestClient = spy(okHttpRestClient); + // Mock asyncPost with ssz to fail + doReturn(SafeFuture.failedFuture(new ConnectException())) + .when(okHttpRestClient) + .postAsync(eq(BuilderApiMethod.REGISTER_VALIDATOR.getPath()), any(), eq(true)); + restBuilderClient = + new RestBuilderClient(restBuilderClientOptions, okHttpRestClient, timeProvider, spec, true); + + final SszList signedValidatorRegistrations = + createSignedValidatorRegistrations(); + + assertThat(restBuilderClient.registerValidators(SLOT, signedValidatorRegistrations)) + .failsWithin(WAIT_FOR_CALL_COMPLETION) + .withThrowableThat() + .withCauseInstanceOf(ConnectException.class); + + // No requests are made (including no fallback to json) + assertThat(mockWebServer.getRequestCount()).isEqualTo(0); + } + @TestTemplate void getHeader_success_doesNotSetUserAgentHeader() { - restBuilderClient = new RestBuilderClient(okHttpRestClient, spec, false); + restBuilderClient = + new RestBuilderClient( + restBuilderClientOptions, okHttpRestClient, timeProvider, spec, false); mockWebServer.enqueue( new MockResponse().setResponseCode(200).setBody(signedBuilderBidResponseJson)); @@ -622,6 +735,10 @@ private void verifyPostRequestJson(final String apiPath, final String requestBod verifyRequest("POST", apiPath, Optional.of(requestBody), Optional.empty()); } + private void verifyPostRequestSsz(final String apiPath, final T requestBody) { + verifyRequest("POST", apiPath, Optional.empty(), Optional.of(requestBody)); + } + private void verifyRequest( final String method, final String apiPath, @@ -725,4 +842,9 @@ private static String changeResponseVersion(final String json, final SpecMilesto return json.replaceFirst( "(?<=version\":\\s?\")\\w+", newVersion.toString().toLowerCase(Locale.ROOT)); } + + private RestBuilderClientOptions forcedTimeoutRestBuilderClientOptions() { + return new RestBuilderClientOptions( + Duration.ofSeconds(0), Duration.ofSeconds(0), Duration.ofSeconds(0), Duration.ofSeconds(0)); + } } diff --git a/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClient.java b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClient.java index acd7d7e0921..2768534e6db 100644 --- a/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClient.java +++ b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClient.java @@ -14,17 +14,16 @@ package tech.pegasys.teku.ethereum.executionclient.rest; import static tech.pegasys.teku.infrastructure.http.RestApiConstants.HEADER_CONSENSUS_VERSION; -import static tech.pegasys.teku.spec.config.Constants.BUILDER_GET_PAYLOAD_TIMEOUT; -import static tech.pegasys.teku.spec.config.Constants.BUILDER_PROPOSAL_DELAY_TOLERANCE; -import static tech.pegasys.teku.spec.config.Constants.BUILDER_REGISTER_VALIDATOR_TIMEOUT; -import static tech.pegasys.teku.spec.config.Constants.BUILDER_STATUS_TIMEOUT; import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.ethereum.executionclient.BuilderApiMethod; @@ -35,6 +34,7 @@ import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.json.types.DeserializableTypeDefinition; import tech.pegasys.teku.infrastructure.ssz.SszList; +import tech.pegasys.teku.infrastructure.time.TimeProvider; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.infrastructure.version.VersionProvider; import tech.pegasys.teku.spec.Spec; @@ -50,6 +50,8 @@ public class RestBuilderClient implements BuilderClient { + private static final Logger LOG = LogManager.getLogger(); + private static final Map.Entry USER_AGENT_HEADER = Map.entry( "User-Agent", @@ -72,13 +74,26 @@ public class RestBuilderClient implements BuilderClient { private final Map> cachedBuilderApiSignedBuilderBidResponseType = new ConcurrentHashMap<>(); + public static final UInt64 REGISTER_VALIDATOR_SSZ_BACKOFF_TIME_MILLIS = + UInt64.valueOf(TimeUnit.DAYS.toMillis(1)); + + private final RestBuilderClientOptions options; private final RestClient restClient; + private final TimeProvider timeProvider; private final Spec spec; private final boolean setUserAgentHeader; + private UInt64 nextSszRegisterValidatorsTryMillis = UInt64.ZERO; + public RestBuilderClient( - final RestClient restClient, final Spec spec, final boolean setUserAgentHeader) { + final RestBuilderClientOptions options, + final RestClient restClient, + final TimeProvider timeProvider, + final Spec spec, + final boolean setUserAgentHeader) { + this.options = options; this.restClient = restClient; + this.timeProvider = timeProvider; this.spec = spec; this.setUserAgentHeader = setUserAgentHeader; } @@ -87,7 +102,7 @@ public RestBuilderClient( public SafeFuture> status() { return restClient .getAsync(BuilderApiMethod.GET_STATUS.getPath()) - .orTimeout(BUILDER_STATUS_TIMEOUT); + .orTimeout(options.builderStatusTimeout()); } @Override @@ -98,10 +113,41 @@ public SafeFuture> registerValidators( return SafeFuture.completedFuture(Response.fromNullPayload()); } - return restClient - .postAsync( - BuilderApiMethod.REGISTER_VALIDATOR.getPath(), signedValidatorRegistrations, false) - .orTimeout(BUILDER_REGISTER_VALIDATOR_TIMEOUT); + if (nextSszRegisterValidatorsTryMillis.isGreaterThan(timeProvider.getTimeInMillis())) { + return registerValidatorsUsingJson(signedValidatorRegistrations) + .orTimeout(options.builderRegisterValidatorTimeout()); + } + + return registerValidatorsUsingSsz(signedValidatorRegistrations) + .thenCompose( + response -> { + // Any failure will trigger a retry, not only Unsupported Media Type (415) + if (response.isFailure()) { + LOG.debug( + "Failed to register validator using SSZ. Will retry using JSON (error: {})", + response.errorMessage()); + + nextSszRegisterValidatorsTryMillis = + timeProvider.getTimeInMillis().plus(REGISTER_VALIDATOR_SSZ_BACKOFF_TIME_MILLIS); + + return registerValidatorsUsingJson(signedValidatorRegistrations); + } + + return SafeFuture.completedFuture(response); + }) + .orTimeout(options.builderRegisterValidatorTimeout()); + } + + private SafeFuture> registerValidatorsUsingJson( + final SszList signedValidatorRegistrations) { + return restClient.postAsync( + BuilderApiMethod.REGISTER_VALIDATOR.getPath(), signedValidatorRegistrations, false); + } + + private SafeFuture> registerValidatorsUsingSsz( + final SszList signedValidatorRegistrations) { + return restClient.postAsync( + BuilderApiMethod.REGISTER_VALIDATOR.getPath(), signedValidatorRegistrations, true); } @Override @@ -138,7 +184,7 @@ public SafeFuture>> getHeader( ? GET_HEADER_HTTP_HEADERS_WITH_USER_AGENT : GET_HEADER_HTTP_HEADERS_WITHOUT_USER_AGENT, responseTypeDefinition) - .orTimeout(BUILDER_PROPOSAL_DELAY_TOLERANCE) + .orTimeout(options.builderProposalDelayTolerance()) .thenApply( response -> response.unwrapVersioned( @@ -174,7 +220,7 @@ public SafeFuture> getPayload( response -> response.unwrapVersioned( this::extractBuilderPayload, milestone, BuilderApiResponse::version, false)) - .orTimeout(BUILDER_GET_PAYLOAD_TIMEOUT); + .orTimeout(options.builderGetPayloadTimeout()); } private diff --git a/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClientOptions.java b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClientOptions.java new file mode 100644 index 00000000000..b71f8cd0d29 --- /dev/null +++ b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/rest/RestBuilderClientOptions.java @@ -0,0 +1,31 @@ +/* + * Copyright Consensys Software Inc., 2025 + * + * 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.ethereum.executionclient.rest; + +import java.time.Duration; +import tech.pegasys.teku.spec.config.Constants; + +public record RestBuilderClientOptions( + Duration builderStatusTimeout, + Duration builderGetPayloadTimeout, + Duration builderRegisterValidatorTimeout, + Duration builderProposalDelayTolerance) { + + public static final RestBuilderClientOptions DEFAULT = + new RestBuilderClientOptions( + Constants.BUILDER_STATUS_TIMEOUT, + Constants.BUILDER_GET_PAYLOAD_TIMEOUT, + Constants.BUILDER_REGISTER_VALIDATOR_TIMEOUT, + Constants.BUILDER_PROPOSAL_DELAY_TOLERANCE); +} diff --git a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java b/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java index 1ef114e771f..6e71469287b 100644 --- a/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java +++ b/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionLayerManagerImpl.java @@ -34,6 +34,7 @@ import tech.pegasys.teku.ethereum.executionclient.metrics.MetricRecordingBuilderClient; import tech.pegasys.teku.ethereum.executionclient.metrics.MetricRecordingExecutionEngineClient; import tech.pegasys.teku.ethereum.executionclient.rest.RestBuilderClient; +import tech.pegasys.teku.ethereum.executionclient.rest.RestBuilderClientOptions; import tech.pegasys.teku.ethereum.executionclient.rest.RestClient; import tech.pegasys.teku.ethereum.executionclient.web3j.Web3JClient; import tech.pegasys.teku.ethereum.executionclient.web3j.Web3JExecutionEngineClient; @@ -130,7 +131,8 @@ public static BuilderClient createBuilderClient( final boolean setUserAgentHeader) { final RestBuilderClient restBuilderClient = - new RestBuilderClient(restClient, spec, setUserAgentHeader); + new RestBuilderClient( + RestBuilderClientOptions.DEFAULT, restClient, timeProvider, spec, setUserAgentHeader); final MetricRecordingBuilderClient metricRecordingBuilderClient = new MetricRecordingBuilderClient(restBuilderClient, timeProvider, metricsSystem); return new ThrottlingBuilderClient(