Remove the requirement to cache seen attestations (#4332)

- AttestationValidator no longer looks for attestations that have been seen, as it is filtered by the gossip cache

Signed-off-by: Paul Harris <[email protected]>

ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/AttestationManager.java

@@ -232,8 +232,6 @@ private void sendToSubscribersIfProducedLocally(ValidateableAttestation attestat
 
     if (attestation.isAggregate()) {
       aggregateValidator.addSeenAggregate(attestation);
-    } else {
-      attestationValidator.addSeenAttestation(attestation);
     }
 
     notifyAttestationsToSendSubscribers(attestation);

ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/AttestationValidator.java

@@ -19,16 +19,12 @@
 import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ZERO;
 import static tech.pegasys.teku.statetransition.validation.ValidationResultCode.ACCEPT;
 import static tech.pegasys.teku.util.config.Constants.ATTESTATION_PROPAGATION_SLOT_RANGE;
-import static tech.pegasys.teku.util.config.Constants.VALID_ATTESTATION_SET_SIZE;
 
 import it.unimi.dsi.fastutil.ints.IntList;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalInt;
-import java.util.Set;
 import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
-import tech.pegasys.teku.infrastructure.collections.LimitedSet;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 import tech.pegasys.teku.spec.Spec;
 import tech.pegasys.teku.spec.datastructures.attestation.ValidateableAttestation;
@@ -41,13 +37,10 @@
 import tech.pegasys.teku.util.config.Constants;
 
 public class AttestationValidator {
-
   private static final UInt64 MAX_FUTURE_SLOT_ALLOWANCE = UInt64.valueOf(3);
   private static final UInt64 MAXIMUM_GOSSIP_CLOCK_DISPARITY =
       UInt64.valueOf(Constants.MAXIMUM_GOSSIP_CLOCK_DISPARITY);
 
-  private final Set<ValidatorAndTargetEpoch> receivedValidAttestations =
-      LimitedSet.create(VALID_ATTESTATION_SET_SIZE);
   private final Spec spec;
   private final RecentChainData recentChainData;
   private final AsyncBLSSignatureVerifier signatureVerifier;
@@ -70,30 +63,7 @@ public SafeFuture<InternalValidationResult> validate(
     }
 
     return singleOrAggregateAttestationChecks(
-            signatureVerifier,
-            validateableAttestation,
-            validateableAttestation.getReceivedSubnetId())
-        .thenApply(
-            result -> {
-              if (result.code() != ACCEPT) {
-                return result;
-              }
-
-              return addAndCheckFirstValidAttestation(attestation);
-            });
-  }
-
-  public void addSeenAttestation(final ValidateableAttestation attestation) {
-    receivedValidAttestations.add(getValidatorAndTargetEpoch(attestation.getAttestation()));
-  }
-
-  private InternalValidationResult addAndCheckFirstValidAttestation(final Attestation attestation) {
-    // The attestation is the first valid attestation received for the participating validator for
-    // the slot, attestation.data.slot.
-    if (!receivedValidAttestations.add(getValidatorAndTargetEpoch(attestation))) {
-      return InternalValidationResult.IGNORE;
-    }
-    return InternalValidationResult.ACCEPT;
+        signatureVerifier, validateableAttestation, validateableAttestation.getReceivedSubnetId());
   }
 
   private InternalValidationResult singleAttestationChecks(final Attestation attestation) {
@@ -103,12 +73,6 @@ private InternalValidationResult singleAttestationChecks(final Attestation attes
     if (bitCount != 1) {
       return InternalValidationResult.reject("Attestation has %s bits set instead of 1", bitCount);
     }
-
-    // The attestation is the first valid attestation received for the participating validator for
-    // the slot, attestation.data.slot.
-    if (receivedValidAttestations.contains(getValidatorAndTargetEpoch(attestation))) {
-      return InternalValidationResult.IGNORE;
-    }
     return InternalValidationResult.ACCEPT;
   }
 
@@ -261,13 +225,6 @@ public SafeFuture<Optional<BeaconState>> resolveStateForAttestation(
     }
   }
 
-  private ValidatorAndTargetEpoch getValidatorAndTargetEpoch(final Attestation attestation) {
-    return new ValidatorAndTargetEpoch(
-        attestation.getData().getTarget().getEpoch(),
-        attestation.getData().getIndex(),
-        attestation.getAggregationBits().streamAllSetBits().findFirst().orElseThrow());
-  }
-
   private boolean isCurrentTimeBeforeMinimumAttestationBroadcastTime(
       final Attestation attestation, final UInt64 currentTimeMillis) {
     final UInt64 minimumBroadcastTimeMillis =
@@ -319,40 +276,6 @@ private UInt64 maximumBroadcastTimeMillis(final UInt64 attestationSlot) {
     return secondsToMillis(lastAllowedTime).plus(MAXIMUM_GOSSIP_CLOCK_DISPARITY);
   }
 
-  private static class ValidatorAndTargetEpoch {
-    private final UInt64 targetEpoch;
-    // Validator is identified via committee index and position to avoid resolving the actual
-    // validator ID before checking for duplicates
-    private final UInt64 committeeIndex;
-    private final int committeePosition;
-
-    private ValidatorAndTargetEpoch(
-        final UInt64 targetEpoch, final UInt64 committeeIndex, final int committeePosition) {
-      this.targetEpoch = targetEpoch;
-      this.committeeIndex = committeeIndex;
-      this.committeePosition = committeePosition;
-    }
-
-    @Override
-    public boolean equals(final Object o) {
-      if (this == o) {
-        return true;
-      }
-      if (o == null || getClass() != o.getClass()) {
-        return false;
-      }
-      final ValidatorAndTargetEpoch that = (ValidatorAndTargetEpoch) o;
-      return committeePosition == that.committeePosition
-          && Objects.equals(targetEpoch, that.targetEpoch)
-          && Objects.equals(committeeIndex, that.committeeIndex);
-    }
-
-    @Override
-    public int hashCode() {
-      return Objects.hash(targetEpoch, committeeIndex, committeePosition);
-    }
-  }
-
   private int secondsPerSlot(final UInt64 slot) {
     return spec.getSecondsPerSlot(slot);
   }

ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/validation/AttestationValidatorTest.java

@@ -19,13 +19,11 @@
 import static org.mockito.Mockito.when;
 import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ONE;
 import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ZERO;
-import static tech.pegasys.teku.spec.datastructures.util.BeaconStateUtil.get_committee_count_per_slot;
 import static tech.pegasys.teku.statetransition.validation.ValidationResultCode.ACCEPT;
 import static tech.pegasys.teku.statetransition.validation.ValidationResultCode.IGNORE;
 import static tech.pegasys.teku.statetransition.validation.ValidationResultCode.REJECT;
 import static tech.pegasys.teku.statetransition.validation.ValidationResultCode.SAVE_FOR_FUTURE;
 import static tech.pegasys.teku.util.config.Constants.ATTESTATION_PROPAGATION_SLOT_RANGE;
-import static tech.pegasys.teku.util.config.Constants.SLOTS_PER_EPOCH;
 
 import java.util.List;
 import java.util.Optional;
@@ -122,11 +120,12 @@ public void shouldReturnValidForValidAttestation() {
   @Test
   public void shouldReturnValidForValidAttestation_whenManyBlocksHaveBeenSkipped() {
     final StateAndBlockSummary head = recentChainData.getChainHead().orElseThrow();
-    final UInt64 currentSlot = head.getSlot().plus(SLOTS_PER_EPOCH * 3);
+    final UInt64 currentSlot = head.getSlot().plus(spec.getSlotsPerEpoch(head.getSlot()) * 3);
     storageSystem.chainUpdater().setCurrentSlot(currentSlot);
 
     final Attestation attestation =
-        attestationGenerator.validAttestation(head, head.getSlot().plus(SLOTS_PER_EPOCH * 3));
+        attestationGenerator.validAttestation(
+            head, head.getSlot().plus(spec.getSlotsPerEpoch(head.getSlot()) * 3));
     assertThat(validate(attestation).code()).isEqualTo(ACCEPT);
   }
 
@@ -208,36 +207,13 @@ public void shouldRejectAggregatedAttestation() {
     assertThat(validate(attestation).code()).isEqualTo(REJECT);
   }
 
-  @Test
-  public void shouldRejectAttestationForSameValidatorAndTargetEpoch() {
-    final StateAndBlockSummary genesis = recentChainData.getChainHead().orElseThrow();
-    chainUpdater.advanceChain(ONE);
-
-    // Slot 1 attestation for the block at slot 1
-    final Attestation attestation1 =
-        attestationGenerator.validAttestation(recentChainData.getChainHead().orElseThrow());
-    // Slot 1 attestation from the same validator claiming no block at slot 1
-    final Attestation attestation2 =
-        attestationGenerator
-            .streamAttestations(genesis, ONE)
-            .filter(attestation -> hasSameValidators(attestation1, attestation))
-            .findFirst()
-            .orElseThrow();
-
-    // Sanity check
-    assertThat(attestation1.getData().getTarget().getEpoch())
-        .isEqualTo(attestation2.getData().getTarget().getEpoch());
-    assertThat(attestation1.getAggregationBits()).isEqualTo(attestation2.getAggregationBits());
-
-    assertThat(validate(attestation1).code()).isEqualTo(ACCEPT);
-    assertThat(validate(attestation2).code()).isEqualTo(IGNORE);
-  }
-
   @Test
   public void shouldAcceptAttestationForSameValidatorButDifferentTargetEpoch() {
     final StateAndBlockSummary genesis = recentChainData.getChainHead().orElseThrow();
     final SignedBeaconBlock nextEpochBlock =
-        chainUpdater.advanceChain(UInt64.valueOf(SLOTS_PER_EPOCH + 1)).getBlock();
+        chainUpdater
+            .advanceChain(UInt64.valueOf(spec.getSlotsPerEpoch(genesis.getSlot()) + 1))
+            .getBlock();
 
     // Slot 0 attestation
     final Attestation attestation1 = attestationGenerator.validAttestation(genesis);
@@ -333,7 +309,7 @@ public void shouldRejectAttestationsWithCommitteeIndexNotInTheExpectedRange() {
                         attestation.getAggregationBits(),
                         new AttestationData(
                             data.getSlot(),
-                            get_committee_count_per_slot(
+                            spec.getCommitteeCountPerSlot(
                                 blockAndState.getState(), data.getTarget().getEpoch()),
                             data.getBeacon_block_root(),
                             data.getSource(),

util/src/main/java/tech/pegasys/teku/util/config/Constants.java

@@ -116,7 +116,6 @@ public class Constants {
 
   // Teku Networking Specific
   public static final int VALID_BLOCK_SET_SIZE = 1000;
-  public static final int VALID_ATTESTATION_SET_SIZE = 21000;
   public static final int VALID_AGGREGATE_SET_SIZE = 1000;
   public static final int VALID_VALIDATOR_SET_SIZE = 10000;
   public static final int VALID_CONTRIBUTION_AND_PROOF_SET_SIZE = 10000;