diff --git a/CHANGELOG.md b/CHANGELOG.md index 465e8ca7c43..d5215b6faff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Breaking Changes ### Additions and Improvements +- Optimized blobs validation pipeline ### Bug Fixes - Updated the gas change check for block building so that warnings only get raised if the change is off spec. diff --git a/beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcher.java b/beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcher.java index ee18ffbc5df..c0d24c06eec 100644 --- a/beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcher.java +++ b/beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcher.java @@ -402,6 +402,7 @@ private void validateBlobSidecars(final SignedBeaconBlock block) { final List blobSidecars = blobSidecarsBySlotToImport.getOrDefault( block.getSlotAndBlockRoot(), Collections.emptyList()); + LOG.trace("Validating {} blob sidecars for block {}", blobSidecars.size(), block.getRoot()); final BlobSidecarsAndValidationResult validationResult = blobSidecarManager.createAvailabilityCheckerAndValidateImmediately(block, blobSidecars); diff --git a/beacon/sync/src/test/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcherTest.java b/beacon/sync/src/test/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcherTest.java index 7fe92edf82a..0121cc26555 100644 --- a/beacon/sync/src/test/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcherTest.java +++ b/beacon/sync/src/test/java/tech/pegasys/teku/beacon/sync/historical/HistoricalBatchFetcherTest.java @@ -117,7 +117,7 @@ public void setup() { .map(SignedBlockAndState::getBlock) .collect(Collectors.toList()); lastBlockInBatch = chainBuilder.getLatestBlockAndState().getBlock(); - firstBlockInBatch = blockBatch.get(0); + firstBlockInBatch = blockBatch.getFirst(); blobSidecarsBatch = chainBuilder .streamBlobSidecars(10, 20) @@ -202,7 +202,7 @@ public void run_returnAllBlocksAndBlobSidecarsOnFirstRequest() { earliestBlobSidecarSlotCaptor.capture()); assertThat(blockCaptor.getValue()).containsExactlyElementsOf(blockBatch); assertThat(blobSidecarCaptor.getValue()).isEqualTo(blobSidecarsBatch); - assertThat(earliestBlobSidecarSlotCaptor.getValue()).contains(blockBatch.get(0).getSlot()); + assertThat(earliestBlobSidecarSlotCaptor.getValue()).contains(blockBatch.getFirst().getSlot()); } @Test 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 5210b3353ad..5ffe6cdec0b 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 @@ -98,12 +98,6 @@ public SafeFuture getAvailabilityCheckResult() return SafeFuture.completedFuture(validateImmediately(block, blobsAndProofs)); } - @Override - public BlobSidecarsAndValidationResult validateImmediately( - final List blobSidecars) { - throw new UnsupportedOperationException("Not available in fork choice reference tests"); - } - private BlobSidecarsAndValidationResult validateImmediately( final SignedBeaconBlock block, final BlobsAndProofs blobsAndProofs) { final List kzgCommitments = 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 812cfeb2c63..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,6 +39,10 @@ public class BlobSidecar SignedBeaconBlockHeader, SszBytes32Vector> { + private volatile boolean kzgValidated = false; + private volatile boolean kzgCommitmentInclusionProofValidated = false; + private volatile boolean signatureValidated = false; + BlobSidecar(final BlobSidecarSchema blobSidecarSchema, final TreeNode backingTreeNode) { super(blobSidecarSchema, backingTreeNode); } @@ -139,6 +143,30 @@ public String toLogString() { getKZGProof().toAbbreviatedString()); } + public boolean isKzgValidated() { + return kzgValidated; + } + + 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; + } + @Override public BlobSidecarSchema getSchema() { return (BlobSidecarSchema) super.getSchema(); 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 559f3a01c87..58ddc35e480 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 @@ -380,16 +380,22 @@ public boolean verifyBlobKzgProofBatch(final KZG kzg, final List bl return false; } - public void validateBlobSidecarsBatchAgainstBlock( - final List blobSidecars, - final BeaconBlock block, - final List kzgCommitmentsFromBlock) { + public boolean verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( + final List blobSidecars, final SignedBeaconBlock signedBeaconBlock) { + return blobSidecars.stream() + .allMatch( + blobSidecar -> + verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( + blobSidecar, signedBeaconBlock)); + } + + public boolean verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( + final BlobSidecar blobSidecar, final SignedBeaconBlock signedBeaconBlock) { throw new UnsupportedOperationException("No Blob Sidecars before Deneb"); } public void verifyBlobSidecarCompleteness( - final List verifiedBlobSidecars, - final List kzgCommitmentsFromBlock) + final List verifiedBlobSidecars, final SignedBeaconBlock signedBeaconBlock) throws IllegalArgumentException { throw new UnsupportedOperationException("No Blob Sidecars before Deneb"); } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsAvailabilityChecker.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsAvailabilityChecker.java index d156f33c890..7456a69cc73 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsAvailabilityChecker.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/deneb/blobs/BlobSidecarsAvailabilityChecker.java @@ -15,9 +15,7 @@ import static tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAndValidationResult.NOT_REQUIRED_RESULT_FUTURE; -import java.util.List; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeader; import tech.pegasys.teku.spec.datastructures.execution.NewPayloadRequest; import tech.pegasys.teku.spec.logic.versions.bellatrix.block.OptimisticExecutionPayloadExecutor; @@ -35,12 +33,6 @@ public boolean initiateDataAvailabilityCheck() { public SafeFuture getAvailabilityCheckResult() { return NOT_REQUIRED_RESULT_FUTURE; } - - @Override - public BlobSidecarsAndValidationResult validateImmediately( - final List blobSidecars) { - return BlobSidecarsAndValidationResult.NOT_REQUIRED; - } }; BlobSidecarsAvailabilityChecker NOT_REQUIRED = NOOP; @@ -55,7 +47,4 @@ public BlobSidecarsAndValidationResult validateImmediately( boolean initiateDataAvailabilityCheck(); SafeFuture getAvailabilityCheckResult(); - - /** Perform the data availability check immediately on the provided blob sidecars */ - BlobSidecarsAndValidationResult validateImmediately(List blobSidecars); } 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..ebf9e188b45 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 @@ -84,10 +84,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,60 +115,48 @@ 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()); - }); + blobSidecars.stream() + .filter(blobSidecar -> !blobSidecar.isKzgValidated()) + .forEach( + blobSidecar -> { + blobs.add(blobSidecar.getBlob().getBytes()); + kzgCommitments.add(blobSidecar.getKZGCommitment()); + kzgProofs.add(blobSidecar.getKZGProof()); + }); + + if (blobs.isEmpty()) { + return true; + } + + final boolean result = kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs); - return kzg.verifyBlobKzgProofBatch(blobs, kzgCommitments, kzgProofs); + if (result) { + blobSidecars.stream() + .filter(blobSidecar -> !blobSidecar.isKzgValidated()) + .forEach(BlobSidecar::markKzgAsValidated); + } + + return result; } - /** - * 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)); - } - - checkArgument( - blobSidecar.getKZGCommitment().equals(kzgCommitmentFromBlock), - "Block and blob sidecar kzg commitments mismatch for %s, blob index %s", - slotAndBlockRoot, - blobIndex); - }); + public boolean verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( + final BlobSidecar blobSidecar, final SignedBeaconBlock signedBeaconBlock) { + if (blobSidecar.isSignatureValidated()) { + return true; + } + + final boolean result = + blobSidecar + .getSignedBeaconBlockHeader() + .hashTreeRoot() + .equals(signedBeaconBlock.hashTreeRoot()); + + if (result) { + blobSidecar.markSignatureAsValidated(); + } + + return result; } /** @@ -166,16 +164,22 @@ public void validateBlobSidecarsBatchAgainstBlock( * * @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. + * @param signedBeaconBlock block with commitments */ @Override public void verifyBlobSidecarCompleteness( final List completeVerifiedBlobSidecars, - final List kzgCommitmentsFromBlock) + final SignedBeaconBlock signedBeaconBlock) throws IllegalArgumentException { + int commitmentCount = + signedBeaconBlock + .getBeaconBlock() + .map(BeaconBlock::getBody) + .flatMap(BeaconBlockBody::getOptionalBlobKzgCommitments) + .orElseThrow() + .size(); checkArgument( - completeVerifiedBlobSidecars.size() == kzgCommitmentsFromBlock.size(), - "Blob sidecars are not complete"); + completeVerifiedBlobSidecars.size() == commitmentCount, "Blob sidecars are not complete"); IntStream.range(0, completeVerifiedBlobSidecars.size()) .forEach( @@ -262,12 +266,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..c47ec6e9546 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 @@ -30,7 +30,6 @@ import tech.pegasys.teku.spec.config.SpecConfigDeneb; 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.blocks.BeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; import tech.pegasys.teku.spec.datastructures.type.SszKZGProof; import tech.pegasys.teku.spec.logic.common.helpers.Predicates; @@ -38,7 +37,6 @@ import tech.pegasys.teku.spec.propertytest.suppliers.blobs.versions.deneb.BlobSidecarIndexSupplier; import tech.pegasys.teku.spec.propertytest.suppliers.blobs.versions.deneb.BlobSidecarSupplier; import tech.pegasys.teku.spec.propertytest.suppliers.blobs.versions.deneb.BlobSupplier; -import tech.pegasys.teku.spec.propertytest.suppliers.blocks.versions.deneb.BeaconBlockSupplier; import tech.pegasys.teku.spec.propertytest.suppliers.blocks.versions.deneb.SignedBeaconBlockSupplier; import tech.pegasys.teku.spec.propertytest.suppliers.type.KZGCommitmentSupplier; import tech.pegasys.teku.spec.propertytest.suppliers.type.SszKZGProofSupplier; @@ -86,28 +84,25 @@ void fuzzVerifyBlobKzgProofBatch( } @Property(tries = 100) - void fuzzValidateBlobSidecarsBatchAgainstBlock( + void fuzzVerifyBlobSidecarCompleteness( @ForAll final List<@From(supplier = BlobSidecarSupplier.class) BlobSidecar> blobSidecars, - @ForAll(supplier = BeaconBlockSupplier.class) final BeaconBlock block, - @ForAll - final List<@From(supplier = KZGCommitmentSupplier.class) KZGCommitment> kzgCommitments) { + @ForAll(supplier = SignedBeaconBlockSupplier.class) + final SignedBeaconBlock signedBeaconBlock) { try { - miscHelpers.validateBlobSidecarsBatchAgainstBlock(blobSidecars, block, kzgCommitments); + miscHelpers.verifyBlobSidecarCompleteness(blobSidecars, signedBeaconBlock); } catch (Exception e) { assertThat(e).isInstanceOf(IllegalArgumentException.class); } } @Property(tries = 100) - void fuzzVerifyBlobSidecarCompleteness( + void fuzzVerifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( @ForAll final List<@From(supplier = BlobSidecarSupplier.class) BlobSidecar> blobSidecars, - @ForAll - final List<@From(supplier = KZGCommitmentSupplier.class) KZGCommitment> kzgCommitments) { - try { - miscHelpers.verifyBlobSidecarCompleteness(blobSidecars, kzgCommitments); - } catch (Exception e) { - assertThat(e).isInstanceOf(IllegalArgumentException.class); - } + @ForAll(supplier = SignedBeaconBlockSupplier.class) + final SignedBeaconBlock signedBeaconBlock) { + + miscHelpers.verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( + blobSidecars, signedBeaconBlock); } @Property(tries = 100) @@ -119,7 +114,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..78ddb7fd975 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 @@ -16,9 +16,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assumptions.assumeThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; 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,108 +69,13 @@ 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()))) + dataStructureUtil.randomSignedBeaconBlockWithCommitments(2))) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Blob sidecars are not complete"); } @@ -181,10 +86,20 @@ void verifyBlobSidecarCompleteness_shouldThrowWhenBlobSidecarIndexIsWrong() { assertThatThrownBy( () -> miscHelpersDeneb.verifyBlobSidecarCompleteness( - blobSidecars, List.of(dataStructureUtil.randomKZGCommitment()))) + blobSidecars, dataStructureUtil.randomSignedBeaconBlockWithCommitments(1))) .isInstanceOf(IllegalArgumentException.class) .hasMessage( - "Blob sidecar index mismatch, expected 0, got %s", blobSidecars.get(0).getIndex()); + "Blob sidecar index mismatch, expected 0, got %s", blobSidecars.getFirst().getIndex()); + } + + @Test + void verifyBlobSidecarCompleteness_shouldNotThrow() { + final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlockWithCommitments(2); + final List blobSidecars = + List.of( + dataStructureUtil.randomBlobSidecarForBlock(block, 0), + dataStructureUtil.randomBlobSidecarForBlock(block, 1)); + assertDoesNotThrow(() -> miscHelpersDeneb.verifyBlobSidecarCompleteness(blobSidecars, block)); } @Test @@ -207,7 +122,8 @@ 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(); + assertThat(blobSidecar.isKzgCommitmentInclusionProofValidated()).isTrue(); } @Test @@ -227,7 +143,7 @@ void shouldThrowWhenConstructingBlobSidecarWithInvalidIndex() { } @Test - void verifyBlobSidecarMerkleProofShouldValidate() { + void verifyBlobKzgCommitmentInclusionProofShouldValidate() { final int numberOfCommitments = 4; final BeaconBlockBodyDeneb beaconBlockBody = BeaconBlockBodyDeneb.required( @@ -266,7 +182,8 @@ void verifyBlobSidecarMerkleProofShouldValidate() { .getBytes()) .kzgCommitmentInclusionProof(merkleProof) .build(); - assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)).isTrue(); + assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecar)).isTrue(); + assertThat(blobSidecar.isKzgCommitmentInclusionProofValidated()).isTrue(); // And the same blobSidecar but with wrong merkle proof for (int j = 0; j < numberOfCommitments; ++j) { @@ -295,8 +212,35 @@ void verifyBlobSidecarMerkleProofShouldValidate() { .getBytes()) .kzgCommitmentInclusionProof(merkleProofWrong) .build(); - assertThat(miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecarWrong)).isFalse(); + assertThat(miscHelpersDeneb.verifyBlobKzgCommitmentInclusionProof(blobSidecarWrong)) + .isFalse(); + assertThat(blobSidecarWrong.isKzgCommitmentInclusionProofValidated()).isFalse(); } } } + + @Test + void verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock_returnsTrue() { + final SignedBeaconBlock signedBeaconBlock = + dataStructureUtil.randomSignedBeaconBlockWithCommitments(1); + final BlobSidecar blobSidecar = + dataStructureUtil.randomBlobSidecarForBlock(signedBeaconBlock, 0); + assertThat( + miscHelpersDeneb.verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( + blobSidecar, signedBeaconBlock)) + .isTrue(); + assertThat(blobSidecar.isSignatureValidated()).isTrue(); + } + + @Test + void verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock_returnsFalse() { + final SignedBeaconBlock signedBeaconBlock = + dataStructureUtil.randomSignedBeaconBlockWithCommitments(1); + final BlobSidecar blobSidecar = dataStructureUtil.randomBlobSidecar(); + assertThat( + miscHelpersDeneb.verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( + blobSidecar, signedBeaconBlock)) + .isFalse(); + assertThat(blobSidecar.isSignatureValidated()).isFalse(); + } } 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 7859aea03c5..fc20d0c60e8 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 @@ -13,12 +13,12 @@ package tech.pegasys.teku.statetransition.blobs; +import com.google.common.annotations.VisibleForTesting; import java.util.List; import java.util.Map; import java.util.Optional; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.ethereum.events.SlotEventsChannel; -import tech.pegasys.teku.infrastructure.async.AsyncRunner; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.subscribers.Subscribers; import tech.pegasys.teku.infrastructure.unsigned.UInt64; @@ -37,34 +37,60 @@ public class BlobSidecarManagerImpl implements BlobSidecarManager, SlotEventsChannel { private final Spec spec; - private final AsyncRunner asyncRunner; private final RecentChainData recentChainData; private final BlobSidecarGossipValidator validator; - private final KZG kzg; private final BlockBlobSidecarsTrackersPool blockBlobSidecarsTrackersPool; private final FutureItems futureBlobSidecars; private final Map invalidBlobSidecarRoots; + private final ForkChoiceBlobSidecarsAvailabilityCheckerProvider + forkChoiceBlobSidecarsAvailabilityCheckerProvider; + private final UnpooledBlockBlobSidecarsTrackerProvider unpooledBlockBlobSidecarsTrackerProvider; private final Subscribers receivedBlobSidecarSubscribers = Subscribers.create(true); public BlobSidecarManagerImpl( final Spec spec, - final AsyncRunner asyncRunner, final RecentChainData recentChainData, final BlockBlobSidecarsTrackersPool blockBlobSidecarsTrackersPool, final BlobSidecarGossipValidator validator, final KZG kzg, final FutureItems futureBlobSidecars, final Map invalidBlobSidecarRoots) { + this( + spec, + recentChainData, + blockBlobSidecarsTrackersPool, + validator, + futureBlobSidecars, + invalidBlobSidecarRoots, + (tracker) -> + new ForkChoiceBlobSidecarsAvailabilityChecker(spec, recentChainData, tracker, kzg), + // we don't care to set maxBlobsPerBlock since it isn't used with this immediate validation + // flow + (block) -> new BlockBlobSidecarsTracker(block.getSlotAndBlockRoot(), UInt64.ZERO)); + } + + @VisibleForTesting + BlobSidecarManagerImpl( + final Spec spec, + final RecentChainData recentChainData, + final BlockBlobSidecarsTrackersPool blockBlobSidecarsTrackersPool, + final BlobSidecarGossipValidator validator, + final FutureItems futureBlobSidecars, + final Map invalidBlobSidecarRoots, + final ForkChoiceBlobSidecarsAvailabilityCheckerProvider + forkChoiceBlobSidecarsAvailabilityCheckerProvider, + final UnpooledBlockBlobSidecarsTrackerProvider unpooledBlockBlobSidecarsTrackerProvider) { this.spec = spec; - this.asyncRunner = asyncRunner; this.recentChainData = recentChainData; this.validator = validator; - this.kzg = kzg; this.blockBlobSidecarsTrackersPool = blockBlobSidecarsTrackersPool; this.futureBlobSidecars = futureBlobSidecars; this.invalidBlobSidecarRoots = invalidBlobSidecarRoots; + this.forkChoiceBlobSidecarsAvailabilityCheckerProvider = + forkChoiceBlobSidecarsAvailabilityCheckerProvider; + this.unpooledBlockBlobSidecarsTrackerProvider = unpooledBlockBlobSidecarsTrackerProvider; } @Override @@ -128,8 +154,7 @@ public BlobSidecarsAvailabilityChecker createAvailabilityChecker(final SignedBea final BlockBlobSidecarsTracker blockBlobSidecarsTracker = blockBlobSidecarsTrackersPool.getOrCreateBlockBlobSidecarsTracker(block); - return new ForkChoiceBlobSidecarsAvailabilityChecker( - spec, asyncRunner, recentChainData, blockBlobSidecarsTracker, kzg); + return forkChoiceBlobSidecarsAvailabilityCheckerProvider.create(blockBlobSidecarsTracker); } @Override @@ -140,15 +165,36 @@ public BlobSidecarsAndValidationResult createAvailabilityCheckerAndValidateImmed return BlobSidecarsAndValidationResult.NOT_REQUIRED; } - // we don't care to set maxBlobsPerBlock since it isn't used with this immediate validation flow final BlockBlobSidecarsTracker blockBlobSidecarsTracker = - new BlockBlobSidecarsTracker(block.getSlotAndBlockRoot(), UInt64.ZERO); - + unpooledBlockBlobSidecarsTrackerProvider.create(block); blockBlobSidecarsTracker.setBlock(block); - return new ForkChoiceBlobSidecarsAvailabilityChecker( - spec, asyncRunner, recentChainData, blockBlobSidecarsTracker, kzg) - .validateImmediately(blobSidecars); + boolean allAdded = blobSidecars.stream().allMatch(blockBlobSidecarsTracker::add); + if (!allAdded) { + return BlobSidecarsAndValidationResult.invalidResult( + blobSidecars, + new IllegalStateException( + "Failed to add all blobs to tracker, possible blobs with same index or index out of blocks commitment range")); + } + + if (!blockBlobSidecarsTracker.isComplete()) { + return BlobSidecarsAndValidationResult.NOT_AVAILABLE; + } + + final ForkChoiceBlobSidecarsAvailabilityChecker forkChoiceBlobSidecarsAvailabilityChecker = + forkChoiceBlobSidecarsAvailabilityCheckerProvider.create(blockBlobSidecarsTracker); + + forkChoiceBlobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck(); + + final SafeFuture availabilityCheckResult = + forkChoiceBlobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); + + if (availabilityCheckResult.isDone()) { + return availabilityCheckResult.join(); + } else { + throw new IllegalStateException( + "Availability check expected to be done synchronously when providing immediate blobs"); + } } @Override @@ -163,4 +209,17 @@ public void onSlot(final UInt64 slot) { validateAndPrepareForBlockImport(blobSidecar, Optional.empty()) .ifExceptionGetsHereRaiseABug()); } + + @VisibleForTesting + @FunctionalInterface + interface ForkChoiceBlobSidecarsAvailabilityCheckerProvider { + ForkChoiceBlobSidecarsAvailabilityChecker create( + final BlockBlobSidecarsTracker blockBlobSidecarsTracker); + } + + @VisibleForTesting + @FunctionalInterface + interface UnpooledBlockBlobSidecarsTrackerProvider { + BlockBlobSidecarsTracker create(final SignedBeaconBlock block); + } } 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 721211524f3..64c89d91ba1 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 @@ -243,7 +243,7 @@ private boolean isExcessiveBlobSidecar(final BlobSidecar blobSidecar) { .orElse(false); } - public boolean isCompleted() { + public boolean isComplete() { return blobSidecarsComplete.isDone(); } @@ -341,7 +341,7 @@ public String toString() { return MoreObjects.toStringHelper(this) .add("slotAndBlockRoot", slotAndBlockRoot) .add("isBlockPresent", block.get().isPresent()) - .add("isCompleted", isCompleted()) + .add("isComplete", isComplete()) .add("rpcFetchTriggered", rpcFetchTriggered) .add("localElFetchTriggered", localElFetchTriggered) .add("blockImportOnCompletionEnabled", blockImportOnCompletionEnabled.get()) 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 d8778c223b5..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 @@ -17,7 +17,6 @@ import static tech.pegasys.teku.infrastructure.logging.P2PLogger.P2P_LOG; import static tech.pegasys.teku.infrastructure.time.TimeUtilities.secondsToMillis; import static tech.pegasys.teku.spec.constants.NetworkConstants.INTERVALS_PER_SLOT; -import static tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsValidationResult.INVALID; import static tech.pegasys.teku.statetransition.forkchoice.StateRootCollector.addParentStateRoots; import com.google.common.annotations.VisibleForTesting; 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 fe6c43a345b..4cba1390a8c 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 @@ -13,35 +13,17 @@ package tech.pegasys.teku.statetransition.forkchoice; -import static com.google.common.base.Preconditions.checkState; - import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Suppliers; import java.time.Duration; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.NavigableMap; -import java.util.Optional; -import java.util.SortedMap; -import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.TimeoutException; -import java.util.function.Supplier; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import tech.pegasys.teku.infrastructure.async.AsyncRunner; 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.kzg.KZGCommitment; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; -import tech.pegasys.teku.spec.datastructures.blocks.BeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; -import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot; -import tech.pegasys.teku.spec.datastructures.blocks.blockbody.versions.deneb.BeaconBlockBodyDeneb; -import tech.pegasys.teku.spec.datastructures.type.SszKZGCommitment; import tech.pegasys.teku.spec.logic.common.helpers.MiscHelpers; import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAndValidationResult; import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAvailabilityChecker; @@ -53,32 +35,21 @@ * href="https://github.com/ethereum/consensus-specs/blob/dev/specs/deneb/fork-choice.md#is_data_available">is_data_available */ public class ForkChoiceBlobSidecarsAvailabilityChecker implements BlobSidecarsAvailabilityChecker { - private static final Logger LOG = LogManager.getLogger(); - private final Spec spec; - private final AsyncRunner asyncRunner; private final RecentChainData recentChainData; private final BlockBlobSidecarsTracker blockBlobSidecarsTracker; private final KZG kzg; - private final NavigableMap validatedBlobSidecars = - new ConcurrentSkipListMap<>(); - private final SafeFuture validationResult = new SafeFuture<>(); - private final Supplier> kzgCommitmentsFromBlockSupplier = - createLazyKzgCommitmentsSupplier(this); - private final Duration waitForTrackerCompletionTimeout; public ForkChoiceBlobSidecarsAvailabilityChecker( final Spec spec, - final AsyncRunner asyncRunner, final RecentChainData recentChainData, final BlockBlobSidecarsTracker blockBlobSidecarsTracker, final KZG kzg) { this.spec = spec; - this.asyncRunner = asyncRunner; this.recentChainData = recentChainData; this.blockBlobSidecarsTracker = blockBlobSidecarsTracker; this.kzg = kzg; @@ -89,13 +60,11 @@ public ForkChoiceBlobSidecarsAvailabilityChecker( @VisibleForTesting ForkChoiceBlobSidecarsAvailabilityChecker( final Spec spec, - final AsyncRunner asyncRunner, final RecentChainData recentChainData, final BlockBlobSidecarsTracker blockBlobSidecarsTracker, final KZG kzg, final Duration waitForTrackerCompletionTimeout) { this.spec = spec; - this.asyncRunner = asyncRunner; this.recentChainData = recentChainData; this.blockBlobSidecarsTracker = blockBlobSidecarsTracker; this.kzg = kzg; @@ -104,258 +73,57 @@ public ForkChoiceBlobSidecarsAvailabilityChecker( @Override public boolean initiateDataAvailabilityCheck() { - asyncRunner - .runAsync(this::validateImmediatelyAvailable) - .thenCompose(this::completeValidation) - .thenApply(this::performCompletenessValidation) + blockBlobSidecarsTracker + .getCompletionFuture() + .orTimeout(waitForTrackerCompletionTimeout) + .thenApply(__ -> validateCompletedBlobSidecars()) + .exceptionallyCompose( + error -> + ExceptionUtil.getCause(error, TimeoutException.class) + .map( + timeoutException -> { + if (isBlockOutsideDataAvailabilityWindow()) { + return SafeFuture.completedFuture( + BlobSidecarsAndValidationResult.NOT_REQUIRED); + } + return SafeFuture.completedFuture( + BlobSidecarsAndValidationResult.notAvailable(timeoutException)); + }) + .orElseGet(() -> SafeFuture.failedFuture(error))) .propagateTo(validationResult); - return true; } - @Override - public SafeFuture getAvailabilityCheckResult() { - return validationResult; - } - - @Override - public BlobSidecarsAndValidationResult validateImmediately(final List blobSidecars) { - - final List kzgCommitmentsFromBlock = kzgCommitmentsFromBlockSupplier.get(); - - if (!blobSidecars.isEmpty()) { - final BlobSidecarsAndValidationResult blobSidecarsAndValidationResult = - validateBatch(blobSidecars); - - return performCompletenessValidation(blobSidecarsAndValidationResult); - } - - // no blob sidecars - - if (isBlockOutsideDataAvailabilityWindow()) { - return BlobSidecarsAndValidationResult.NOT_REQUIRED; - } - - if (kzgCommitmentsFromBlock.isEmpty()) { - return BlobSidecarsAndValidationResult.validResult(List.of()); - } - - return BlobSidecarsAndValidationResult.NOT_AVAILABLE; - } - - private BlobSidecarsAndValidationResult validateBatch(final List blobSidecars) { - final BeaconBlock block = blockBlobSidecarsTracker.getBlock().orElseThrow().getMessage(); - final SlotAndBlockRoot slotAndBlockRoot = blockBlobSidecarsTracker.getSlotAndBlockRoot(); - - final MiscHelpers miscHelpers = spec.atSlot(slotAndBlockRoot.getSlot()).miscHelpers(); + private BlobSidecarsAndValidationResult validateCompletedBlobSidecars() { + final MiscHelpers miscHelpers = + spec.atSlot(blockBlobSidecarsTracker.getSlotAndBlockRoot().getSlot()).miscHelpers(); + final List blobSidecars = + List.copyOf(blockBlobSidecarsTracker.getBlobSidecars().values()); + final SignedBeaconBlock block = blockBlobSidecarsTracker.getBlock().orElseThrow(); try { - miscHelpers.validateBlobSidecarsBatchAgainstBlock( - blobSidecars, block, kzgCommitmentsFromBlockSupplier.get()); - if (!miscHelpers.verifyBlobKzgProofBatch(kzg, blobSidecars)) { return BlobSidecarsAndValidationResult.invalidResult(blobSidecars); } + + miscHelpers.verifyBlobSidecarCompleteness(blobSidecars, block); } catch (final Exception ex) { return BlobSidecarsAndValidationResult.invalidResult(blobSidecars, ex); } - return BlobSidecarsAndValidationResult.validResult(blobSidecars); - } - - /** - * Step 1 of 2 - * - *

