From 544984308ef1820294daf3dc0a7b5809ae1983b9 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Wed, 4 Dec 2024 12:13:13 +0100 Subject: [PATCH 01/10] Implements SingleAttestation handling --- .../chains/ThrottlingSyncSource.java | 8 + .../attestation/ValidatableAttestation.java | 28 ++- .../electra/util/AttestationUtilElectra.java | 158 +++++++++++++++ .../common/util/AttestationUtilTest.java | 188 ++++++++++++++++-- .../teku/spec/util/DataStructureUtil.java | 15 +- .../validation/AttestationValidator.java | 25 ++- .../AttestationSubnetSubscriptions.java | 15 +- .../AttestationSubnetSubscriptionsTest.java | 28 ++- .../AttestationProductionDuty.java | 64 +++++- 9 files changed, 492 insertions(+), 37 deletions(-) diff --git a/beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/forward/multipeer/chains/ThrottlingSyncSource.java b/beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/forward/multipeer/chains/ThrottlingSyncSource.java index 2f6ab09a4e5..b8be60d4fb8 100644 --- a/beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/forward/multipeer/chains/ThrottlingSyncSource.java +++ b/beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/forward/multipeer/chains/ThrottlingSyncSource.java @@ -65,6 +65,10 @@ public SafeFuture requestBlocksByRange( LOG.debug("Sending request for {} blocks", count); return delegate.requestBlocksByRange(startSlot, count, listener); } else { + LOG.debug( + "Rate limiting request for {} blocks. Retry in {} seconds", + count, + PEER_REQUEST_DELAY.toSeconds()); return asyncRunner.runAfterDelay( () -> requestBlocksByRange(startSlot, count, listener), PEER_REQUEST_DELAY); } @@ -77,6 +81,10 @@ public SafeFuture requestBlobSidecarsByRange( LOG.debug("Sending request for {} blob sidecars", count); return delegate.requestBlobSidecarsByRange(startSlot, count, listener); } else { + LOG.debug( + "Rate limiting request for {} blob sidecars. Retry in {} seconds", + count, + PEER_REQUEST_DELAY.toSeconds()); return asyncRunner.runAfterDelay( () -> requestBlobSidecarsByRange(startSlot, count, listener), PEER_REQUEST_DELAY); } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java index 04ed1897c58..c0a7c298485 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java @@ -13,6 +13,8 @@ package tech.pegasys.teku.spec.datastructures.attestation; +import static com.google.common.base.Preconditions.checkState; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; @@ -24,6 +26,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import org.apache.tuweni.bytes.Bytes32; +import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.constants.Domain; @@ -31,11 +34,12 @@ import tech.pegasys.teku.spec.datastructures.operations.AttestationData; import tech.pegasys.teku.spec.datastructures.operations.IndexedAttestation; import tech.pegasys.teku.spec.datastructures.operations.SignedAggregateAndProof; +import tech.pegasys.teku.spec.datastructures.operations.versions.electra.AttestationElectraSchema; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; public class ValidatableAttestation { private final Spec spec; - private final Attestation attestation; + private volatile Attestation attestation; private final Optional maybeAggregate; private final Supplier hashTreeRoot; private final AtomicBoolean gossiped = new AtomicBoolean(false); @@ -49,6 +53,28 @@ public class ValidatableAttestation { private volatile Optional committeeShufflingSeed = Optional.empty(); private volatile Optional committeesSize = Optional.empty(); + public void convertFromSingleAttestation(final SszBitlist singleAttestationAggregationBits) { + final Attestation localAttestation = attestation; + checkState(localAttestation.isSingleAttestation()); + + final AttestationElectraSchema attestationElectraSchema = + spec.atSlot(localAttestation.getData().getSlot()) + .getSchemaDefinitions() + .getAttestationSchema() + .toVersionElectra() + .orElseThrow(); + + attestation = + attestationElectraSchema.create( + singleAttestationAggregationBits, + localAttestation.getData(), + localAttestation.getAggregateSignature(), + attestationElectraSchema + .getCommitteeBitsSchema() + .orElseThrow() + .ofBits(localAttestation.getFirstCommitteeIndex().intValue())); + } + public static ValidatableAttestation from(final Spec spec, final Attestation attestation) { return new ValidatableAttestation( spec, attestation, Optional.empty(), OptionalInt.empty(), false); diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java index 943536e7bf6..059b61f4917 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java @@ -13,23 +13,44 @@ package tech.pegasys.teku.spec.logic.versions.electra.util; +import static com.google.common.base.Preconditions.checkArgument; +import static tech.pegasys.teku.infrastructure.async.SafeFuture.completedFuture; + import it.unimi.dsi.fastutil.ints.IntArrayList; import it.unimi.dsi.fastutil.ints.IntList; import java.util.List; +import java.util.Optional; import java.util.stream.IntStream; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.tuweni.bytes.Bytes; +import org.apache.tuweni.bytes.Bytes32; +import tech.pegasys.teku.bls.BLSPublicKey; +import tech.pegasys.teku.bls.BLSSignature; +import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.config.SpecConfig; +import tech.pegasys.teku.spec.constants.Domain; +import tech.pegasys.teku.spec.datastructures.attestation.ValidatableAttestation; import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlockSummary; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationData; +import tech.pegasys.teku.spec.datastructures.operations.IndexedAttestation; +import tech.pegasys.teku.spec.datastructures.operations.IndexedAttestationSchema; +import tech.pegasys.teku.spec.datastructures.operations.SingleAttestation; +import tech.pegasys.teku.spec.datastructures.state.Fork; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; +import tech.pegasys.teku.spec.datastructures.util.AttestationProcessingResult; import tech.pegasys.teku.spec.logic.common.helpers.BeaconStateAccessors; import tech.pegasys.teku.spec.logic.common.helpers.MiscHelpers; +import tech.pegasys.teku.spec.logic.common.util.AsyncBLSSignatureVerifier; import tech.pegasys.teku.spec.logic.versions.deneb.util.AttestationUtilDeneb; import tech.pegasys.teku.spec.schemas.SchemaDefinitions; public class AttestationUtilElectra extends AttestationUtilDeneb { + private static final Logger LOG = LogManager.getLogger(); + public AttestationUtilElectra( final SpecConfig specConfig, final SchemaDefinitions schemaDefinitions, @@ -93,4 +114,141 @@ public AttestationData getGenericAttestationData( final UInt64 committeeIndex) { return super.getGenericAttestationData(slot, state, block, UInt64.ZERO); } + + @Override + public IndexedAttestation getIndexedAttestation( + final BeaconState state, final Attestation attestation) { + if (attestation.isSingleAttestation()) { + return getIndexedAttestationFromSingleAttestation(attestation.toSingleAttestationRequired()); + } + return super.getIndexedAttestation(state, attestation); + } + + private IndexedAttestation getIndexedAttestationFromSingleAttestation( + final SingleAttestation attestation) { + final IndexedAttestationSchema indexedAttestationSchema = + schemaDefinitions.getIndexedAttestationSchema(); + + return indexedAttestationSchema.create( + indexedAttestationSchema + .getAttestingIndicesSchema() + .of(attestation.getValidatorIndexRequired()), + attestation.getData(), + attestation.getSignature()); + } + + @Override + public SafeFuture isValidIndexedAttestationAsync( + final Fork fork, + final BeaconState state, + final ValidatableAttestation attestation, + final AsyncBLSSignatureVerifier blsSignatureVerifier) { + + if (!attestation.getAttestation().isSingleAttestation()) { + // we can use the default implementation for aggregate attestation + return super.isValidIndexedAttestationAsync(fork, state, attestation, blsSignatureVerifier); + } + + // single attestation flow + + // 1. verify signature first + // 2. verify call getSingleAttestationAggregationBits which also validates the validatorIndex + // and the committee against the state + // 3. convert attestation inside ValidatableAttestation to AttestationElectra + // 4. set the indexed attestation into ValidatableAttestation + // 5. set the attestation as valid indexed attestation + + return validateSingleAttestationSignature( + fork, + state, + attestation.getAttestation().toSingleAttestationRequired(), + blsSignatureVerifier) + .thenApply( + result -> { + if (result.isSuccessful()) { + final IndexedAttestation indexedAttestation = + getIndexedAttestationFromSingleAttestation( + attestation.getAttestation().toSingleAttestationRequired()); + + final SszBitlist singleAttestationAggregationBits = + getSingleAttestationAggregationBits(state, attestation.getAttestation()); + + attestation.convertFromSingleAttestation(singleAttestationAggregationBits); + attestation.saveCommitteeShufflingSeedAndCommitteesSize(state); + attestation.setIndexedAttestation(indexedAttestation); + attestation.setValidIndexedAttestation(); + } + return result; + }) + .exceptionallyCompose( + err -> { + if (err.getCause() instanceof IllegalArgumentException) { + LOG.debug("on_attestation: Attestation is not valid: ", err); + return SafeFuture.completedFuture( + AttestationProcessingResult.invalid(err.getMessage())); + } else { + return SafeFuture.failedFuture(err); + } + }); + } + + private SafeFuture validateSingleAttestationSignature( + final Fork fork, + final BeaconState state, + final SingleAttestation singleAttestation, + final AsyncBLSSignatureVerifier signatureVerifier) { + final Optional pubkey = + beaconStateAccessors.getValidatorPubKey( + state, singleAttestation.getValidatorIndexRequired()); + + if (pubkey.isEmpty()) { + return completedFuture( + AttestationProcessingResult.invalid("Attesting index include non-existent validator")); + } + + final BLSSignature signature = singleAttestation.getSignature(); + final Bytes32 domain = + beaconStateAccessors.getDomain( + Domain.BEACON_ATTESTER, + singleAttestation.getData().getTarget().getEpoch(), + fork, + state.getGenesisValidatorsRoot()); + final Bytes signingRoot = miscHelpers.computeSigningRoot(singleAttestation.getData(), domain); + + return signatureVerifier + .verify(pubkey.get(), signingRoot, signature) + .thenApply( + isValidSignature -> { + if (isValidSignature) { + return AttestationProcessingResult.SUCCESSFUL; + } else { + return AttestationProcessingResult.invalid("Signature is invalid"); + } + }); + } + + private SszBitlist getSingleAttestationAggregationBits( + final BeaconState state, final Attestation attestation) { + checkArgument(attestation.isSingleAttestation(), "Expecting single attestation"); + + final IntList committee = + beaconStateAccessors.getBeaconCommittee( + state, attestation.getData().getSlot(), attestation.getFirstCommitteeIndex()); + + int validatorIndex = attestation.getValidatorIndexRequired().intValue(); + + int validatorCommitteeBit = committee.indexOf(validatorIndex); + + checkArgument( + validatorCommitteeBit >= 0, + "Validator index %s is not part of the committee %s", + validatorIndex, + attestation.getFirstCommitteeIndex()); + + return schemaDefinitions + .toVersionElectra() + .orElseThrow() + .getAttestationSchema() + .createAggregationBitsOf(committee.size(), validatorCommitteeBit); + } } diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java index 7d98b7fbc15..6532534020a 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java @@ -16,36 +16,49 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; +import static tech.pegasys.teku.spec.SpecMilestone.DENEB; +import static tech.pegasys.teku.spec.SpecMilestone.ELECTRA; +import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; +import it.unimi.dsi.fastutil.ints.Int2IntOpenHashMap; import it.unimi.dsi.fastutil.ints.IntList; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import java.util.stream.IntStream; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; +import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.bls.BLSSignature; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.ssz.Merkleizable; +import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; -import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.SpecVersion; import tech.pegasys.teku.spec.TestSpecContext; import tech.pegasys.teku.spec.TestSpecInvocationContextProvider.SpecContext; import tech.pegasys.teku.spec.datastructures.attestation.ValidatableAttestation; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.IndexedAttestation; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; import tech.pegasys.teku.spec.datastructures.util.AttestationProcessingResult; import tech.pegasys.teku.spec.logic.common.helpers.BeaconStateAccessors; import tech.pegasys.teku.spec.logic.common.helpers.MiscHelpers; +import tech.pegasys.teku.spec.logic.versions.deneb.util.AttestationUtilDeneb; +import tech.pegasys.teku.spec.logic.versions.electra.util.AttestationUtilElectra; import tech.pegasys.teku.spec.logic.versions.phase0.util.AttestationUtilPhase0; import tech.pegasys.teku.spec.util.DataStructureUtil; -@TestSpecContext(milestone = {SpecMilestone.PHASE0}) +@TestSpecContext(milestone = {PHASE0, DENEB, ELECTRA}) class AttestationUtilTest { private final MiscHelpers miscHelpers = mock(MiscHelpers.class); @@ -63,8 +76,11 @@ void setUp(final SpecContext specContext) { spec = specContext.getSpec(); dataStructureUtil = specContext.getDataStructureUtil(); final SpecVersion specVersion = spec.forMilestone(specContext.getSpecMilestone()); - final IntList beaconCommittee = createBeaconCommittee(specVersion); - when(beaconStateAccessors.getBeaconCommittee(any(), any(), any())).thenReturn(beaconCommittee); + doAnswer(invocation -> createBeaconCommittee(specVersion, invocation.getArgument(2))) + .when(beaconStateAccessors) + .getBeaconCommittee(any(), any(), any()); + when(beaconStateAccessors.getBeaconCommitteesSize(any(), any())) + .thenReturn(new Int2IntOpenHashMap()); when(beaconStateAccessors.getValidatorPubKey(any(), any())) .thenReturn(Optional.of(dataStructureUtil.randomPublicKey())); when(beaconStateAccessors.getDomain(any(), any(), any(), any())) @@ -84,15 +100,29 @@ void setUp(final SpecContext specContext) { beaconStateAccessors, miscHelpers); break; + case DENEB: + attestationUtil = + new AttestationUtilDeneb( + spec.getGenesisSpecConfig(), + specVersion.getSchemaDefinitions(), + beaconStateAccessors, + miscHelpers); + break; + case ELECTRA: + attestationUtil = + new AttestationUtilElectra( + spec.getGenesisSpecConfig(), + specVersion.getSchemaDefinitions(), + beaconStateAccessors, + miscHelpers); + break; default: throw new UnsupportedOperationException("unsupported milestone"); } } @TestTemplate - void noValidationIsDoneIfAttestationIsAlreadyValidAndIndexedAttestationIsPresent( - final SpecContext specContext) { - specContext.assumeIsOneOf(SpecMilestone.PHASE0); + void noValidationIsDoneIfAttestationIsAlreadyValidAndIndexedAttestationIsPresent() { final ValidatableAttestation validatableAttestation = ValidatableAttestation.from(spec, dataStructureUtil.randomAttestation()); validatableAttestation.setValidIndexedAttestation(); @@ -112,10 +142,13 @@ void noValidationIsDoneIfAttestationIsAlreadyValidAndIndexedAttestationIsPresent @TestTemplate void createsAndValidatesIndexedAttestation(final SpecContext specContext) { - specContext.assumeIsOneOf(SpecMilestone.PHASE0); + final Spec mockedSpec = spy(spec); + final Int2IntOpenHashMap committeesSize = new Int2IntOpenHashMap(); + doAnswer(invocation -> committeesSize).when(mockedSpec).getBeaconCommitteesSize(any(), any()); + final Attestation attestation = dataStructureUtil.randomAttestation(); final ValidatableAttestation validatableAttestation = - ValidatableAttestation.from(spec, attestation); + ValidatableAttestation.from(mockedSpec, attestation); final SafeFuture result = executeValidation(validatableAttestation); @@ -125,20 +158,82 @@ void createsAndValidatesIndexedAttestation(final SpecContext specContext) { assertThat(validatableAttestation.isValidIndexedAttestation()).isTrue(); assertThat(validatableAttestation.getIndexedAttestation()).isPresent(); assertThat(validatableAttestation.getCommitteeShufflingSeed()).isPresent(); - assertThat(validatableAttestation.getCommitteesSize()).isEmpty(); + if (specContext.getSpecMilestone().isGreaterThanOrEqualTo(ELECTRA)) { + assertThat(validatableAttestation.getCommitteesSize()).contains(committeesSize); + } else { + assertThat(validatableAttestation.getCommitteesSize()).isEmpty(); + } verify(asyncBLSSignatureVerifier).verify(anyList(), any(Bytes.class), any(BLSSignature.class)); } + @TestTemplate + void createsValidatesIndexedAttestationAndConcertsFromSingleAttestation( + final SpecContext specContext) { + specContext.assumeElectraActive(); + + final Spec mockedSpec = spy(spec); + final Int2IntOpenHashMap committeesSize = new Int2IntOpenHashMap(); + doAnswer(invocation -> committeesSize).when(mockedSpec).getBeaconCommitteesSize(any(), any()); + + final UInt64 committeeIndex = UInt64.valueOf(2); + final UInt64 validatorIndex = UInt64.valueOf(5); + + final Attestation attestation = + dataStructureUtil.randomSingleAttestation(validatorIndex, committeeIndex); + final ValidatableAttestation validatableAttestation = + ValidatableAttestation.fromNetwork(mockedSpec, attestation, 1); + + // single pubkey signature verification + when(asyncBLSSignatureVerifier.verify( + any(BLSPublicKey.class), any(Bytes.class), any(BLSSignature.class))) + .thenReturn(SafeFuture.completedFuture(true)); + + // let validator be the second in the committee + doAnswer(invocation -> IntList.of(validatorIndex.intValue() + 1, validatorIndex.intValue())) + .when(beaconStateAccessors) + .getBeaconCommittee(any(), any(), any()); + + final SafeFuture result = + executeValidation(validatableAttestation); + + assertThat(result).isCompletedWithValue(AttestationProcessingResult.SUCCESSFUL); + + assertThat(validatableAttestation.getAttestation().isSingleAttestation()) + .describedAs("converted to regular attestation") + .isFalse(); + assertThat(validatableAttestation.getAttestation().getAggregationBits().getBitCount()) + .describedAs("Refers to a single validator") + .isEqualTo(1); + assertThat(validatableAttestation.getAttestation().getCommitteeIndicesRequired()) + .describedAs("Refers to the correct committee") + .containsExactly(UInt64.valueOf(2)); + + assertThat(validatableAttestation.isValidIndexedAttestation()).isTrue(); + assertThat(validatableAttestation.getIndexedAttestation()).isPresent(); + assertThat(validatableAttestation.getCommitteeShufflingSeed()).isPresent(); + if (specContext.getSpecMilestone().isGreaterThanOrEqualTo(ELECTRA)) { + assertThat(validatableAttestation.getCommitteesSize()).contains(committeesSize); + } else { + assertThat(validatableAttestation.getCommitteesSize()).isEmpty(); + } + + verify(asyncBLSSignatureVerifier) + .verify(any(BLSPublicKey.class), any(Bytes.class), any(BLSSignature.class)); + } + @TestTemplate void createsButDoesNotValidateIndexedAttestationBecauseItHasAlreadyBeenValidated( final SpecContext specContext) { - specContext.assumeIsOneOf(SpecMilestone.PHASE0); final Attestation attestation = dataStructureUtil.randomAttestation(); + final Spec mockedSpec = spy(spec); + final Int2IntOpenHashMap committeesSize = new Int2IntOpenHashMap(); + doAnswer(invocation -> committeesSize).when(mockedSpec).getBeaconCommitteesSize(any(), any()); + // reorged block does not require indexed attestation validation, however it requires the // creation of it final ValidatableAttestation validatableAttestation = - ValidatableAttestation.fromReorgedBlock(spec, attestation); + ValidatableAttestation.fromReorgedBlock(mockedSpec, attestation); final SafeFuture result = executeValidation(validatableAttestation); @@ -148,11 +243,59 @@ void createsButDoesNotValidateIndexedAttestationBecauseItHasAlreadyBeenValidated assertThat(validatableAttestation.isValidIndexedAttestation()).isTrue(); assertThat(validatableAttestation.getIndexedAttestation()).isPresent(); assertThat(validatableAttestation.getCommitteeShufflingSeed()).isPresent(); - assertThat(validatableAttestation.getCommitteesSize()).isEmpty(); + if (specContext.getSpecMilestone().isGreaterThanOrEqualTo(ELECTRA)) { + assertThat(validatableAttestation.getCommitteesSize()).contains(committeesSize); + } else { + assertThat(validatableAttestation.getCommitteesSize()).isEmpty(); + } verifyNoInteractions(miscHelpers, asyncBLSSignatureVerifier); } + @TestTemplate + void getIndexedAttestationGetsBeaconCommitteeWhenAttestationIsNotSingle( + final SpecContext specContext) { + final Attestation attestation = dataStructureUtil.randomAttestation(); + final BeaconState beaconState = dataStructureUtil.randomBeaconState(); + final IndexedAttestation indexedAttestation = + attestationUtil.getIndexedAttestation(beaconState, attestation); + + if (specContext.getSpecMilestone().isGreaterThanOrEqualTo(ELECTRA)) { + attestation + .getCommitteeIndicesRequired() + .forEach( + index -> + verify(beaconStateAccessors) + .getBeaconCommittee(beaconState, attestation.getData().getSlot(), index)); + } else { + verify(beaconStateAccessors) + .getBeaconCommittee( + beaconState, attestation.getData().getSlot(), attestation.getData().getIndex()); + } + + assertThat(indexedAttestation.getData()).isEqualTo(attestation.getData()); + assertThat(indexedAttestation.getSignature()).isEqualTo(attestation.getAggregateSignature()); + } + + @TestTemplate + void getIndexedAttestationNoQueryToBeaconCommitteeWhenSingleAttestation( + final SpecContext specContext) { + specContext.assumeElectraActive(); + + final Attestation attestation = dataStructureUtil.randomSingleAttestation(); + final BeaconState beaconState = dataStructureUtil.randomBeaconState(); + + final IndexedAttestation indexedAttestation = + attestationUtil.getIndexedAttestation(beaconState, attestation); + + verify(beaconStateAccessors, never()).getBeaconCommittee(any(), any(), any()); + + assertThat(indexedAttestation.getAttestingIndices().streamUnboxed()) + .containsExactly(attestation.getValidatorIndexRequired()); + assertThat(indexedAttestation.getData()).isEqualTo(attestation.getData()); + assertThat(indexedAttestation.getSignature()).isEqualTo(attestation.getAggregateSignature()); + } + private SafeFuture executeValidation( final ValidatableAttestation validatableAttestation) { return attestationUtil.isValidIndexedAttestationAsync( @@ -162,9 +305,24 @@ private SafeFuture executeValidation( asyncBLSSignatureVerifier); } - private IntList createBeaconCommittee(final SpecVersion specVersion) { + static final List KNOWN_COMMITTEES = new ArrayList<>(); + + private IntList createBeaconCommittee( + final SpecVersion specVersion, final UInt64 committeeIndex) { + // we have to generate non overlapping committees, committeeIndex is a random UInt64, so we + // can't use it directly + int position = KNOWN_COMMITTEES.indexOf(committeeIndex); + if (position == -1) { + KNOWN_COMMITTEES.add(committeeIndex); + position = KNOWN_COMMITTEES.size() - 1; + } + + final int maxValidatorsPerCommittee = specVersion.getConfig().getMaxValidatorsPerCommittee(); + final int validatorIndexTranslation = position * maxValidatorsPerCommittee; + final int[] committee = - IntStream.rangeClosed(0, specVersion.getConfig().getMaxValidatorsPerCommittee() - 1) + IntStream.rangeClosed(0, maxValidatorsPerCommittee - 1) + .map(i -> i + validatorIndexTranslation) .toArray(); return IntList.of(committee); } 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 b1f39032a9a..122fbd9e617 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 @@ -825,7 +825,20 @@ public SingleAttestation randomSingleAttestation() { .toVersionElectra() .orElseThrow() .getSingleAttestationSchema() - .create(randomUInt64(), randomUInt64(), randomAttestationData(), randomSignature()); + .create( + randomUInt64(Integer.MAX_VALUE), + randomUInt64(Integer.MAX_VALUE), + randomAttestationData(), + randomSignature()); + } + + public SingleAttestation randomSingleAttestation( + final UInt64 validatorIndex, final UInt64 committeeIndex) { + return spec.getGenesisSchemaDefinitions() + .toVersionElectra() + .orElseThrow() + .getSingleAttestationSchema() + .create(committeeIndex, validatorIndex, randomAttestationData(), randomSignature()); } public Attestation randomAttestation(final long slot) { diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/AttestationValidator.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/AttestationValidator.java index f652f597585..041d1f9cb07 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/AttestationValidator.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/AttestationValidator.java @@ -72,6 +72,11 @@ public SafeFuture validate( } private InternalValidationResult singleAttestationChecks(final Attestation attestation) { + // if it is a SingleAttestation type we are guaranteed to be a valid single attestation + if (attestation.isSingleAttestation()) { + return InternalValidationResult.ACCEPT; + } + // The attestation is unaggregated -- that is, it has exactly one participating validator // (len([bit for bit in attestation.aggregation_bits if bit == 0b1]) == 1). final int bitCount = attestation.getAggregationBits().getBitCount(); @@ -167,15 +172,17 @@ SafeFuture singleOrAggregateAttestationChecks attestation.getFirstCommitteeIndex(), receivedOnSubnetId.getAsInt())); } - // [REJECT] The number of aggregation bits matches the committee size - final IntList committee = - spec.getBeaconCommittee( - state, data.getSlot(), attestation.getFirstCommitteeIndex()); - if (committee.size() != attestation.getAggregationBits().size()) { - return completedFuture( - InternalValidationResultWithState.reject( - "Aggregation bit size %s is greater than committee size %s", - attestation.getAggregationBits().size(), committee.size())); + if (!attestation.isSingleAttestation()) { + // [REJECT] The number of aggregation bits matches the committee size + final IntList committee = + spec.getBeaconCommittee( + state, data.getSlot(), attestation.getFirstCommitteeIndex()); + if (committee.size() != attestation.getAggregationBits().size()) { + return completedFuture( + InternalValidationResultWithState.reject( + "Aggregation bit size %s is greater than committee size %s", + attestation.getAggregationBits().size(), committee.size())); + } } return spec.isValidIndexedAttestation( diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptions.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptions.java index de1231d38c0..6df760cb70a 100644 --- a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptions.java +++ b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptions.java @@ -30,6 +30,8 @@ import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationSchema; import tech.pegasys.teku.spec.datastructures.state.ForkInfo; +import tech.pegasys.teku.spec.schemas.SchemaDefinitions; +import tech.pegasys.teku.spec.schemas.SchemaDefinitionsElectra; import tech.pegasys.teku.statetransition.util.DebugDataDumper; import tech.pegasys.teku.storage.client.RecentChainData; @@ -58,8 +60,14 @@ public AttestationSubnetSubscriptions( this.recentChainData = recentChainData; this.processor = processor; this.forkInfo = forkInfo; + final SchemaDefinitions schemaDefinitions = + spec.atEpoch(forkInfo.getFork().getEpoch()).getSchemaDefinitions(); attestationSchema = - spec.atEpoch(forkInfo.getFork().getEpoch()).getSchemaDefinitions().getAttestationSchema(); + schemaDefinitions + .toVersionElectra() + .>map( + SchemaDefinitionsElectra::getSingleAttestationSchema) + .orElse(schemaDefinitions.getAttestationSchema()); this.debugDataDumper = debugDataDumper; } @@ -110,4 +118,9 @@ private SafeFuture> computeSubnetForAttestation(final Attestat state.map( s -> recentChainData.getSpec().computeSubnetForAttestation(s, attestation))); } + + @VisibleForTesting + AttestationSchema getAttestationSchema() { + return attestationSchema; + } } diff --git a/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptionsTest.java b/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptionsTest.java index 4f9d5256248..aad37fc0c4d 100644 --- a/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptionsTest.java +++ b/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptionsTest.java @@ -37,17 +37,17 @@ import tech.pegasys.teku.spec.datastructures.attestation.ValidatableAttestation; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.util.DataStructureUtil; -import tech.pegasys.teku.statetransition.BeaconChainUtil; import tech.pegasys.teku.statetransition.util.DebugDataDumper; -import tech.pegasys.teku.storage.client.MemoryOnlyRecentChainData; import tech.pegasys.teku.storage.client.RecentChainData; +import tech.pegasys.teku.storage.storageSystem.InMemoryStorageSystemBuilder; +import tech.pegasys.teku.storage.storageSystem.StorageSystem; public class AttestationSubnetSubscriptionsTest { private final Spec spec = TestSpecFactory.createMinimalPhase0(); - + private final StorageSystem storageSystem = InMemoryStorageSystemBuilder.buildDefault(spec); private final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec); private final StubAsyncRunner asyncRunner = new StubAsyncRunner(); - private final RecentChainData recentChainData = MemoryOnlyRecentChainData.create(spec); + private final RecentChainData recentChainData = storageSystem.recentChainData(); private final GossipNetwork gossipNetwork = mock(GossipNetwork.class); private final GossipEncoding gossipEncoding = GossipEncoding.SSZ_SNAPPY; @@ -59,7 +59,7 @@ public class AttestationSubnetSubscriptionsTest { @BeforeEach void setUp() { - BeaconChainUtil.create(spec, 0, recentChainData).initializeStorage(); + storageSystem.chainUpdater().initializeGenesis(); subnetSubscriptions = new AttestationSubnetSubscriptions( spec, @@ -148,6 +148,24 @@ void shouldUnsubscribeFromOnlyCommitteeOnSubnet() { verify(topicChannel).close(); } + @Test + void shouldCreateHandlerForSingleAttestationWhenMilestoneIsElectra() { + final Spec spec = TestSpecFactory.createMinimalElectra(); + final StorageSystem storageSystem = InMemoryStorageSystemBuilder.buildDefault(spec); + storageSystem.chainUpdater().initializeGenesis(); + subnetSubscriptions = + new AttestationSubnetSubscriptions( + spec, + asyncRunner, + gossipNetwork, + gossipEncoding, + storageSystem.recentChainData(), + processor, + storageSystem.recentChainData().getCurrentForkInfo().orElseThrow(), + DebugDataDumper.NOOP); + subnetSubscriptions.getAttestationSchema().toSingleAttestationSchemaRequired(); + } + private int computeSubnetId(final Attestation attestation) { return spec.computeSubnetForAttestation( safeJoin(recentChainData.getBestState().orElseThrow()), attestation); diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java index c6d97eab41e..2e4b3e11bd7 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java @@ -35,7 +35,9 @@ import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationData; import tech.pegasys.teku.spec.datastructures.operations.AttestationSchema; +import tech.pegasys.teku.spec.datastructures.operations.SingleAttestationSchema; import tech.pegasys.teku.spec.datastructures.state.ForkInfo; +import tech.pegasys.teku.spec.schemas.SchemaDefinitions; import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.client.ForkProvider; import tech.pegasys.teku.validator.client.Validator; @@ -136,19 +138,44 @@ private List>> produceAttestationsForCo CREATE_TOTAL); unsignedAttestationFuture.propagateTo(committee.getAttestationDataFuture()); + final SignedAttestationProducer signedAttestationProducer = + selectSignedAttestationProducer(slot); + return committee.getValidators().stream() .map( validator -> signAttestationForValidatorInCommittee( - slot, forkInfo, committeeIndex, validator, unsignedAttestationFuture)) + slot, + forkInfo, + committeeIndex, + validator, + signedAttestationProducer, + unsignedAttestationFuture)) .toList(); } + private SignedAttestationProducer selectSignedAttestationProducer(final UInt64 slot) { + final SchemaDefinitions schemaDefinitions = spec.atSlot(slot).getSchemaDefinitions(); + + return schemaDefinitions + .toVersionElectra() + .map( + schemaDefinitionsElectra -> + (a, b, c) -> + createSignedSingleAttestation( + schemaDefinitionsElectra.getSingleAttestationSchema(), a, b, c)) + .orElseGet( + () -> + (a, b, c) -> + createSignedAttestation(schemaDefinitions.getAttestationSchema(), a, b, c)); + } + private SafeFuture> signAttestationForValidatorInCommittee( final UInt64 slot, final ForkInfo forkInfo, final int committeeIndex, final ValidatorWithAttestationDutyInfo validator, + final SignedAttestationProducer signedAttestationProducer, final SafeFuture> attestationDataFuture) { return attestationDataFuture .thenCompose( @@ -159,7 +186,11 @@ private SafeFuture> signAttestationForValidatorInC validateAttestationData(slot, attestationData); return validatorDutyMetrics.record( () -> - signAttestationForValidator(forkInfo, attestationData, validator), + signAttestationForValidator( + signedAttestationProducer, + forkInfo, + attestationData, + validator), this, ValidatorDutyMetricsSteps.SIGN); }) @@ -187,13 +218,17 @@ private static void validateAttestationData( } private SafeFuture> signAttestationForValidator( + final SignedAttestationProducer signedAttestationProducer, final ForkInfo forkInfo, final AttestationData attestationData, final ValidatorWithAttestationDutyInfo validator) { return validator .signer() .signAttestationData(attestationData, forkInfo) - .thenApply(signature -> createSignedAttestation(attestationData, validator, signature)) + .thenApply( + signature -> + signedAttestationProducer.createSignedAttestation( + attestationData, validator, signature)) .thenApply( attestation -> ProductionResult.success( @@ -201,11 +236,10 @@ private SafeFuture> signAttestationForValidator( } private Attestation createSignedAttestation( + final AttestationSchema attestationSchema, final AttestationData attestationData, final ValidatorWithAttestationDutyInfo validator, final BLSSignature signature) { - final AttestationSchema attestationSchema = - spec.atSlot(attestationData.getSlot()).getSchemaDefinitions().getAttestationSchema(); final SszBitlist aggregationBits = attestationSchema .getAggregationBitsSchema() @@ -220,4 +254,24 @@ private Attestation createSignedAttestation( return attestationSchema.create( aggregationBits, attestationData, signature, committeeBitsSupplier); } + + private Attestation createSignedSingleAttestation( + final SingleAttestationSchema attestationSchema, + final AttestationData attestationData, + final ValidatorWithAttestationDutyInfo validator, + final BLSSignature signature) { + return attestationSchema.create( + UInt64.valueOf(validator.committeeIndex()), + UInt64.valueOf(validator.validatorIndex()), + attestationData, + signature); + } + + @FunctionalInterface + private interface SignedAttestationProducer { + Attestation createSignedAttestation( + final AttestationData attestationData, + final ValidatorWithAttestationDutyInfo validator, + final BLSSignature signature); + } } From 96d307f59b000050d809f5595d5e4fbd7df47a04 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Wed, 4 Dec 2024 17:42:31 +0100 Subject: [PATCH 02/10] fix tests --- .../duties/AttestationProductionDutyTest.java | 134 ++++++++++++++---- 1 file changed, 105 insertions(+), 29 deletions(-) diff --git a/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/AttestationProductionDutyTest.java b/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/AttestationProductionDutyTest.java index 6a2d0a9e215..2d3c3310df7 100644 --- a/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/AttestationProductionDutyTest.java +++ b/validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/AttestationProductionDutyTest.java @@ -38,7 +38,6 @@ import java.util.List; import java.util.Optional; import java.util.Set; -import java.util.function.Supplier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; import org.mockito.ArgumentCaptor; @@ -47,7 +46,6 @@ import tech.pegasys.teku.infrastructure.logging.ValidatorLogger; import tech.pegasys.teku.infrastructure.metrics.StubMetricsSystem; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; -import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.TestSpecContext; @@ -55,7 +53,9 @@ import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationData; import tech.pegasys.teku.spec.datastructures.operations.AttestationSchema; +import tech.pegasys.teku.spec.datastructures.operations.SingleAttestationSchema; import tech.pegasys.teku.spec.datastructures.state.ForkInfo; +import tech.pegasys.teku.spec.schemas.SchemaDefinitions; import tech.pegasys.teku.spec.signatures.Signer; import tech.pegasys.teku.spec.util.DataStructureUtil; import tech.pegasys.teku.validator.api.FileBackedGraffitiProvider; @@ -137,8 +137,10 @@ public void shouldFailWhenUnsignedAttestationCanNotBeCreated() { public void shouldPublishProducedAttestationsWhenSomeUnsignedAttestationsCanNotBeCreated() { final Validator validator1 = createValidator(); final Validator validator2 = createValidator(); + final int validator1Index = 10; final int validator1CommitteeIndex = 0; final int validator1CommitteePosition = 5; + final int validator2Index = 20; final int validator2CommitteeIndex = 1; final int validator2CommitteePosition = 3; final int validator2CommitteeSize = 8; @@ -148,6 +150,7 @@ public void shouldPublishProducedAttestationsWhenSomeUnsignedAttestationsCanNotB final Attestation expectedAttestation = expectSignAttestation( validator2, + validator2Index, validator2CommitteeIndex, validator2CommitteePosition, validator2CommitteeSize, @@ -155,13 +158,13 @@ public void shouldPublishProducedAttestationsWhenSomeUnsignedAttestationsCanNotB final SafeFuture> attestationResult1 = duty.addValidator( - validator1, validator1CommitteeIndex, validator1CommitteePosition, 10, 11); + validator1, validator1CommitteeIndex, validator1CommitteePosition, validator1Index, 11); final SafeFuture> attestationResult2 = duty.addValidator( validator2, validator2CommitteeIndex, validator2CommitteePosition, - 10, + validator2Index, validator2CommitteeSize); performAndReportDuty(); @@ -190,8 +193,10 @@ public void shouldPublishProducedAttestationsWhenSomeUnsignedAttestationsCanNotB public void shouldPublishProducedAttestationsWhenSomeUnsignedAttestationsFail() { final Validator validator1 = createValidator(); final Validator validator2 = createValidator(); + final int validator1Index = 10; final int validator1CommitteeIndex = 0; final int validator1CommitteePosition = 5; + final int validator2Index = 20; final int validator2CommitteeIndex = 1; final int validator2CommitteePosition = 3; final int validator2CommitteeSize = 12; @@ -202,6 +207,7 @@ public void shouldPublishProducedAttestationsWhenSomeUnsignedAttestationsFail() final Attestation expectedAttestation = expectSignAttestation( validator2, + validator2Index, validator2CommitteeIndex, validator2CommitteePosition, validator2CommitteeSize, @@ -209,13 +215,13 @@ public void shouldPublishProducedAttestationsWhenSomeUnsignedAttestationsFail() final SafeFuture> attestationResult1 = duty.addValidator( - validator1, validator1CommitteeIndex, validator1CommitteePosition, 10, 11); + validator1, validator1CommitteeIndex, validator1CommitteePosition, validator1Index, 11); final SafeFuture> attestationResult2 = duty.addValidator( validator2, validator2CommitteeIndex, validator2CommitteePosition, - 10, + validator2Index, validator2CommitteeSize); performAndReportDuty(); @@ -243,8 +249,10 @@ public void shouldPublishProducedAttestationsWhenSignerFailsForSomeAttestations( final Validator validator1 = createValidator(); final Validator validator2 = createValidator(); final int committeeIndex = 0; + final int validator1Index = 10; final int validator1CommitteePosition = 5; final int validator2CommitteePosition = 3; + final int validator2Index = 20; final int validator1CommitteeSize = 23; final int validator2CommitteeSize = 39; final AttestationData attestationData = expectCreateAttestationData(committeeIndex); @@ -254,6 +262,7 @@ public void shouldPublishProducedAttestationsWhenSignerFailsForSomeAttestations( final Attestation expectedAttestation = expectSignAttestation( validator2, + validator2Index, committeeIndex, validator2CommitteePosition, validator2CommitteeSize, @@ -261,10 +270,18 @@ public void shouldPublishProducedAttestationsWhenSignerFailsForSomeAttestations( final SafeFuture> attestationResult1 = duty.addValidator( - validator1, committeeIndex, validator1CommitteePosition, 10, validator1CommitteeSize); + validator1, + committeeIndex, + validator1CommitteePosition, + validator1Index, + validator1CommitteeSize); final SafeFuture> attestationResult2 = duty.addValidator( - validator2, committeeIndex, validator2CommitteePosition, 10, validator2CommitteeSize); + validator2, + committeeIndex, + validator2CommitteePosition, + validator2Index, + validator2CommitteeSize); performAndReportDuty(); assertThat(attestationResult1).isCompletedWithValue(Optional.of(attestationData)); @@ -288,6 +305,7 @@ public void shouldPublishProducedAttestationsWhenSignerFailsForSomeAttestations( @TestTemplate public void shouldCreateAttestationForSingleValidator() { + final int validatorIndex = 10; final int committeeIndex = 3; final int committeePosition = 6; final int committeeSize = 22; @@ -296,10 +314,16 @@ public void shouldCreateAttestationForSingleValidator() { final AttestationData attestationData = expectCreateAttestationData(committeeIndex); final Attestation expectedAttestation = expectSignAttestation( - validator, committeeIndex, committeePosition, committeeSize, attestationData); + validator, + validatorIndex, + committeeIndex, + committeePosition, + committeeSize, + attestationData); final SafeFuture> attestationResult = - duty.addValidator(validator, committeeIndex, committeePosition, 10, committeeSize); + duty.addValidator( + validator, committeeIndex, committeePosition, validatorIndex, committeeSize); performAndReportDuty(); assertThat(attestationResult).isCompletedWithValue(Optional.of(attestationData)); @@ -316,6 +340,7 @@ public void shouldCreateAttestationForSingleValidator() { @TestTemplate void shouldReportFailureWhenAttestationIsInvalid() { + final int validatorIndex = 10; final int committeeIndex = 3; final int committeePosition = 6; final int committeeSize = 22; @@ -324,7 +349,7 @@ void shouldReportFailureWhenAttestationIsInvalid() { final AttestationData attestationData = expectCreateAttestationData(committeeIndex); final Attestation expectedAttestation = expectSignAttestation( - validator, committeeIndex, committeePosition, committeeSize, attestationData); + validator, 10, committeeIndex, committeePosition, committeeSize, attestationData); when(validatorApiChannel.sendSignedAttestations(List.of(expectedAttestation))) .thenReturn( @@ -332,7 +357,8 @@ void shouldReportFailureWhenAttestationIsInvalid() { List.of(new SubmitDataError(UInt64.ZERO, "Naughty attestation")))); final SafeFuture> attestationResult = - duty.addValidator(validator, committeeIndex, committeePosition, 10, committeeSize); + duty.addValidator( + validator, committeeIndex, committeePosition, validatorIndex, committeeSize); performAndReportDuty(); assertThat(attestationResult).isCompletedWithValue(Optional.of(attestationData)); @@ -358,6 +384,9 @@ void shouldReportFailureWhenAttestationIsInvalid() { public void shouldCreateAttestationForMultipleValidatorsInSameCommittee() { final int committeeIndex = 3; final int committeeSize = 33; + final int validator1Index = 10; + final int validator2Index = 20; + final int validator3Index = 30; final int validator1CommitteePosition = 6; final int validator2CommitteePosition = 2; final int validator3CommitteePosition = 5; @@ -369,6 +398,7 @@ public void shouldCreateAttestationForMultipleValidatorsInSameCommittee() { final Attestation expectedAttestation1 = expectSignAttestation( validator1, + validator1Index, committeeIndex, validator1CommitteePosition, committeeSize, @@ -376,6 +406,7 @@ public void shouldCreateAttestationForMultipleValidatorsInSameCommittee() { final Attestation expectedAttestation2 = expectSignAttestation( validator2, + validator2Index, committeeIndex, validator2CommitteePosition, committeeSize, @@ -383,6 +414,7 @@ public void shouldCreateAttestationForMultipleValidatorsInSameCommittee() { final Attestation expectedAttestation3 = expectSignAttestation( validator3, + validator3Index, committeeIndex, validator3CommitteePosition, committeeSize, @@ -390,13 +422,25 @@ public void shouldCreateAttestationForMultipleValidatorsInSameCommittee() { final SafeFuture> attestationResult1 = duty.addValidator( - validator1, committeeIndex, validator1CommitteePosition, 10, committeeSize); + validator1, + committeeIndex, + validator1CommitteePosition, + validator1Index, + committeeSize); final SafeFuture> attestationResult2 = duty.addValidator( - validator2, committeeIndex, validator2CommitteePosition, 10, committeeSize); + validator2, + committeeIndex, + validator2CommitteePosition, + validator2Index, + committeeSize); final SafeFuture> attestationResult3 = duty.addValidator( - validator3, committeeIndex, validator3CommitteePosition, 10, committeeSize); + validator3, + committeeIndex, + validator3CommitteePosition, + validator3Index, + committeeSize); performAndReportDuty(); assertThat(attestationResult1).isCompletedWithValue(Optional.of(attestationData)); assertThat(attestationResult2).isCompletedWithValue(Optional.of(attestationData)); @@ -428,6 +472,9 @@ public void shouldCreateAttestationForMultipleValidatorsInDifferentCommittees() final int committeeIndex2 = 1; final int committeeSize1 = 15; final int committeeSize2 = 20; + final int validator1Index = 10; + final int validator2Index = 20; + final int validator3Index = 30; final int validator1CommitteePosition = 6; final int validator2CommitteePosition = 2; final int validator3CommitteePosition = 5; @@ -440,6 +487,7 @@ public void shouldCreateAttestationForMultipleValidatorsInDifferentCommittees() final Attestation expectedAttestation1 = expectSignAttestation( validator1, + validator1Index, committeeIndex1, validator1CommitteePosition, committeeSize1, @@ -447,6 +495,7 @@ public void shouldCreateAttestationForMultipleValidatorsInDifferentCommittees() final Attestation expectedAttestation2 = expectSignAttestation( validator2, + validator2Index, committeeIndex2, validator2CommitteePosition, committeeSize2, @@ -454,6 +503,7 @@ public void shouldCreateAttestationForMultipleValidatorsInDifferentCommittees() final Attestation expectedAttestation3 = expectSignAttestation( validator3, + validator3Index, committeeIndex1, validator3CommitteePosition, committeeSize1, @@ -461,13 +511,25 @@ public void shouldCreateAttestationForMultipleValidatorsInDifferentCommittees() final SafeFuture> attestationResult1 = duty.addValidator( - validator1, committeeIndex1, validator1CommitteePosition, 10, committeeSize1); + validator1, + committeeIndex1, + validator1CommitteePosition, + validator1Index, + committeeSize1); final SafeFuture> attestationResult2 = duty.addValidator( - validator2, committeeIndex2, validator2CommitteePosition, 10, committeeSize2); + validator2, + committeeIndex2, + validator2CommitteePosition, + validator2Index, + committeeSize2); final SafeFuture> attestationResult3 = duty.addValidator( - validator3, committeeIndex1, validator3CommitteePosition, 10, committeeSize1); + validator3, + committeeIndex1, + validator3CommitteePosition, + validator3Index, + committeeSize1); performAndReportDuty(); assertThat(attestationResult1).isCompletedWithValue(Optional.of(unsignedAttestation1)); @@ -507,6 +569,7 @@ private Validator createValidator() { private Attestation expectSignAttestation( final Validator validator, + final int validatorIndex, final int committeeIndex, final int committeePosition, final int committeeSize, @@ -516,7 +579,12 @@ private Attestation expectSignAttestation( .thenReturn(completedFuture(signature)); return createExpectedAttestation( - attestationData, committeeIndex, committeePosition, committeeSize, signature); + attestationData, + validatorIndex, + committeeIndex, + committeePosition, + committeeSize, + signature); } private AttestationData expectCreateAttestationData(final int committeeIndex) { @@ -528,26 +596,34 @@ private AttestationData expectCreateAttestationData(final int committeeIndex) { private Attestation createExpectedAttestation( final AttestationData attestationData, + final int validatorIndex, final int committeeIndex, final int committeePosition, final int committeeSize, final BLSSignature signature) { + final SchemaDefinitions schemaDefinitions = + spec.atSlot(attestationData.getSlot()).getSchemaDefinitions(); + + if (schemaDefinitions.toVersionElectra().isPresent()) { + final SingleAttestationSchema attestationSchema = + schemaDefinitions.toVersionElectra().orElseThrow().getSingleAttestationSchema(); + + return attestationSchema.create( + UInt64.valueOf(committeeIndex), + UInt64.valueOf(validatorIndex), + attestationData, + signature); + } + + // Phase 0 + final AttestationSchema attestationSchema = spec.atSlot(attestationData.getSlot()).getSchemaDefinitions().getAttestationSchema(); final SszBitlist expectedAggregationBits = attestationSchema.getAggregationBitsSchema().ofBits(committeeSize, committeePosition); - final Supplier committeeBits; - - if (spec.atSlot(attestationData.getSlot()).getMilestone().isGreaterThanOrEqualTo(ELECTRA)) { - committeeBits = - () -> attestationSchema.getCommitteeBitsSchema().orElseThrow().ofBits(committeeIndex); - } else { - committeeBits = () -> null; - } - return attestationSchema.create( - expectedAggregationBits, attestationData, signature, committeeBits); + expectedAggregationBits, attestationData, signature, () -> null); } private void performAndReportDuty() { From 03d2ecd5d093feedb159685cf51544aec8ce26da Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Wed, 4 Dec 2024 18:22:54 +0100 Subject: [PATCH 03/10] tests cleanup --- .../common/util/AttestationUtilTest.java | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java index 6532534020a..b751ce14678 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java @@ -65,6 +65,7 @@ class AttestationUtilTest { private final BeaconStateAccessors beaconStateAccessors = mock(BeaconStateAccessors.class); private final AsyncBLSSignatureVerifier asyncBLSSignatureVerifier = mock(AsyncBLSSignatureVerifier.class); + final Int2IntOpenHashMap committeesSize = new Int2IntOpenHashMap(); private Spec spec; private DataStructureUtil dataStructureUtil; @@ -73,14 +74,13 @@ class AttestationUtilTest { @BeforeEach void setUp(final SpecContext specContext) { - spec = specContext.getSpec(); + spec = spy(specContext.getSpec()); dataStructureUtil = specContext.getDataStructureUtil(); final SpecVersion specVersion = spec.forMilestone(specContext.getSpecMilestone()); doAnswer(invocation -> createBeaconCommittee(specVersion, invocation.getArgument(2))) .when(beaconStateAccessors) .getBeaconCommittee(any(), any(), any()); - when(beaconStateAccessors.getBeaconCommitteesSize(any(), any())) - .thenReturn(new Int2IntOpenHashMap()); + doAnswer(invocation -> committeesSize).when(spec).getBeaconCommitteesSize(any(), any()); when(beaconStateAccessors.getValidatorPubKey(any(), any())) .thenReturn(Optional.of(dataStructureUtil.randomPublicKey())); when(beaconStateAccessors.getDomain(any(), any(), any(), any())) @@ -142,13 +142,9 @@ void noValidationIsDoneIfAttestationIsAlreadyValidAndIndexedAttestationIsPresent @TestTemplate void createsAndValidatesIndexedAttestation(final SpecContext specContext) { - final Spec mockedSpec = spy(spec); - final Int2IntOpenHashMap committeesSize = new Int2IntOpenHashMap(); - doAnswer(invocation -> committeesSize).when(mockedSpec).getBeaconCommitteesSize(any(), any()); - final Attestation attestation = dataStructureUtil.randomAttestation(); final ValidatableAttestation validatableAttestation = - ValidatableAttestation.from(mockedSpec, attestation); + ValidatableAttestation.from(spec, attestation); final SafeFuture result = executeValidation(validatableAttestation); @@ -172,17 +168,13 @@ void createsValidatesIndexedAttestationAndConcertsFromSingleAttestation( final SpecContext specContext) { specContext.assumeElectraActive(); - final Spec mockedSpec = spy(spec); - final Int2IntOpenHashMap committeesSize = new Int2IntOpenHashMap(); - doAnswer(invocation -> committeesSize).when(mockedSpec).getBeaconCommitteesSize(any(), any()); - final UInt64 committeeIndex = UInt64.valueOf(2); final UInt64 validatorIndex = UInt64.valueOf(5); final Attestation attestation = dataStructureUtil.randomSingleAttestation(validatorIndex, committeeIndex); final ValidatableAttestation validatableAttestation = - ValidatableAttestation.fromNetwork(mockedSpec, attestation, 1); + ValidatableAttestation.fromNetwork(spec, attestation, 1); // single pubkey signature verification when(asyncBLSSignatureVerifier.verify( @@ -226,14 +218,10 @@ void createsValidatesIndexedAttestationAndConcertsFromSingleAttestation( void createsButDoesNotValidateIndexedAttestationBecauseItHasAlreadyBeenValidated( final SpecContext specContext) { final Attestation attestation = dataStructureUtil.randomAttestation(); - final Spec mockedSpec = spy(spec); - final Int2IntOpenHashMap committeesSize = new Int2IntOpenHashMap(); - doAnswer(invocation -> committeesSize).when(mockedSpec).getBeaconCommitteesSize(any(), any()); - // reorged block does not require indexed attestation validation, however it requires the // creation of it final ValidatableAttestation validatableAttestation = - ValidatableAttestation.fromReorgedBlock(mockedSpec, attestation); + ValidatableAttestation.fromReorgedBlock(spec, attestation); final SafeFuture result = executeValidation(validatableAttestation); From b19e00c546c5e8b728d9b1bfd3b7fa389cec5a44 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 5 Dec 2024 09:55:29 +0100 Subject: [PATCH 04/10] change conversion approach --- .../v1/events/EventSubscriptionManager.java | 8 ++-- .../attestation/ValidatableAttestation.java | 40 ++++++------------ .../electra/util/AttestationUtilElectra.java | 42 ++++++++++++++----- .../common/util/AttestationUtilTest.java | 8 +++- .../AggregatingAttestationPoolTest.java | 38 +++++++++++++++-- .../MatchingDataAttestationGroupTest.java | 37 +++++++++++++--- 6 files changed, 123 insertions(+), 50 deletions(-) diff --git a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriptionManager.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriptionManager.java index fb0c4238425..640b1c87afc 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriptionManager.java +++ b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriptionManager.java @@ -43,6 +43,7 @@ import tech.pegasys.teku.spec.datastructures.attestation.ValidatableAttestation; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; +import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttesterSlashing; import tech.pegasys.teku.spec.datastructures.operations.ProposerSlashing; import tech.pegasys.teku.spec.datastructures.operations.SignedBlsToExecutionChange; @@ -211,12 +212,13 @@ protected void onSyncCommitteeContribution( } protected void onNewAttestation(final ValidatableAttestation attestation) { - if (!attestation.getAttestation().isSingleAttestation()) { - final AttestationEvent attestationEvent = new AttestationEvent(attestation.getAttestation()); + final Attestation actualAttestation = attestation.getUnconvertedAttestation(); + if (!actualAttestation.isSingleAttestation()) { + final AttestationEvent attestationEvent = new AttestationEvent(actualAttestation); notifySubscribersOfEvent(EventType.attestation, attestationEvent); } else { final SingleAttestationEvent attestationEvent = - new SingleAttestationEvent(attestation.getAttestation().toSingleAttestationRequired()); + new SingleAttestationEvent(actualAttestation.toSingleAttestationRequired()); notifySubscribersOfEvent(EventType.single_attestation, attestationEvent); } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java index c0a7c298485..539c056409a 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java @@ -13,8 +13,6 @@ package tech.pegasys.teku.spec.datastructures.attestation; -import static com.google.common.base.Preconditions.checkState; - import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; @@ -26,7 +24,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import org.apache.tuweni.bytes.Bytes32; -import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.constants.Domain; @@ -34,12 +31,11 @@ import tech.pegasys.teku.spec.datastructures.operations.AttestationData; import tech.pegasys.teku.spec.datastructures.operations.IndexedAttestation; import tech.pegasys.teku.spec.datastructures.operations.SignedAggregateAndProof; -import tech.pegasys.teku.spec.datastructures.operations.versions.electra.AttestationElectraSchema; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; public class ValidatableAttestation { private final Spec spec; - private volatile Attestation attestation; + private final Attestation attestation; private final Optional maybeAggregate; private final Supplier hashTreeRoot; private final AtomicBoolean gossiped = new AtomicBoolean(false); @@ -52,27 +48,11 @@ public class ValidatableAttestation { private volatile Optional indexedAttestation = Optional.empty(); private volatile Optional committeeShufflingSeed = Optional.empty(); private volatile Optional committeesSize = Optional.empty(); + private volatile Optional aggregatedFormatFromSingleAttestation = Optional.empty(); - public void convertFromSingleAttestation(final SszBitlist singleAttestationAggregationBits) { - final Attestation localAttestation = attestation; - checkState(localAttestation.isSingleAttestation()); - - final AttestationElectraSchema attestationElectraSchema = - spec.atSlot(localAttestation.getData().getSlot()) - .getSchemaDefinitions() - .getAttestationSchema() - .toVersionElectra() - .orElseThrow(); - - attestation = - attestationElectraSchema.create( - singleAttestationAggregationBits, - localAttestation.getData(), - localAttestation.getAggregateSignature(), - attestationElectraSchema - .getCommitteeBitsSchema() - .orElseThrow() - .ofBits(localAttestation.getFirstCommitteeIndex().intValue())); + public void setAggregatedFormatFromSingleAttestation( + final Attestation aggregatedFormatFromSingleAttestation) { + this.aggregatedFormatFromSingleAttestation = Optional.of(aggregatedFormatFromSingleAttestation); } public static ValidatableAttestation from(final Spec spec, final Attestation attestation) { @@ -204,7 +184,7 @@ public void saveCommitteeShufflingSeedAndCommitteesSize(final BeaconState state) saveCommitteeShufflingSeed(state); // The committees size is only required when the committee_bits field is present in the // Attestation - if (attestation.requiresCommitteeBits()) { + if (attestation.isSingleAttestation() || attestation.requiresCommitteeBits()) { saveCommitteesSize(state); } } @@ -243,6 +223,10 @@ public boolean isAggregate() { } public Attestation getAttestation() { + return aggregatedFormatFromSingleAttestation.orElse(attestation); + } + + public Attestation getUnconvertedAttestation() { return attestation; } @@ -272,10 +256,9 @@ public boolean equals(final Object o) { if (this == o) { return true; } - if (!(o instanceof ValidatableAttestation)) { + if (!(o instanceof ValidatableAttestation that)) { return false; } - ValidatableAttestation that = (ValidatableAttestation) o; return Objects.equal(getAttestation(), that.getAttestation()) && Objects.equal(maybeAggregate, that.maybeAggregate); } @@ -299,6 +282,7 @@ public String toString() { .add("committeeShufflingSeed", committeeShufflingSeed) .add("committeesSize", committeesSize) .add("receivedSubnetId", receivedSubnetId) + .add("aggregatedFormatFromSingleAttestation", aggregatedFormatFromSingleAttestation) .toString(); } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java index 059b61f4917..d38429ba858 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java @@ -39,6 +39,7 @@ import tech.pegasys.teku.spec.datastructures.operations.IndexedAttestation; import tech.pegasys.teku.spec.datastructures.operations.IndexedAttestationSchema; import tech.pegasys.teku.spec.datastructures.operations.SingleAttestation; +import tech.pegasys.teku.spec.datastructures.operations.versions.electra.AttestationElectraSchema; import tech.pegasys.teku.spec.datastructures.state.Fork; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; import tech.pegasys.teku.spec.datastructures.util.AttestationProcessingResult; @@ -166,14 +167,19 @@ public SafeFuture isValidIndexedAttestationAsync( .thenApply( result -> { if (result.isSuccessful()) { + final SingleAttestation singleAttestation = + attestation.getAttestation().toSingleAttestationRequired(); final IndexedAttestation indexedAttestation = - getIndexedAttestationFromSingleAttestation( - attestation.getAttestation().toSingleAttestationRequired()); + getIndexedAttestationFromSingleAttestation(singleAttestation); final SszBitlist singleAttestationAggregationBits = - getSingleAttestationAggregationBits(state, attestation.getAttestation()); + getSingleAttestationAggregationBits(state, singleAttestation); - attestation.convertFromSingleAttestation(singleAttestationAggregationBits); + final Attestation convertedAttestation = + convertSingleAttestationToAggregated( + singleAttestation, singleAttestationAggregationBits); + + attestation.setAggregatedFormatFromSingleAttestation(convertedAttestation); attestation.saveCommitteeShufflingSeedAndCommitteesSize(state); attestation.setIndexedAttestation(indexedAttestation); attestation.setValidIndexedAttestation(); @@ -192,6 +198,22 @@ public SafeFuture isValidIndexedAttestationAsync( }); } + private Attestation convertSingleAttestationToAggregated( + final SingleAttestation singleAttestation, + final SszBitlist singleAttestationAggregationBits) { + final AttestationElectraSchema attestationElectraSchema = + schemaDefinitions.getAttestationSchema().toVersionElectra().orElseThrow(); + + return attestationElectraSchema.create( + singleAttestationAggregationBits, + singleAttestation.getData(), + singleAttestation.getAggregateSignature(), + attestationElectraSchema + .getCommitteeBitsSchema() + .orElseThrow() + .ofBits(singleAttestation.getFirstCommitteeIndex().intValue())); + } + private SafeFuture validateSingleAttestationSignature( final Fork fork, final BeaconState state, @@ -228,14 +250,14 @@ private SafeFuture validateSingleAttestationSignatu } private SszBitlist getSingleAttestationAggregationBits( - final BeaconState state, final Attestation attestation) { - checkArgument(attestation.isSingleAttestation(), "Expecting single attestation"); - + final BeaconState state, final SingleAttestation singleAttestation) { final IntList committee = beaconStateAccessors.getBeaconCommittee( - state, attestation.getData().getSlot(), attestation.getFirstCommitteeIndex()); + state, + singleAttestation.getData().getSlot(), + singleAttestation.getFirstCommitteeIndex()); - int validatorIndex = attestation.getValidatorIndexRequired().intValue(); + int validatorIndex = singleAttestation.getValidatorIndexRequired().intValue(); int validatorCommitteeBit = committee.indexOf(validatorIndex); @@ -243,7 +265,7 @@ private SszBitlist getSingleAttestationAggregationBits( validatorCommitteeBit >= 0, "Validator index %s is not part of the committee %s", validatorIndex, - attestation.getFirstCommitteeIndex()); + singleAttestation.getFirstCommitteeIndex()); return schemaDefinitions .toVersionElectra() diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java index b751ce14678..c7bd424612c 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java @@ -151,6 +151,9 @@ void createsAndValidatesIndexedAttestation(final SpecContext specContext) { assertThat(result).isCompletedWithValue(AttestationProcessingResult.SUCCESSFUL); + assertThat(validatableAttestation.getAttestation()) + .isSameAs(validatableAttestation.getUnconvertedAttestation()); + assertThat(validatableAttestation.getAttestation().isSingleAttestation()).isFalse(); assertThat(validatableAttestation.isValidIndexedAttestation()).isTrue(); assertThat(validatableAttestation.getIndexedAttestation()).isPresent(); assertThat(validatableAttestation.getCommitteeShufflingSeed()).isPresent(); @@ -191,8 +194,11 @@ void createsValidatesIndexedAttestationAndConcertsFromSingleAttestation( assertThat(result).isCompletedWithValue(AttestationProcessingResult.SUCCESSFUL); + assertThat(validatableAttestation.getUnconvertedAttestation().isSingleAttestation()) + .describedAs("Original is still single attestation") + .isTrue(); assertThat(validatableAttestation.getAttestation().isSingleAttestation()) - .describedAs("converted to regular attestation") + .describedAs("Aggregated format is not single attestation") .isFalse(); assertThat(validatableAttestation.getAttestation().getAggregationBits().getBitCount()) .describedAs("Refers to a single validator") 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 05d82c5c265..354236821d0 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 @@ -51,6 +51,8 @@ import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationData; import tech.pegasys.teku.spec.datastructures.operations.AttestationSchema; +import tech.pegasys.teku.spec.datastructures.operations.SingleAttestation; +import tech.pegasys.teku.spec.datastructures.operations.SingleAttestationSchema; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; import tech.pegasys.teku.spec.logic.common.operations.validation.AttestationDataValidator.AttestationInvalidReason; import tech.pegasys.teku.spec.util.DataStructureUtil; @@ -630,9 +632,24 @@ private Attestation addAttestationFromValidators( final AttestationData data, final Spec spec, final int... validators) { - final Attestation attestation = createAttestation(data, spec, validators); - ValidatableAttestation validatableAttestation = - ValidatableAttestation.from(spec, attestation, committeeSizes); + final Attestation attestationFromValidators; + final Attestation attestation; + if (specMilestone.isGreaterThanOrEqualTo(ELECTRA) && validators.length == 1) { + attestationFromValidators = createSingleAttestation(data, validators[0]); + } else { + attestationFromValidators = createAttestation(data, spec, validators); + } + + final ValidatableAttestation validatableAttestation = + ValidatableAttestation.from(spec, attestationFromValidators, committeeSizes); + + if (attestationFromValidators.isSingleAttestation()) { + attestation = createAttestation(data, spec, validators); + validatableAttestation.setAggregatedFormatFromSingleAttestation(attestation); + } else { + attestation = attestationFromValidators; + } + validatableAttestation.saveCommitteeShufflingSeedAndCommitteesSize( dataStructureUtil.randomBeaconState(100, 15, data.getSlot())); aggregatingAttestationPool.add(validatableAttestation); @@ -643,6 +660,21 @@ private Attestation createAttestation(final AttestationData data, final int... v return createAttestation(data, spec, validators); } + private SingleAttestation createSingleAttestation( + final AttestationData data, final int validatorIndex) { + final SingleAttestationSchema attestationSchema = + spec.getGenesisSchemaDefinitions() + .toVersionElectra() + .orElseThrow() + .getSingleAttestationSchema(); + + return attestationSchema.create( + committeeIndex.orElseThrow(), + UInt64.valueOf(validatorIndex), + data, + dataStructureUtil.randomSignature()); + } + private Attestation createAttestation( final AttestationData data, final Spec spec, final int... validators) { final AttestationSchema attestationSchema = diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java index c19dd5af076..59903d4457d 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java @@ -353,7 +353,27 @@ private ValidatableAttestation createAttestation( final Optional committeeIndex, final int... validators) { final SszBitlist aggregationBits = attestationSchema.getAggregationBitsSchema().ofBits(10, validators); + final boolean isElectra = spec.atSlot(SLOT).getMilestone().isGreaterThanOrEqualTo(ELECTRA); final Supplier committeeBits; + final Optional singleAttestation; + final Attestation attestation; + final int resolvedCommitteeIndex = committeeIndex.orElse(0); + + if (validators.length == 1 && isElectra) { + singleAttestation = + Optional.of( + spec.getGenesisSchemaDefinitions() + .toVersionElectra() + .orElseThrow() + .getSingleAttestationSchema() + .create( + UInt64.valueOf(resolvedCommitteeIndex), + UInt64.valueOf(validators[0]), + attestationData, + dataStructureUtil.randomSignature())); + } else { + singleAttestation = Optional.empty(); + } if (spec.atSlot(SLOT).getMilestone().isGreaterThanOrEqualTo(ELECTRA)) { committeeBits = @@ -361,14 +381,21 @@ private ValidatableAttestation createAttestation( attestationSchema .getCommitteeBitsSchema() .orElseThrow() - .ofBits(committeeIndex.orElse(0)); + .ofBits(resolvedCommitteeIndex); } else { committeeBits = () -> null; } - return ValidatableAttestation.from( - spec, + + attestation = attestationSchema.create( - aggregationBits, attestationData, dataStructureUtil.randomSignature(), committeeBits), - committeeSizes); + aggregationBits, attestationData, dataStructureUtil.randomSignature(), committeeBits); + + final ValidatableAttestation validatableAttestation = + ValidatableAttestation.from(spec, attestation, committeeSizes); + + singleAttestation.ifPresent( + __ -> validatableAttestation.setAggregatedFormatFromSingleAttestation(attestation)); + + return validatableAttestation; } } From 1ae0bf2a964d85a471ed8e9d3d5ca9fe982d9288 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 5 Dec 2024 12:45:34 +0100 Subject: [PATCH 05/10] fix test --- .../attestation/MatchingDataAttestationGroupTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java index 59903d4457d..e593bfea855 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java @@ -356,7 +356,6 @@ private ValidatableAttestation createAttestation( final boolean isElectra = spec.atSlot(SLOT).getMilestone().isGreaterThanOrEqualTo(ELECTRA); final Supplier committeeBits; final Optional singleAttestation; - final Attestation attestation; final int resolvedCommitteeIndex = committeeIndex.orElse(0); if (validators.length == 1 && isElectra) { @@ -386,12 +385,12 @@ private ValidatableAttestation createAttestation( committeeBits = () -> null; } - attestation = + final Attestation attestation = attestationSchema.create( aggregationBits, attestationData, dataStructureUtil.randomSignature(), committeeBits); final ValidatableAttestation validatableAttestation = - ValidatableAttestation.from(spec, attestation, committeeSizes); + ValidatableAttestation.from(spec, singleAttestation.orElse(attestation), committeeSizes); singleAttestation.ifPresent( __ -> validatableAttestation.setAggregatedFormatFromSingleAttestation(attestation)); From c4674d785b9c50802aa5c6a661c1a0cf33544810 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 5 Dec 2024 12:58:05 +0100 Subject: [PATCH 06/10] improvement --- .../attestation/ValidatableAttestation.java | 26 ++++++++++++++----- .../electra/util/AttestationUtilElectra.java | 2 +- .../AggregatingAttestationPoolTest.java | 2 +- .../MatchingDataAttestationGroupTest.java | 2 +- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java index 539c056409a..a276002d22e 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/attestation/ValidatableAttestation.java @@ -13,6 +13,8 @@ package tech.pegasys.teku.spec.datastructures.attestation; +import static com.google.common.base.Preconditions.checkState; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; @@ -24,6 +26,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import org.apache.tuweni.bytes.Bytes32; +import org.jetbrains.annotations.NotNull; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.constants.Domain; @@ -35,24 +38,30 @@ public class ValidatableAttestation { private final Spec spec; - private final Attestation attestation; private final Optional maybeAggregate; private final Supplier hashTreeRoot; private final AtomicBoolean gossiped = new AtomicBoolean(false); private final boolean producedLocally; private final OptionalInt receivedSubnetId; + private final Attestation unconvertedAttestation; + + @NotNull // will help us not forget to initialize if a new constructor is added + private volatile Attestation attestation; + private volatile boolean isValidIndexedAttestation = false; private volatile boolean acceptedAsGossip = false; private volatile Optional indexedAttestation = Optional.empty(); private volatile Optional committeeShufflingSeed = Optional.empty(); private volatile Optional committeesSize = Optional.empty(); - private volatile Optional aggregatedFormatFromSingleAttestation = Optional.empty(); - public void setAggregatedFormatFromSingleAttestation( + public void convertToAggregatedFormatFromSingleAttestation( final Attestation aggregatedFormatFromSingleAttestation) { - this.aggregatedFormatFromSingleAttestation = Optional.of(aggregatedFormatFromSingleAttestation); + checkState( + attestation.isSingleAttestation(), + "Attestation must be a single attestation to convert to aggregated format"); + this.attestation = aggregatedFormatFromSingleAttestation; } public static ValidatableAttestation from(final Spec spec, final Attestation attestation) { @@ -119,6 +128,7 @@ private ValidatableAttestation( this.spec = spec; this.maybeAggregate = aggregateAndProof; this.attestation = attestation; + this.unconvertedAttestation = attestation; this.receivedSubnetId = receivedSubnetId; this.hashTreeRoot = Suppliers.memoize(attestation::hashTreeRoot); this.producedLocally = producedLocally; @@ -134,6 +144,7 @@ private ValidatableAttestation( this.spec = spec; this.maybeAggregate = aggregateAndProof; this.attestation = attestation; + this.unconvertedAttestation = attestation; this.receivedSubnetId = receivedSubnetId; this.hashTreeRoot = Suppliers.memoize(attestation::hashTreeRoot); this.producedLocally = producedLocally; @@ -222,12 +233,13 @@ public boolean isAggregate() { return maybeAggregate.isPresent(); } + @NotNull public Attestation getAttestation() { - return aggregatedFormatFromSingleAttestation.orElse(attestation); + return attestation; } public Attestation getUnconvertedAttestation() { - return attestation; + return unconvertedAttestation; } public SignedAggregateAndProof getSignedAggregateAndProof() { @@ -282,7 +294,7 @@ public String toString() { .add("committeeShufflingSeed", committeeShufflingSeed) .add("committeesSize", committeesSize) .add("receivedSubnetId", receivedSubnetId) - .add("aggregatedFormatFromSingleAttestation", aggregatedFormatFromSingleAttestation) + .add("unconvertedAttestation", unconvertedAttestation) .toString(); } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java index d38429ba858..878d1948fb5 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java @@ -179,7 +179,7 @@ public SafeFuture isValidIndexedAttestationAsync( convertSingleAttestationToAggregated( singleAttestation, singleAttestationAggregationBits); - attestation.setAggregatedFormatFromSingleAttestation(convertedAttestation); + attestation.convertToAggregatedFormatFromSingleAttestation(convertedAttestation); attestation.saveCommitteeShufflingSeedAndCommitteesSize(state); attestation.setIndexedAttestation(indexedAttestation); attestation.setValidIndexedAttestation(); 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 354236821d0..d5f7acae9ad 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 @@ -645,7 +645,7 @@ private Attestation addAttestationFromValidators( if (attestationFromValidators.isSingleAttestation()) { attestation = createAttestation(data, spec, validators); - validatableAttestation.setAggregatedFormatFromSingleAttestation(attestation); + validatableAttestation.convertToAggregatedFormatFromSingleAttestation(attestation); } else { attestation = attestationFromValidators; } diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java index e593bfea855..a85cade50f7 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java @@ -393,7 +393,7 @@ private ValidatableAttestation createAttestation( ValidatableAttestation.from(spec, singleAttestation.orElse(attestation), committeeSizes); singleAttestation.ifPresent( - __ -> validatableAttestation.setAggregatedFormatFromSingleAttestation(attestation)); + __ -> validatableAttestation.convertToAggregatedFormatFromSingleAttestation(attestation)); return validatableAttestation; } From 66dfac10baad88b3ca0cdaa42e288f1cfc556380 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 5 Dec 2024 16:46:56 +0100 Subject: [PATCH 07/10] feedback and test improvement --- .../logic/common/util/AttestationUtil.java | 27 ++++++++++---- .../electra/util/AttestationUtilElectra.java | 35 +++++-------------- .../common/util/AttestationUtilTest.java | 27 +++++++++----- .../AttestationSubnetSubscriptionsTest.java | 4 ++- 4 files changed, 52 insertions(+), 41 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtil.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtil.java index 5da5b45f023..a46f3006e90 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtil.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtil.java @@ -252,24 +252,39 @@ public SafeFuture isValidIndexedAttestationAsync( AttestationProcessingResult.invalid("Attesting indices include non-existent validator")); } - final BLSSignature signature = indexedAttestation.getSignature(); + return validateAttestationDataSignature( + fork, + state, + pubkeys, + indexedAttestation.getSignature(), + indexedAttestation.getData(), + signatureVerifier); + } + + protected SafeFuture validateAttestationDataSignature( + final Fork fork, + final BeaconState state, + final List publicKeys, + final BLSSignature signature, + final AttestationData attestationData, + final AsyncBLSSignatureVerifier signatureVerifier) { + final Bytes32 domain = beaconStateAccessors.getDomain( Domain.BEACON_ATTESTER, - indexedAttestation.getData().getTarget().getEpoch(), + attestationData.getTarget().getEpoch(), fork, state.getGenesisValidatorsRoot()); - final Bytes signingRoot = miscHelpers.computeSigningRoot(indexedAttestation.getData(), domain); + final Bytes signingRoot = miscHelpers.computeSigningRoot(attestationData, domain); return signatureVerifier - .verify(pubkeys, signingRoot, signature) + .verify(publicKeys, signingRoot, signature) .thenApply( isValidSignature -> { if (isValidSignature) { return AttestationProcessingResult.SUCCESSFUL; } else { - LOG.debug( - "AttestationUtil.is_valid_indexed_attestation: Verify aggregate signature"); + LOG.debug("AttestationUtil.validateAttestationSignature: Verify signature"); return AttestationProcessingResult.invalid("Signature is invalid"); } }); diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java index 878d1948fb5..dfa67c18f36 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/electra/util/AttestationUtilElectra.java @@ -23,15 +23,11 @@ import java.util.stream.IntStream; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.tuweni.bytes.Bytes; -import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.bls.BLSPublicKey; -import tech.pegasys.teku.bls.BLSSignature; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitlist; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.config.SpecConfig; -import tech.pegasys.teku.spec.constants.Domain; import tech.pegasys.teku.spec.datastructures.attestation.ValidatableAttestation; import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlockSummary; import tech.pegasys.teku.spec.datastructures.operations.Attestation; @@ -228,25 +224,13 @@ private SafeFuture validateSingleAttestationSignatu AttestationProcessingResult.invalid("Attesting index include non-existent validator")); } - final BLSSignature signature = singleAttestation.getSignature(); - final Bytes32 domain = - beaconStateAccessors.getDomain( - Domain.BEACON_ATTESTER, - singleAttestation.getData().getTarget().getEpoch(), - fork, - state.getGenesisValidatorsRoot()); - final Bytes signingRoot = miscHelpers.computeSigningRoot(singleAttestation.getData(), domain); - - return signatureVerifier - .verify(pubkey.get(), signingRoot, signature) - .thenApply( - isValidSignature -> { - if (isValidSignature) { - return AttestationProcessingResult.SUCCESSFUL; - } else { - return AttestationProcessingResult.invalid("Signature is invalid"); - } - }); + return validateAttestationDataSignature( + fork, + state, + List.of(pubkey.get()), + singleAttestation.getSignature(), + singleAttestation.getData(), + signatureVerifier); } private SszBitlist getSingleAttestationAggregationBits( @@ -257,9 +241,8 @@ private SszBitlist getSingleAttestationAggregationBits( singleAttestation.getData().getSlot(), singleAttestation.getFirstCommitteeIndex()); - int validatorIndex = singleAttestation.getValidatorIndexRequired().intValue(); - - int validatorCommitteeBit = committee.indexOf(validatorIndex); + final int validatorIndex = singleAttestation.getValidatorIndexRequired().intValue(); + final int validatorCommitteeBit = committee.indexOf(validatorIndex); checkArgument( validatorCommitteeBit >= 0, diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java index c7bd424612c..ef831e9fdf0 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java @@ -37,7 +37,6 @@ import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.TestTemplate; -import tech.pegasys.teku.bls.BLSPublicKey; import tech.pegasys.teku.bls.BLSSignature; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.ssz.Merkleizable; @@ -179,10 +178,12 @@ void createsValidatesIndexedAttestationAndConcertsFromSingleAttestation( final ValidatableAttestation validatableAttestation = ValidatableAttestation.fromNetwork(spec, attestation, 1); - // single pubkey signature verification - when(asyncBLSSignatureVerifier.verify( - any(BLSPublicKey.class), any(Bytes.class), any(BLSSignature.class))) - .thenReturn(SafeFuture.completedFuture(true)); + // we want to make sure that we do signature verification before we do the committee lookup, + // that may trigger shuffling calculation + // To do that, let's control signature verification result + final SafeFuture signatureVerificationResult = new SafeFuture<>(); + when(asyncBLSSignatureVerifier.verify(anyList(), any(Bytes.class), any(BLSSignature.class))) + .thenReturn(signatureVerificationResult); // let validator be the second in the committee doAnswer(invocation -> IntList.of(validatorIndex.intValue() + 1, validatorIndex.intValue())) @@ -192,6 +193,19 @@ void createsValidatesIndexedAttestationAndConcertsFromSingleAttestation( final SafeFuture result = executeValidation(validatableAttestation); + // no beacon committee lookup before signature verification + verify(beaconStateAccessors, never()).getBeaconCommittee(any(), any(), any()); + + // validation still in progress + assertThat(result).isNotDone(); + + // signature verification completed + signatureVerificationResult.complete(true); + + // now we should have the beacon committee lookup + verify(beaconStateAccessors).getBeaconCommittee(any(), any(), any()); + + // now we have successful validation assertThat(result).isCompletedWithValue(AttestationProcessingResult.SUCCESSFUL); assertThat(validatableAttestation.getUnconvertedAttestation().isSingleAttestation()) @@ -215,9 +229,6 @@ void createsValidatesIndexedAttestationAndConcertsFromSingleAttestation( } else { assertThat(validatableAttestation.getCommitteesSize()).isEmpty(); } - - verify(asyncBLSSignatureVerifier) - .verify(any(BLSPublicKey.class), any(Bytes.class), any(BLSSignature.class)); } @TestTemplate diff --git a/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptionsTest.java b/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptionsTest.java index aad37fc0c4d..fae952c80e1 100644 --- a/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptionsTest.java +++ b/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/gossip/subnets/AttestationSubnetSubscriptionsTest.java @@ -14,6 +14,7 @@ package tech.pegasys.teku.networking.eth2.gossip.subnets; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.contains; @@ -163,7 +164,8 @@ void shouldCreateHandlerForSingleAttestationWhenMilestoneIsElectra() { processor, storageSystem.recentChainData().getCurrentForkInfo().orElseThrow(), DebugDataDumper.NOOP); - subnetSubscriptions.getAttestationSchema().toSingleAttestationSchemaRequired(); + assertDoesNotThrow( + () -> subnetSubscriptions.getAttestationSchema().toSingleAttestationSchemaRequired()); } private int computeSubnetId(final Attestation attestation) { From 4ac92629ec3e99ddf9b303c45cf3199b14002159 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 5 Dec 2024 16:51:39 +0100 Subject: [PATCH 08/10] additional feedback --- .../attestations/AttestationProductionDuty.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java index 2e4b3e11bd7..1f9661f660b 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/duties/attestations/AttestationProductionDuty.java @@ -161,13 +161,20 @@ private SignedAttestationProducer selectSignedAttestationProducer(final UInt64 s .toVersionElectra() .map( schemaDefinitionsElectra -> - (a, b, c) -> + (attestationData, validator, signature) -> createSignedSingleAttestation( - schemaDefinitionsElectra.getSingleAttestationSchema(), a, b, c)) + schemaDefinitionsElectra.getSingleAttestationSchema(), + attestationData, + validator, + signature)) .orElseGet( () -> - (a, b, c) -> - createSignedAttestation(schemaDefinitions.getAttestationSchema(), a, b, c)); + (attestationData, validator, signature) -> + createSignedAttestation( + schemaDefinitions.getAttestationSchema(), + attestationData, + validator, + signature)); } private SafeFuture> signAttestationForValidatorInCommittee( From f4c838a12ab207ea985bcf81c8aff40960e4fc0b Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 5 Dec 2024 16:54:34 +0100 Subject: [PATCH 09/10] fix debug log --- .../pegasys/teku/spec/logic/common/util/AttestationUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtil.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtil.java index a46f3006e90..ecd8906aa7b 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtil.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtil.java @@ -284,7 +284,7 @@ protected SafeFuture validateAttestationDataSignatu if (isValidSignature) { return AttestationProcessingResult.SUCCESSFUL; } else { - LOG.debug("AttestationUtil.validateAttestationSignature: Verify signature"); + LOG.debug("AttestationUtil.validateAttestationDataSignature: Verify signature"); return AttestationProcessingResult.invalid("Signature is invalid"); } }); From 83338373b531f4ad348301487156229ca7c61699 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 5 Dec 2024 17:05:57 +0100 Subject: [PATCH 10/10] fix typo --- .../teku/spec/logic/common/util/AttestationUtilTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java index ef831e9fdf0..6ff63c43341 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/common/util/AttestationUtilTest.java @@ -166,7 +166,7 @@ void createsAndValidatesIndexedAttestation(final SpecContext specContext) { } @TestTemplate - void createsValidatesIndexedAttestationAndConcertsFromSingleAttestation( + void createsValidatesIndexedAttestationAndConvertsFromSingleAttestation( final SpecContext specContext) { specContext.assumeElectraActive();