Fix get committees size using the state (#9249)

* Fix getCommitteesSizeUsingTheState

* fix meaning of optional current epoch

also changed messages to debug as they're useless to users, which I had left previously but this will make more sense in debug.

The test cases had to be updated to return an epoch to make the code exercised.

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

---------

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

Author: StefanBratanov

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

@@ -34,7 +34,6 @@
 import org.apache.tuweni.bytes.Bytes32;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
 import tech.pegasys.teku.ethereum.events.SlotEventsChannel;
-import tech.pegasys.teku.infrastructure.async.SafeFuture;
 import tech.pegasys.teku.infrastructure.metrics.SettableGauge;
 import tech.pegasys.teku.infrastructure.metrics.TekuMetricCategory;
 import tech.pegasys.teku.infrastructure.ssz.SszList;
@@ -173,42 +172,49 @@ private Optional<Int2IntMap> getCommitteesSizeUsingTheState(
       final AttestationData attestationData) {
     // we can use the first state of the epoch to get committees for an attestation
     final MiscHelpers miscHelpers = spec.atSlot(attestationData.getSlot()).miscHelpers();
-    final UInt64 currentEpoch = miscHelpers.computeEpochAtSlot(attestationData.getSlot());
+    final Optional<UInt64> maybeEpoch = recentChainData.getCurrentEpoch();
+    // the only reason this can happen is we don't have a store yet.
+    if (maybeEpoch.isEmpty()) {
+      return Optional.empty();
+    }
+    final UInt64 currentEpoch = maybeEpoch.get();
     final UInt64 attestationEpoch = miscHelpers.computeEpochAtSlot(attestationData.getSlot());
 
     LOG.debug("currentEpoch {}, attestationEpoch {}", currentEpoch, attestationEpoch);
     if (attestationEpoch.equals(currentEpoch)
-        || attestationEpoch.equals(currentEpoch.decrement())) {
-
-      final Optional<SafeFuture<BeaconState>> maybeFuture = recentChainData.getBestState();
-      if (maybeFuture.isEmpty()) {
-        return Optional.empty();
-      } else {
-        try {
-          final BeaconState state = maybeFuture.get().getImmediately();
-          return Optional.of(spec.getBeaconCommitteesSize(state, attestationData.getSlot()));
-        } catch (IllegalStateException e) {
-          LOG.info(
-              "Couldn't retrieve state for committee calculation of slot {}",
-              attestationData.getSlot());
-          return Optional.empty();
-        }
-      }
+        || attestationEpoch.equals(currentEpoch.minusMinZero(1))) {
+
+      return recentChainData
+          .getBestState()
+          .flatMap(
+              state -> {
+                try {
+                  return Optional.of(
+                      spec.getBeaconCommitteesSize(
+                          state.getImmediately(), attestationData.getSlot()));
+                } catch (IllegalStateException e) {
+                  LOG.debug(
+                      "Couldn't retrieve state for committee calculation of slot {}",
+                      attestationData.getSlot());
+                  return Optional.empty();
+                }
+              });
     }
 
     // attestation is not from the current or previous epoch
-    LOG.debug("State at slot {} needed", miscHelpers.computeStartSlotAtEpoch(attestationEpoch));
-    final SafeFuture<Optional<BeaconState>> optionalFutureBeaconState =
-        recentChainData.retrieveStateInEffectAtSlot(
-            miscHelpers.computeStartSlotAtEpoch(attestationEpoch));
+    // this is really an edge case because the current or previous epoch is at least 31 slots
+    // and the attestation is only valid for 64 slots, so it may be epoch-2 but not beyond.
+    final UInt64 attestationEpochStartSlot = miscHelpers.computeStartSlotAtEpoch(attestationEpoch);
+    LOG.debug("State at slot {} needed", attestationEpochStartSlot);
     try {
-      final Optional<BeaconState> maybeState = optionalFutureBeaconState.getImmediately();
-      return maybeState.map(
-          state -> spec.getBeaconCommitteesSize(state, attestationData.getSlot()));
-    } catch (IllegalStateException e) {
-      LOG.info(
+      return recentChainData
+          .retrieveStateInEffectAtSlot(attestationEpochStartSlot)
+          .getImmediately()
+          .map(state -> spec.getBeaconCommitteesSize(state, attestationData.getSlot()));
+    } catch (final IllegalStateException e) {
+      LOG.debug(
           "Couldn't retrieve state in effect at slot {} for committee calculation of slot {}",
-          miscHelpers.computeStartSlotAtEpoch(attestationEpoch),
+          attestationEpochStartSlot,
           attestationData.getSlot());
       return Optional.empty();
     }

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

@@ -400,9 +400,11 @@ public void getSize_shouldIncludeAttestationsAdded() {
 
   @TestTemplate
   public void getSize_shouldDecreaseWhenAttestationsRemoved() {
-    final AttestationData attestationData = dataStructureUtil.randomAttestationData();
+    final AttestationData attestationData = dataStructureUtil.randomAttestationData(ZERO);
     addAttestationFromValidators(attestationData, 1, 2, 3, 4);
     final Attestation attestationToRemove = addAttestationFromValidators(attestationData, 2, 5);
+
+    when(mockRecentChainData.getCurrentEpoch()).thenReturn(Optional.of(ZERO));
     aggregatingPool.onAttestationsIncludedInBlock(ZERO, List.of(attestationToRemove));
     assertThat(aggregatingPool.getSize()).isEqualTo(1);
   }
@@ -418,12 +420,14 @@ public void getSize_shouldNotIncrementWhenAttestationAlreadyExists() {
 
   @TestTemplate
   public void getSize_shouldDecrementForAllRemovedAttestations() {
-    final AttestationData attestationData = dataStructureUtil.randomAttestationData();
+    final AttestationData attestationData = dataStructureUtil.randomAttestationData(ZERO);
     addAttestationFromValidators(attestationData, 1, 2, 3);
     addAttestationFromValidators(attestationData, 4, 5);
     assertThat(aggregatingPool.getSize()).isEqualTo(2);
     final Attestation attestationToRemove =
         addAttestationFromValidators(attestationData, 1, 2, 3, 4, 5);
+
+    when(mockRecentChainData.getCurrentEpoch()).thenReturn(Optional.of(ZERO));
     aggregatingPool.onAttestationsIncludedInBlock(ZERO, List.of(attestationToRemove));
     assertThat(aggregatingPool.getSize()).isEqualTo(0);
   }
@@ -441,7 +445,7 @@ public void getSize_shouldAddTheRightData() {
 
   @TestTemplate
   public void getSize_shouldDecrementForAllRemovedAttestationsWhileKeepingOthers() {
-    final AttestationData attestationData = dataStructureUtil.randomAttestationData();
+    final AttestationData attestationData = dataStructureUtil.randomAttestationData(ZERO);
 
     addAttestationFromValidators(attestationData, 1, 2, 3);
     addAttestationFromValidators(attestationData, 4, 5);
@@ -452,6 +456,7 @@ public void getSize_shouldDecrementForAllRemovedAttestationsWhileKeepingOthers()
         addAttestationFromValidators(attestationData, 1, 2, 3, 4, 5);
     assertThat(aggregatingPool.getSize()).isEqualTo(5);
 
+    when(mockRecentChainData.getCurrentEpoch()).thenReturn(Optional.of(ONE));
     aggregatingPool.onAttestationsIncludedInBlock(ZERO, List.of(attestationToRemove));
     assertThat(aggregatingPool.getSize()).isEqualTo(2);
   }
@@ -627,6 +632,7 @@ public void getAttestations_shouldReturnAttestationsForGivenSlotOnly() {
   @TestTemplate
   void onAttestationsIncludedInBlock_shouldNotAddAttestationsAlreadySeenInABlock() {
     final AttestationData attestationData = dataStructureUtil.randomAttestationData(ZERO);
+    when(mockRecentChainData.getCurrentEpoch()).thenReturn(Optional.of(ZERO));
     // Included in block before we see any attestations with this data
     aggregatingPool.onAttestationsIncludedInBlock(
         ONE, List.of(createAttestation(attestationData, 1, 2, 3, 4)));
@@ -639,6 +645,7 @@ void onAttestationsIncludedInBlock_shouldNotAddAttestationsAlreadySeenInABlock()
   @TestTemplate
   void onAttestationsIncludedInBlock_shouldRemoveAttestationsWhenSeenInABlock() {
     final AttestationData attestationData = dataStructureUtil.randomAttestationData(ZERO);
+    when(mockRecentChainData.getCurrentEpoch()).thenReturn(Optional.of(ZERO));
     addAttestationFromValidators(attestationData, 2, 3);
 
     aggregatingPool.onAttestationsIncludedInBlock(