This function performs a partial validation with blobs that are currently available from the - * tracker. If all blobs are already available, it performs a full validation. - * - * @return a validation result only in case it is a definitive result. If the validation needs to - * be completed it returns empty - */ - private Optional validateImmediatelyAvailable() { - final List kzgCommitmentsInBlock = kzgCommitmentsFromBlockSupplier.get(); - - final List blobSidecarsToValidate; - - final boolean performCompleteValidation = blockBlobSidecarsTracker.isCompleted(); - - if (performCompleteValidation) { - // the tracker contains all required blobs for our block: we can perform a complete validation - blobSidecarsToValidate = List.copyOf(blockBlobSidecarsTracker.getBlobSidecars().values()); - LOG.debug( - "The expected {} BlobSidecars have already been received. Performing full validation.", - blobSidecarsToValidate.size()); - } else { - // prepare partial validation by matching the currently available blobs with the corresponding - // commitments from the block - blobSidecarsToValidate = - blockBlobSidecarsTracker.getBlobSidecars().values().stream().toList(); - - LOG.debug( - "{} out of {} BlobSidecars have been received so far. Performing partial validation.", - blobSidecarsToValidate.size(), - kzgCommitmentsInBlock.size()); - - if (blobSidecarsToValidate.isEmpty() - && !kzgCommitmentsInBlock.isEmpty() - && isBlockOutsideDataAvailabilityWindow()) { - // there are no available blobs so far, but we are outside the availability window. We can - // skip additional checks - return Optional.of(BlobSidecarsAndValidationResult.NOT_REQUIRED); - } - } - - // perform the actual validation - final BlobSidecarsAndValidationResult result = validateBatch(blobSidecarsToValidate); - - if (result.isFailure()) { - return Optional.of(result); - } - - if (performCompleteValidation) { - return Optional.of(result); - } - - // cache partially validated blobs - blobSidecarsToValidate.forEach( - blobSidecar -> validatedBlobSidecars.put(blobSidecar.getIndex(), blobSidecar)); - - return Optional.empty(); - } - - /** - * Step 2 of 2 - * - *

This function completes the validation if necessary. If there are still blobs to validate, - * it waits for the tracker to complete and then performs the final validation by computing and - * validating the non-yet validated blobs-commitments, and then computes the final validation - * result with the entire list of blobs - * - * @param maybeBlobSidecarsAndValidationResult is the validation result coming from Step 1. - * @return final validation result - */ - private SafeFuture completeValidation( - final Optional maybeBlobSidecarsAndValidationResult) { - - return maybeBlobSidecarsAndValidationResult - .map(SafeFuture::completedFuture) - .orElseGet( - () -> - blockBlobSidecarsTracker - .getCompletionFuture() - .orTimeout(waitForTrackerCompletionTimeout) - .thenApplyChecked(__ -> computeAndValidateRemaining()) - .thenApply(this::computeFinalValidationResult) - .exceptionallyCompose( - error -> - ExceptionUtil.getCause(error, TimeoutException.class) - .map( - timeoutException -> - SafeFuture.completedFuture( - BlobSidecarsAndValidationResult.notAvailable( - timeoutException))) - .orElseGet(() -> SafeFuture.failedFuture(error)))); - } - - /** - * Knowing that: - * - *

(1) `validatedBlobSidecars` contains already validated blobSidecars - * - *

(2) `blockBlobSidecarsTracker` is now completed - * - *

This function computes and validates a batch of BlobSidecars-kzgCommitments that needs to be - * additionally validated to verify the entire set - * - * @return validation result for the batch - */ - private BlobSidecarsAndValidationResult computeAndValidateRemaining() { - checkState( - blockBlobSidecarsTracker.isCompleted(), - "BlobSidecar tracker assumed to be completed but it is not."); - - final List additionalBlobSidecarsToBeValidated = new ArrayList<>(); - - final List kzgCommitmentsInBlock = kzgCommitmentsFromBlockSupplier.get(); - final SortedMap completeBlobSidecars = - blockBlobSidecarsTracker.getBlobSidecars(); - - UInt64.range(UInt64.ZERO, UInt64.valueOf(kzgCommitmentsInBlock.size())) - .forEachOrdered( - index -> { - if (!validatedBlobSidecars.containsKey(index)) { - - Optional.ofNullable(completeBlobSidecars.get(index)) - .ifPresentOrElse( - additionalBlobSidecarsToBeValidated::add, - () -> - LOG.error( - "Index {} not found in blob sidecar returned by tracker for block {}", - index, - blockBlobSidecarsTracker.getSlotAndBlockRoot().toLogString())); - } - }); - - LOG.debug( - "Remaining {} out of {} BlobSidecars have been received. Completing validation.", - additionalBlobSidecarsToBeValidated.size(), - kzgCommitmentsInBlock.size()); - - return validateBatch(additionalBlobSidecarsToBeValidated); - } - - /** - * Computes the final validation result combining the already validated blobs from - * `validatedBlobSidecars` - */ - private BlobSidecarsAndValidationResult computeFinalValidationResult( - final BlobSidecarsAndValidationResult additionalBlobSidecarsAndValidationResult) { - if (additionalBlobSidecarsAndValidationResult.isFailure()) { - return additionalBlobSidecarsAndValidationResult; + if (!miscHelpers.verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock( + blobSidecars, block)) { + return BlobSidecarsAndValidationResult.invalidResult( + blobSidecars, + new IllegalStateException("Blob sidecars block header does not match signed block")); } - // put all additional validated blobSidecar in validatedBlobSidecars which now should - // be complete - additionalBlobSidecarsAndValidationResult - .getBlobSidecars() - .forEach(blobSidecar -> validatedBlobSidecars.put(blobSidecar.getIndex(), blobSidecar)); - - final List completeValidatedBlobSidecars = - new ArrayList<>(validatedBlobSidecars.values()); - - return BlobSidecarsAndValidationResult.validResult( - Collections.unmodifiableList(completeValidatedBlobSidecars)); + return BlobSidecarsAndValidationResult.validResult(blobSidecars); } - /** Makes sure that final blob sidecar list is complete */ - private BlobSidecarsAndValidationResult performCompletenessValidation( - final BlobSidecarsAndValidationResult blobSidecarsAndValidationResult) { - - if (blobSidecarsAndValidationResult.isFailure() - || blobSidecarsAndValidationResult.isNotRequired()) { - return blobSidecarsAndValidationResult; - } - - try { - spec.atSlot(blockBlobSidecarsTracker.getSlotAndBlockRoot().getSlot()) - .miscHelpers() - .verifyBlobSidecarCompleteness( - blobSidecarsAndValidationResult.getBlobSidecars(), - kzgCommitmentsFromBlockSupplier.get()); - } catch (final IllegalArgumentException ex) { - if (!isBlockOutsideDataAvailabilityWindow()) { - return BlobSidecarsAndValidationResult.invalidResult( - blobSidecarsAndValidationResult.getBlobSidecars(), - new IllegalArgumentException( - "Validated blobs are less than commitments present in block.", ex)); - } - - LOG.error( - "Inconsistent state detected: validated blobs is less then commitments in block. Since slot is outside availability the window we can consider blobs as NOT_REQUIRED."); - return BlobSidecarsAndValidationResult.NOT_REQUIRED; - } - - return blobSidecarsAndValidationResult; + @Override + public SafeFuture getAvailabilityCheckResult() { + return validationResult; } private boolean isBlockOutsideDataAvailabilityWindow() { @@ -363,23 +131,6 @@ private boolean isBlockOutsideDataAvailabilityWindow() { recentChainData.getStore(), blockBlobSidecarsTracker.getSlotAndBlockRoot().getSlot()); } - static Supplier> createLazyKzgCommitmentsSupplier( - final ForkChoiceBlobSidecarsAvailabilityChecker availabilityChecker) { - return Suppliers.memoize( - () -> - BeaconBlockBodyDeneb.required( - availabilityChecker - .blockBlobSidecarsTracker - .getBlock() - .flatMap(SignedBeaconBlock::getBeaconBlock) - .map(BeaconBlock::getBody) - .orElseThrow()) - .getBlobKzgCommitments() - .stream() - .map(SszKZGCommitment::getKZGCommitment) - .toList()); - } - static Duration calculateCompletionTimeout(final Spec spec, final UInt64 slot) { return Duration.ofMillis((spec.atSlot(slot).getConfig().getSecondsPerSlot() * 1000L) / 3); } 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 e056f36c1d6..0bfb291d0e2 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 @@ -340,7 +340,7 @@ public synchronized void onCompletedBlockAndBlobSidecars( totalBlobSidecars += (int) addedBlobs; sizeGauge.set(totalBlobSidecars, GAUGE_BLOB_SIDECARS_LABEL); - if (!blobSidecarsTracker.isCompleted()) { + if (!blobSidecarsTracker.isComplete()) { LOG.error( "Tracker for block {} is supposed to be completed but it is not. Missing blob sidecars: {}", block.toLogString(), @@ -510,7 +510,7 @@ private BlockBlobSidecarsTracker internalOnNewBlock( // if we attempted to fetch via RPC, we missed the opportunity to complete the // tracker via local EL (local El fetch requires the block to be known) // Let's try now - if (!existingTracker.isLocalElFetchTriggered() && !existingTracker.isCompleted()) { + if (!existingTracker.isLocalElFetchTriggered() && !existingTracker.isComplete()) { fetchMissingContentFromLocalEL(slotAndBlockRoot) .finish(this::logLocalElBlobsLookupFailure); } @@ -650,7 +650,7 @@ private synchronized SafeFuture fetchMissingContentFromLocalEL( return SafeFuture.COMPLETE; } - if (blockBlobSidecarsTracker.isCompleted()) { + if (blockBlobSidecarsTracker.isComplete()) { return SafeFuture.COMPLETE; } @@ -721,7 +721,6 @@ private synchronized SafeFuture fetchMissingContentFromLocalEL( blobAndProof.get(), beaconBlockBodyDeneb, signedBeaconBlockHeader); - onNewBlobSidecar(blobSidecar, LOCAL_EL); } }); @@ -738,14 +737,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( @@ -757,7 +764,7 @@ private synchronized void fetchMissingContentFromRemotePeers( return; } - if (blockBlobSidecarsTracker.isCompleted()) { + if (blockBlobSidecarsTracker.isComplete()) { return; } 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 9baad8da958..fa573f7cb17 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 @@ -213,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")); } @@ -255,8 +255,7 @@ public SafeFuture validate(final BlobSidecar blobSidec * [REJECT] The proposer signature of `blob_sidecar.signed_block_header`, is valid * with respect to the `block_header.proposer_index` pubkey. */ - if (!verifyBlockHeaderSignature( - postState, blobSidecar.getSignedBeaconBlockHeader())) { + if (!verifyBlockHeaderSignature(postState, blobSidecar)) { return reject("BlobSidecar block header signature is invalid."); } @@ -291,10 +290,12 @@ public boolean markForEquivocation(final BlobSidecar blobSidecar) { private SafeFuture validateBlobSidecarWithKnownValidHeader( final BlobSidecar blobSidecar, final BeaconBlockHeader blockHeader) { + blobSidecar.markSignatureAsValidated(); + /* * [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")); } @@ -330,7 +331,8 @@ private SafeFuture validateBlobSidecarWithKnownValidHe } private boolean verifyBlockHeaderSignature( - final BeaconState state, final SignedBeaconBlockHeader signedBlockHeader) { + final BeaconState state, final BlobSidecar blobSidecar) { + final SignedBeaconBlockHeader signedBlockHeader = blobSidecar.getSignedBeaconBlockHeader(); final Bytes32 domain = spec.getDomain( Domain.BEACON_PROPOSER, @@ -339,11 +341,18 @@ private boolean verifyBlockHeaderSignature( state.getGenesisValidatorsRoot()); final Bytes signingRoot = spec.computeSigningRoot(signedBlockHeader.getMessage(), domain); - return gossipValidationHelper.isSignatureValidWithRespectToProposerIndex( - signingRoot, - signedBlockHeader.getMessage().getProposerIndex(), - signedBlockHeader.getSignature(), - state); + final boolean result = + gossipValidationHelper.isSignatureValidWithRespectToProposerIndex( + signingRoot, + signedBlockHeader.getMessage().getProposerIndex(), + signedBlockHeader.getSignature(), + state); + + if (result) { + blobSidecar.markSignatureAsValidated(); + } + + return result; } private boolean isFirstValidForSlotProposerIndexAndBlobIndex( diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/blobs/BlobSidecarManagerTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/blobs/BlobSidecarManagerTest.java index 576c65827b2..cac650a982e 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/blobs/BlobSidecarManagerTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/blobs/BlobSidecarManagerTest.java @@ -14,6 +14,7 @@ package tech.pegasys.teku.statetransition.blobs; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -31,9 +32,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.async.StubAsyncRunner; import tech.pegasys.teku.infrastructure.unsigned.UInt64; -import tech.pegasys.teku.kzg.KZG; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.TestSpecFactory; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; @@ -43,22 +42,21 @@ import tech.pegasys.teku.spec.util.DataStructureUtil; import tech.pegasys.teku.statetransition.blobs.BlobSidecarManager.ReceivedBlobSidecarListener; import tech.pegasys.teku.statetransition.blobs.BlobSidecarManager.RemoteOrigin; +import tech.pegasys.teku.statetransition.blobs.BlobSidecarManagerImpl.ForkChoiceBlobSidecarsAvailabilityCheckerProvider; +import tech.pegasys.teku.statetransition.blobs.BlobSidecarManagerImpl.UnpooledBlockBlobSidecarsTrackerProvider; import tech.pegasys.teku.statetransition.forkchoice.ForkChoiceBlobSidecarsAvailabilityChecker; import tech.pegasys.teku.statetransition.util.BlockBlobSidecarsTrackersPoolImpl; import tech.pegasys.teku.statetransition.util.FutureItems; import tech.pegasys.teku.statetransition.validation.BlobSidecarGossipValidator; import tech.pegasys.teku.statetransition.validation.InternalValidationResult; import tech.pegasys.teku.storage.client.RecentChainData; -import tech.pegasys.teku.storage.store.UpdatableStore; public class BlobSidecarManagerTest { private final Spec spec = TestSpecFactory.createMinimalDeneb(); private final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec); - private final StubAsyncRunner asyncRunner = new StubAsyncRunner(); private final RecentChainData recentChainData = mock(RecentChainData.class); private final BlobSidecarGossipValidator blobSidecarValidator = mock(BlobSidecarGossipValidator.class); - private final KZG kzg = mock(KZG.class); private final BlockBlobSidecarsTrackersPoolImpl blockBlobSidecarsTrackersPool = mock(BlockBlobSidecarsTrackersPoolImpl.class); private final Map invalidBlobSidecarRoots = new HashMap<>(); @@ -66,21 +64,29 @@ public class BlobSidecarManagerTest { @SuppressWarnings("unchecked") private final FutureItems futureBlobSidecars = mock(FutureItems.class); + private final ForkChoiceBlobSidecarsAvailabilityCheckerProvider + forkChoiceBlobSidecarsAvailabilityCheckerProvider = + mock(ForkChoiceBlobSidecarsAvailabilityCheckerProvider.class); + private final UnpooledBlockBlobSidecarsTrackerProvider unpooledBlockBlobSidecarsTrackerProvider = + mock(UnpooledBlockBlobSidecarsTrackerProvider.class); + private final BlobSidecarManagerImpl blobSidecarManager = new BlobSidecarManagerImpl( spec, - asyncRunner, recentChainData, blockBlobSidecarsTrackersPool, blobSidecarValidator, - kzg, futureBlobSidecars, - invalidBlobSidecarRoots); + invalidBlobSidecarRoots, + forkChoiceBlobSidecarsAvailabilityCheckerProvider, + unpooledBlockBlobSidecarsTrackerProvider); private final ReceivedBlobSidecarListener receivedBlobSidecarListener = mock(ReceivedBlobSidecarListener.class); private final BlobSidecar blobSidecar = dataStructureUtil.randomBlobSidecar(); + private final List blobSidecars = List.of(blobSidecar); + private final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(UInt64.ONE); @BeforeEach void setUp() { @@ -171,7 +177,6 @@ void validateAndPrepareForBlockImport_shouldRejectKnownInvalidBlobs() { @Test void prepareForBlockImport_shouldAddToPoolAndNotify() { - blobSidecarManager.prepareForBlockImport(blobSidecar, RemoteOrigin.GOSSIP); verify(receivedBlobSidecarListener).onBlobSidecarReceived(blobSidecar); @@ -190,8 +195,8 @@ void onSlot_shouldInteractWithPoolAndFutureBlobs() { verify(futureBlobSidecars).onSlot(UInt64.ONE); verify(blockBlobSidecarsTrackersPool) - .onNewBlobSidecar(futureBlobSidecarsList.get(0), RemoteOrigin.GOSSIP); - verify(receivedBlobSidecarListener).onBlobSidecarReceived(futureBlobSidecarsList.get(0)); + .onNewBlobSidecar(futureBlobSidecarsList.getFirst(), RemoteOrigin.GOSSIP); + verify(receivedBlobSidecarListener).onBlobSidecarReceived(futureBlobSidecarsList.getFirst()); } @Test @@ -217,30 +222,142 @@ void createAvailabilityChecker_shouldReturnANotRequiredAvailabilityCheckerWhenBl @Test void createAvailabilityChecker_shouldReturnAnAvailabilityChecker() { - final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(UInt64.ONE); - + final ForkChoiceBlobSidecarsAvailabilityChecker forkChoiceBlobSidecarsAvailabilityChecker = + mock(ForkChoiceBlobSidecarsAvailabilityChecker.class); final BlockBlobSidecarsTracker blockBlobSidecarsTracker = mock(BlockBlobSidecarsTracker.class); - when(blockBlobSidecarsTracker.getSlotAndBlockRoot()).thenReturn(block.getSlotAndBlockRoot()); + when(forkChoiceBlobSidecarsAvailabilityCheckerProvider.create(blockBlobSidecarsTracker)) + .thenReturn(forkChoiceBlobSidecarsAvailabilityChecker); when(blockBlobSidecarsTrackersPool.getOrCreateBlockBlobSidecarsTracker(block)) .thenReturn(blockBlobSidecarsTracker); assertThat(blobSidecarManager.createAvailabilityChecker(block)) - .isInstanceOf(ForkChoiceBlobSidecarsAvailabilityChecker.class); + .isSameAs(forkChoiceBlobSidecarsAvailabilityChecker); + } + + @Test + void createAvailabilityCheckerAndValidateImmediately_shouldReturnValidWhenComplete() { + final BlockBlobSidecarsTracker blockBlobSidecarsTracker = mock(BlockBlobSidecarsTracker.class); + final ForkChoiceBlobSidecarsAvailabilityChecker forkChoiceBlobSidecarsAvailabilityChecker = + mock(ForkChoiceBlobSidecarsAvailabilityChecker.class); + + when(forkChoiceBlobSidecarsAvailabilityCheckerProvider.create(blockBlobSidecarsTracker)) + .thenReturn(forkChoiceBlobSidecarsAvailabilityChecker); + when(unpooledBlockBlobSidecarsTrackerProvider.create(block)) + .thenReturn(blockBlobSidecarsTracker); + when(blockBlobSidecarsTracker.add(any())).thenReturn(true); + when(blockBlobSidecarsTracker.isComplete()).thenReturn(true); + when(forkChoiceBlobSidecarsAvailabilityChecker.getAvailabilityCheckResult()) + .thenReturn( + SafeFuture.completedFuture(BlobSidecarsAndValidationResult.validResult(blobSidecars))); + + assertThat( + blobSidecarManager.createAvailabilityCheckerAndValidateImmediately(block, blobSidecars)) + .matches(BlobSidecarsAndValidationResult::isValid) + .matches( + result -> { + assertThat(result.getBlobSidecars()).containsExactlyElementsOf(blobSidecars); + return true; + }); + + verifyNoInteractions(blockBlobSidecarsTrackersPool); + } + + @Test + void createAvailabilityCheckerAndValidateImmediately_shouldReturnNotRequiredWhenPreDeneb() { + final SignedBeaconBlock preDenebBlock = + new DataStructureUtil(TestSpecFactory.createMinimalCapella()).randomSignedBeaconBlock(); + + assertThat( + blobSidecarManager.createAvailabilityCheckerAndValidateImmediately( + preDenebBlock, blobSidecars)) + .matches(BlobSidecarsAndValidationResult::isNotRequired); + + verifyNoInteractions(blockBlobSidecarsTrackersPool); } @Test void - createAvailabilityCheckerAndValidateImmediately_shouldReturnABlobSidecarsAndValidationResult() { - final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock(UInt64.ONE); + createAvailabilityCheckerAndValidateImmediately_shouldReturnInvalidWhenBlobsHaveDuplicatedIndices() { + final BlockBlobSidecarsTracker blockBlobSidecarsTracker = mock(BlockBlobSidecarsTracker.class); + final ForkChoiceBlobSidecarsAvailabilityChecker forkChoiceBlobSidecarsAvailabilityChecker = + mock(ForkChoiceBlobSidecarsAvailabilityChecker.class); - final UpdatableStore store = mock(UpdatableStore.class); - when(recentChainData.getStore()).thenReturn(store); - when(store.getTimeSeconds()).thenReturn(UInt64.ONE); - when(store.getGenesisTime()).thenReturn(UInt64.ZERO); + when(forkChoiceBlobSidecarsAvailabilityCheckerProvider.create(blockBlobSidecarsTracker)) + .thenReturn(forkChoiceBlobSidecarsAvailabilityChecker); + when(unpooledBlockBlobSidecarsTrackerProvider.create(block)) + .thenReturn(blockBlobSidecarsTracker); + when(blockBlobSidecarsTracker.add(any())).thenReturn(false); + when(blockBlobSidecarsTracker.isComplete()).thenReturn(true); + + assertThat( + blobSidecarManager.createAvailabilityCheckerAndValidateImmediately(block, blobSidecars)) + .matches(BlobSidecarsAndValidationResult::isInvalid) + .matches( + result -> { + assertThat(result.getCause()).isPresent(); + assertThat(result.getCause().orElseThrow()) + .matches(cause -> cause instanceof IllegalStateException) + .hasMessage( + "Failed to add all blobs to tracker, possible blobs with same index or index out of blocks commitment range"); + return true; + }); - assertThat(blobSidecarManager.createAvailabilityCheckerAndValidateImmediately(block, List.of())) - .isInstanceOf(BlobSidecarsAndValidationResult.class); + verifyNoInteractions(blockBlobSidecarsTrackersPool); + } + + @Test + void + createAvailabilityCheckerAndValidateImmediately_shouldReturnNotAvailableWhenBlobsAreIncomplete() { + final BlockBlobSidecarsTracker blockBlobSidecarsTracker = mock(BlockBlobSidecarsTracker.class); + final ForkChoiceBlobSidecarsAvailabilityChecker forkChoiceBlobSidecarsAvailabilityChecker = + mock(ForkChoiceBlobSidecarsAvailabilityChecker.class); + + when(forkChoiceBlobSidecarsAvailabilityCheckerProvider.create(blockBlobSidecarsTracker)) + .thenReturn(forkChoiceBlobSidecarsAvailabilityChecker); + when(unpooledBlockBlobSidecarsTrackerProvider.create(block)) + .thenReturn(blockBlobSidecarsTracker); + when(blockBlobSidecarsTracker.add(any())).thenReturn(true); + when(blockBlobSidecarsTracker.isComplete()).thenReturn(false); + + assertThat( + blobSidecarManager.createAvailabilityCheckerAndValidateImmediately(block, blobSidecars)) + .matches(BlobSidecarsAndValidationResult::isNotAvailable); + + verifyNoInteractions(blockBlobSidecarsTrackersPool); + } + + @Test + void + createAvailabilityCheckerAndValidateImmediately_shouldReturnTheAvailabilityCheckValidationResult() { + final BlockBlobSidecarsTracker blockBlobSidecarsTracker = mock(BlockBlobSidecarsTracker.class); + final ForkChoiceBlobSidecarsAvailabilityChecker forkChoiceBlobSidecarsAvailabilityChecker = + mock(ForkChoiceBlobSidecarsAvailabilityChecker.class); + + when(forkChoiceBlobSidecarsAvailabilityCheckerProvider.create(blockBlobSidecarsTracker)) + .thenReturn(forkChoiceBlobSidecarsAvailabilityChecker); + when(unpooledBlockBlobSidecarsTrackerProvider.create(block)) + .thenReturn(blockBlobSidecarsTracker); + when(blockBlobSidecarsTracker.add(any())).thenReturn(true); + when(blockBlobSidecarsTracker.isComplete()).thenReturn(true); + + final SafeFuture result = new SafeFuture<>(); + + when(forkChoiceBlobSidecarsAvailabilityChecker.getAvailabilityCheckResult()).thenReturn(result); + + assertThatThrownBy( + () -> + blobSidecarManager.createAvailabilityCheckerAndValidateImmediately( + block, blobSidecars)) + .isInstanceOf(IllegalStateException.class) + .hasMessage( + "Availability check expected to be done synchronously when providing immediate blobs"); + + result.complete(BlobSidecarsAndValidationResult.validResult(blobSidecars)); + + assertThat( + blobSidecarManager.createAvailabilityCheckerAndValidateImmediately(block, blobSidecars)) + .isSameAs(result.join()); verifyNoInteractions(blockBlobSidecarsTrackersPool); } diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTrackerTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTrackerTest.java index d5d702acbc9..275fb2fcada 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTrackerTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/blobs/BlockBlobSidecarsTrackerTest.java @@ -123,7 +123,7 @@ void setBlock_immediatelyCompletesWithBlockWithoutBlobs() { blockBlobSidecarsTracker.setBlock(block); SafeFutureAssert.assertThatSafeFuture(completionFuture).isCompleted(); - assertThat(blockBlobSidecarsTracker.isCompleted()).isTrue(); + assertThat(blockBlobSidecarsTracker.isComplete()).isTrue(); assertThat(blockBlobSidecarsTracker.getMissingBlobSidecars()).isEmpty(); assertThat(blockBlobSidecarsTracker.getBlobSidecars()).isEmpty(); @@ -210,15 +210,15 @@ void add_shouldWorkTillCompletionWhenAddingBlobsBeforeBlockIsSet() { if (idx == blobIdentifiersForBlock.size() - 1) { SafeFutureAssert.assertThatSafeFuture(completionFuture).isCompleted(); - assertThat(blockBlobSidecarsTracker.isCompleted()).isTrue(); + assertThat(blockBlobSidecarsTracker.isComplete()).isTrue(); } else { SafeFutureAssert.assertThatSafeFuture(completionFuture).isNotCompleted(); - assertThat(blockBlobSidecarsTracker.isCompleted()).isFalse(); + assertThat(blockBlobSidecarsTracker.isComplete()).isFalse(); } } SafeFutureAssert.assertThatSafeFuture(completionFuture).isCompleted(); - assertThat(blockBlobSidecarsTracker.isCompleted()).isTrue(); + assertThat(blockBlobSidecarsTracker.isComplete()).isTrue(); } @Test @@ -416,7 +416,7 @@ void enableBlockImportOnCompletion_shouldImportOnlyOnceWhenCalled() { blobSidecarsForBlock.forEach(blockBlobSidecarsTracker::add); - assertThat(blockBlobSidecarsTracker.isCompleted()).isTrue(); + assertThat(blockBlobSidecarsTracker.isComplete()).isTrue(); verify(blockImportChannel, times(1)).importBlock(block); } diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/block/BlockManagerTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/block/BlockManagerTest.java index 1095051d219..daf13b3633f 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/block/BlockManagerTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/block/BlockManagerTest.java @@ -32,9 +32,7 @@ import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.assertThatSafeFuture; import static tech.pegasys.teku.spec.config.SpecConfig.GENESIS_SLOT; import static tech.pegasys.teku.spec.datastructures.validator.BroadcastValidationLevel.GOSSIP; -import static tech.pegasys.teku.spec.logic.common.statetransition.results.BlockImportResult.FailureReason.FAILED_DATA_AVAILABILITY_CHECK_INVALID; import static tech.pegasys.teku.spec.logic.common.statetransition.results.BlockImportResult.FailureReason.FAILED_DATA_AVAILABILITY_CHECK_NOT_AVAILABLE; -import static tech.pegasys.teku.spec.logic.common.statetransition.results.BlockImportResult.FailureReason.UNKNOWN_PARENT; import static tech.pegasys.teku.statetransition.block.BlockImportPerformance.ARRIVAL_EVENT_LABEL; import static tech.pegasys.teku.statetransition.block.BlockImportPerformance.BEGIN_IMPORTING_LABEL; import static tech.pegasys.teku.statetransition.block.BlockImportPerformance.COMPLETED_EVENT_LABEL; @@ -969,52 +967,6 @@ void onDeneb_shouldStoreBlockWhenBlobSidecarsNotRequired() { .isCompletedWithValueMatching(Optional::isEmpty); } - @Test - void onDeneb_shouldNotStoreBlockWhenBlobSidecarsIsInvalid() { - // If we start genesis with Deneb, 0 will be earliestBlobSidecarSlot, so started on epoch 1 - setupWithSpec(TestSpecFactory.createMinimalWithDenebForkEpoch(UInt64.valueOf(1))); - final UInt64 slotsPerEpoch = UInt64.valueOf(spec.slotsPerEpoch(UInt64.ZERO)); - incrementSlotTo(slotsPerEpoch); - - final SignedBlockAndState signedBlockAndState1 = - localChain - .chainBuilder() - .generateBlockAtSlot(currentSlot, BlockOptions.create().setGenerateRandomBlobs(true)); - final SignedBlockAndState signedBlockAndState2 = - localChain - .chainBuilder() - .generateBlockAtSlot( - currentSlot.increment(), BlockOptions.create().setGenerateRandomBlobs(true)); - - final List blobSidecars1 = - localChain.chainBuilder().getBlobSidecars(signedBlockAndState1.getRoot()); - assertThatNothingStoredForSlotRoot(signedBlockAndState1.getSlotAndBlockRoot()); - assertThat(blobSidecars1).isNotEmpty(); - assertThat(localRecentChainData.retrieveEarliestBlobSidecarSlot()) - .isCompletedWithValueMatching(Optional::isEmpty); - - final SignedBeaconBlock block1 = signedBlockAndState1.getBlock(); - final BlobSidecarsAvailabilityChecker blobSidecarsAvailabilityChecker1 = - createAvailabilityCheckerWithInvalidBlobSidecars(block1, blobSidecars1); - - assertThatBlockImport(block1) - .isCompletedWithValueMatching( - cause -> cause.getFailureReason().equals(FAILED_DATA_AVAILABILITY_CHECK_INVALID)); - verify(blobSidecarsAvailabilityChecker1).getAvailabilityCheckResult(); - assertThat(localRecentChainData.retrieveBlockByRoot(block1.getRoot())) - .isCompletedWithValue(Optional.empty()); - assertThat(localRecentChainData.getBlobSidecars(block1.getSlotAndBlockRoot())).isEmpty(); - assertThat(localRecentChainData.retrieveEarliestBlobSidecarSlot()) - .isCompletedWithValueMatching(Optional::isEmpty); - - verify(blockBlobSidecarsTrackersPool).removeAllForBlock(block1.getRoot()); - assertThat(invalidBlockRoots).doesNotContainKeys(block1.getRoot()); - - // if we receive a block building on top of block1, we should trigger unknown parent flow - assertThatBlockImport(signedBlockAndState2.getBlock()) - .isCompletedWithValueMatching(cause -> cause.getFailureReason().equals(UNKNOWN_PARENT)); - } - @Test void onDeneb_shouldNotStoreBlockWhenBlobSidecarsIsNotAvailable() { // If we start genesis with Deneb, 0 will be earliestBlobSidecarSlot, so started on epoch 1 @@ -1115,21 +1067,6 @@ private BlobSidecarsAvailabilityChecker createAvailabilityCheckerWithNotAvailabl return blobSidecarsAvailabilityChecker; } - private BlobSidecarsAvailabilityChecker createAvailabilityCheckerWithInvalidBlobSidecars( - final SignedBeaconBlock block, final List blobSidecars) { - reset(blobSidecarManager); - final BlobSidecarsAvailabilityChecker blobSidecarsAvailabilityChecker = - mock(BlobSidecarsAvailabilityChecker.class); - when(blobSidecarManager.createAvailabilityChecker(eq(block))) - .thenReturn(blobSidecarsAvailabilityChecker); - when(blobSidecarsAvailabilityChecker.getAvailabilityCheckResult()) - .thenReturn( - SafeFuture.completedFuture( - BlobSidecarsAndValidationResult.invalidResult( - blobSidecars, new RuntimeException("ouch")))); - return blobSidecarsAvailabilityChecker; - } - private void assertThatStored( final BeaconBlock beaconBlock, final List blobSidecars) { assertThat(localRecentChainData.retrieveBlockByRoot(beaconBlock.getRoot())) diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceBlobSidecarsAvailabilityCheckerTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceBlobSidecarsAvailabilityCheckerTest.java index 10acbc68200..b2406f0e9da 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceBlobSidecarsAvailabilityCheckerTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceBlobSidecarsAvailabilityCheckerTest.java @@ -15,45 +15,31 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.ArgumentMatchers.assertArg; -import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.reset; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.assertThatSafeFuture; import com.google.common.collect.ImmutableSortedMap; import java.time.Duration; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeoutException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EnumSource; -import org.mockito.stubbing.OngoingStubbing; import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.async.SafeFutureAssert; -import tech.pegasys.teku.infrastructure.async.StubAsyncRunner; import tech.pegasys.teku.infrastructure.async.Waiter; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.kzg.KZG; -import tech.pegasys.teku.kzg.KZGCommitment; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.SpecVersion; import tech.pegasys.teku.spec.TestSpecFactory; import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; -import tech.pegasys.teku.spec.datastructures.type.SszKZGCommitment; import tech.pegasys.teku.spec.logic.common.helpers.MiscHelpers; import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsAndValidationResult; import tech.pegasys.teku.spec.logic.versions.deneb.blobs.BlobSidecarsValidationResult; @@ -69,20 +55,15 @@ public class ForkChoiceBlobSidecarsAvailabilityCheckerTest { private final Spec spec = mock(Spec.class); private final SpecVersion specVersion = mock(SpecVersion.class); + private final MiscHelpers miscHelpers = mock(MiscHelpers.class); private final KZG kzg = mock(KZG.class); - private final StubAsyncRunner asyncRunner = new StubAsyncRunner(); private final UpdatableStore store = mock(UpdatableStore.class); private final BlockBlobSidecarsTracker blockBlobSidecarsTracker = mock(BlockBlobSidecarsTracker.class); - private final MiscHelpers miscHelpers = mock(MiscHelpers.class); private final RecentChainData recentChainData = mock(RecentChainData.class); private SignedBeaconBlock block; private List blobSidecarsComplete; - private List kzgCommitmentsComplete; - - private List blobSidecarsInitial; - private List blobSidecarsAdditional; private final SafeFuture trackerCompletionFuture = new SafeFuture<>(); @@ -96,315 +77,152 @@ void setUp() { when(blockBlobSidecarsTracker.getCompletionFuture()).thenReturn(trackerCompletionFuture); } - @ParameterizedTest - @EnumSource(Availability.class) - void shouldVerifyAvailableBlobsInTwoBatches(final Availability availability) throws Exception { - prepareInitialAvailability(availability); - - final SafeFuture availabilityCheckResult = - blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); - - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - - // initiate availability check - assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); - - // all validation on a separate thread, so no interaction so far. - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - verifyDataAvailabilityNeverCalled(); - verify(blockBlobSidecarsTracker, never()).getBlobSidecars(); - - // mock kzg availability check to be OK for the initial set - whenDataAvailability(blobSidecarsInitial).thenReturn(true); - - // let availability check to be performed. - asyncRunner.executeDueActions(); - - // verify that kzg validation has been performed for the initial batch - verifyValidationAndDataAvailabilityCall(blobSidecarsInitial, false); - - // mock the additional check to be OK. - whenDataAvailability(blobSidecarsAdditional).thenReturn(true); - - // let the tracker complete with all blobSidecars - completeTrackerWith(blobSidecarsComplete); + @Test + void shouldVerifyValidAvailableBlobs() throws Exception { + prepareInitialAvailability(); - Waiter.waitFor(availabilityCheckResult); + when(miscHelpers.verifyBlobKzgProofBatch(any(), any())).thenReturn(true); + when(miscHelpers.verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock(anyList(), any())) + .thenReturn(true); - // verify that kzg validation has been performed for the additional batch - verifyValidationAndDataAvailabilityCall(blobSidecarsAdditional, true); + final SafeFuture availabilityCheckResult = + runAvailabilityCheck(); assertAvailable(availabilityCheckResult); - - // no interaction since last verify - verifyNoInteractions(miscHelpers); } @Test - void shouldVerifyAvailableBlobsInOneBatch() throws Exception { - prepareInitialAvailability(Availability.FULL); - - final SafeFuture availabilityCheckResult = - blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); - - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - - // initiate availability check - assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); + void shouldVerifyInvalidBlobsDueToWrongBlockHeader() throws Exception { + prepareInitialAvailability(); - // mock kzg availability check to be OK for the initial set - whenDataAvailability(blobSidecarsComplete).thenReturn(true); + when(miscHelpers.verifyBlobKzgProofBatch(any(), any())).thenReturn(true); + when(miscHelpers.verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock(anyList(), any())) + .thenReturn(false); - // tracker is completed in advance - completeTrackerWith(blobSidecarsComplete); - - // let availability check to be performed. - asyncRunner.executeDueActions(); - - // verify that kzg validation has been performed for the initial batch - verifyValidationAndDataAvailabilityCall(blobSidecarsComplete, true); - - Waiter.waitFor(availabilityCheckResult); - - assertAvailable(availabilityCheckResult); + final SafeFuture availabilityCheckResult = + runAvailabilityCheck(); - // no interaction since last verify - verifyNoInteractions(miscHelpers); + assertInvalid( + availabilityCheckResult, + blobSidecarsComplete, + Optional.of( + new IllegalStateException("Blob sidecars block header does not match signed block"))); } @Test - void shouldReturnNotAvailableOnTimeout() throws Exception { - prepareForImmediateTimeout(); + void shouldVerifyInvalidBlobsDueToWrongKzg() throws Exception { + prepareInitialAvailability(); + + when(miscHelpers.verifyBlobKzgProofBatch(any(), any())).thenReturn(false); + when(miscHelpers.verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock(anyList(), any())) + .thenReturn(true); final SafeFuture availabilityCheckResult = - blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); + runAvailabilityCheck(); - assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); + assertInvalid(availabilityCheckResult, blobSidecarsComplete, Optional.empty()); + } - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - verifyDataAvailabilityNeverCalled(); - verify(blockBlobSidecarsTracker, never()).getBlobSidecars(); + @Test + void shouldVerifyInvalidBlobsWhenKzgValidationThrows() throws Exception { + prepareInitialAvailability(); - whenDataAvailability(blobSidecarsInitial).thenReturn(true); + final RuntimeException error = new RuntimeException("oops"); - asyncRunner.executeDueActions(); + when(miscHelpers.verifyBlobKzgProofBatch(any(), any())).thenThrow(error); - Waiter.waitFor(availabilityCheckResult); + final SafeFuture availabilityCheckResult = + runAvailabilityCheck(); - assertNotAvailableDueToTimeout(availabilityCheckResult); + assertInvalid(availabilityCheckResult, blobSidecarsComplete, Optional.of(error)); } @Test - void shouldReturnNotRequiredWhenBlockIsOutsideAvailabilityWindow() { - prepareBlockAndBlobSidecarsOutsideAvailabilityWindow(); - - final SafeFuture availabilityCheckResult = - blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); - - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - - assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - asyncRunner.executeDueActions(); - assertNotRequired(availabilityCheckResult); - } + void shouldVerifyInvalidBlobsWhenCompletenessValidationThrows() throws Exception { + prepareInitialAvailability(); - @ParameterizedTest - @EnumSource(value = BatchFailure.class) - void shouldReturnNotAvailableIfFirstBatchFails(final BatchFailure batchFailure) { - prepareInitialAvailability(Availability.PARTIAL); + final RuntimeException error = new RuntimeException("oops"); - final Optional cause = - switch (batchFailure) { - case BLOB_SIDECAR_VALIDATION_EXCEPTION, IS_DATA_AVAILABLE_EXCEPTION -> - Optional.of(new RuntimeException("oops")); - default -> Optional.empty(); - }; + when(miscHelpers.verifyBlobKzgProofBatch(any(), any())).thenReturn(true); + when(miscHelpers.verifyBlobSidecarBlockHeaderSignatureViaValidatedSignedBlock(anyList(), any())) + .thenReturn(true); + doThrow(error).when(miscHelpers).verifyBlobSidecarCompleteness(anyList(), any()); final SafeFuture availabilityCheckResult = - blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); - - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - - // initiate availability check - assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); - - switch (batchFailure) { - // blobsidecar validation check failure for the initial set - case BLOB_SIDECAR_VALIDATION_EXCEPTION -> - throwWhenValidatingBlobSidecarsBatchAgainstBlock(blobSidecarsInitial, cause.get()); - // mock kzg availability check failure for the initial set - case IS_DATA_AVAILABLE_EXCEPTION -> - whenDataAvailability(blobSidecarsInitial).thenThrow(cause.get()); - case IS_DATA_AVAILABLE_RETURN_FALSE -> - whenDataAvailability(blobSidecarsInitial).thenReturn(false); - } - - asyncRunner.executeDueActions(); + runAvailabilityCheck(); - assertInvalid(availabilityCheckResult, blobSidecarsInitial, cause); + assertInvalid(availabilityCheckResult, blobSidecarsComplete, Optional.of(error)); } - @ParameterizedTest - @EnumSource(value = BatchFailure.class) - void shouldReturnNotAvailableIfSecondBatchFails(final BatchFailure batchFailure) { - prepareInitialAvailability(Availability.PARTIAL); - - final Optional cause = - switch (batchFailure) { - case BLOB_SIDECAR_VALIDATION_EXCEPTION, IS_DATA_AVAILABLE_EXCEPTION -> - Optional.of(new RuntimeException("oops")); - default -> Optional.empty(); - }; + @Test + void shouldFailIfTrackerCompletesWithFailure() { + prepareInitialAvailability(); final SafeFuture availabilityCheckResult = blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); + assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); // initiate availability check assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); - // mock kzg availability check to be OK for the initial set - whenDataAvailability(blobSidecarsInitial).thenReturn(true); - - // let availability check to be performed. - asyncRunner.executeDueActions(); - - switch (batchFailure) { - // blobsidecar validation check failure for the additional set - case BLOB_SIDECAR_VALIDATION_EXCEPTION -> - throwWhenValidatingBlobSidecarsBatchAgainstBlock(blobSidecarsAdditional, cause.get()); - // mock kzg availability check failure for the additional set - case IS_DATA_AVAILABLE_EXCEPTION -> - whenDataAvailability(blobSidecarsAdditional).thenThrow(cause.get()); - case IS_DATA_AVAILABLE_RETURN_FALSE -> - whenDataAvailability(blobSidecarsAdditional).thenReturn(false); - } + assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); + verify(blockBlobSidecarsTracker, never()).getBlobSidecars(); // let the tracker complete with all blobSidecars - completeTrackerWith(blobSidecarsComplete); + trackerCompletionFuture.completeExceptionally(new RuntimeException("oops")); - assertInvalid(availabilityCheckResult, blobSidecarsAdditional, cause); + assertThatSafeFuture(availabilityCheckResult).isCompletedExceptionallyWithMessage("oops"); } @Test - void shouldReturnInvalidIfTrackerLiesWithCompletionButItIsNot() { - prepareInitialAvailability(Availability.PARTIAL); + void shouldReturnNotAvailableOnTimeout() throws Exception { + prepareForImmediateTimeout(); final SafeFuture availabilityCheckResult = blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - - // initiate availability check assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); - // all validation on a separate thread, so no interaction so far. - SafeFutureAssert.assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - verifyDataAvailabilityNeverCalled(); verify(blockBlobSidecarsTracker, never()).getBlobSidecars(); - // mock kzg availability check to be OK for the initial set - whenDataAvailability(blobSidecarsInitial).thenReturn(true); - - // let availability check to be performed. - asyncRunner.executeDueActions(); - - // verify that kzg validation has been performed for the initial batch - verifyValidationAndDataAvailabilityCall(blobSidecarsInitial, false); - - // we complete the blobs without index 3 - final List partialBlobs = blobSidecarsComplete.subList(1, 2); - // we lie on availability check too (not actually possible) - whenDataAvailability(partialBlobs).thenReturn(true); - - final List expectedIncompleteBlobSidecar = new ArrayList<>(); - expectedIncompleteBlobSidecar.add(blobSidecarsComplete.get(0)); // blob 0 - expectedIncompleteBlobSidecar.add(blobSidecarsComplete.get(1)); // blob 1 - expectedIncompleteBlobSidecar.add(blobSidecarsComplete.get(2)); // blob 2 - - final Throwable cause = new IllegalArgumentException("oops"); - - throwWhenVerifyingBlobSidecarCompleteness(expectedIncompleteBlobSidecar, cause); - - // let the tracker complete with all blobSidecars - completeTrackerWith(partialBlobs); + Waiter.waitFor(availabilityCheckResult); - assertInvalid( - availabilityCheckResult, - expectedIncompleteBlobSidecar, - Optional.of( - new IllegalArgumentException( - "Validated blobs are less than commitments present in block.", cause))); + assertNotAvailableDueToTimeout(availabilityCheckResult); } @Test - void validateImmediately_shouldReturnAvailable() { - prepareInitialAvailability(Availability.FULL); - - whenDataAvailability(blobSidecarsComplete).thenReturn(true); + void shouldReturnNotRequiredWhenBlockIsOutsideAvailabilityWindow() throws Exception { + prepareForImmediateTimeoutWithBlockAndBlobSidecarsOutsideAvailabilityWindow(); - assertAvailable( - SafeFuture.completedFuture( - blobSidecarsAvailabilityChecker.validateImmediately(blobSidecarsComplete))); - } - - @Test - void validateImmediately_shouldReturnInvalidIfCompletenessCheckFails() { - prepareInitialAvailability(Availability.FULL); + final SafeFuture availabilityCheckResult = + blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); - whenDataAvailability(blobSidecarsComplete).thenReturn(true); - final Throwable cause = new IllegalArgumentException("oops"); - doThrow(cause).when(miscHelpers).verifyBlobSidecarCompleteness(any(), any()); + assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); - final BlobSidecarsAndValidationResult availabilityCheckResult = - blobSidecarsAvailabilityChecker.validateImmediately(blobSidecarsComplete); + Waiter.waitFor(availabilityCheckResult); - assertInvalid( - SafeFuture.completedFuture(availabilityCheckResult), - blobSidecarsComplete, - Optional.of( - new IllegalArgumentException( - "Validated blobs are less than commitments present in block.", cause))); + assertNotRequired(availabilityCheckResult); } - @Test - void validateImmediately_shouldReturnNotAvailableWithEmptyBlobsButRequired() { - prepareInitialAvailability(Availability.FULL); + private SafeFuture runAvailabilityCheck() throws Exception { + final SafeFuture availabilityCheckResult = + blobSidecarsAvailabilityChecker.getAvailabilityCheckResult(); - assertNotAvailable( - SafeFuture.completedFuture( - blobSidecarsAvailabilityChecker.validateImmediately(Collections.emptyList()))); - } + assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); - @Test - void validateImmediately_shouldReturnAvailableOnEmptyBlobs() { - prepareInitialAvailabilityWithEmptyCommitmentsBlock(); + // initiate availability check + assertThat(blobSidecarsAvailabilityChecker.initiateDataAvailabilityCheck()).isTrue(); - assertAvailable( - SafeFuture.completedFuture( - blobSidecarsAvailabilityChecker.validateImmediately(Collections.emptyList()))); - } + assertThatSafeFuture(availabilityCheckResult).isNotCompleted(); + verify(blockBlobSidecarsTracker, never()).getBlobSidecars(); - @Test - void validateImmediately_shouldReturnNotRequiredWhenBlockIsOutsideAvailabilityWindow() { - prepareBlockAndBlobSidecarsOutsideAvailabilityWindow(); + // let the tracker complete with all blobSidecars + completeTrackerWith(blobSidecarsComplete); - assertNotRequired( - SafeFuture.completedFuture( - blobSidecarsAvailabilityChecker.validateImmediately(Collections.emptyList()))); - } + Waiter.waitFor(availabilityCheckResult); - private void assertNotRequired( - final SafeFuture availabilityOrValidityCheck) { - assertThat(availabilityOrValidityCheck) - .isCompletedWithValueMatching(result -> !result.isFailure(), "is not failure") - .isCompletedWithValueMatching(result -> !result.isValid(), "is not valid") - .isCompletedWithValueMatching( - BlobSidecarsAndValidationResult::isNotRequired, "is not required") - .isCompletedWithValueMatching( - result -> result.getBlobSidecars().isEmpty(), "doesn't have blob sidecars"); + return availabilityCheckResult; } private void assertInvalid( @@ -435,6 +253,16 @@ private void assertInvalid( "matches the cause"); } + private void assertNotRequired( + final SafeFuture availabilityOrValidityCheck) { + assertThat(availabilityOrValidityCheck) + .isCompletedWithValueMatching(result -> !result.isValid(), "is not valid") + .isCompletedWithValueMatching( + BlobSidecarsAndValidationResult::isNotRequired, "is not required") + .isCompletedWithValueMatching( + result -> result.getBlobSidecars().isEmpty(), "doesn't have blob sidecars"); + } + private void assertNotAvailableDueToTimeout( final SafeFuture availabilityOrValidityCheck) { assertNotAvailable(availabilityOrValidityCheck); @@ -446,7 +274,6 @@ private void assertNotAvailableDueToTimeout( private void assertNotAvailable( final SafeFuture availabilityOrValidityCheck) { assertThat(availabilityOrValidityCheck) - .isCompletedWithValueMatching(BlobSidecarsAndValidationResult::isFailure, "is failure") .isCompletedWithValueMatching(result -> !result.isValid(), "is not valid") .isCompletedWithValueMatching( result -> result.getValidationResult() == BlobSidecarsValidationResult.NOT_AVAILABLE, @@ -458,7 +285,6 @@ private void assertNotAvailable( private void assertAvailable( final SafeFuture availabilityOrValidityCheck) { assertThat(availabilityOrValidityCheck) - .isCompletedWithValueMatching(result -> !result.isFailure(), "is not failure") .isCompletedWithValueMatching(BlobSidecarsAndValidationResult::isValid, "is valid") .isCompletedWithValueMatching( result -> result.getValidationResult() == BlobSidecarsValidationResult.VALID, @@ -467,71 +293,28 @@ private void assertAvailable( result -> result.getBlobSidecars().equals(blobSidecarsComplete), "has blob sidecars"); } - private void prepareInitialAvailabilityWithEmptyCommitmentsBlock() { - prepareInitialAvailability( - Availability.FULL, - Optional.of(dataStructureUtil.randomSignedBeaconBlockWithEmptyCommitments())); - } - private void prepareForImmediateTimeout() { - prepareInitialAvailability(Availability.PARTIAL, Optional.empty(), Duration.ZERO); + prepareInitialAvailability(Optional.empty(), Duration.ZERO); } - private void prepareInitialAvailability(final Availability blobsAvailability) { - prepareInitialAvailability(blobsAvailability, Optional.empty(), Duration.ofSeconds(30)); - } - - private void prepareInitialAvailability( - final Availability blobsAvailability, final Optional providedBlock) { - prepareInitialAvailability(blobsAvailability, providedBlock, Duration.ofSeconds(30)); + private void prepareInitialAvailability() { + prepareInitialAvailability(Optional.empty(), Duration.ofSeconds(30)); } private void prepareInitialAvailability( - final Availability blobsAvailability, - final Optional providedBlock, - final Duration timeout) { + final Optional providedBlock, final Duration timeout) { block = providedBlock.orElse(dataStructureUtil.randomSignedBeaconBlockWithCommitments(4)); blobSidecarsComplete = dataStructureUtil.randomBlobSidecarsForBlock(block); - kzgCommitmentsComplete = - block - .getBeaconBlock() - .orElseThrow() - .getBody() - .toVersionDeneb() - .orElseThrow() - .getBlobKzgCommitments() - .stream() - .map(SszKZGCommitment::getKZGCommitment) - .toList(); when(spec.isAvailabilityOfBlobSidecarsRequiredAtSlot(store, block.getSlot())).thenReturn(true); - switch (blobsAvailability) { - case FULL -> { - blobSidecarsInitial = blobSidecarsComplete; - blobSidecarsAdditional = List.of(); - } - case EMPTY -> { - blobSidecarsInitial = List.of(); - blobSidecarsAdditional = blobSidecarsComplete; - } - case PARTIAL -> { - blobSidecarsInitial = List.of(blobSidecarsComplete.get(0), blobSidecarsComplete.get(2)); - blobSidecarsAdditional = List.of(blobSidecarsComplete.get(1), blobSidecarsComplete.get(3)); - } - } - - final ImmutableSortedMap.Builder mapBuilder = - ImmutableSortedMap.naturalOrder(); - blobSidecarsInitial.forEach(blobSidecar -> mapBuilder.put(blobSidecar.getIndex(), blobSidecar)); - when(blockBlobSidecarsTracker.getBlock()).thenReturn(Optional.of(block)); - when(blockBlobSidecarsTracker.getBlobSidecars()).thenReturn(mapBuilder.build()); + when(blockBlobSidecarsTracker.getBlobSidecars()).thenReturn(ImmutableSortedMap.of()); when(blockBlobSidecarsTracker.getSlotAndBlockRoot()).thenReturn(block.getSlotAndBlockRoot()); blobSidecarsAvailabilityChecker = new ForkChoiceBlobSidecarsAvailabilityChecker( - spec, asyncRunner, recentChainData, blockBlobSidecarsTracker, kzg, timeout); + spec, recentChainData, blockBlobSidecarsTracker, kzg, timeout); } private void completeTrackerWith(final List blobSidecars) { @@ -539,68 +322,11 @@ private void completeTrackerWith(final List blobSidecars) { ImmutableSortedMap.naturalOrder(); blobSidecars.forEach(blobSidecar -> mapBuilder.put(blobSidecar.getIndex(), blobSidecar)); when(blockBlobSidecarsTracker.getBlobSidecars()).thenReturn(mapBuilder.build()); - when(blockBlobSidecarsTracker.isCompleted()).thenReturn(true); + when(blockBlobSidecarsTracker.isComplete()).thenReturn(true); trackerCompletionFuture.complete(null); } - private OngoingStubbing whenDataAvailability(final List blobSidecars) { - return when(miscHelpers.verifyBlobKzgProofBatch(kzg, blobSidecars)); - } - - private void throwWhenValidatingBlobSidecarsBatchAgainstBlock( - final List blobSidecars, final Throwable cause) { - doThrow(cause) - .when(miscHelpers) - .validateBlobSidecarsBatchAgainstBlock( - eq(blobSidecars), - argThat(block -> block.equals(this.block.getBeaconBlock().orElseThrow())), - assertArg( - kzgCommitmentsArg -> - assertThat(kzgCommitmentsArg).isEqualTo(kzgCommitmentsComplete))); - } - - private void throwWhenVerifyingBlobSidecarCompleteness( - final List blobSidecars, final Throwable cause) { - doThrow(cause) - .when(miscHelpers) - .verifyBlobSidecarCompleteness( - eq(blobSidecars), - assertArg( - kzgCommitmentsArg -> - assertThat(kzgCommitmentsArg).isEqualTo(kzgCommitmentsComplete))); - } - - private void verifyValidationAndDataAvailabilityCall( - final List blobSidecars, final boolean isFinalValidation) { - verify(miscHelpers, times(1)) - .validateBlobSidecarsBatchAgainstBlock( - eq(blobSidecars), - argThat(block -> block.equals(this.block.getBeaconBlock().orElseThrow())), - assertArg( - kzgCommitmentsArg -> - assertThat(kzgCommitmentsArg).isEqualTo(kzgCommitmentsComplete))); - - verify(miscHelpers, times(1)).verifyBlobKzgProofBatch(kzg, blobSidecars); - - if (isFinalValidation) { - verify(miscHelpers, times(1)) - .verifyBlobSidecarCompleteness( - eq(blobSidecarsComplete), - assertArg( - kzgCommitmentsArg -> - assertThat(kzgCommitmentsArg).isEqualTo(kzgCommitmentsComplete))); - } - - // assume we verified all interaction before resetting - verifyNoMoreInteractions(miscHelpers); - reset(miscHelpers); - } - - private void verifyDataAvailabilityNeverCalled() { - verify(miscHelpers, never()).verifyBlobKzgProofBatch(eq(kzg), any()); - } - - private void prepareBlockAndBlobSidecarsOutsideAvailabilityWindow() { + private void prepareForImmediateTimeoutWithBlockAndBlobSidecarsOutsideAvailabilityWindow() { block = dataStructureUtil.randomSignedBeaconBlock(); blobSidecarsComplete = dataStructureUtil.randomBlobSidecarsForBlock(block); @@ -611,29 +337,11 @@ private void prepareBlockAndBlobSidecarsOutsideAvailabilityWindow() { when(spec.isAvailabilityOfBlobSidecarsRequiredAtSlot(store, block.getSlot())).thenReturn(false); when(blockBlobSidecarsTracker.getBlock()).thenReturn(Optional.of(block)); - when(blockBlobSidecarsTracker.getCompletionFuture()).thenReturn(SafeFuture.COMPLETE); when(blockBlobSidecarsTracker.getBlobSidecars()).thenReturn(ImmutableSortedMap.of()); when(blockBlobSidecarsTracker.getSlotAndBlockRoot()).thenReturn(block.getSlotAndBlockRoot()); blobSidecarsAvailabilityChecker = new ForkChoiceBlobSidecarsAvailabilityChecker( - spec, - asyncRunner, - recentChainData, - blockBlobSidecarsTracker, - kzg, - Duration.ofSeconds(30)); - } - - private enum Availability { - EMPTY, - PARTIAL, - FULL - } - - private enum BatchFailure { - BLOB_SIDECAR_VALIDATION_EXCEPTION, - IS_DATA_AVAILABLE_EXCEPTION, - IS_DATA_AVAILABLE_RETURN_FALSE, + spec, recentChainData, blockBlobSidecarsTracker, kzg, Duration.ZERO); } } diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java index fda3089f451..589b3b0e151 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java @@ -247,31 +247,6 @@ void onBlock_shouldFailIfBlobsAreNotAvailable() { verify(blobSidecarsAvailabilityChecker).getAvailabilityCheckResult(); } - @Test - void onBlock_shouldFailIfBlobsAreInvalid() { - setupWithSpec(TestSpecFactory.createMinimalDeneb()); - final SignedBlockAndState blockAndState = chainBuilder.generateBlockAtSlot(ONE); - storageSystem.chainUpdater().advanceCurrentSlotToAtLeast(blockAndState.getSlot()); - final List blobSidecars = - storageSystem - .chainStorage() - .getBlobSidecarsBySlotAndBlockRoot(blockAndState.getSlotAndBlockRoot()) - .join(); - - when(blobSidecarsAvailabilityChecker.getAvailabilityCheckResult()) - .thenReturn( - SafeFuture.completedFuture( - BlobSidecarsAndValidationResult.invalidResult(blobSidecars))); - - importBlockAndAssertFailure( - blockAndState, FailureReason.FAILED_DATA_AVAILABILITY_CHECK_INVALID); - - verify(blobSidecarManager).createAvailabilityChecker(blockAndState.getBlock()); - verify(blobSidecarsAvailabilityChecker).initiateDataAvailabilityCheck(); - verify(blobSidecarsAvailabilityChecker).getAvailabilityCheckResult(); - verify(debugDataDumper).saveInvalidBlobSidecars(blobSidecars, blockAndState.getBlock()); - } - @Test void onBlock_consensusValidationShouldNotResolveWhenDataAvailabilityFails() { setupWithSpec(TestSpecFactory.createMinimalDeneb()); 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..1a92bd8b42e 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,13 +111,14 @@ 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 void shouldAccept() { SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar)) .isCompletedWithValueMatching(InternalValidationResult::isAccept); + assertThat(blobSidecar.isSignatureValidated()).isTrue(); } @TestTemplate @@ -184,6 +185,8 @@ void shouldRejectIfSignatureIsInvalid() { SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar)) .isCompletedWithValueMatching(InternalValidationResult::isReject); + + assertThat(blobSidecar.isSignatureValidated()).isFalse(); } @TestTemplate @@ -224,7 +227,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 +295,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 +307,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 +320,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,12 +339,15 @@ 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()); verify(gossipValidationHelper, never()) .isSignatureValidWithRespectToProposerIndex(any(), any(), any(), any()); + + assertThat(blobSidecar.isSignatureValidated()).isTrue(); + clearInvocations(gossipValidationHelper); // BlobSidecar from the new block @@ -365,7 +371,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 {}", diff --git a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java index 47e0d1540e4..5fc563a9f4b 100644 --- a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java +++ b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java @@ -586,7 +586,6 @@ protected void initBlobSidecarManager() { final BlobSidecarManagerImpl blobSidecarManagerImpl = new BlobSidecarManagerImpl( spec, - beaconAsyncRunner, recentChainData, blockBlobSidecarsTrackersPool, blobSidecarValidator,