From c9b2960278a65a4e3c3be957025c05f75332f8fe Mon Sep 17 00:00:00 2001 From: Mehdi AOUADI Date: Wed, 24 Jul 2024 15:20:27 +0200 Subject: [PATCH] refactor attestations filtering --- CHANGELOG.md | 1 - .../teku/spec/util/DataStructureUtil.java | 6 +++- .../AggregatingAttestationPool.java | 32 +++++++++++++++---- .../AggregatingAttestationPoolTest.java | 21 ++++++++++-- 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cc6790106d..c698bc4eecf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ ### Additions and Improvements - Added a state pruner that can limit the number of finalized states stored when running an archive node. - - Updated bootnodes for Sepolia network. - Implemented [GetBlockAttestationV2](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/getBlockAttestationsV2) (adding support for Electra attestations) - Implemented [GetAttestationsV2](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/getPoolAttestationsV2) (adding support for Electra attestations) diff --git a/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java b/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java index 1e6ee19fefc..c09fcbe8a10 100644 --- a/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java +++ b/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java @@ -790,7 +790,11 @@ public AttestationData randomAttestationData() { public AttestationData randomAttestationData(final UInt64 slot) { return new AttestationData( - slot, randomUInt64(), randomBytes32(), randomCheckpoint(), randomCheckpoint()); + slot, + randomUInt64(getMaxCommitteesPerSlot()), + randomBytes32(), + randomCheckpoint(), + randomCheckpoint()); } public AttestationData randomAttestationData(final UInt64 slot, final Bytes32 blockRoot) { diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPool.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPool.java index c488a5d8c8f..c8e9f98cd45 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPool.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPool.java @@ -25,7 +25,9 @@ import java.util.Set; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Stream; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes; @@ -273,17 +275,23 @@ public synchronized List getAttestations( final Predicate>> filterForSlot = (entry) -> maybeSlot.map(slot -> entry.getKey().equals(slot)).orElse(true); - final Predicate filterForCommitteeIndex = - (group) -> - maybeCommitteeIndex - .map(index -> group.getAttestationData().getIndex().equals(index)) - .orElse(true); final UInt64 slot = maybeSlot.orElse(recentChainData.getCurrentSlot().orElse(UInt64.ZERO)); final SchemaDefinitions schemaDefinitions = spec.atSlot(slot).getSchemaDefinitions(); final boolean requestRequiresAttestationWithCommitteeBits = schemaDefinitions.getAttestationSchema().requiresCommitteeBits(); + // Committee index filter predicate is used for pre Electra attestations only (the filter is + // applied to the index at the attestation data level) + // Post Electra the index is always set to 0 at the attestation data level (the filter + // is rather applied to the committee bits) + final Predicate filterForCommitteeIndex = + (group) -> + requestRequiresAttestationWithCommitteeBits + || maybeCommitteeIndex + .map(index -> group.getAttestationData().getIndex().equals(index)) + .orElse(true); + return dataHashBySlot.descendingMap().entrySet().stream() .filter(filterForSlot) .map(Map.Entry::getValue) @@ -291,7 +299,9 @@ public synchronized List getAttestations( .map(attestationGroupByDataHash::get) .filter(Objects::nonNull) .filter(filterForCommitteeIndex) - .flatMap(MatchingDataAttestationGroup::stream) + .flatMap( + streamMatchingAttestations( + maybeCommitteeIndex, requestRequiresAttestationWithCommitteeBits)) .map(ValidatableAttestation::getAttestation) .filter( attestation -> @@ -299,6 +309,16 @@ public synchronized List getAttestations( .toList(); } + private Function> + streamMatchingAttestations( + final Optional maybeCommitteeIndex, + final boolean requestRequiresAttestationWithCommitteeBits) { + return matchingDataAttestationGroup -> + requestRequiresAttestationWithCommitteeBits + ? matchingDataAttestationGroup.stream(maybeCommitteeIndex) + : matchingDataAttestationGroup.stream(); + } + private boolean isValid( final BeaconState stateAtBlockSlot, final AttestationData attestationData) { return spec.validateAttestation(stateAtBlockSlot, attestationData).isEmpty(); diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPoolTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPoolTest.java index b804792a9c9..05d82c5c265 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPoolTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPoolTest.java @@ -522,7 +522,10 @@ public void getAttestations_shouldReturnAllAttestations() { } @TestTemplate - public void getAttestations_shouldReturnAttestationsForGivenCommitteeIndexOnly() { + public void getAttestations_shouldReturnAttestationsForGivenCommitteeIndexOnly_PreElectra() { + assumeThat(specMilestone).isLessThan(ELECTRA); + // Pre Electra the committee index filter is applied to the index set at the attestation data + // level final AttestationData attestationData1 = dataStructureUtil.randomAttestationData(); final AttestationData attestationData2 = new AttestationData( @@ -531,7 +534,7 @@ public void getAttestations_shouldReturnAttestationsForGivenCommitteeIndexOnly() attestationData1.getBeaconBlockRoot(), attestationData1.getSource(), attestationData1.getTarget()); - Attestation attestation1 = addAttestationFromValidators(attestationData1, 1, 2, 3); + final Attestation attestation1 = addAttestationFromValidators(attestationData1, 1, 2, 3); addAttestationFromValidators(attestationData2, 4, 5, 6); assertThat( aggregatingPool.getAttestations( @@ -539,6 +542,20 @@ public void getAttestations_shouldReturnAttestationsForGivenCommitteeIndexOnly() .containsExactly(attestation1); } + @TestTemplate + public void getAttestations_shouldReturnAttestationsForGivenCommitteeIndexOnly_PostElectra() { + assumeThat(specMilestone).isGreaterThanOrEqualTo(ELECTRA); + // Post Electra the committee index filter is applied to the committee bits + final AttestationData attestationData1 = dataStructureUtil.randomAttestationData(); + final AttestationData attestationData2 = dataStructureUtil.randomAttestationData(); + final Attestation attestation1 = addAttestationFromValidators(attestationData1, 1, 2, 3); + final Optional committeeIndexFilter = committeeIndex; + committeeIndex = Optional.of(committeeIndex.get().plus(1)); + addAttestationFromValidators(attestationData2, 4, 5, 6); + assertThat(aggregatingPool.getAttestations(Optional.empty(), committeeIndexFilter)) + .containsExactly(attestation1); + } + @TestTemplate public void getAttestations_shouldReturnAttestationsForGivenSlotOnly() { final AttestationData attestationData1 = dataStructureUtil.randomAttestationData();