Do not add Electra attestations if committees size is not retrievable (…

…#8323)

Co-authored-by: Enrico Del Fante <enrico.delfante@consensys.net>

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

@@ -71,7 +71,7 @@ public ValidatableAttestation buildAggregate() {
             .create(
                 currentAggregateBits.getAggregationBits(),
                 attestationData,
-                () -> currentAggregateBits.getCommitteeBits(),
+                currentAggregateBits::getCommitteeBits,
                 BLS.aggregate(
                     includedAttestations.stream()
                         .map(ValidatableAttestation::getAttestation)

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

@@ -26,7 +26,6 @@
 import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Predicate;
-import java.util.function.Supplier;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
@@ -107,18 +106,17 @@ public AggregatingAttestationPool(
   }
 
   public synchronized void add(final ValidatableAttestation attestation) {
-    final AttestationData attestationData = attestation.getAttestation().getData();
-    final Supplier<Int2IntMap> committeesSizeSupplier =
-        attestation
-            .getCommitteesSize()
-            .<Supplier<Int2IntMap>>map(committeesSize -> () -> committeesSize)
-            .orElseGet(() -> getCommitteesSizeSupplierUsingTheState(attestationData));
-    final boolean add =
-        getOrCreateAttestationGroup(attestationData, committeesSizeSupplier).add(attestation);
-    if (add) {
-      updateSize(1);
-    }
-    // Always keep the latest slot attestations so we don't discard everything
+    final Optional<Int2IntMap> committeesSize =
+        attestation.getCommitteesSize().or(() -> getCommitteesSize(attestation.getAttestation()));
+    getOrCreateAttestationGroup(attestation.getAttestation(), committeesSize)
+        .ifPresent(
+            attestationGroup -> {
+              final boolean added = attestationGroup.add(attestation);
+              if (added) {
+                updateSize(1);
+              }
+            });
+    // Always keep the latest slot attestations, so we don't discard everything
     int currentSize = getSize();
     while (dataHashBySlot.size() > 1 && currentSize > maximumAttestationCount) {
       LOG.trace("Attestation cache at {} exceeds {}, ", currentSize, maximumAttestationCount);
@@ -128,39 +126,48 @@ public synchronized void add(final ValidatableAttestation attestation) {
     }
   }
 
+  private Optional<Int2IntMap> getCommitteesSize(final Attestation attestation) {
+    if (attestation.requiresCommitteeBits()) {
+      return getCommitteesSizeUsingTheState(attestation.getData());
+    }
+    return Optional.empty();
+  }
+
   /**
-   * @param committeesSizeSupplier Required for aggregating attestations as per <a
+   * @param committeesSize Required for aggregating attestations as per <a
    *     href="https://eips.ethereum.org/EIPS/eip-7549">EIP-7549</a>
    */
-  private MatchingDataAttestationGroup getOrCreateAttestationGroup(
-      final AttestationData attestationData, final Supplier<Int2IntMap> committeesSizeSupplier) {
+  private Optional<MatchingDataAttestationGroup> getOrCreateAttestationGroup(
+      final Attestation attestation, final Optional<Int2IntMap> committeesSize) {
+    final AttestationData attestationData = attestation.getData();
+    // if an attestation has committee bits, committees size should have been computed. If this is
+    // not the case, we should ignore this attestation and not add it to the pool
+    if (attestation.requiresCommitteeBits() && committeesSize.isEmpty()) {
+      LOG.warn(
+          "Committees size couldn't be retrieved for attestation at slot {}, block root {} and target root {}. Will NOT add this attestation to the pool.",
+          attestationData.getSlot(),
+          attestationData.getBeaconBlockRoot(),
+          attestationData.getTarget().getRoot());
+      return Optional.empty();
+    }
     dataHashBySlot
         .computeIfAbsent(attestationData.getSlot(), slot -> new HashSet<>())
         .add(attestationData.hashTreeRoot());
-    return attestationGroupByDataHash.computeIfAbsent(
-        attestationData.hashTreeRoot(),
-        key -> new MatchingDataAttestationGroup(spec, attestationData, committeesSizeSupplier));
+    final MatchingDataAttestationGroup attestationGroup =
+        attestationGroupByDataHash.computeIfAbsent(
+            attestationData.hashTreeRoot(),
+            key -> new MatchingDataAttestationGroup(spec, attestationData, committeesSize));
+    return Optional.of(attestationGroup);
   }
 
   // We only have the committees size already available via attestations received in the gossip
   // flow and have been successfully validated, so querying the state is required for other cases
-  private Supplier<Int2IntMap> getCommitteesSizeSupplierUsingTheState(
+  private Optional<Int2IntMap> getCommitteesSizeUsingTheState(
       final AttestationData attestationData) {
-    return () -> {
-      final Bytes32 targetRoot = attestationData.getTarget().getRoot();
-      LOG.debug(
-          "Committees size was not readily available for attestation with target root {}. Will attempt to retrieve it using the relevant state.",
-          targetRoot);
-      final BeaconState state =
-          recentChainData
-              .getStore()
-              .getBlockStateIfAvailable(targetRoot)
-              .orElseThrow(
-                  () ->
-                      new IllegalStateException(
-                          "No state available for attestation with target root " + targetRoot));
-      return spec.getBeaconCommitteesSize(state, attestationData.getSlot());
-    };
+    return recentChainData
+        .getStore()
+        .getBlockStateIfAvailable(attestationData.getTarget().getRoot())
+        .map(state -> spec.getBeaconCommitteesSize(state, attestationData.getSlot()));
   }
 
   @Override
@@ -198,12 +205,13 @@ public synchronized void onAttestationsIncludedInBlock(
   }
 
   private void onAttestationIncludedInBlock(final UInt64 slot, final Attestation attestation) {
-    final Supplier<Int2IntMap> committeesSizeSupplier =
-        getCommitteesSizeSupplierUsingTheState(attestation.getData());
-    final MatchingDataAttestationGroup attestations =
-        getOrCreateAttestationGroup(attestation.getData(), committeesSizeSupplier);
-    final int numRemoved = attestations.onAttestationIncludedInBlock(slot, attestation);
-    updateSize(-numRemoved);
+    getOrCreateAttestationGroup(attestation, getCommitteesSize(attestation))
+        .ifPresent(
+            attestationGroup -> {
+              final int numRemoved =
+                  attestationGroup.onAttestationIncludedInBlock(slot, attestation);
+              updateSize(-numRemoved);
+            });
   }
 
   private void updateSize(final int delta) {
@@ -264,7 +272,7 @@ public synchronized List<Attestation> getAttestations(
     final Predicate<Map.Entry<UInt64, Set<Bytes>>> filterForSlot =
         (entry) -> maybeSlot.map(slot -> entry.getKey().equals(slot)).orElse(true);
 
-    // TODO fix for electra
+    // TODO fix for electra (only used in Beacon API)
     final Predicate<MatchingDataAttestationGroup> filterForCommitteeIndex =
         (group) ->
             maybeCommitteeIndex

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

@@ -24,7 +24,6 @@
 import java.util.Spliterator;
 import java.util.Spliterators;
 import java.util.TreeMap;
-import java.util.function.Supplier;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 import org.apache.tuweni.bytes.Bytes32;
@@ -56,14 +55,14 @@ public class MatchingDataAttestationGroup implements Iterable<ValidatableAttesta
   private final Spec spec;
   private Optional<Bytes32> committeeShufflingSeed = Optional.empty();
   private final AttestationData attestationData;
-  private final Supplier<Int2IntMap> commiteesSizeSupplier;
+  private final Optional<Int2IntMap> committeesSize;
 
   /**
    * Tracks which validators were included in attestations at a given slot on the canonical chain.
    *
    * <p>When a reorg occurs we can accurately compute the set of included validators at the common
    * ancestor by removing blocks in slots after the ancestor then recalculating {@link
-   * #includedValidators}. Otherwise we might remove a validator from the included list because it
+   * #includedValidators}. Otherwise, we might remove a validator from the included list because it
    * was in a block moved off the canonical chain even though that validator was also included in an
    * earlier block which is still on the canonical chain.
    *
@@ -79,17 +78,17 @@ public class MatchingDataAttestationGroup implements Iterable<ValidatableAttesta
   public MatchingDataAttestationGroup(
       final Spec spec,
       final AttestationData attestationData,
-      final Supplier<Int2IntMap> commiteesSizeSupplier) {
+      final Optional<Int2IntMap> committeesSize) {
     this.spec = spec;
     this.attestationData = attestationData;
-    this.commiteesSizeSupplier = commiteesSizeSupplier;
-    this.includedValidators = createEmptyAttestationBits();
+    this.committeesSize = committeesSize;
+    includedValidators = createEmptyAttestationBits();
   }
 
   private AttestationBitsAggregator createEmptyAttestationBits() {
     return AttestationBitsAggregator.fromEmptyFromAttestationSchema(
         spec.atSlot(attestationData.getSlot()).getSchemaDefinitions().getAttestationSchema(),
-        commiteesSizeSupplier);
+        committeesSize);
   }
 
   public AttestationData getAttestationData() {
@@ -180,7 +179,7 @@ public int onAttestationIncludedInBlock(final UInt64 slot, final Attestation att
         slot,
         (__, attestationBitsCalculator) -> {
           if (attestationBitsCalculator == null) {
-            return AttestationBitsAggregator.of(attestation, commiteesSizeSupplier);
+            return AttestationBitsAggregator.of(attestation, committeesSize);
           }
           attestationBitsCalculator.or(attestation);
           return attestationBitsCalculator;

ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregator.java

@@ -14,7 +14,7 @@
 package tech.pegasys.teku.statetransition.attestation.utils;
 
 import it.unimi.dsi.fastutil.ints.Int2IntMap;
-import java.util.function.Supplier;
+import java.util.Optional;
 import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist;
 import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector;
 import tech.pegasys.teku.spec.datastructures.attestation.ValidatableAttestation;
@@ -24,13 +24,13 @@
 public interface AttestationBitsAggregator {
 
   static AttestationBitsAggregator fromEmptyFromAttestationSchema(
-      AttestationSchema<?> attestationSchema, Supplier<Int2IntMap> committeesSize) {
+      AttestationSchema<?> attestationSchema, Optional<Int2IntMap> committeesSize) {
     return attestationSchema
         .toVersionElectra()
         .map(
             schema ->
                 AttestationBitsAggregatorElectra.fromAttestationSchema(
-                    schema, committeesSize.get()))
+                    schema, committeesSize.orElseThrow()))
         .orElseGet(() -> AttestationBitsAggregatorPhase0.fromAttestationSchema(attestationSchema));
   }
 
@@ -52,14 +52,13 @@ static AttestationBitsAggregator of(ValidatableAttestation attestation) {
   }
 
   static AttestationBitsAggregator of(
-      Attestation attestation, Supplier<Int2IntMap> committeesSize) {
+      Attestation attestation, Optional<Int2IntMap> committeesSize) {
     return attestation
         .getCommitteeBits()
-        .map(
+        .<AttestationBitsAggregator>map(
             committeeBits ->
-                (AttestationBitsAggregator)
-                    new AttestationBitsAggregatorElectra(
-                        attestation.getAggregationBits(), committeeBits, committeesSize.get()))
+                new AttestationBitsAggregatorElectra(
+                    attestation.getAggregationBits(), committeeBits, committeesSize.orElseThrow()))
         .orElseGet(() -> new AttestationBitsAggregatorPhase0(attestation.getAggregationBits()));
   }
 

ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java

@@ -58,7 +58,7 @@ public void setUp(final SpecContext specContext) {
     committeeSizes = new Int2IntOpenHashMap();
     committeeSizes.put(0, 10);
     committeeSizes.put(1, 10);
-    group = new MatchingDataAttestationGroup(spec, attestationData, () -> committeeSizes);
+    group = new MatchingDataAttestationGroup(spec, attestationData, Optional.of(committeeSizes));
   }
 
   @TestTemplate

ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java

@@ -66,7 +66,7 @@ void aggregateFromEmpty() {
 
     AttestationBitsAggregator aggregator =
         AttestationBitsAggregator.fromEmptyFromAttestationSchema(
-            attestationSchema, () -> committeeSizes);
+            attestationSchema, Optional.of(committeeSizes));
 
     assertThat(aggregator.aggregateWith(initialAttestation.getAttestation())).isTrue();
 

validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java

@@ -214,10 +214,8 @@ private Attestation createSignedAttestation(
     final Supplier<SszBitvector> committeeBitsSupplier =
         attestationSchema
             .getCommitteeBitsSchema()
-            .map(
-                committeeBitsSchema ->
-                    (Supplier<SszBitvector>)
-                        () -> committeeBitsSchema.ofBits(validator.committeeIndex()))
+            .<Supplier<SszBitvector>>map(
+                committeeBitsSchema -> () -> committeeBitsSchema.ofBits(validator.committeeIndex()))
             .orElse(() -> null);
     return attestationSchema.create(
         aggregationBits, attestationData, committeeBitsSupplier, signature);