update current or next slot rule

Author: mehdi-aouadi

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

@@ -104,9 +104,7 @@ public SafeFuture<InternalValidationResult> validate(
      * [IGNORE] bid.slot is the current slot or the next slot.
      */
 
-    // This check considers the gossip clock disparity allowance and hence accepts bids with current
-    // slot, next slot but not too early, previous slot but not too late
-    if (!gossipValidationHelper.isCurrentSlotWithGossipDisparityAllowance(bid.getSlot())) {
+    if (!gossipValidationHelper.isSlotCurrentOrNext(bid.getSlot())) {
       LOG.trace("Bid must be for current or next slot but was for slot {}", bid.getSlot());
       return completedFuture(
           ignore("Bid must be for current or next slot but was for slot %s", bid.getSlot()));

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

@@ -13,6 +13,8 @@
 
 package tech.pegasys.teku.statetransition.validation;
 
+import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ONE;
+
 import java.util.Optional;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
@@ -165,29 +167,20 @@ public SafeFuture<Optional<BeaconState>> getStateAtSlotAndBlockRoot(
   }
 
   public boolean isCurrentSlotWithGossipDisparityAllowance(final UInt64 slot) {
-    final int maximumGossipClockDisparityMillis =
-        spec.getNetworkingConfig().getMaximumGossipClockDisparity();
-    return isTimeWithinSlotWindow(slot, maximumGossipClockDisparityMillis);
+    return isTimeWithinSlotWindow(slot, maxOffsetTimeInMillis);
+  }
+
+  public boolean isSlotCurrentOrNext(final UInt64 slot) {
+    return getCurrentSlot()
+        .map(currentSlot -> slot.equals(currentSlot) || slot.equals(currentSlot.plus(ONE)))
+        .orElse(false);
   }
 
   public boolean isValidatorInPayloadTimelinessCommittee(
       final UInt64 validatorIndex, final BeaconState state, final UInt64 slot) {
     return spec.getPtc(state, slot).contains(validatorIndex.intValue());
   }
 
-  private boolean isTimeWithinSlotWindow(final UInt64 slot, final int disparity) {
-    if (recentChainData.getCurrentSlot().isEmpty()) {
-      return false;
-    }
-    final UInt64 slotStartTimeMillis =
-        spec.computeTimeMillisAtSlot(slot, recentChainData.getGenesisTimeMillis());
-    final UInt64 slotEndTimeMillis = slotStartTimeMillis.plus(spec.getSlotDurationMillis(slot));
-    final UInt64 currentTimeMillis = recentChainData.getStore().getTimeInMillis();
-
-    return currentTimeMillis.isGreaterThanOrEqualTo(slotStartTimeMillis.minusMinZero(disparity))
-        && currentTimeMillis.isLessThanOrEqualTo(slotEndTimeMillis.plus(disparity));
-  }
-
   public boolean hasBuilderWithdrawalCredential(
       final UInt64 builderIndex, final BeaconState state, final UInt64 slot) {
     return MiscHelpersGloas.required(spec.atSlot(slot).miscHelpers())
@@ -208,4 +201,21 @@ public boolean isBlockHashKnown(final Bytes32 blockHash, final Bytes32 blockRoot
         recentChainData.getExecutionBlockHashForBlockRoot(blockRoot);
     return maybeBlockHash.isPresent() && blockHash.equals(maybeBlockHash.get());
   }
+
+  private Optional<UInt64> getCurrentSlot() {
+    return recentChainData.getCurrentSlot();
+  }
+
+  private boolean isTimeWithinSlotWindow(final UInt64 slot, final int disparity) {
+    if (getCurrentSlot().isEmpty()) {
+      return false;
+    }
+    final UInt64 slotStartTimeMillis =
+        spec.computeTimeMillisAtSlot(slot, recentChainData.getGenesisTimeMillis());
+    final UInt64 slotEndTimeMillis = slotStartTimeMillis.plus(spec.getSlotDurationMillis(slot));
+    final UInt64 currentTimeMillis = recentChainData.getStore().getTimeInMillis();
+
+    return currentTimeMillis.isGreaterThanOrEqualTo(slotStartTimeMillis.minusMinZero(disparity))
+        && currentTimeMillis.isLessThanOrEqualTo(slotEndTimeMillis.plus(disparity));
+  }
 }

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

@@ -70,7 +70,7 @@ void setup(final TestSpecInvocationContextProvider.SpecContext specContext) {
     parentBlockHash = bid.getParentBlockHash();
     postState = dataStructureUtil.randomBeaconState();
 
-    when(gossipValidationHelper.isCurrentSlotWithGossipDisparityAllowance(slot)).thenReturn(true);
+    when(gossipValidationHelper.isSlotCurrentOrNext(slot)).thenReturn(true);
     when(gossipValidationHelper.isBlockHashKnown(parentBlockHash, parentBlockRoot))
         .thenReturn(true);
     when(gossipValidationHelper.getSlotForBlockRoot(parentBlockRoot))
@@ -114,8 +114,8 @@ void shouldReject_whenExecutionPaymentIsNonZero() {
   }
 
   @TestTemplate
-  void shouldIgnore_whenSlotIsNotWithinGossipWindow() {
-    when(gossipValidationHelper.isCurrentSlotWithGossipDisparityAllowance(slot)).thenReturn(false);
+  void shouldIgnore_whenSlotIsNotCurrentOrNext() {
+    when(gossipValidationHelper.isSlotCurrentOrNext(slot)).thenReturn(false);
     assertThatSafeFuture(bidValidator.validate(signedBid))
         .isCompletedWithValue(
             ignore("Bid must be for current or next slot but was for slot %s", slot));

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

@@ -312,36 +312,26 @@ void currentFinalizedCheckpointIsAncestorOfBlock_shouldReturnValid() {
   }
 
   @TestTemplate
-  void isForCurrentSlot_shouldRejectOutsideLowerBound() {
+  void isForCurrentSlotWithGossipDisparityAllowance() {
     final UInt64 slot = UInt64.valueOf(1000);
     final UInt64 slotStartTimeMillis = getSlotStartTimeMillis(slot);
-    final UInt64 currentTime = slotStartTimeMillis.minus(maximumGossipClockDisparity).decrement();
-    assertIsCurrentSlot(slot, currentTime, false);
-  }
-
-  @TestTemplate
-  void isForCurrentSlot_shouldAcceptLowerBound() {
-    final UInt64 slot = UInt64.valueOf(1000);
-    final UInt64 slotStartTimeMillis = getSlotStartTimeMillis(slot);
-    final UInt64 currentTime = slotStartTimeMillis.minus(maximumGossipClockDisparity);
-    assertIsCurrentSlot(slot, currentTime, true);
-  }
-
-  @TestTemplate
-  void isForCurrentSlot_shouldAcceptUpperBound() {
-    final UInt64 slot = UInt64.valueOf(1000);
+    assertIsCurrentSlot(
+        slot, slotStartTimeMillis.minus(maximumGossipClockDisparity).decrement(), false);
+    assertIsCurrentSlot(slot, slotStartTimeMillis.minus(maximumGossipClockDisparity), true);
     final UInt64 nextSlotStartTimeMillis = getSlotStartTimeMillis(slot.increment());
-    final UInt64 currentTime = nextSlotStartTimeMillis.plus(maximumGossipClockDisparity);
-    assertIsCurrentSlot(slot, currentTime, true);
+    assertIsCurrentSlot(slot, nextSlotStartTimeMillis.plus(maximumGossipClockDisparity), true);
+    assertIsCurrentSlot(
+        slot, nextSlotStartTimeMillis.plus(maximumGossipClockDisparity).increment(), false);
   }
 
   @TestTemplate
-  void isForCurrentSlot_shouldRejectOutsideUpperBound() {
-    final UInt64 slot = UInt64.valueOf(1000);
-    final UInt64 nextSlotStartTimeMillis = getSlotStartTimeMillis(slot.increment());
-    final UInt64 currentTime =
-        nextSlotStartTimeMillis.plus(maximumGossipClockDisparity).increment();
-    assertIsCurrentSlot(slot, currentTime, false);
+  void isCurrentOrNextSlot() {
+    final UInt64 currentSlot = UInt64.valueOf(10);
+    storageSystem.chainUpdater().setCurrentSlot(currentSlot);
+    assertThat(gossipValidationHelper.isSlotCurrentOrNext(currentSlot)).isTrue();
+    assertThat(gossipValidationHelper.isSlotCurrentOrNext(currentSlot.plus(ONE))).isTrue();
+    assertThat(gossipValidationHelper.isSlotCurrentOrNext(currentSlot.minus(ONE))).isFalse();
+    assertThat(gossipValidationHelper.isSlotCurrentOrNext(currentSlot.plus(2))).isFalse();
   }
 
   private UInt64 getSlotStartTimeMillis(final UInt64 slot) {