From a7274a556042795aeaa83f996c884499aceaffcf Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Wed, 29 May 2024 14:38:50 +0200 Subject: [PATCH 1/7] tmp --- .../MatchingDataAttestationGroup.java | 2 +- .../AttestationBitsAggregatorElectra.java | 27 ++++++++++--------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroup.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroup.java index 3604b2ad716..b840d82e033 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroup.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroup.java @@ -82,7 +82,7 @@ public MatchingDataAttestationGroup( this.spec = spec; this.attestationData = attestationData; this.committeesSize = committeesSize; - includedValidators = createEmptyAttestationBits(); + this.includedValidators = createEmptyAttestationBits(); } private AttestationBitsAggregator createEmptyAttestationBits() { diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java index c825c4dcc39..2a318427f40 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java @@ -29,6 +29,7 @@ class AttestationBitsAggregatorElectra implements AttestationBitsAggregator { private SszBitlist aggregationBits; private SszBitvector committeeBits; + private Int2IntMap committeeBitsStartingPositions; private final Int2IntMap committeesSize; AttestationBitsAggregatorElectra( @@ -38,6 +39,7 @@ class AttestationBitsAggregatorElectra implements AttestationBitsAggregator { this.aggregationBits = aggregationBits; this.committeeBits = committeeBits; this.committeesSize = committeesSize; + this.committeeBitsStartingPositions = calculateCommitteeStartingPositions(committeeBits); } static AttestationBitsAggregator fromAttestationSchema( @@ -72,21 +74,11 @@ private boolean or( final SszBitvector aggregatedCommitteeBits = committeeBits.or(otherCommitteeBits); - final Int2IntMap committeeBitsStartingPositions = - calculateCommitteeStartingPositions(committeeBits); final Int2IntMap otherCommitteeBitsStartingPositions = calculateCommitteeStartingPositions(otherCommitteeBits); final Int2IntMap aggregatedCommitteeBitsStartingPositions = calculateCommitteeStartingPositions(aggregatedCommitteeBits); - final IntList aggregatedCommitteeIndices = aggregatedCommitteeBits.getAllSetBits(); - - // create an aggregation bit big as last boundary for last committee bit - final int lastCommitteeIndex = - aggregatedCommitteeIndices.getInt(aggregatedCommitteeIndices.size() - 1); - final int lastCommitteeStartingPosition = - aggregatedCommitteeBitsStartingPositions.get(lastCommitteeIndex); - final IntList aggregationIndices = new IntArrayList(); // aggregateBits contains a new set of bits @@ -127,13 +119,23 @@ private boolean or( return false; } + final IntList aggregatedCommitteeIndices = aggregatedCommitteeBits.getAllSetBits(); + + // create an aggregation bit big as last boundary for last committee bit + final int lastCommitteeIndex = + aggregatedCommitteeIndices.getInt(aggregatedCommitteeIndices.size() - 1); + final int lastCommitteeStartingPosition = + aggregatedCommitteeBitsStartingPositions.get(lastCommitteeIndex); + final int aggregationBitsSize = lastCommitteeStartingPosition + committeesSize.get(lastCommitteeIndex); + committeeBits = aggregatedCommitteeBits; aggregationBits = aggregationBits .getSchema() .ofBits( - lastCommitteeStartingPosition + committeesSize.get(lastCommitteeIndex), + aggregationBitsSize, aggregationIndices.toIntArray()); + committeeBitsStartingPositions = aggregatedCommitteeBitsStartingPositions; return true; } @@ -181,8 +183,6 @@ public boolean isSuperSetOf(final Attestation other) { final SszBitvector otherCommitteeBits = other.getCommitteeBitsRequired(); - final Int2IntMap committeeBitsStartingPositions = - calculateCommitteeStartingPositions(committeeBits); final Int2IntMap otherCommitteeBitsStartingPositions = calculateCommitteeStartingPositions(otherCommitteeBits); @@ -235,6 +235,7 @@ public String toString() { .add("aggregationBits", aggregationBits) .add("committeeBits", committeeBits) .add("committeesSize", committeesSize) + .add("committeeBitsStartingPositions", committeeBitsStartingPositions) .toString(); } } From 4a8d89e5b2c785ba96aeb32be915682a2638f102 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Wed, 29 May 2024 17:00:30 +0200 Subject: [PATCH 2/7] electra attestation aggregation improvements --- .../AttestationBitsAggregatorElectra.java | 22 +-- .../AttestationBitsAggregatorElectraTest.java | 157 ++++++++++++------ .../ssz/collections/SszBitvector.java | 3 + .../ssz/collections/impl/BitvectorImpl.java | 11 +- .../collections/impl/SszBitvectorImpl.java | 5 + .../ssz/collections/SszBitvectorTest.java | 8 + 6 files changed, 137 insertions(+), 69 deletions(-) diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java index 2a318427f40..4aea4642927 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java @@ -119,22 +119,16 @@ private boolean or( return false; } - final IntList aggregatedCommitteeIndices = aggregatedCommitteeBits.getAllSetBits(); - // create an aggregation bit big as last boundary for last committee bit - final int lastCommitteeIndex = - aggregatedCommitteeIndices.getInt(aggregatedCommitteeIndices.size() - 1); + final int lastCommitteeIndex = aggregatedCommitteeBits.getLastSetBitIndex(); final int lastCommitteeStartingPosition = - aggregatedCommitteeBitsStartingPositions.get(lastCommitteeIndex); - final int aggregationBitsSize = lastCommitteeStartingPosition + committeesSize.get(lastCommitteeIndex); + aggregatedCommitteeBitsStartingPositions.get(lastCommitteeIndex); + final int aggregationBitsSize = + lastCommitteeStartingPosition + committeesSize.get(lastCommitteeIndex); committeeBits = aggregatedCommitteeBits; aggregationBits = - aggregationBits - .getSchema() - .ofBits( - aggregationBitsSize, - aggregationIndices.toIntArray()); + aggregationBits.getSchema().ofBits(aggregationBitsSize, aggregationIndices.toIntArray()); committeeBitsStartingPositions = aggregatedCommitteeBitsStartingPositions; return true; @@ -177,7 +171,9 @@ public boolean isSuperSetOf(final Attestation other) { return false; } - if (committeeBits.equals(other.getCommitteeBitsRequired())) { + if (committeeBits.getBitCount() == other.getCommitteeBitsRequired().getBitCount()) { + // this committeeBits is a superset of the other, and bit count is the same, so they are the + // same set and we can directly compare aggregation bits. return aggregationBits.isSuperSetOf(other.getAggregationBits()); } @@ -235,7 +231,7 @@ public String toString() { .add("aggregationBits", aggregationBits) .add("committeeBits", committeeBits) .add("committeesSize", committeesSize) - .add("committeeBitsStartingPositions", committeeBitsStartingPositions) + .add("committeeBitsStartingPositions", committeeBitsStartingPositions) .toString(); } } diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java index 375ecb88cf0..2e377bbdfe8 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Optional; import java.util.function.Supplier; +import java.util.regex.Pattern; import java.util.stream.IntStream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -62,9 +63,9 @@ void aggregateFromEmpty() { 012 <- committee 1 indices 011 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(1), 1, 2); + final ValidatableAttestation initialAttestation = createAttestation(List.of(1), 1, 2); - AttestationBitsAggregator aggregator = + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.fromEmptyFromAttestationSchema( attestationSchema, Optional.of(committeeSizes)); @@ -81,15 +82,15 @@ void cannotAggregateSameCommitteesWithOverlappingAggregates() { 012 <- committee 1 indices 011 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(1), 1, 2); + final ValidatableAttestation initialAttestation = createAttestation(List.of(1), 1, 2); /* 012 <- committee 1 indices 110 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0, 1); + final ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0, 1); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isFalse(); } @@ -100,15 +101,15 @@ void aggregateOnSameCommittee() { 012 <- committee 1 indices 011 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(1), 1, 2); + final ValidatableAttestation initialAttestation = createAttestation(List.of(1), 1, 2); /* 012 <- committee 1 indices 100 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0); + final ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isTrue(); @@ -127,15 +128,15 @@ void aggregateOnMultipleOverlappingCommitteeBits() { 01|234 <- committee 0 and 1 indices 10|100 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); /* 01|234 <- committee 0 and 1 indices 01|010 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1), 1, 3); + final ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1), 1, 3); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isTrue(); @@ -154,15 +155,15 @@ void aggregateOnMultipleOverlappingCommitteeBitsVariation() { 01|234 <- committee 0 and 1 indices 10|100 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); /* 01|234|5678 <- committee 0, 1 and 2 indices 01|011|0001 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1, 2), 1, 3, 4, 8); + final ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1, 2), 1, 3, 4, 8); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isTrue(); @@ -182,15 +183,15 @@ void cannotAggregateOnMultipleOverlappingCommitteeBitsButWithSomeOfAggregationOv 01|234 <- committee 0 and 1 indices 10|100 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); /* 01|234|5678 <- committee 0, 1 and 2 indices 01|110|0001 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1, 2), 1, 2, 3, 8); + final ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1, 2), 1, 2, 3, 8); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isFalse(); @@ -206,16 +207,20 @@ void aggregateOnMultipleOverlappingCommitteeBitsButWithSomeOfAggregationOverlapp 01|234 <- committee 0 and 1 indices 10|100 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); /* 01|234|5678 <- committee 0, 1 and 2 indices 01|110|0001 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1, 2), 1, 2, 3, 8); + final ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1, 2), 1, 2, 3, 8); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + + // cannot aggregate + assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isFalse(); + // calculate the or aggregator.or(otherAttestation.getAttestation()); /* @@ -234,16 +239,20 @@ void aggregateOnMultipleOverlappingCommitteeBitsButWithSomeOfAggregationOverlapp 0123 <- committee 2 indices 0100 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(2), 1); + final ValidatableAttestation initialAttestation = createAttestation(List.of(2), 1); /* 0123 <- committee 2 indices 1101 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(2), 0, 1, 3); + final ValidatableAttestation otherAttestation = createAttestation(List.of(2), 0, 1, 3); AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + // cannot aggregate + assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isFalse(); + + // calculate the or aggregator.or(otherAttestation.getAttestation()); /* @@ -261,15 +270,15 @@ void aggregateOnMultipleDisjointedCommitteeBits() { 01|234 <- committee 0 and 1 indices 10|100 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2); /* 0123 <- committee 1 indices 1101 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(2), 0, 1, 3); + final ValidatableAttestation otherAttestation = createAttestation(List.of(2), 0, 1, 3); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isTrue(); @@ -288,13 +297,13 @@ void aggregateOnMultipleDisjointedCommitteeBitsVariation() { 0123 <- committee 2 indices 0001 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(2), 3); + final ValidatableAttestation initialAttestation = createAttestation(List.of(2), 3); /* 01 <- committee 0 indices 01 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(0), 1); + final ValidatableAttestation otherAttestation = createAttestation(List.of(0), 1); AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); @@ -313,7 +322,7 @@ void aggregateOnMultipleDisjointedCommitteeBitsVariation() { void bigAggregation() { committeeSizes = new Int2IntOpenHashMap(); committeeSizes.put(0, 50); - committeeSizes.put(1, 50); + committeeSizes.put(1, 49); committeeSizes.put(2, 50); committeeSizes.put(3, 50); committeeSizes.put(4, 50); @@ -327,17 +336,61 @@ void bigAggregation() { committeeSizes.put(12, 50); committeeSizes.put(13, 50); committeeSizes.put(14, 50); - committeeSizes.put(15, 50); + committeeSizes.put(15, 51); - ValidatableAttestation initialAttestation = + final ValidatableAttestation initialAttestation = createAttestation( "1111111111111111", - "11111111111111111111111111111111111111111111111111000000000000000000000000000000000000000000000000100000000000000000000010000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000001000000000000000000000000000000010000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000001000100000000000000000000000000000000000000000000000000000001000000000000000000000000000000100000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000100000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000"); - ValidatableAttestation att = + """ + 11111111111111111111111111111111111111111111111111\ + 0000000000000000000000000000000000000000000000010\ + 00000000000000000000100000000000000000000000000000\ + 00000000000010000000000000000000000000000000000000\ + 00000000000000000000000000000000000000000000000001\ + 00000000000000000000000000000000000010000000000000\ + 00000000000000000010000000000000000000000000000000\ + 00000000000000000001000000000000000000000000000000\ + 00000000000000010001000000000000000000000000000000\ + 00000000000000000000000001000000000000000000000000\ + 00000010000000000000000000000000000000000000000000\ + 00010000000000000000000000000000000000000000000000\ + 00000000000000100000000000000000000000000000000000\ + 00000000000010000000000000000000000000000000000000\ + 00100000000000000000000000000000000000000000000000\ + 000000000000001000000000000000000000000000000000001\ + """); + final ValidatableAttestation att = createAttestation("0001000000000000", "00000000000000000000000100000000000000000000000000"); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.aggregateWith(att.getAttestation())).isTrue(); + + final ValidatableAttestation result = + createAttestation( + "1111111111111111", + """ + 11111111111111111111111111111111111111111111111111\ + 0000000000000000000000000000000000000000000000010\ + 00000000000000000000100000000000000000000000000000\ + 00000000000010000000000100000000000000000000000000\ + 00000000000000000000000000000000000000000000000001\ + 00000000000000000000000000000000000010000000000000\ + 00000000000000000010000000000000000000000000000000\ + 00000000000000000001000000000000000000000000000000\ + 00000000000000010001000000000000000000000000000000\ + 00000000000000000000000001000000000000000000000000\ + 00000010000000000000000000000000000000000000000000\ + 00010000000000000000000000000000000000000000000000\ + 00000000000000100000000000000000000000000000000000\ + 00000000000010000000000000000000000000000000000000\ + 00100000000000000000000000000000000000000000000000\ + 000000000000001000000000000000000000000000000000001\ + """); + + assertThat(aggregator.getCommitteeBits()) + .isEqualTo(result.getAttestation().getCommitteeBitsRequired()); + assertThat(aggregator.getAggregationBits()) + .isEqualTo(result.getAttestation().getAggregationBits()); } @Test @@ -347,13 +400,13 @@ void isSuperSetOf1() { 01|234 <- committee 0 and 1 indices 10|101 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2, 4); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2, 4); /* 01|234 <- committee 0 and 1 indices 10|100 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1), 0, 2); + final ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1), 0, 2); AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); @@ -367,15 +420,15 @@ void isSuperSetOf2() { 01|234 <- committee 0 and 1 indices 10|101 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2, 4); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2, 4); /* 01|234 <- committee 0 and 1 indices 10|101 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1), 0, 2, 4); + final ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1), 0, 2, 4); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.isSuperSetOf(otherAttestation.getAttestation())).isTrue(); } @@ -387,15 +440,15 @@ void isSuperSetOf3() { 01|234 <- committee 0 and 1 indices 10|101 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2, 4); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2, 4); /* 01|234 <- committee 0 and 1 indices 10|111 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1), 0, 2, 3, 4); + final ValidatableAttestation otherAttestation = createAttestation(List.of(0, 1), 0, 2, 3, 4); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.isSuperSetOf(otherAttestation.getAttestation())).isFalse(); } @@ -407,13 +460,13 @@ void isSuperSetOf4() { 01|234 <- committee 0 and 1 indices 10|101 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2, 4); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1), 0, 2, 4); /* 012 <- committee 1 indices 111 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0, 1, 2); + final ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0, 1, 2); AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); @@ -427,15 +480,15 @@ void isSuperSetOf5() { 01 <- committee 0 10 <- bits */ - ValidatableAttestation initialAttestation = createAttestation(List.of(0), 0); + final ValidatableAttestation initialAttestation = createAttestation(List.of(0), 0); /* 012 <- committee 1 indices 100 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0); + final ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.isSuperSetOf(otherAttestation.getAttestation())).isFalse(); } @@ -447,21 +500,23 @@ void isSuperSetOf6() { 01|234|5678 <- committee 0, 1 and 2 indices 11|111|1111 <- bits */ - ValidatableAttestation initialAttestation = + final ValidatableAttestation initialAttestation = createAttestation(List.of(0, 1, 2), 0, 1, 2, 3, 4, 5, 6, 7, 8); /* 012 <- committee 1 indices 100 <- bits */ - ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0); + final ValidatableAttestation otherAttestation = createAttestation(List.of(1), 0); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.isSuperSetOf(otherAttestation.getAttestation())).isTrue(); } private ValidatableAttestation createAttestation(final String commBits, final String aggBits) { + assertThat(commBits).matches(Pattern.compile("^[0-1]+$")); + assertThat(aggBits).matches(Pattern.compile("^[0-1]+$")); final List commBitList = IntStream.range(0, commBits.length()) .mapToObj(index -> commBits.charAt(index) == '1' ? index : -1) @@ -480,9 +535,7 @@ private ValidatableAttestation createAttestation( final SszBitlist aggregationBits = attestationSchema .getAggregationBitsSchema() - .ofBits( - Math.toIntExact(attestationSchema.getAggregationBitsSchema().getMaxLength()), - validators); + .ofBits(committeeSizes.values().intStream().sum(), validators); final Supplier committeeBits = () -> attestationSchema.getCommitteeBitsSchema().orElseThrow().ofBits(committeeIndices); diff --git a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitvector.java b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitvector.java index 43d4c770a24..a2e2dbae237 100644 --- a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitvector.java +++ b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitvector.java @@ -59,6 +59,9 @@ default boolean isSet(final int i) { /** Returns indices of all bits set in this {@link SszBitvector} */ IntList getAllSetBits(); + /** Returns last bit set index, -1 if no bits are set * */ + int getLastSetBitIndex(); + /** Streams indices of all bits set in this {@link SszBitvector} */ @Override IntStream streamAllSetBits(); diff --git a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/BitvectorImpl.java b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/BitvectorImpl.java index 7df277e5c58..3ef294ac532 100644 --- a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/BitvectorImpl.java +++ b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/BitvectorImpl.java @@ -116,6 +116,10 @@ public int getSize() { return size; } + public int getLastSetBitIndex() { + return data.length() - 1; + } + public IntStream streamAllSetBits() { return data.stream(); } @@ -128,14 +132,13 @@ public Bytes serialize() { } public BitvectorImpl rightShift(final int i) { - int length = this.getSize(); - BitSet newData = new BitSet(getSize()); - for (int j = 0; j < length - i; j++) { + final BitSet newData = new BitSet(size); + for (int j = 0; j < size - i; j++) { if (this.getBit(j)) { newData.set(j + i); } } - return new BitvectorImpl(newData, getSize()); + return new BitvectorImpl(newData, size); } @Override diff --git a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/SszBitvectorImpl.java b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/SszBitvectorImpl.java index fe43b7e1e4e..45dac26fe81 100644 --- a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/SszBitvectorImpl.java +++ b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/SszBitvectorImpl.java @@ -79,6 +79,11 @@ public IntList getAllSetBits() { return IntArrayList.toList(value.streamAllSetBits()); } + @Override + public int getLastSetBitIndex() { + return value.getLastSetBitIndex(); + } + @Override public IntStream streamAllSetBits() { return value.streamAllSetBits(); diff --git a/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitvectorTest.java b/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitvectorTest.java index 56326b5a878..58464ba66c8 100644 --- a/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitvectorTest.java +++ b/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitvectorTest.java @@ -226,6 +226,14 @@ void testOrWithEmptyBitvector(SszBitvector bitvector) { assertThat(bitvector.or(empty)).isEqualTo(bitvector); } + @ParameterizedTest + @MethodSource("bitvectorArgs") + void testGetLastSetBitIndex(SszBitvector bitvector) { + int result = bitvector.getLastSetBitIndex(); + int expected = bitvector.streamAllSetBits().reduce((first, second) -> second).orElse(-1); + assertThat(result).isEqualTo(expected); + } + @ParameterizedTest @MethodSource("bitvectorArgs") void get_shouldThrowIndexOutOfBounds(SszBitvector vector) { From b8c0a9d772a8d9a4db108253464077edd6929caf Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Wed, 29 May 2024 21:36:58 +0200 Subject: [PATCH 3/7] optimizations --- .../AttestationBitsAggregatorElectra.java | 77 +++++++++++-------- .../AttestationBitsAggregatorElectraTest.java | 2 +- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java index 4aea4642927..12b5e1bfe65 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java @@ -18,8 +18,6 @@ import it.unimi.dsi.fastutil.ints.Int2IntOpenHashMap; import it.unimi.dsi.fastutil.ints.IntArrayList; import it.unimi.dsi.fastutil.ints.IntList; -import java.util.ArrayList; -import java.util.List; import java.util.stream.IntStream; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector; @@ -91,29 +89,38 @@ private boolean or( int committeeSize = committeesSize.get(committeeIndex); int destinationStart = aggregatedCommitteeBitsStartingPositions.get(committeeIndex); - final List sources = new ArrayList<>(); - final List sourcesStartingPosition = new ArrayList<>(); + SszBitlist source1 = null, maybeSource2 = null; + int source1StartingPosition = 0, source2StartingPosition = 0; + if (committeeBitsStartingPositions.containsKey(committeeIndex)) { - sources.add(aggregationBits); - sourcesStartingPosition.add(committeeBitsStartingPositions.get(committeeIndex)); + source1 = aggregationBits; + source1StartingPosition = committeeBitsStartingPositions.get(committeeIndex); } if (otherCommitteeBitsStartingPositions.containsKey(committeeIndex)) { - sources.add(otherAggregatedBits); - sourcesStartingPosition.add( - otherCommitteeBitsStartingPositions.get(committeeIndex)); + if (source1 != null) { + maybeSource2 = otherAggregatedBits; + source2StartingPosition = + otherCommitteeBitsStartingPositions.get(committeeIndex); + } else { + source1 = otherAggregatedBits; + source1StartingPosition = + otherCommitteeBitsStartingPositions.get(committeeIndex); + } } - IntStream.range(0, committeeSize) - .forEach( - positionInCommittee -> { - if (orSingleBit( - positionInCommittee, - sources, - sourcesStartingPosition, - isAggregation)) { - aggregationIndices.add(destinationStart + positionInCommittee); - } - }); + for (int positionInCommittee = 0; + positionInCommittee < committeeSize; + positionInCommittee++) { + if (orSingleBit( + positionInCommittee, + source1, + source1StartingPosition, + maybeSource2, + source2StartingPosition, + isAggregation)) { + aggregationIndices.add(destinationStart + positionInCommittee); + } + } }); } catch (final CannotAggregateException __) { return false; @@ -136,21 +143,25 @@ private boolean or( private boolean orSingleBit( final int positionInCommittee, - final List sources, - final List sourcesStartingPosition, + final SszBitlist source1, + final int source1StartingPosition, + final SszBitlist maybeSource2, + final int source2StartingPosition, final boolean isAggregating) { - boolean aggregatedBit = false; - for (int s = 0; s < sources.size(); s++) { - final boolean sourceBit = - sources.get(s).getBit(sourcesStartingPosition.get(s) + positionInCommittee); - - if (!aggregatedBit && sourceBit) { - aggregatedBit = true; - } else if (isAggregating && aggregatedBit && sourceBit) { - throw new CannotAggregateException(); - } + + final boolean source1Bit = source1.getBit(source1StartingPosition + positionInCommittee); + + if (maybeSource2 == null) { + return source1Bit; } - return aggregatedBit; + + final boolean source2Bit = maybeSource2.getBit(source2StartingPosition + positionInCommittee); + + if (isAggregating && source1Bit && source2Bit) { + throw new CannotAggregateException(); + } + + return source1Bit || source2Bit; } private Int2IntMap calculateCommitteeStartingPositions(final SszBitvector committeeBits) { diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java index 2e377bbdfe8..7dab2b056b7 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java @@ -305,7 +305,7 @@ void aggregateOnMultipleDisjointedCommitteeBitsVariation() { */ final ValidatableAttestation otherAttestation = createAttestation(List.of(0), 1); - AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(initialAttestation); assertThat(aggregator.aggregateWith(otherAttestation.getAttestation())).isTrue(); From 8d04a83da99c0c29d6e98231c137723dab3bf2da Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 30 May 2024 09:53:47 +0200 Subject: [PATCH 4/7] optimizations2 --- .../AttestationBitsAggregatorElectra.java | 22 +++++------ .../AttestationBitsAggregatorElectraTest.java | 39 +++++++++++++++++++ .../ssz/collections/impl/BitlistImpl.java | 8 ++++ .../ssz/collections/impl/SszBitlistImpl.java | 6 +++ .../schema/collections/SszBitlistSchema.java | 3 ++ .../impl/SszBitlistSchemaImpl.java | 7 ++++ .../ssz/collections/SszBitlistTest.java | 26 ++++++++++++- 7 files changed, 98 insertions(+), 13 deletions(-) diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java index 12b5e1bfe65..ad920570666 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java @@ -16,8 +16,8 @@ import com.google.common.base.MoreObjects; import it.unimi.dsi.fastutil.ints.Int2IntMap; import it.unimi.dsi.fastutil.ints.Int2IntOpenHashMap; -import it.unimi.dsi.fastutil.ints.IntArrayList; import it.unimi.dsi.fastutil.ints.IntList; +import java.util.BitSet; import java.util.stream.IntStream; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector; @@ -77,7 +77,14 @@ private boolean or( final Int2IntMap aggregatedCommitteeBitsStartingPositions = calculateCommitteeStartingPositions(aggregatedCommitteeBits); - final IntList aggregationIndices = new IntArrayList(); + // create an aggregation bit big as last boundary for last committee bit + final int lastCommitteeIndex = aggregatedCommitteeBits.getLastSetBitIndex(); + final int lastCommitteeStartingPosition = + aggregatedCommitteeBitsStartingPositions.get(lastCommitteeIndex); + final int aggregationBitsSize = + lastCommitteeStartingPosition + committeesSize.get(lastCommitteeIndex); + + final BitSet aggregationIndices = new BitSet(aggregationBitsSize); // aggregateBits contains a new set of bits @@ -118,7 +125,7 @@ private boolean or( maybeSource2, source2StartingPosition, isAggregation)) { - aggregationIndices.add(destinationStart + positionInCommittee); + aggregationIndices.set(destinationStart + positionInCommittee); } } }); @@ -126,16 +133,9 @@ private boolean or( return false; } - // create an aggregation bit big as last boundary for last committee bit - final int lastCommitteeIndex = aggregatedCommitteeBits.getLastSetBitIndex(); - final int lastCommitteeStartingPosition = - aggregatedCommitteeBitsStartingPositions.get(lastCommitteeIndex); - final int aggregationBitsSize = - lastCommitteeStartingPosition + committeesSize.get(lastCommitteeIndex); - committeeBits = aggregatedCommitteeBits; aggregationBits = - aggregationBits.getSchema().ofBits(aggregationBitsSize, aggregationIndices.toIntArray()); + aggregationBits.getSchema().wrapBitSet(aggregationBitsSize, aggregationIndices); committeeBitsStartingPositions = aggregatedCommitteeBitsStartingPositions; return true; diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java index 7dab2b056b7..80db17f0051 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java @@ -393,6 +393,45 @@ void bigAggregation() { .isEqualTo(result.getAttestation().getAggregationBits()); } + @Test + void asd() { + committeeSizes = new Int2IntOpenHashMap(); + IntStream.range(0, 64).forEach(i -> committeeSizes.put(i, 600)); + + List committee1 = + IntStream.range(0, 64).mapToObj(__ -> dataStructureUtil.randomPositiveInt(64)).toList(); + int[] aggregation1 = + IntStream.range(0, 600).map(__ -> dataStructureUtil.randomPositiveInt(600)).toArray(); + + List committee2 = + IntStream.range(0, 64).mapToObj(__ -> dataStructureUtil.randomPositiveInt(64)).toList(); + int[] aggregation2 = + IntStream.range(0, 600).map(__ -> dataStructureUtil.randomPositiveInt(600)).toArray(); + + ValidatableAttestation asd = createAttestation(committee1, aggregation1); + + committee2 = committee1.subList(0, committee1.size() - 1); + + ValidatableAttestation asd2 = createAttestation(committee2, aggregation2); + final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(asd); + + long now = System.currentTimeMillis(); + for (int i = 0; i < 100000; i++) { + aggregator.or(asd2.getAttestation()); + } + long finalTime = System.currentTimeMillis() - now; + System.out.println("time: " + finalTime + " ms - ops per millisecond: " + 100000f / finalTime); + + // now = System.currentTimeMillis(); + // for(int i = 0; i < 10000; i++) { + // + // asd2.getAttestation().getAggregationBits().or(asd.getAttestation().getAggregationBits()); + // } + // finalTime = System.currentTimeMillis() - now; + // System.out.println("time: " + finalTime + " ms - ops per millisecond: " + 10000f / + // finalTime); + } + @Test void isSuperSetOf1() { diff --git a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/BitlistImpl.java b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/BitlistImpl.java index a0740e33c80..289fad8436e 100644 --- a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/BitlistImpl.java +++ b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/BitlistImpl.java @@ -51,6 +51,14 @@ public BitlistImpl(final int size, final long maxSize, final int... bitIndices) } } + public BitlistImpl(final int size, final long maxSize, final BitSet bitSet) { + checkArgument(size >= 0, "Negative size"); + checkArgument(maxSize >= size, "maxSize should be >= size"); + this.size = size; + this.data = bitSet; + this.maxSize = maxSize; + } + private BitlistImpl(final int size, final BitSet data, final long maxSize) { this.size = size; this.data = data; diff --git a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/SszBitlistImpl.java b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/SszBitlistImpl.java index 71c2a006d9d..754c19125dd 100644 --- a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/SszBitlistImpl.java +++ b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/collections/impl/SszBitlistImpl.java @@ -16,6 +16,7 @@ import static com.google.common.base.Preconditions.checkArgument; import it.unimi.dsi.fastutil.ints.IntList; +import java.util.BitSet; import java.util.stream.IntStream; import javax.annotation.Nullable; import org.apache.tuweni.bytes.Bytes; @@ -72,6 +73,11 @@ public static SszBitlistImpl ofBits( return new SszBitlistImpl(schema, new BitlistImpl(size, schema.getMaxLength(), bits)); } + public static SszBitlistImpl wrapBitSet( + final SszBitlistSchema schema, final int size, final BitSet bitSet) { + return new SszBitlistImpl(schema, new BitlistImpl(size, schema.getMaxLength(), bitSet)); + } + private final BitlistImpl value; public SszBitlistImpl(final SszListSchema schema, final TreeNode backingNode) { diff --git a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/SszBitlistSchema.java b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/SszBitlistSchema.java index 1e42598eb6b..c3061b6910c 100644 --- a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/SszBitlistSchema.java +++ b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/SszBitlistSchema.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.infrastructure.ssz.schema.collections; +import java.util.BitSet; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; import tech.pegasys.teku.infrastructure.ssz.primitive.SszBit; import tech.pegasys.teku.infrastructure.ssz.schema.collections.impl.SszBitlistSchemaImpl; @@ -29,4 +30,6 @@ default SszBitlistT empty() { } SszBitlistT ofBits(int size, int... setBitIndices); + + SszBitlistT wrapBitSet(int size, BitSet bitSet); } diff --git a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/impl/SszBitlistSchemaImpl.java b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/impl/SszBitlistSchemaImpl.java index 14bbe94353d..66bc8e1ac82 100644 --- a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/impl/SszBitlistSchemaImpl.java +++ b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/impl/SszBitlistSchemaImpl.java @@ -17,6 +17,7 @@ import com.google.common.base.Preconditions; import java.util.ArrayList; +import java.util.BitSet; import java.util.List; import java.util.stream.IntStream; import org.apache.tuweni.bytes.Bytes; @@ -56,6 +57,12 @@ public SszBitlist ofBits(final int size, final int... setBitIndices) { return SszBitlistImpl.ofBits(this, size, setBitIndices); } + @Override + public SszBitlist wrapBitSet(int size, BitSet bitSet) { + Preconditions.checkArgument(size <= getMaxLength(), "size > maxLength"); + return SszBitlistImpl.wrapBitSet(this, size, bitSet); + } + @Override public SszBitlist createFromElements(final List elements) { return ofBits( diff --git a/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitlistTest.java b/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitlistTest.java index 85bdc4c7c39..71a50f5f418 100644 --- a/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitlistTest.java +++ b/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitlistTest.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static tech.pegasys.teku.infrastructure.collections.PrimitiveCollectionAssert.assertThatIntCollection; +import java.util.BitSet; import java.util.OptionalInt; import java.util.Random; import java.util.stream.IntStream; @@ -29,6 +30,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import tech.pegasys.teku.infrastructure.crypto.Hash; +import tech.pegasys.teku.infrastructure.ssz.SszCollection; import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.infrastructure.ssz.SszTestUtils; import tech.pegasys.teku.infrastructure.ssz.impl.AbstractSszPrimitive; @@ -86,11 +88,11 @@ public Stream bitlistArgs() { } public Stream nonEmptyBitlistArgs() { - return sszData().filter(b -> b.size() > 0).map(Arguments::of); + return sszData().filter(b -> !b.isEmpty()).map(Arguments::of); } public Stream emptyBitlistArgs() { - return sszData().filter(b -> b.size() == 0).map(Arguments::of); + return sszData().filter(SszCollection::isEmpty).map(Arguments::of); } @ParameterizedTest @@ -113,6 +115,26 @@ void testSszRoundtrip(SszBitlist bitlist1) { assertThat(ssz2).isEqualTo(ssz1); } + @ParameterizedTest + @MethodSource("bitlistArgs") + void testWrapBitSet(SszBitlist bitlist1) { + final BitSet bits = new BitSet(bitlist1.size()); + + bitlist1.streamAllSetBits().forEach(bits::set); + + final SszBitlist bitlist2 = bitlist1.getSchema().wrapBitSet(bitlist1.size(), bits); + + for (int i = 0; i < bitlist1.size(); i++) { + assertThat(bitlist2.getBit(i)).isEqualTo(bitlist1.getBit(i)); + assertThat(bitlist2.get(i)).isEqualTo(bitlist1.get(i)); + } + + assertThat(bitlist2).isEqualTo(bitlist1); + assertThat(bitlist2.hashCode()).isEqualTo(bitlist1.hashCode()); + assertThat(bitlist2.hashTreeRoot()).isEqualTo(bitlist1.hashTreeRoot()); + assertThat(bitlist2.sszSerialize()).isEqualTo(bitlist1.sszSerialize()); + } + @ParameterizedTest @MethodSource("bitlistArgs") void testTreeRoundtrip(SszBitlist bitlist1) { From f03f780de0d8208cef372d5db0ac9fe1853a8f9a Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 30 May 2024 11:18:25 +0200 Subject: [PATCH 5/7] optimization3 --- .../utils/AttestationBitsAggregatorElectra.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java index ad920570666..22676e3cae9 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java @@ -16,7 +16,6 @@ import com.google.common.base.MoreObjects; import it.unimi.dsi.fastutil.ints.Int2IntMap; import it.unimi.dsi.fastutil.ints.Int2IntOpenHashMap; -import it.unimi.dsi.fastutil.ints.IntList; import java.util.BitSet; import java.util.stream.IntStream; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; @@ -166,12 +165,14 @@ private boolean orSingleBit( private Int2IntMap calculateCommitteeStartingPositions(final SszBitvector committeeBits) { final Int2IntMap committeeBitsStartingPositions = new Int2IntOpenHashMap(); - final IntList committeeIndices = committeeBits.getAllSetBits(); - int currentOffset = 0; - for (final int index : committeeIndices) { - committeeBitsStartingPositions.put(index, currentOffset); - currentOffset += committeesSize.get(index); - } + final int[] currentOffset = {0}; + committeeBits + .streamAllSetBits() + .forEach( + index -> { + committeeBitsStartingPositions.put(index, currentOffset[0]); + currentOffset[0] += committeesSize.get(index); + }); return committeeBitsStartingPositions; } From 50fda0dba3502b1c7fc517585200ae4623db2aa4 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 30 May 2024 12:47:51 +0200 Subject: [PATCH 6/7] improvements --- .../AttestationBitsAggregatorElectraTest.java | 39 ------------------- .../schema/collections/SszBitlistSchema.java | 9 +++++ .../ssz/collections/SszBitlistTest.java | 22 +++++++++++ 3 files changed, 31 insertions(+), 39 deletions(-) diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java index 80db17f0051..7dab2b056b7 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectraTest.java @@ -393,45 +393,6 @@ void bigAggregation() { .isEqualTo(result.getAttestation().getAggregationBits()); } - @Test - void asd() { - committeeSizes = new Int2IntOpenHashMap(); - IntStream.range(0, 64).forEach(i -> committeeSizes.put(i, 600)); - - List committee1 = - IntStream.range(0, 64).mapToObj(__ -> dataStructureUtil.randomPositiveInt(64)).toList(); - int[] aggregation1 = - IntStream.range(0, 600).map(__ -> dataStructureUtil.randomPositiveInt(600)).toArray(); - - List committee2 = - IntStream.range(0, 64).mapToObj(__ -> dataStructureUtil.randomPositiveInt(64)).toList(); - int[] aggregation2 = - IntStream.range(0, 600).map(__ -> dataStructureUtil.randomPositiveInt(600)).toArray(); - - ValidatableAttestation asd = createAttestation(committee1, aggregation1); - - committee2 = committee1.subList(0, committee1.size() - 1); - - ValidatableAttestation asd2 = createAttestation(committee2, aggregation2); - final AttestationBitsAggregator aggregator = AttestationBitsAggregator.of(asd); - - long now = System.currentTimeMillis(); - for (int i = 0; i < 100000; i++) { - aggregator.or(asd2.getAttestation()); - } - long finalTime = System.currentTimeMillis() - now; - System.out.println("time: " + finalTime + " ms - ops per millisecond: " + 100000f / finalTime); - - // now = System.currentTimeMillis(); - // for(int i = 0; i < 10000; i++) { - // - // asd2.getAttestation().getAggregationBits().or(asd.getAttestation().getAggregationBits()); - // } - // finalTime = System.currentTimeMillis() - now; - // System.out.println("time: " + finalTime + " ms - ops per millisecond: " + 10000f / - // finalTime); - } - @Test void isSuperSetOf1() { diff --git a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/SszBitlistSchema.java b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/SszBitlistSchema.java index c3061b6910c..2c4f53750e3 100644 --- a/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/SszBitlistSchema.java +++ b/infrastructure/ssz/src/main/java/tech/pegasys/teku/infrastructure/ssz/schema/collections/SszBitlistSchema.java @@ -31,5 +31,14 @@ default SszBitlistT empty() { SszBitlistT ofBits(int size, int... setBitIndices); + /** + * Creates a SszBitlist by wrapping a given bitSet. This is an optimized constructor that DOES NOT + * clone the bitSet. It used in aggregating attestation pool. DO NOT MUTATE the after the + * wrapping!! SszBitlist is supposed to be immutable. + * + * @param size size of the SszBitlist + * @param bitSet data backing the ssz + * @return SszBitlist instance + */ SszBitlistT wrapBitSet(int size, BitSet bitSet); } diff --git a/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitlistTest.java b/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitlistTest.java index 71a50f5f418..6489373bdae 100644 --- a/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitlistTest.java +++ b/infrastructure/ssz/src/test/java/tech/pegasys/teku/infrastructure/ssz/collections/SszBitlistTest.java @@ -135,6 +135,28 @@ void testWrapBitSet(SszBitlist bitlist1) { assertThat(bitlist2.sszSerialize()).isEqualTo(bitlist1.sszSerialize()); } + @Test + void wrapBitSet_shouldDropBitsIfBitSetIsLarger() { + final BitSet bitSet = new BitSet(100); + bitSet.set(99); + assertThat(bitSet.stream().count()).isEqualTo(1); + + final SszBitlist sszBitlist = schema.wrapBitSet(10, bitSet); + final SszBitlist expectedSszBitlist = schema.ofBits(10); + + assertThat(sszBitlist).isEqualTo(expectedSszBitlist); + assertThat(sszBitlist.hashCode()).isEqualTo(expectedSszBitlist.hashCode()); + assertThat(sszBitlist.hashTreeRoot()).isEqualTo(expectedSszBitlist.hashTreeRoot()); + assertThat(sszBitlist.sszSerialize()).isEqualTo(expectedSszBitlist.sszSerialize()); + } + + @Test + void wrapBitSet_shouldThrowIfSizeIsLargerThanSchemaMaxLength() { + assertThatThrownBy( + () -> schema.wrapBitSet(Math.toIntExact(schema.getMaxLength() + 1), new BitSet())) + .isInstanceOf(IllegalArgumentException.class); + } + @ParameterizedTest @MethodSource("bitlistArgs") void testTreeRoundtrip(SszBitlist bitlist1) { From 93108bc75daab95a4e2c9964f1bb43cf78d1466e Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 30 May 2024 14:45:41 +0200 Subject: [PATCH 7/7] some refactoring and comments --- .../AttestationBitsAggregatorElectra.java | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java index 22676e3cae9..4242922ed8a 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/attestation/utils/AttestationBitsAggregatorElectra.java @@ -69,26 +69,26 @@ private boolean or( final SszBitlist otherAggregatedBits, final boolean isAggregation) { - final SszBitvector aggregatedCommitteeBits = committeeBits.or(otherCommitteeBits); + final SszBitvector combinedCommitteeBits = committeeBits.or(otherCommitteeBits); final Int2IntMap otherCommitteeBitsStartingPositions = calculateCommitteeStartingPositions(otherCommitteeBits); final Int2IntMap aggregatedCommitteeBitsStartingPositions = - calculateCommitteeStartingPositions(aggregatedCommitteeBits); + calculateCommitteeStartingPositions(combinedCommitteeBits); // create an aggregation bit big as last boundary for last committee bit - final int lastCommitteeIndex = aggregatedCommitteeBits.getLastSetBitIndex(); + final int lastCommitteeIndex = combinedCommitteeBits.getLastSetBitIndex(); final int lastCommitteeStartingPosition = aggregatedCommitteeBitsStartingPositions.get(lastCommitteeIndex); - final int aggregationBitsSize = + final int combinedAggregationBitsSize = lastCommitteeStartingPosition + committeesSize.get(lastCommitteeIndex); - final BitSet aggregationIndices = new BitSet(aggregationBitsSize); - - // aggregateBits contains a new set of bits + final BitSet combinedAggregationIndices = new BitSet(combinedAggregationBitsSize); + // let's go over all aggregated committees to calculate indices for the combined aggregation + // bits try { - aggregatedCommitteeBits + combinedCommitteeBits .streamAllSetBits() .forEach( committeeIndex -> { @@ -114,6 +114,10 @@ private boolean or( } } + // Now that we know: + // 1. which aggregationBits (this or other or both) will contribute to the result + // 2. the offset of the committee for each contributing aggregation bits + // We can go over the committee and calculate the combined aggregate bits for (int positionInCommittee = 0; positionInCommittee < committeeSize; positionInCommittee++) { @@ -124,7 +128,7 @@ private boolean or( maybeSource2, source2StartingPosition, isAggregation)) { - aggregationIndices.set(destinationStart + positionInCommittee); + combinedAggregationIndices.set(destinationStart + positionInCommittee); } } }); @@ -132,9 +136,11 @@ private boolean or( return false; } - committeeBits = aggregatedCommitteeBits; + committeeBits = combinedCommitteeBits; aggregationBits = - aggregationBits.getSchema().wrapBitSet(aggregationBitsSize, aggregationIndices); + aggregationBits + .getSchema() + .wrapBitSet(combinedAggregationBitsSize, combinedAggregationIndices); committeeBitsStartingPositions = aggregatedCommitteeBitsStartingPositions; return true; @@ -146,7 +152,7 @@ private boolean orSingleBit( final int source1StartingPosition, final SszBitlist maybeSource2, final int source2StartingPosition, - final boolean isAggregating) { + final boolean isAggregation) { final boolean source1Bit = source1.getBit(source1StartingPosition + positionInCommittee); @@ -156,7 +162,7 @@ private boolean orSingleBit( final boolean source2Bit = maybeSource2.getBit(source2StartingPosition + positionInCommittee); - if (isAggregating && source1Bit && source2Bit) { + if (isAggregation && source1Bit && source2Bit) { throw new CannotAggregateException(); }