From 39ecb9983fbd057335d37b6974f051668f57f8bd Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Fri, 22 Nov 2024 18:54:04 +0100 Subject: [PATCH] tmp change approach --- .../BlockOperationSelectorFactoryTest.java | 6 +- .../handlers/v1/events/EventSubscriber.java | 2 +- .../forkchoice/StubBlobSidecarManager.java | 14 +- .../blobs/versions/deneb/BlobSidecar.java | 19 ++- .../logic/common/helpers/MiscHelpers.java | 14 -- .../BlobSidecarsAndValidationResult.java | 24 +++ .../blobs/BlobSidecarsValidationResult.java | 1 + .../deneb/helpers/MiscHelpersDeneb.java | 143 ++++++------------ .../helpers/MiscHelpersDenebPropertyTest.java | 2 +- .../deneb/helpers/MiscHelpersDenebTest.java | 130 +--------------- .../blobs/BlobSidecarManagerImpl.java | 6 +- .../blobs/BlockBlobSidecarsTracker.java | 4 - .../forkchoice/ForkChoice.java | 17 ++- ...ChoiceBlobSidecarsAvailabilityChecker.java | 20 ++- .../BlockBlobSidecarsTrackersPoolImpl.java | 29 ++-- .../BlobSidecarGossipValidator.java | 11 +- ...BlockBlobSidecarsTrackersPoolImplTest.java | 64 +------- .../BlobSidecarGossipValidatorTest.java | 14 +- .../AbstractBlobSidecarsValidator.java | 2 +- 19 files changed, 168 insertions(+), 354 deletions(-) diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java index cd18566109b..35c58e2a331 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java @@ -801,7 +801,8 @@ void shouldCreateBlobSidecarsForBlockContents() { assertThat(blobSidecar.getSszKZGCommitment()) .isEqualTo(expectedCommitments.get(index)); // verify the merkle proof - assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)).isTrue(); + assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar)) + .isTrue(); }); } @@ -921,7 +922,8 @@ void shouldCreateBlobSidecarsForBlindedBlock(final boolean useLocalFallback) { assertThat(blobSidecar.getSszKZGCommitment()) .isEqualTo(expectedCommitments.get(index)); // verify the merkle proof - assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)).isTrue(); + assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar)) + .isTrue(); }); } diff --git a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriber.java b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriber.java index 420dee5349c..4be2966528c 100644 --- a/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriber.java +++ b/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/events/EventSubscriber.java @@ -45,7 +45,7 @@ public class EventSubscriber { private final AtomicBoolean processingQueue; private final AsyncRunner asyncRunner; private final AtomicLong excessiveQueueingDisconnectionTime = new AtomicLong(Long.MAX_VALUE); - private volatile AtomicInteger successiveFailureCounter = new AtomicInteger(0); + private final AtomicInteger successiveFailureCounter = new AtomicInteger(0); public EventSubscriber( final List eventTypes, diff --git a/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/StubBlobSidecarManager.java b/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/StubBlobSidecarManager.java index a8608be6960..f2d520a1fb4 100644 --- a/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/StubBlobSidecarManager.java +++ b/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/StubBlobSidecarManager.java @@ -95,11 +95,7 @@ public SafeFuture getAvailabilityCheckResult() if (blobsAndProofs == null) { return SafeFuture.completedFuture(BlobSidecarsAndValidationResult.NOT_REQUIRED); } - try { - return SafeFuture.completedFuture(validateImmediately(block, blobsAndProofs)); - } catch (RuntimeException e) { - return SafeFuture.completedFuture(BlobSidecarsAndValidationResult.notAvailable(e)); - } + return SafeFuture.completedFuture(validateImmediately(block, blobsAndProofs)); } private BlobSidecarsAndValidationResult validateImmediately( @@ -111,8 +107,12 @@ private BlobSidecarsAndValidationResult validateImmediately( .map(SszKZGCommitment::getKZGCommitment) .toList(); final List blobs = blobsAndProofs.blobs.stream().map(Blob::getBytes).toList(); - if (!kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, blobsAndProofs.proofs)) { - throw new RuntimeException("Invalid KZG proof"); + try { + if (!kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, blobsAndProofs.proofs)) { + return BlobSidecarsAndValidationResult.invalidResult(Collections.emptyList()); + } + } catch (final Exception ex) { + return BlobSidecarsAndValidationResult.invalidResult(Collections.emptyList(), ex); } return BlobSidecarsAndValidationResult.validResult(Collections.emptyList()); } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blobs/versions/deneb/BlobSidecar.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blobs/versions/deneb/BlobSidecar.java index 0693ce6e016..aae359ffd10 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blobs/versions/deneb/BlobSidecar.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/blobs/versions/deneb/BlobSidecar.java @@ -39,7 +39,8 @@ public class BlobSidecar SignedBeaconBlockHeader, SszBytes32Vector> { - private volatile boolean kzgAndInclusionProofValidated = false; + private volatile boolean kzgValidated = false; + private volatile boolean kzgCommitmentInclusionProofValidated = false; private volatile boolean signatureValidated = false; BlobSidecar(final BlobSidecarSchema blobSidecarSchema, final TreeNode backingTreeNode) { @@ -142,18 +143,26 @@ public String toLogString() { getKZGProof().toAbbreviatedString()); } - public boolean isKzgAndInclusionProofValidated() { - return kzgAndInclusionProofValidated; + public boolean isKzgValidated() { + return kzgValidated; } - public void markKzgAndInclusionProofAsValidated() { - kzgAndInclusionProofValidated = true; + public boolean isKzgCommitmentInclusionProofValidated() { + return kzgCommitmentInclusionProofValidated; } public boolean isSignatureValidated() { return signatureValidated; } + public void markKzgAsValidated() { + kzgValidated = true; + } + + public void markKzgCommitmentInclusionProofAsValidated() { + kzgCommitmentInclusionProofValidated = true; + } + public void markSignatureAsValidated() { signatureValidated = true; } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/helpers/MiscHelpers.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/helpers/MiscHelpers.java index 3ff2ab071bf..1a19c14b10a 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/helpers/MiscHelpers.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/helpers/MiscHelpers.java @@ -366,20 +366,6 @@ public boolean verifyBlobKzgProofBatch(final KZG kzg, final List bl return false; } - public void validateBlobSidecarsBatchAgainstBlock( - final List blobSidecars, - final BeaconBlock block, - final List kzgCommitmentsFromBlock) { - throw new UnsupportedOperationException("No Blob Sidecars before Deneb"); - } - - public void verifyBlobSidecarCompleteness( - final List verifiedBlobSidecars, - final List kzgCommitmentsFromBlock) - throws IllegalArgumentException { - throw new UnsupportedOperationException("No Blob Sidecars before Deneb"); - } - public VersionedHash kzgCommitmentToVersionedHash(final KZGCommitment kzgCommitment) { throw new UnsupportedOperationException("No KZGCommitments before Deneb"); } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsAndValidationResult.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsAndValidationResult.java index 2eebaa18e12..09bdb79ef8e 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsAndValidationResult.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsAndValidationResult.java @@ -43,6 +43,18 @@ public static BlobSidecarsAndValidationResult validResult(final List blobSidecars) { + return new BlobSidecarsAndValidationResult( + BlobSidecarsValidationResult.INVALID, blobSidecars, Optional.empty()); + } + + public static BlobSidecarsAndValidationResult invalidResult( + final List blobSidecars, final Throwable cause) { + return new BlobSidecarsAndValidationResult( + BlobSidecarsValidationResult.INVALID, blobSidecars, Optional.of(cause)); + } + public static BlobSidecarsAndValidationResult notAvailable(final Throwable cause) { return new BlobSidecarsAndValidationResult( BlobSidecarsValidationResult.NOT_AVAILABLE, Collections.emptyList(), Optional.of(cause)); @@ -77,6 +89,18 @@ public boolean isNotRequired() { return validationResult.equals(BlobSidecarsValidationResult.NOT_REQUIRED); } + public boolean isInvalid() { + return validationResult.equals(BlobSidecarsValidationResult.INVALID); + } + + public boolean isNotAvailable() { + return validationResult.equals(BlobSidecarsValidationResult.NOT_AVAILABLE); + } + + public boolean isFailure() { + return isInvalid() || isNotAvailable(); + } + public boolean isSuccess() { return isValid() || isNotRequired(); } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsValidationResult.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsValidationResult.java index 72257425801..e40f5734d30 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsValidationResult.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsValidationResult.java @@ -16,5 +16,6 @@ public enum BlobSidecarsValidationResult { NOT_REQUIRED, NOT_AVAILABLE, + INVALID, VALID } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDeneb.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDeneb.java index c1a09477d72..e7a7592e050 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDeneb.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDeneb.java @@ -13,14 +13,12 @@ package tech.pegasys.teku.spec.logic.versions.deneb.helpers; -import static com.google.common.base.Preconditions.checkArgument; import static tech.pegasys.teku.spec.config.SpecConfigDeneb.VERSIONED_HASH_VERSION_KZG; import java.util.ArrayList; import java.util.List; import java.util.NoSuchElementException; import java.util.Optional; -import java.util.stream.IntStream; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.infrastructure.crypto.Hash; @@ -35,7 +33,6 @@ import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.Blob; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecarSchema; -import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.blockbody.BeaconBlockBody; import tech.pegasys.teku.spec.datastructures.blocks.blockbody.versions.deneb.BeaconBlockBodySchemaDeneb; @@ -84,10 +81,20 @@ public MiscHelpersDeneb( */ @Override public boolean verifyBlobKzgProof(final KZG kzg, final BlobSidecar blobSidecar) { - return kzg.verifyBlobKzgProof( - blobSidecar.getBlob().getBytes(), - blobSidecar.getKZGCommitment(), - blobSidecar.getKZGProof()); + if (blobSidecar.isKzgValidated()) { + return true; + } + final boolean result = + kzg.verifyBlobKzgProof( + blobSidecar.getBlob().getBytes(), + blobSidecar.getKZGCommitment(), + blobSidecar.getKZGProof()); + + if (result) { + blobSidecar.markKzgAsValidated(); + } + + return result; } /** @@ -105,90 +112,28 @@ public boolean verifyBlobKzgProofBatch(final KZG kzg, final List bl final List kzgCommitments = new ArrayList<>(); final List kzgProofs = new ArrayList<>(); - blobSidecars.forEach( - blobSidecar -> { - blobs.add(blobSidecar.getBlob().getBytes()); - kzgCommitments.add(blobSidecar.getKZGCommitment()); - kzgProofs.add(blobSidecar.getKZGProof()); - }); - - return kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs); - } - - /** - * Validates blob sidecars against block. We need to check block root and kzg commitment, it's - * enough to guarantee BlobSidecars belong to block - * - * @param blobSidecars blob sidecars to validate - * @param block block to validate blob sidecar against - * @param kzgCommitmentsFromBlock kzg commitments from block. They could be extracted from block - * but since we already have them we can avoid extracting them again. - */ - @Override - public void validateBlobSidecarsBatchAgainstBlock( - final List blobSidecars, - final BeaconBlock block, - final List kzgCommitmentsFromBlock) { - - final String slotAndBlockRoot = block.getSlotAndBlockRoot().toLogString(); - - blobSidecars.forEach( - blobSidecar -> { - final UInt64 blobIndex = blobSidecar.getIndex(); - - checkArgument( - blobSidecar.getBlockRoot().equals(block.getRoot()), - "Block and blob sidecar root mismatch for %s, blob index %s", - slotAndBlockRoot, - blobIndex); - - final KZGCommitment kzgCommitmentFromBlock; - - try { - kzgCommitmentFromBlock = kzgCommitmentsFromBlock.get(blobIndex.intValue()); - } catch (IndexOutOfBoundsException e) { - throw new IllegalArgumentException( - String.format( - "Blob sidecar index out of bound with respect to block %s, blob index %s", - slotAndBlockRoot, blobIndex)); - } + blobSidecars.stream() + .filter(blobSidecar -> !blobSidecar.isKzgValidated()) + .forEach( + blobSidecar -> { + blobs.add(blobSidecar.getBlob().getBytes()); + kzgCommitments.add(blobSidecar.getKZGCommitment()); + kzgProofs.add(blobSidecar.getKZGProof()); + }); - checkArgument( - blobSidecar.getKZGCommitment().equals(kzgCommitmentFromBlock), - "Block and blob sidecar kzg commitments mismatch for %s, blob index %s", - slotAndBlockRoot, - blobIndex); - }); - } + if (blobs.isEmpty()) { + return true; + } - /** - * Verifies that blob sidecars are complete and with expected indexes - * - * @param completeVerifiedBlobSidecars blob sidecars to verify, It is assumed that it is an - * ordered list based on BlobSidecar index - * @param kzgCommitmentsFromBlock kzg commitments from block. - */ - @Override - public void verifyBlobSidecarCompleteness( - final List completeVerifiedBlobSidecars, - final List kzgCommitmentsFromBlock) - throws IllegalArgumentException { - checkArgument( - completeVerifiedBlobSidecars.size() == kzgCommitmentsFromBlock.size(), - "Blob sidecars are not complete"); + final boolean result = kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs); - IntStream.range(0, completeVerifiedBlobSidecars.size()) - .forEach( - index -> { - final BlobSidecar blobSidecar = completeVerifiedBlobSidecars.get(index); - final UInt64 blobIndex = blobSidecar.getIndex(); + if (result) { + blobSidecars.stream() + .filter(blobSidecar -> !blobSidecar.isKzgValidated()) + .forEach(BlobSidecar::markKzgAsValidated); + } - checkArgument( - blobIndex.longValue() == index, - "Blob sidecar index mismatch, expected %s, got %s", - index, - blobIndex); - }); + return result; } /** @@ -262,12 +207,22 @@ public BlobSidecar constructBlobSidecar( index, blob, commitment, proof, signedBeaconBlock.asHeader(), kzgCommitmentInclusionProof); } - public boolean verifyBlobSidecarMerkleProof(final BlobSidecar blobSidecar) { - return predicates.isValidMerkleBranch( - blobSidecar.getSszKZGCommitment().hashTreeRoot(), - blobSidecar.getKzgCommitmentInclusionProof(), - SpecConfigDeneb.required(specConfig).getKzgCommitmentInclusionProofDepth(), - getBlobSidecarKzgCommitmentGeneralizedIndex(blobSidecar.getIndex()), - blobSidecar.getBlockBodyRoot()); + public boolean verifyBlobKzgCommitmentInclusionProof(final BlobSidecar blobSidecar) { + if (blobSidecar.isKzgCommitmentInclusionProofValidated()) { + return true; + } + final boolean result = + predicates.isValidMerkleBranch( + blobSidecar.getSszKZGCommitment().hashTreeRoot(), + blobSidecar.getKzgCommitmentInclusionProof(), + SpecConfigDeneb.required(specConfig).getKzgCommitmentInclusionProofDepth(), + getBlobSidecarKzgCommitmentGeneralizedIndex(blobSidecar.getIndex()), + blobSidecar.getBlockBodyRoot()); + + if (result) { + blobSidecar.markKzgCommitmentInclusionProofAsValidated(); + } + + return result; } } diff --git a/ethereum/spec/src/property-test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebPropertyTest.java b/ethereum/spec/src/property-test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebPropertyTest.java index f29ee89cd4b..746c802a9d6 100644 --- a/ethereum/spec/src/property-test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebPropertyTest.java +++ b/ethereum/spec/src/property-test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebPropertyTest.java @@ -119,7 +119,7 @@ void fuzzConstructBlobSidecarAndVerifyMerkleProof( try { final BlobSidecar blobSidecar = miscHelpers.constructBlobSidecar(signedBeaconBlock, index, blob, proof); - miscHelpers.verifyBlobSidecarMerkleProof(blobSidecar); + miscHelpers.verifyBlobKzgCommitmentInclusionProof(blobSidecar); } catch (Exception e) { assertThat(e).isInstanceOf(IllegalArgumentException.class); } diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebTest.java index d6712b89470..6fab0c8710d 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/logic/versions/deneb/helpers/MiscHelpersDenebTest.java @@ -18,7 +18,6 @@ import static org.assertj.core.api.Assumptions.assumeThat; import static tech.pegasys.teku.spec.config.SpecConfigDeneb.VERSIONED_HASH_VERSION_KZG; -import java.util.ArrayList; import java.util.List; import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.Test; @@ -69,124 +68,6 @@ public void versionedHash() { assertThat(actual).isEqualTo(VERSIONED_HASH); } - @Test - void validateBlobSidecarsAgainstBlock_shouldNotThrowOnValidBlobSidecar() { - final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(); - final List blobSidecars = dataStructureUtil.randomBlobSidecarsForBlock(block); - - // make sure we are testing something - assertThat(blobSidecars).isNotEmpty(); - - miscHelpersDeneb.validateBlobSidecarsBatchAgainstBlock( - blobSidecars, - block.getMessage(), - BeaconBlockBodyDeneb.required(block.getMessage().getBody()).getBlobKzgCommitments().stream() - .map(SszKZGCommitment::getKZGCommitment) - .toList()); - } - - @Test - void validateBlobSidecarsAgainstBlock_shouldThrowOnNotMatchingBlockRoot() { - - final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(); - - final List kzgCommitments = - BeaconBlockBodyDeneb.required(block.getMessage().getBody()).getBlobKzgCommitments().stream() - .map(SszKZGCommitment::getKZGCommitment) - .toList(); - - // make sure we are testing something - assertThat(kzgCommitments).isNotEmpty(); - - final int indexToBeAltered = - Math.toIntExact(dataStructureUtil.randomPositiveLong(kzgCommitments.size())); - - // let's create blobs with only one altered with the given alteration - final List blobSidecars = - dataStructureUtil.randomBlobSidecarsForBlock( - block, - (index, randomBlobSidecarBuilder) -> { - if (!index.equals(indexToBeAltered)) { - return randomBlobSidecarBuilder; - } - - return randomBlobSidecarBuilder.signedBeaconBlockHeader( - dataStructureUtil.randomSignedBeaconBlockHeader( - block.getSlot(), block.getProposerIndex())); - }); - - assertThatThrownBy( - () -> - miscHelpersDeneb.validateBlobSidecarsBatchAgainstBlock( - blobSidecars, block.getMessage(), kzgCommitments)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Block and blob sidecar root mismatch for") - .hasMessageEndingWith("blob index %s", indexToBeAltered); - } - - @Test - void validateBlobSidecarsAgainstBlock_shouldThrowOnNotMatchingKzgCommitments() { - - final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(); - - final List kzgCommitments = - BeaconBlockBodyDeneb.required(block.getMessage().getBody()).getBlobKzgCommitments().stream() - .map(SszKZGCommitment::getKZGCommitment) - .toList(); - - // make sure we are testing something - assertThat(kzgCommitments).isNotEmpty(); - - final int indexToBeAltered = - Math.toIntExact(dataStructureUtil.randomPositiveLong(kzgCommitments.size())); - final List alteredKzgCommitments = new ArrayList<>(kzgCommitments); - alteredKzgCommitments.remove(indexToBeAltered); - alteredKzgCommitments.add(indexToBeAltered, dataStructureUtil.randomKZGCommitment()); - - final List blobSidecars = dataStructureUtil.randomBlobSidecarsForBlock(block); - - assertThatThrownBy( - () -> - miscHelpersDeneb.validateBlobSidecarsBatchAgainstBlock( - blobSidecars, block.getMessage(), List.of())) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Blob sidecar index out of bound with respect to block") - .hasMessageEndingWith("blob index %s", 0); - - assertThatThrownBy( - () -> - miscHelpersDeneb.validateBlobSidecarsBatchAgainstBlock( - blobSidecars, block.getMessage(), alteredKzgCommitments)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Block and blob sidecar kzg commitments mismatch for") - .hasMessageEndingWith("blob index %s", indexToBeAltered); - } - - @Test - void verifyBlobSidecarCompleteness_shouldThrowWhenSizesDoNotMatch() { - assertThatThrownBy( - () -> - miscHelpersDeneb.verifyBlobSidecarCompleteness( - dataStructureUtil.randomBlobSidecars(1), - List.of( - dataStructureUtil.randomKZGCommitment(), - dataStructureUtil.randomKZGCommitment()))) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Blob sidecars are not complete"); - } - - @Test - void verifyBlobSidecarCompleteness_shouldThrowWhenBlobSidecarIndexIsWrong() { - final List blobSidecars = dataStructureUtil.randomBlobSidecars(1); - assertThatThrownBy( - () -> - miscHelpersDeneb.verifyBlobSidecarCompleteness( - blobSidecars, List.of(dataStructureUtil.randomKZGCommitment()))) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Blob sidecar index mismatch, expected 0, got %s", blobSidecars.get(0).getIndex()); - } - @Test void shouldConstructValidBlobSidecar() { final SignedBeaconBlock signedBeaconBlock = @@ -207,7 +88,7 @@ void shouldConstructValidBlobSidecar() { assertThat(blobSidecar.getSszKZGCommitment()).isEqualTo(expectedCommitment); assertThat(blobSidecar.getSignedBeaconBlockHeader()).isEqualTo(signedBeaconBlock.asHeader()); // verify the merkle proof - assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)).isTrue(); + assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar)).isTrue(); } @Test @@ -227,7 +108,7 @@ void shouldThrowWhenConstructingBlobSidecarWithInvalidIndex() { } @Test - void verifyBlobSidecarMerkleProofShouldValidate() { + void verifyBlobKzgCommitmentInclusionProofShouldValidate() { final int numberOfCommitments = 4; final BeaconBlockBodyDeneb beaconBlockBody = BeaconBlockBodyDeneb.required( @@ -266,7 +147,7 @@ void verifyBlobSidecarMerkleProofShouldValidate() { .getBytes()) .kzgCommitmentInclusionProof(merkleProof) .build(); - assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)).isTrue(); + assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar)).isTrue(); // And the same blobSidecar but with wrong merkle proof for (int j = 0; j < numberOfCommitments; ++j) { @@ -295,8 +176,11 @@ void verifyBlobSidecarMerkleProofShouldValidate() { .getBytes()) .kzgCommitmentInclusionProof(merkleProofWrong) .build(); - assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecarWrong)).isFalse(); + assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecarWrong)) + .isFalse(); } } } + + // TODO test mark as validated } diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlobSidecarManagerImpl.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlobSidecarManagerImpl.java index 610529ba125..8cd875f4aea 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlobSidecarManagerImpl.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlobSidecarManagerImpl.java @@ -20,6 +20,7 @@ import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.subscribers.Subscribers; import tech.pegasys.teku.infrastructure.unsigned.UInt64; +import tech.pegasys.teku.kzg.KZG; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; @@ -35,6 +36,7 @@ public class BlobSidecarManagerImpl implements BlobSidecarManager, SlotEventsCha private final Spec spec; private final RecentChainData recentChainData; private final BlobSidecarGossipValidator validator; + private final KZG kzg; private final BlockBlobSidecarsTrackersPool blockBlobSidecarsTrackersPool; private final FutureItems futureBlobSidecars; private final Map invalidBlobSidecarRoots; @@ -47,11 +49,13 @@ public BlobSidecarManagerImpl( final RecentChainData recentChainData, final BlockBlobSidecarsTrackersPool blockBlobSidecarsTrackersPool, final BlobSidecarGossipValidator validator, + final KZG kzg, final FutureItems futureBlobSidecars, final Map invalidBlobSidecarRoots) { this.spec = spec; this.recentChainData = recentChainData; this.validator = validator; + this.kzg = kzg; this.blockBlobSidecarsTrackersPool = blockBlobSidecarsTrackersPool; this.futureBlobSidecars = futureBlobSidecars; this.invalidBlobSidecarRoots = invalidBlobSidecarRoots; @@ -119,7 +123,7 @@ public BlobSidecarsAvailabilityChecker createAvailabilityChecker(final SignedBea blockBlobSidecarsTrackersPool.getOrCreateBlockBlobSidecarsTracker(block); return new ForkChoiceBlobSidecarsAvailabilityChecker( - spec, recentChainData, blockBlobSidecarsTracker); + spec, recentChainData, blockBlobSidecarsTracker, kzg); } @Override diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTracker.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTracker.java index a9dc3c9cadc..721211524f3 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTracker.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTracker.java @@ -142,10 +142,6 @@ public boolean add(final BlobSidecar blobSidecar) { blobSidecar.getBlockRoot().equals(slotAndBlockRoot.getBlockRoot()), "Wrong blobSidecar block root"); - checkArgument( - blobSidecar.isKzgAndInclusionProofValidated(), - "BlobSidecar must be validated before adding"); - if (blobSidecarsComplete.isDone()) { // already completed return false; diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java index 05660d23cb5..bb31b9e2205 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoice.java @@ -71,7 +71,6 @@ import tech.pegasys.teku.spec.logic.common.util.ForkChoiceUtil; import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAndValidationResult; import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAvailabilityChecker; -import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsValidationResult; import tech.pegasys.teku.statetransition.attestation.DeferredAttestations; import tech.pegasys.teku.statetransition.blobs.BlobSidecarManager; import tech.pegasys.teku.statetransition.block.BlockImportPerformance; @@ -543,10 +542,18 @@ private BlockImportResult importBlockAndState( LOG.debug("blobSidecars validation result: {}", blobSidecarsAndValidationResult::toLogString); - if (blobSidecarsAndValidationResult.getValidationResult() - == BlobSidecarsValidationResult.NOT_AVAILABLE) { - return BlockImportResult.failedDataAvailabilityCheckNotAvailable( - blobSidecarsAndValidationResult.getCause()); + switch (blobSidecarsAndValidationResult.getValidationResult()) { + case NOT_AVAILABLE -> { + return BlockImportResult.failedDataAvailabilityCheckNotAvailable( + blobSidecarsAndValidationResult.getCause()); + } + case INVALID -> { + debugDataDumper.saveInvalidBlobSidecars( + blobSidecarsAndValidationResult.getBlobSidecars(), block); + return BlockImportResult.failedDataAvailabilityCheckInvalid( + blobSidecarsAndValidationResult.getCause()); + } + default -> {} } final ForkChoiceStrategy forkChoiceStrategy = getForkChoiceStrategy(); diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceBlobSidecarsAvailabilityChecker.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceBlobSidecarsAvailabilityChecker.java index cdff49d3963..f6f8f8713b1 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceBlobSidecarsAvailabilityChecker.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceBlobSidecarsAvailabilityChecker.java @@ -20,6 +20,7 @@ import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.exceptions.ExceptionUtil; import tech.pegasys.teku.infrastructure.unsigned.UInt64; +import tech.pegasys.teku.kzg.KZG; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; @@ -36,6 +37,7 @@ public class ForkChoiceBlobSidecarsAvailabilityChecker implements BlobSidecarsAv private final Spec spec; private final RecentChainData recentChainData; private final BlockBlobSidecarsTracker blockBlobSidecarsTracker; + private final KZG kzg; private final SafeFuture validationResult = new SafeFuture<>(); @@ -44,10 +46,12 @@ public class ForkChoiceBlobSidecarsAvailabilityChecker implements BlobSidecarsAv public ForkChoiceBlobSidecarsAvailabilityChecker( final Spec spec, final RecentChainData recentChainData, - final BlockBlobSidecarsTracker blockBlobSidecarsTracker) { + final BlockBlobSidecarsTracker blockBlobSidecarsTracker, + final KZG kzg) { this.spec = spec; this.recentChainData = recentChainData; this.blockBlobSidecarsTracker = blockBlobSidecarsTracker; + this.kzg = kzg; this.waitForTrackerCompletionTimeout = calculateCompletionTimeout(spec, blockBlobSidecarsTracker.getSlotAndBlockRoot().getSlot()); } @@ -57,10 +61,12 @@ public ForkChoiceBlobSidecarsAvailabilityChecker( final Spec spec, final RecentChainData recentChainData, final BlockBlobSidecarsTracker blockBlobSidecarsTracker, + final KZG kzg, final Duration waitForTrackerCompletionTimeout) { this.spec = spec; this.recentChainData = recentChainData; this.blockBlobSidecarsTracker = blockBlobSidecarsTracker; + this.kzg = kzg; this.waitForTrackerCompletionTimeout = waitForTrackerCompletionTimeout; } @@ -92,11 +98,15 @@ private BlobSidecarsAndValidationResult validateCompletedBlobSidecars() { List.copyOf(blockBlobSidecarsTracker.getBlobSidecars().values()); final SignedBeaconBlock block = blockBlobSidecarsTracker.getBlock().orElseThrow(); - for (final BlobSidecar blobSidecar : blobSidecars) { - if (!blobSidecar.isKzgAndInclusionProofValidated()) { - return BlobSidecarsAndValidationResult.notAvailable( - new IllegalStateException("Blob sidecars are not validated")); + try { + if (!spec.atSlot(block.getSlot()).miscHelpers().verifyBlobKzgProofBatch(kzg, blobSidecars)) { + return BlobSidecarsAndValidationResult.invalidResult(blobSidecars); } + } catch (final Exception ex) { + return BlobSidecarsAndValidationResult.invalidResult(blobSidecars, ex); + } + + for (final BlobSidecar blobSidecar : blobSidecars) { if (!blobSidecar.isSignatureValidated() && !blobSidecar .getSignedBeaconBlockHeader() diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/BlockBlobSidecarsTrackersPoolImpl.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/BlockBlobSidecarsTrackersPoolImpl.java index fc31381b744..4d9e376406b 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/BlockBlobSidecarsTrackersPoolImpl.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/util/BlockBlobSidecarsTrackersPoolImpl.java @@ -720,11 +720,6 @@ private synchronized SafeFuture fetchMissingContentFromLocalEL( blobAndProof.get(), beaconBlockBodyDeneb, signedBeaconBlockHeader); - - blobSidecar.markSignatureAsValidated(); - // assume kzg validation done by local EL - blobSidecar.markKzgAndInclusionProofAsValidated(); - onNewBlobSidecar(blobSidecar, LOCAL_EL); } }); @@ -741,14 +736,22 @@ private BlobSidecar createBlobSidecarFromBlobAndProof( final SszKZGCommitment sszKZGCommitment = beaconBlockBodyDeneb.getBlobKzgCommitments().get(blobIdentifier.getIndex().intValue()); - return blobSidecarSchema.create( - blobIdentifier.getIndex(), - blobAndProof.blob(), - sszKZGCommitment, - new SszKZGProof(blobAndProof.proof()), - signedBeaconBlockHeader, - miscHelpersDeneb.computeKzgCommitmentInclusionProof( - blobIdentifier.getIndex(), beaconBlockBodyDeneb)); + final BlobSidecar blobSidecar = + blobSidecarSchema.create( + blobIdentifier.getIndex(), + blobAndProof.blob(), + sszKZGCommitment, + new SszKZGProof(blobAndProof.proof()), + signedBeaconBlockHeader, + miscHelpersDeneb.computeKzgCommitmentInclusionProof( + blobIdentifier.getIndex(), beaconBlockBodyDeneb)); + + blobSidecar.markSignatureAsValidated(); + blobSidecar.markKzgCommitmentInclusionProofAsValidated(); + // assume kzg validation done by local EL + blobSidecar.markKzgAsValidated(); + + return blobSidecar; } private synchronized void fetchMissingContentFromRemotePeers( diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidator.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidator.java index b13094e11d1..f058d8e2375 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidator.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidator.java @@ -159,7 +159,6 @@ public SafeFuture validate(final BlobSidecar blobSidec // Optimization: If we have already completely verified BlobSidecar with the same // SignedBlockHeader, we can skip most steps and jump to shortened validation if (validSignedBlockHeaders.contains(blobSidecar.getSignedBeaconBlockHeader().hashTreeRoot())) { - blobSidecar.markSignatureAsValidated(); return validateBlobSidecarWithKnownValidHeader(blobSidecar, blockHeader); } @@ -214,7 +213,7 @@ public SafeFuture validate(final BlobSidecar blobSidec /* * [REJECT] The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof(blob_sidecar)`. */ - if (!miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)) { + if (!miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar)) { return completedFuture(reject("BlobSidecar inclusion proof validation failed")); } @@ -226,8 +225,6 @@ public SafeFuture validate(final BlobSidecar blobSidec return completedFuture(reject("BlobSidecar does not pass kzg validation")); } - blobSidecar.markKzgAndInclusionProofAsValidated(); - return gossipValidationHelper .getParentStateInBlockEpoch( parentBlockSlot, blockHeader.getParentRoot(), blockHeader.getSlot()) @@ -263,8 +260,6 @@ public SafeFuture validate(final BlobSidecar blobSidec return reject("BlobSidecar block header signature is invalid."); } - blobSidecar.markSignatureAsValidated(); - /* * Checking it again at the very end because whole method is not synchronized * @@ -299,7 +294,7 @@ private SafeFuture validateBlobSidecarWithKnownValidHe /* * [REJECT] The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof(blob_sidecar)`. */ - if (!miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)) { + if (!miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar)) { return completedFuture(reject("BlobSidecar inclusion proof validation failed")); } @@ -322,8 +317,6 @@ private SafeFuture validateBlobSidecarWithKnownValidHe reject("BlobSidecar block header does not descend from finalized checkpoint")); } - blobSidecar.markKzgAndInclusionProofAsValidated(); - /* * [IGNORE] The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, blob_sidecar.index) * with valid header signature, sidecar inclusion proof, and kzg proof. diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/BlockBlobSidecarsTrackersPoolImplTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/BlockBlobSidecarsTrackersPoolImplTest.java index ee8b0a6906b..2dad121a9eb 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/BlockBlobSidecarsTrackersPoolImplTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/util/BlockBlobSidecarsTrackersPoolImplTest.java @@ -14,9 +14,7 @@ package tech.pegasys.teku.statetransition.util; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -156,21 +154,6 @@ public void onNewBlock_addTrackerWithBlock() { assertBlobSidecarsTrackersCount(1); } - @Test - public void onNewBlobSidecar_shouldThrowIfNotMarkedKzgAndInclusionProofAsValidated() { - final BlobSidecar blobSidecar = - dataStructureUtil - .createRandomBlobSidecarBuilder() - .signedBeaconBlockHeader(dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot)) - .build(); - - assertThat(blobSidecar.isKzgAndInclusionProofValidated()).isFalse(); - - assertThrows( - IllegalArgumentException.class, - () -> blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar, RemoteOrigin.GOSSIP)); - } - @Test public void onNewBlobSidecar_addTrackerWithBlobSidecarIgnoringDuplicates() { final BlobSidecar blobSidecar = @@ -179,8 +162,6 @@ public void onNewBlobSidecar_addTrackerWithBlobSidecarIgnoringDuplicates() { .signedBeaconBlockHeader(dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot)) .build(); - blobSidecar.markKzgAndInclusionProofAsValidated(); - blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar, RemoteOrigin.GOSSIP); assertThat( @@ -209,8 +190,6 @@ public void onNewBlobSidecar_shouldIgnoreDuplicates() { .signedBeaconBlockHeader(dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot)) .build(); - blobSidecar.markKzgAndInclusionProofAsValidated(); - blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar, RemoteOrigin.GOSSIP); blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar, RemoteOrigin.GOSSIP); @@ -246,10 +225,6 @@ public void onNewBlobSidecar_shouldMarkForEquivocationAndPublishWhenOriginIsLoca .signedBeaconBlockHeader(dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot)) .build(); - blobSidecar1.markKzgAndInclusionProofAsValidated(); - blobSidecar2.markKzgAndInclusionProofAsValidated(); - blobSidecar3.markKzgAndInclusionProofAsValidated(); - when(blobSidecarGossipValidator.markForEquivocation(blobSidecar1)).thenReturn(true); blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar1, RemoteOrigin.LOCAL_EL); @@ -271,8 +246,6 @@ public void onNewBlobSidecar_shouldPublishWhenOriginIsLocalELAndEquivocating() { .signedBeaconBlockHeader(dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot)) .build(); - blobSidecar1.markKzgAndInclusionProofAsValidated(); - when(blobSidecarGossipValidator.markForEquivocation(blobSidecar1)).thenReturn(false); blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar1, RemoteOrigin.LOCAL_EL); @@ -292,8 +265,6 @@ public void onNewBlobSidecar_shouldNotPublishWhenOriginIsLocalELIsNotCurrentSlot .signedBeaconBlockHeader(dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot)) .build(); - blobSidecar1.markKzgAndInclusionProofAsValidated(); - when(blobSidecarGossipValidator.markForEquivocation(blobSidecar1)).thenReturn(false); blockBlobSidecarsTrackersPool.onSlot(currentSlot.plus(1)); @@ -381,8 +352,6 @@ public void onNewBlobSidecarOnNewBlock_addTrackerWithBothBlockAndBlobSidecar() { final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(currentSlot); final BlobSidecar blobSidecar = dataStructureUtil.randomBlobSidecarsForBlock(block).get(0); - blobSidecar.markKzgAndInclusionProofAsValidated(); - blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar, RemoteOrigin.GOSSIP); blockBlobSidecarsTrackersPool.onNewBlock(block, Optional.empty()); @@ -425,10 +394,6 @@ public void twoOnNewBlobSidecar_addTrackerWithBothBlobSidecars() { .index(UInt64.ONE) .build(); - blobSidecar0.markKzgAndInclusionProofAsValidated(); - blobSidecar1.markKzgAndInclusionProofAsValidated(); - blobSidecar1bis.markKzgAndInclusionProofAsValidated(); - blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar0, RemoteOrigin.GOSSIP); blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar1, RemoteOrigin.GOSSIP); blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar1bis, RemoteOrigin.GOSSIP); @@ -482,8 +447,6 @@ public void onCompletedBlockAndBlobSidecars_shouldCreateTrackerIgnoringHistorica final List blobSidecars = dataStructureUtil.randomBlobSidecarsForBlock(block); - blobSidecars.forEach(BlobSidecar::markKzgAndInclusionProofAsValidated); - blockBlobSidecarsTrackersPool.onCompletedBlockAndBlobSidecars(block, blobSidecars); assertThat(asyncRunner.hasDelayedActions()).isFalse(); @@ -751,21 +714,8 @@ void shouldFetchMissingBlobSidecarsFromLocalELFirst() { engineGetBlobsResponse.complete(blobAndProofsFromEL); - verify(tracker) - .add( - argThat( - b -> - b.equals(missingBlobSidecars.getFirst()) - && b.isKzgAndInclusionProofValidated() - && b.isSignatureValidated())); // 0 - verify(tracker) - .add( - argThat( - b -> - b.equals(missingBlobSidecars.getLast()) - && b.isKzgAndInclusionProofValidated() - && b.isSignatureValidated())); // 2) - + verify(tracker).add(missingBlobSidecars.getFirst()); // 0 + verify(tracker).add(missingBlobSidecars.getLast()); // 2 verify(tracker, times(2)).add(any()); assertStats("blob_sidecar", "local_el_fetch", 3); @@ -977,8 +927,6 @@ public void shouldFetchContentWhenBlobSidecarIsNotForCurrentSlot() { .signedBeaconBlockHeader(dataStructureUtil.randomSignedBeaconBlockHeader(slot)) .build(); - blobSidecar.markKzgAndInclusionProofAsValidated(); - blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar, RemoteOrigin.GOSSIP); assertThat(asyncRunner.hasDelayedActions()).isTrue(); @@ -1297,8 +1245,6 @@ void stats_onNewBlobSidecar() { .signedBeaconBlockHeader(dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot)) .build(); - blobSidecar.markKzgAndInclusionProofAsValidated(); - // new from gossip blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar, RemoteOrigin.GOSSIP); @@ -1324,8 +1270,6 @@ void stats_onNewBlobSidecar() { dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot.increment())) .build(); - blobSidecar2.markKzgAndInclusionProofAsValidated(); - // new from RPC blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar2, RemoteOrigin.RPC); @@ -1341,8 +1285,6 @@ void stats_onNewBlobSidecar() { dataStructureUtil.randomSignedBeaconBlockHeader(currentSlot.increment())) .build(); - blobSidecar3.markKzgAndInclusionProofAsValidated(); - // new from LOCAL_EL blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar3, RemoteOrigin.LOCAL_EL); assertStats("blob_sidecar", "local_el", 1); @@ -1407,8 +1349,6 @@ void stats_onNewBlock() { .signedBeaconBlockHeader(block4.asHeader()) .build(); - blobSidecar4.markKzgAndInclusionProofAsValidated(); - blockBlobSidecarsTrackersPool.onNewBlobSidecar(blobSidecar4, RemoteOrigin.RPC); blockBlobSidecarsTrackersPool.onNewBlock(block4, Optional.of(RemoteOrigin.GOSSIP)); diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidatorTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidatorTest.java index 4abe8e6db2c..6e021da1f08 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidatorTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidatorTest.java @@ -111,7 +111,7 @@ void setup(final SpecContext specContext) { any(), eq(proposerIndex), any(), eq(postState))) .thenReturn(true); when(miscHelpersDeneb.verifyBlobKzgProof(any(), any(BlobSidecar.class))).thenReturn(true); - when(miscHelpersDeneb.verifyBlobSidecarMerkleProof(any())).thenReturn(true); + when(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(any())).thenReturn(true); } @TestTemplate @@ -224,7 +224,7 @@ void shouldRejectIfFinalizedCheckpointIsNotAnAncestorOfBlobSidecarsBlock() { @TestTemplate void shouldRejectWhenInclusionProofFailsValidation() { - when(miscHelpersDeneb.verifyBlobSidecarMerkleProof(any())).thenReturn(false); + when(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(any())).thenReturn(false); SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar)) .isCompletedWithValueMatching(InternalValidationResult::isReject); @@ -292,7 +292,7 @@ void shouldIgnoreImmediatelyWhenBlobFromValidInfoSet() { SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar)) .isCompletedWithValueMatching(InternalValidationResult::isAccept); - verify(miscHelpersDeneb).verifyBlobSidecarMerkleProof(blobSidecar); + verify(miscHelpersDeneb).verifyBlobKzgCommitmentInclusionProof(blobSidecar); verify(miscHelpersDeneb).verifyBlobKzgProof(kzg, blobSidecar); verify(gossipValidationHelper).getParentStateInBlockEpoch(any(), any(), any()); verify(gossipValidationHelper).isProposerTheExpectedProposer(any(), any(), any()); @@ -304,7 +304,7 @@ void shouldIgnoreImmediatelyWhenBlobFromValidInfoSet() { SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar)) .isCompletedWithValueMatching(InternalValidationResult::isIgnore); - verify(miscHelpersDeneb, never()).verifyBlobSidecarMerkleProof(blobSidecar); + verify(miscHelpersDeneb, never()).verifyBlobKzgCommitmentInclusionProof(blobSidecar); verify(miscHelpersDeneb, never()).verifyBlobKzgProof(kzg, blobSidecar); verify(gossipValidationHelper, never()).getParentStateInBlockEpoch(any(), any(), any()); verify(gossipValidationHelper, never()).isProposerTheExpectedProposer(any(), any(), any()); @@ -317,7 +317,7 @@ void shouldNotVerifyKnownValidSignedHeader() { SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar)) .isCompletedWithValueMatching(InternalValidationResult::isAccept); - verify(miscHelpersDeneb).verifyBlobSidecarMerkleProof(blobSidecar); + verify(miscHelpersDeneb).verifyBlobKzgCommitmentInclusionProof(blobSidecar); verify(miscHelpersDeneb).verifyBlobKzgProof(kzg, blobSidecar); verify(gossipValidationHelper).getParentStateInBlockEpoch(any(), any(), any()); verify(gossipValidationHelper).isProposerTheExpectedProposer(any(), any(), any()); @@ -336,7 +336,7 @@ void shouldNotVerifyKnownValidSignedHeader() { SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar0)) .isCompletedWithValueMatching(InternalValidationResult::isAccept); - verify(miscHelpersDeneb).verifyBlobSidecarMerkleProof(blobSidecar0); + verify(miscHelpersDeneb).verifyBlobKzgCommitmentInclusionProof(blobSidecar0); verify(miscHelpersDeneb).verifyBlobKzgProof(kzg, blobSidecar0); verify(gossipValidationHelper, never()).getParentStateInBlockEpoch(any(), any(), any()); verify(gossipValidationHelper, never()).isProposerTheExpectedProposer(any(), any(), any()); @@ -365,7 +365,7 @@ void shouldNotVerifyKnownValidSignedHeader() { SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecarNew)) .isCompletedWithValueMatching(InternalValidationResult::isIgnore); - verify(miscHelpersDeneb).verifyBlobSidecarMerkleProof(blobSidecarNew); + verify(miscHelpersDeneb).verifyBlobKzgCommitmentInclusionProof(blobSidecarNew); verify(miscHelpersDeneb).verifyBlobKzgProof(kzg, blobSidecarNew); verify(gossipValidationHelper).getParentStateInBlockEpoch(any(), any(), any()); } diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/AbstractBlobSidecarsValidator.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/AbstractBlobSidecarsValidator.java index 368f50d5e32..41d49539313 100644 --- a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/AbstractBlobSidecarsValidator.java +++ b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/AbstractBlobSidecarsValidator.java @@ -65,7 +65,7 @@ private boolean verifyBlobKzgProof(final BlobSidecar blobSidecar) { private boolean verifyBlobSidecarInclusionProof(final BlobSidecar blobSidecar) { try { return MiscHelpersDeneb.required(spec.atSlot(blobSidecar.getSlot()).miscHelpers()) - .verifyBlobSidecarMerkleProof(blobSidecar); + .verifyBlobKzgCommitmentInclusionProof(blobSidecar); } catch (final Exception ex) { LOG.debug( "Block inclusion proof verification failed for BlobSidecar {}",