Silencing unecessary warning when handling sync committee messages (C…

…onsensys#7577)
courtneyeh · Oct 5, 2023 · aa5a2d0 · aa5a2d0
1 parent 0905125
commit aa5a2d0
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 4 deletions.
diff --git a/...c/main/java/tech/pegasys/teku/statetransition/synccommittee/SyncCommitteeMessagePool.java b/...c/main/java/tech/pegasys/teku/statetransition/synccommittee/SyncCommitteeMessagePool.java
@@ -25,6 +25,8 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.TreeMap;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes32;
 import tech.pegasys.teku.bls.BLS;
 import tech.pegasys.teku.bls.BLSSignature;
@@ -40,6 +42,9 @@
 import tech.pegasys.teku.statetransition.validation.InternalValidationResult;
 
 public class SyncCommitteeMessagePool implements SlotEventsChannel {
+
+  private static final Logger LOG = LogManager.getLogger();
+
   private final Subscribers<OperationAddedSubscriber<ValidatableSyncCommitteeMessage>> subscribers =
       Subscribers.create(true);
 
@@ -175,11 +180,14 @@ private static class ContributionData {
     public void add(final IntSet participationIndices, final BLSSignature signature) {
       IntIterator iterator = participationIndices.iterator();
       while (iterator.hasNext()) {
-        int index = iterator.nextInt();
-        if (!this.participationIndices.add(index)) {
-          throw new IllegalStateException("Already added " + index);
+        final int index = iterator.nextInt();
+        if (this.participationIndices.add(index)) {
+          this.signatures.add(signature);
+        } else {
+          LOG.trace(
+              "Ignoring already aggregated signature from subcommittee participant index = {}",
+              index);
         }
-        this.signatures.add(signature);
       }
     }
 

diff --git a/...st/java/tech/pegasys/teku/statetransition/synccommittee/SyncCommitteeMessagePoolTest.java b/...st/java/tech/pegasys/teku/statetransition/synccommittee/SyncCommitteeMessagePoolTest.java
@@ -246,6 +246,34 @@ void shouldIncludeRemoteMessageInContributionOnlyOnReceivedSubnet() {
     assertThat(pool.createContribution(slot, blockRoot, 5)).isEmpty();
   }
 
+  @Test
+  public void
+      shouldNotAggregateSignaturesFromRepeatedMessagesWhenSameSubcommitteeParticipationId() {
+    final SyncCommitteeMessage syncCommitteeMessage =
+        dataStructureUtil.randomSyncCommitteeMessage();
+
+    final ValidatableSyncCommitteeMessage message =
+        ValidatableSyncCommitteeMessage.fromNetwork(syncCommitteeMessage, 1);
+    message.setSubcommitteeAssignments(
+        SyncSubcommitteeAssignments.builder().addAssignment(1, 1).build());
+
+    addValidRemote(message);
+    // Duplicated message, should not aggregate signature a second time
+    addValidRemote(message);
+
+    final BLSSignature signature = message.getMessage().getSignature();
+    // Single signature expected on aggregate
+    final BLSSignature expectedAggregate = BLS.aggregate(List.of(signature));
+
+    final UInt64 slot = message.getSlot();
+    final Bytes32 blockRoot = message.getBeaconBlockRoot();
+
+    final Optional<SyncCommitteeContribution> contribution =
+        pool.createContribution(slot, blockRoot, 1);
+    assertThat(contribution).isPresent();
+    assertThat(contribution.orElseThrow().getSignature()).isEqualTo(expectedAggregate);
+  }
+
   @Test
   void shouldAggregateSignatureMultipleTimesWhenValidatorInSameSubcommitteeMultipleTimes() {
     final ValidatableSyncCommitteeMessage message =