Fix SingleAttestation in PerformanceTracker bug (#8902)

Consensys · Dec 9, 2024 · d10a47d · d10a47d
1 parent bf649d0
commit d10a47d
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 3 deletions.
diff --git a/.../validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java b/.../validator/src/main/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandler.java
@@ -586,13 +586,20 @@ public SafeFuture<List<SubmitDataError>> sendSignedAttestations(
   }
 
   private SafeFuture<InternalValidationResult> processAttestation(final Attestation attestation) {
+    final ValidatableAttestation validatableAttestation =
+        ValidatableAttestation.fromValidator(spec, attestation);
     return attestationManager
-        .addAttestation(ValidatableAttestation.fromValidator(spec, attestation), Optional.empty())
+        .addAttestation(validatableAttestation, Optional.empty())
         .thenPeek(
             result -> {
               if (!result.isReject()) {
-                dutyMetrics.onAttestationPublished(attestation.getData().getSlot());
-                performanceTracker.saveProducedAttestation(attestation);
+                // When saving the attestation in performance tracker, we want to make sure we save
+                // the converted attestation.
+                // The conversion happens during processing and is saved in the validatable
+                // attestation.
+                final Attestation convertedAttestation = validatableAttestation.getAttestation();
+                dutyMetrics.onAttestationPublished(convertedAttestation.getData().getSlot());
+                performanceTracker.saveProducedAttestation(convertedAttestation);
               } else {
                 VALIDATOR_LOGGER.producedInvalidAttestation(
                     attestation.getData().getSlot(),

diff --git a/...n/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java b/...n/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java
@@ -14,6 +14,7 @@
 package tech.pegasys.teku.validator.coordinator.performance;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.annotations.VisibleForTesting;
 import it.unimi.dsi.fastutil.ints.Int2IntMap;
@@ -424,6 +425,7 @@ private SafeFuture<Map<UInt64, List<Attestation>>> getAttestationsIncludedInEpoc
 
   @Override
   public void saveProducedAttestation(final Attestation attestation) {
+    checkState(!attestation.isSingleAttestation(), "Single attestation is not supported");
     final UInt64 epoch = spec.computeEpochAtSlot(attestation.getData().getSlot());
     final Set<Attestation> attestationsInEpoch =
         producedAttestationsByEpoch.computeIfAbsent(epoch, __ -> concurrentSet());

diff --git a/...idator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java b/...idator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java
@@ -819,6 +819,53 @@ public void sendSignedAttestations_shouldAddAttestationToAttestationManager() {
         .addAttestation(ValidatableAttestation.from(spec, attestation), Optional.empty());
   }
 
+  @Test
+  void sendSignedAttestations_shouldSaveConvertedAttestationFromSingleAttestation() {
+    spec = TestSpecFactory.createMinimalElectra();
+    dataStructureUtil = new DataStructureUtil(spec);
+    validatorApiHandler =
+        new ValidatorApiHandler(
+            chainDataProvider,
+            nodeDataProvider,
+            networkDataProvider,
+            chainDataClient,
+            syncStateProvider,
+            blockFactory,
+            attestationPool,
+            attestationManager,
+            attestationTopicSubscriptions,
+            activeValidatorTracker,
+            dutyMetrics,
+            performanceTracker,
+            spec,
+            forkChoiceTrigger,
+            proposersDataManager,
+            syncCommitteeMessagePool,
+            syncCommitteeContributionPool,
+            syncCommitteeSubscriptionManager,
+            blockProductionPerformanceFactory,
+            blockPublisher);
+
+    final Attestation attestation = dataStructureUtil.randomSingleAttestation();
+    final Attestation convertedAttestation = dataStructureUtil.randomAttestation();
+    doAnswer(
+            invocation -> {
+              invocation
+                  .getArgument(0, ValidatableAttestation.class)
+                  .convertToAggregatedFormatFromSingleAttestation(convertedAttestation);
+              return completedFuture(InternalValidationResult.ACCEPT);
+            })
+        .when(attestationManager)
+        .addAttestation(any(ValidatableAttestation.class), any());
+
+    final SafeFuture<List<SubmitDataError>> result =
+        validatorApiHandler.sendSignedAttestations(List.of(attestation));
+    assertThat(result).isCompletedWithValue(emptyList());
+
+    verify(dutyMetrics).onAttestationPublished(convertedAttestation.getData().getSlot());
+    verify(performanceTracker).saveProducedAttestation(convertedAttestation);
+  }
+
   @Test
   void sendSignedAttestations_shouldAddToDutyMetricsAndPerformanceTrackerWhenNotInvalid() {
     final Attestation attestation = dataStructureUtil.randomAttestation();