Don't send exited validators registrations to builders

beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java

@@ -28,6 +28,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.BiFunction;
@@ -53,6 +54,7 @@
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockAndState;
 import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration;
+import tech.pegasys.teku.spec.datastructures.builder.ValidatorRegistration;
 import tech.pegasys.teku.spec.datastructures.genesis.GenesisData;
 import tech.pegasys.teku.spec.datastructures.operations.Attestation;
 import tech.pegasys.teku.spec.datastructures.operations.AttestationData;
@@ -246,10 +248,6 @@ public SafeFuture<Optional<ProposerDuties>> getProposerDuties(final UInt64 epoch
                 optionalState.map(state -> getProposerDutiesFromIndicesAndState(state, epoch)));
   }
 
-  public Optional<ProposerDuties> getProposerDuties(final BeaconState state, final UInt64 epoch) {
-    return Optional.of(getProposerDutiesFromIndicesAndState(state, epoch));
-  }
-
   @Override
   public SafeFuture<Optional<Map<BLSPublicKey, ValidatorStatus>>> getValidatorStatuses(
       Collection<BLSPublicKey> validatorIdentifiers) {
@@ -640,8 +638,26 @@ public SafeFuture<Void> prepareBeaconProposer(
   @Override
   public SafeFuture<Void> registerValidators(
       final SszList<SignedValidatorRegistration> validatorRegistrations) {
-    return proposersDataManager.updateValidatorRegistrations(
-        validatorRegistrations, combinedChainDataClient.getCurrentSlot());
+    final List<BLSPublicKey> validatorIdentifiers =
+        validatorRegistrations.stream()
+            .map(SignedValidatorRegistration::getMessage)
+            .map(ValidatorRegistration::getPublicKey)
+            .collect(toList());
+    return getValidatorStatuses(validatorIdentifiers)
+        .thenCompose(
+            maybeValidatorStatuses -> {
+              if (maybeValidatorStatuses.isEmpty()) {
+                final String errorMessage =
+                    "Couldn't retrieve validator statuses during registering. Most likely the BN is still syncing.";
+                return SafeFuture.failedFuture(new IllegalStateException(errorMessage));
+              }
+              final SszList<SignedValidatorRegistration> applicableValidatorRegistrations =
+                  getApplicableValidatorRegistrations(
+                      validatorRegistrations, maybeValidatorStatuses.get());
+
+              return proposersDataManager.updateValidatorRegistrations(
+                  applicableValidatorRegistrations, combinedChainDataClient.getCurrentSlot());
+            });
   }
 
   private Optional<SubmitDataError> fromInternalValidationResult(
@@ -785,6 +801,37 @@ private Optional<SyncCommitteeDuty> getSyncCommitteeDuty(
             state.getValidators().get(validatorIndex).getPublicKey(), validatorIndex, duties));
   }
 
+  private SszList<SignedValidatorRegistration> getApplicableValidatorRegistrations(
+      final SszList<SignedValidatorRegistration> validatorRegistrations,
+      final Map<BLSPublicKey, ValidatorStatus> validatorStatuses) {
+    final List<SignedValidatorRegistration> applicableValidatorRegistrations =
+        validatorRegistrations.stream()
+            .filter(
+                signedValidatorRegistration -> {
+                  final BLSPublicKey validatorIdentifier =
+                      signedValidatorRegistration.getMessage().getPublicKey();
+                  final ValidatorStatus validatorStatus =
+                      validatorStatuses.get(validatorIdentifier);
+                  final boolean hasExited = validatorHasExited(validatorStatus);
+                  if (hasExited) {
+                    LOG.debug(
+                        "Validator {} has exited. It will be skipped for registering.",
+                        validatorIdentifier.toAbbreviatedString());
+                  }
+                  return !hasExited;
+                })
+            .collect(toList());
+    if (validatorRegistrations.size() == applicableValidatorRegistrations.size()) {
+      return validatorRegistrations;
+    }
+    return validatorRegistrations.getSchema().createFromElements(applicableValidatorRegistrations);
+  }
+
+  private boolean validatorHasExited(final ValidatorStatus validatorStatus) {
+    return Objects.equals(validatorStatus, ValidatorStatus.exited_slashed)
+        || Objects.equals(validatorStatus, ValidatorStatus.exited_unslashed);
+  }
+
   private static <A, B, R> Optional<R> combine(
       Optional<A> a, Optional<B> b, BiFunction<A, B, R> fun) {
     if (a.isEmpty() || b.isEmpty()) {

beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java

@@ -14,6 +14,7 @@
 package tech.pegasys.teku.validator.coordinator;
 
 import static java.util.Collections.emptyList;
+import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
@@ -36,33 +37,46 @@
 
 import it.unimi.dsi.fastutil.ints.IntList;
 import it.unimi.dsi.fastutil.ints.IntSet;
+import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 import org.apache.tuweni.bytes.Bytes32;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 import tech.pegasys.teku.api.ChainDataProvider;
+import tech.pegasys.teku.api.migrated.StateValidatorData;
+import tech.pegasys.teku.api.response.v1.beacon.ValidatorStatus;
 import tech.pegasys.teku.beacon.sync.events.SyncState;
 import tech.pegasys.teku.beacon.sync.events.SyncStateProvider;
 import tech.pegasys.teku.bls.BLSPublicKey;
 import tech.pegasys.teku.bls.BLSSignature;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
+import tech.pegasys.teku.infrastructure.async.SafeFutureAssert;
+import tech.pegasys.teku.infrastructure.ssz.SszList;
 import tech.pegasys.teku.infrastructure.ssz.SszMutableList;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 import tech.pegasys.teku.networking.eth2.gossip.BlockGossipChannel;
 import tech.pegasys.teku.networking.eth2.gossip.subnets.AttestationTopicSubscriber;
 import tech.pegasys.teku.networking.eth2.gossip.subnets.SyncCommitteeSubscriptionManager;
 import tech.pegasys.teku.spec.Spec;
+import tech.pegasys.teku.spec.SpecMilestone;
 import tech.pegasys.teku.spec.TestSpecFactory;
 import tech.pegasys.teku.spec.config.SpecConfig;
 import tech.pegasys.teku.spec.config.SpecConfigAltair;
 import tech.pegasys.teku.spec.datastructures.attestation.ValidateableAttestation;
 import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock;
 import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockAndState;
+import tech.pegasys.teku.spec.datastructures.builder.SignedValidatorRegistration;
+import tech.pegasys.teku.spec.datastructures.builder.ValidatorRegistration;
+import tech.pegasys.teku.spec.datastructures.metadata.ObjectAndMetaData;
 import tech.pegasys.teku.spec.datastructures.operations.Attestation;
 import tech.pegasys.teku.spec.datastructures.operations.AttestationData;
 import tech.pegasys.teku.spec.datastructures.operations.SignedAggregateAndProof;
@@ -159,6 +173,8 @@ public void setUp() {
     doAnswer(invocation -> SafeFuture.completedFuture(invocation.getArgument(0)))
         .when(blockFactory)
         .unblindSignedBeaconBlockIfBlinded(any());
+    when(proposersDataManager.updateValidatorRegistrations(any(), any()))
+        .thenReturn(SafeFuture.COMPLETE);
   }
 
   @Test
@@ -920,6 +936,75 @@ void sendSignedContributionAndProofs_shouldReportErrors() {
         .hasMessageContainingAll("Bad", "Worse");
   }
 
+  @Test
+  void registerValidators_shouldUpdateRegistrations() {
+    final SszList<SignedValidatorRegistration> validatorRegistrations =
+        dataStructureUtil.randomSignedValidatorRegistrations(4);
+
+    setupValidatorsState(validatorRegistrations);
+
+    when(chainDataClient.getCurrentSlot()).thenReturn(ONE);
+
+    final SafeFuture<Void> result = validatorApiHandler.registerValidators(validatorRegistrations);
+
+    assertThat(result).isCompleted();
+
+    verify(proposersDataManager).updateValidatorRegistrations(validatorRegistrations, ONE);
+  }
+
+  @Test
+  void registerValidators_shouldIgnoreExitedValidators() {
+    final SszList<SignedValidatorRegistration> validatorRegistrations =
+        dataStructureUtil.randomSignedValidatorRegistrations(4);
+
+    final BLSPublicKey exitedUnslashedValidatorKey =
+        validatorRegistrations.get(2).getMessage().getPublicKey();
+    final BLSPublicKey exitedSlashedValidatorKey =
+        validatorRegistrations.get(3).getMessage().getPublicKey();
+
+    setupValidatorsState(
+        validatorRegistrations,
+        Map.of(
+            exitedUnslashedValidatorKey,
+            ValidatorStatus.exited_unslashed,
+            exitedSlashedValidatorKey,
+            ValidatorStatus.exited_slashed));
+
+    when(chainDataClient.getCurrentSlot()).thenReturn(ONE);
+
+    final SafeFuture<Void> result = validatorApiHandler.registerValidators(validatorRegistrations);
+
+    assertThat(result).isCompleted();
+
+    @SuppressWarnings("unchecked")
+    final ArgumentCaptor<SszList<SignedValidatorRegistration>> argumentCaptor =
+        ArgumentCaptor.forClass(SszList.class);
+
+    verify(proposersDataManager).updateValidatorRegistrations(argumentCaptor.capture(), eq(ONE));
+
+    final SszList<SignedValidatorRegistration> capturedRegistrations = argumentCaptor.getValue();
+
+    assertThat(capturedRegistrations)
+        .hasSize(2)
+        .map(signedRegistration -> signedRegistration.getMessage().getPublicKey())
+        .doesNotContain(exitedUnslashedValidatorKey, exitedSlashedValidatorKey);
+  }
+
+  @Test
+  void registerValidators_shouldReportErrorIfCannotRetrieveValidatorStatuses() {
+    final SszList<SignedValidatorRegistration> validatorRegistrations =
+        dataStructureUtil.randomSignedValidatorRegistrations(4);
+
+    when(chainDataProvider.getStateValidators(eq("head"), any(), any()))
+        .thenReturn(SafeFuture.completedFuture(Optional.empty()));
+
+    final SafeFuture<Void> result = validatorApiHandler.registerValidators(validatorRegistrations);
+
+    SafeFutureAssert.assertThatSafeFuture(result)
+        .isCompletedExceptionallyWithMessage(
+            "Couldn't retrieve validator statuses during registering. Most likely the BN is still syncing.");
+  }
+
   private <T> Optional<T> assertCompletedSuccessfully(final SafeFuture<Optional<T>> result) {
     assertThat(result).isCompleted();
     return result.join();
@@ -948,4 +1033,41 @@ private BeaconState createStateWithActiveValidators(final UInt64 slot) {
               }
             });
   }
+
+  private void setupValidatorsState(
+      final SszList<SignedValidatorRegistration> validatorRegistrations) {
+    setupValidatorsState(validatorRegistrations, Collections.emptyMap());
+  }
+
+  private void setupValidatorsState(
+      final SszList<SignedValidatorRegistration> validatorRegistrations,
+      final Map<BLSPublicKey, ValidatorStatus> statusOverrides) {
+    final List<StateValidatorData> data =
+        IntStream.range(0, 4)
+            .mapToObj(
+                index -> {
+                  final SignedValidatorRegistration validatorRegistration =
+                      validatorRegistrations.get(index);
+                  final BLSPublicKey publicKey = validatorRegistration.getMessage().getPublicKey();
+                  return new StateValidatorData(
+                      UInt64.valueOf(index),
+                      dataStructureUtil.randomUInt64(),
+                      statusOverrides.getOrDefault(publicKey, ValidatorStatus.active_ongoing),
+                      dataStructureUtil.randomValidator(publicKey));
+                })
+            .collect(Collectors.toList());
+
+    final ObjectAndMetaData<List<StateValidatorData>> stateValidators =
+        new ObjectAndMetaData<>(data, SpecMilestone.BELLATRIX, false, true);
+
+    final List<String> validators =
+        validatorRegistrations.stream()
+            .map(SignedValidatorRegistration::getMessage)
+            .map(ValidatorRegistration::getPublicKey)
+            .map(BLSPublicKey::toString)
+            .collect(toList());
+
+    when(chainDataProvider.getStateValidators("head", validators, new HashSet<>()))
+        .thenReturn(SafeFuture.completedFuture(Optional.of(stateValidators)));
+  }
 }

data/beaconrestapi/src/integration-test/resources/tech/pegasys/teku/beaconrestapi/beacon/paths/_eth_v1_validator_register_validator.json

@@ -2,7 +2,7 @@
   "post" : {
     "tags" : [ "Validator", "Validator Required Api" ],
     "summary" : "Register validators with builder",
-    "description" : "Prepares the beacon node for potential proposers by supplying information required when proposing blocks for the given validators. The information supplied for each validator index is considered persistent until overwritten by new information for the given validator index, or until the beacon node restarts.\n\nNote that because the information is not persistent across beacon node restarts it is recommended that either the beacon node is monitored for restarts or this information is refreshed by resending this request periodically (for example, each epoch).\n\nAlso note that requests containing currently inactive or unknown validator indices will be accepted, as they may become active at a later epoch.",
+    "description" : "Prepares the beacon node for potential proposers by supplying information required when proposing blocks for the given validators. The information supplied for each validator index is considered persistent until overwritten by new information for the given validator index, or until the beacon node restarts.\n\nNote that because the information is not persistent across beacon node restarts it is recommended that either the beacon node is monitored for restarts or this information is refreshed by resending this request periodically (for example, each epoch).\n\nAlso note that registrations for exited validators will be filtered out and not sent to the builder network. However, registrations for inactive or unknown validators will be sent, as they may become active at a later epoch.",
     "operationId" : "postEthV1ValidatorRegister_validator",
     "requestBody" : {
       "content" : {

data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/validator/PostRegisterValidator.java

@@ -82,7 +82,7 @@ public PostRegisterValidator(final ValidatorDataProvider validatorDataProvider)
       description =
           "Prepares the beacon node for potential proposers by supplying information required when proposing blocks for the given validators. The information supplied for each validator index is considered persistent until overwritten by new information for the given validator index, or until the beacon node restarts.\n\n"
               + "Note that because the information is not persistent across beacon node restarts it is recommended that either the beacon node is monitored for restarts or this information is refreshed by resending this request periodically (for example, each epoch).\n\n"
-              + "Also note that requests containing currently inactive or unknown validator indices will be accepted, as they may become active at a later epoch.",
+              + "Also note that registrations for exited validators will be filtered out and not sent to the builder network. However, registrations for inactive or unknown validators will be sent, as they may become active at a later epoch.",
       responses = {
         @OpenApiResponse(
             status = RES_OK,

ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java

@@ -1151,8 +1151,12 @@ public List<DepositWithIndex> newDeposits(int numDeposits) {
   }
 
   public Validator randomValidator() {
+    return randomValidator(randomPublicKey());
+  }
+
+  public Validator randomValidator(final BLSPublicKey publicKey) {
     return new Validator(
-        randomPublicKeyBytes(),
+        publicKey,
         randomBytes32(),
         getMaxEffectiveBalance(),
         false,