Skip to content

Commit

Permalink
Optimize BlobSidecarGossipValidator (Consensys#7749)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zilm13
zilm13 authored Nov 23, 2023
1 parent 576e4b8 commit 82c578d
Show file tree
Hide file tree
Showing 3 changed files with 239 additions and 4 deletions.
3 changes: 3 additions & 0 deletions ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/Constants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ public class Constants {
// sync committee size.
public static final int VALID_CONTRIBUTION_AND_PROOF_SET_SIZE = 512;
public static final int VALID_SYNC_COMMITTEE_MESSAGE_SET_SIZE = 512;
// When finalization is at its best case with 100% of votes we could have up to 3 full
// epochs of non-finalized blocks
public static final int BEST_CASE_NON_FINALIZED_EPOCHS = 3;

public static final Duration ETH1_INDIVIDUAL_BLOCK_RETRY_TIMEOUT = Duration.ofMillis(500);
public static final Duration ETH1_DEPOSIT_REQUEST_RETRY_TIMEOUT = Duration.ofSeconds(2);
Expand Down
72 changes: 69 additions & 3 deletions ...rc/main/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
package tech.pegasys.teku.statetransition.validation;

import static tech.pegasys.teku.infrastructure.async.SafeFuture.completedFuture;
import static tech.pegasys.teku.spec.config.Constants.BEST_CASE_NON_FINALIZED_EPOCHS;
import static tech.pegasys.teku.spec.config.Constants.VALID_BLOCK_SET_SIZE;
import static tech.pegasys.teku.statetransition.validation.InternalValidationResult.ACCEPT;
import static tech.pegasys.teku.statetransition.validation.InternalValidationResult.ignore;
import static tech.pegasys.teku.statetransition.validation.InternalValidationResult.reject;

Expand Down Expand Up @@ -48,6 +50,7 @@ public class BlobSidecarGossipValidator {

private final Spec spec;
private final Set<SlotProposerIndexAndBlobIndex> receivedValidBlobSidecarInfoSet;
private final Set<Bytes32> validSignedBlockHeaders;
private final GossipValidationHelper gossipValidationHelper;
private final Map<Bytes32, BlockImportResult> invalidBlockRoots;
private final MiscHelpersDeneb miscHelpersDeneb;
Expand All @@ -63,14 +66,18 @@ public static BlobSidecarGossipValidator create(
final Optional<Integer> maybeMaxBlobsPerBlock = spec.getMaxBlobsPerBlock();

final int validInfoSize = VALID_BLOCK_SET_SIZE * maybeMaxBlobsPerBlock.orElse(1);
// It's not fatal if we miss something and we don't need finalized data
final int validSignedBlockHeadersSize =
spec.getGenesisSpec().getSlotsPerEpoch() * BEST_CASE_NON_FINALIZED_EPOCHS;

return new BlobSidecarGossipValidator(
spec,
invalidBlockRoots,
validationHelper,
miscHelpersDeneb,
kzg,
LimitedSet.createSynchronized(validInfoSize));
LimitedSet.createSynchronized(validInfoSize),
LimitedSet.createSynchronized(validSignedBlockHeadersSize));
}

@VisibleForTesting
Expand All @@ -84,13 +91,15 @@ private BlobSidecarGossipValidator(
final GossipValidationHelper gossipValidationHelper,
final MiscHelpersDeneb miscHelpersDeneb,
final KZG kzg,
final Set<SlotProposerIndexAndBlobIndex> receivedValidBlobSidecarInfoSet) {
final Set<SlotProposerIndexAndBlobIndex> receivedValidBlobSidecarInfoSet,
final Set<Bytes32> validSignedBlockHeaders) {
this.spec = spec;
this.invalidBlockRoots = invalidBlockRoots;
this.gossipValidationHelper = gossipValidationHelper;
this.miscHelpersDeneb = miscHelpersDeneb;
this.kzg = kzg;
this.receivedValidBlobSidecarInfoSet = receivedValidBlobSidecarInfoSet;
this.validSignedBlockHeaders = validSignedBlockHeaders;
}

public SafeFuture<InternalValidationResult> validate(final BlobSidecar blobSidecar) {
Expand Down Expand Up @@ -134,6 +143,12 @@ public SafeFuture<InternalValidationResult> validate(final BlobSidecar blobSidec
return completedFuture(InternalValidationResult.IGNORE);
}

// Optimization: If we have already completely verified BlobSidecar with the same
// SignedBlockHeader, we can skip most steps and jump to shortened validation
if (validSignedBlockHeaders.contains(blobSidecar.getSignedBeaconBlockHeader().hashTreeRoot())) {
return validateBlobSidecarWithKnownValidHeader(blobSidecar, blockHeader);
}

/*
* [REJECT] The proposer signature of `blob_sidecar.signed_block_header`, is valid with respect
* to the `block_header.proposer_index` pubkey.
Expand Down Expand Up @@ -241,6 +256,12 @@ public SafeFuture<InternalValidationResult> validate(final BlobSidecar blobSidec
return reject("BlobSidecar block header signature is invalid.");
}

/*
* Checking it again at the very end because whole method is not synchronized
*
* [IGNORE] The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, blob_sidecar.index)
* with valid header signature, sidecar inclusion proof, and kzg proof.
*/
if (!receivedValidBlobSidecarInfoSet.add(
new SlotProposerIndexAndBlobIndex(
blockHeader.getSlot(),
Expand All @@ -250,10 +271,55 @@ public SafeFuture<InternalValidationResult> validate(final BlobSidecar blobSidec
"BlobSidecar is not the first valid for its slot and index. It will be dropped.");
}

return InternalValidationResult.ACCEPT;
validSignedBlockHeaders.add(blobSidecar.getSignedBeaconBlockHeader().hashTreeRoot());

return ACCEPT;
});
}

private SafeFuture<InternalValidationResult> validateBlobSidecarWithKnownValidHeader(
final BlobSidecar blobSidecar, final BeaconBlockHeader blockHeader) {

/*
* [REJECT] The sidecar's inclusion proof is valid as verified by `verify_blob_sidecar_inclusion_proof(blob_sidecar)`.
*/
if (!miscHelpersDeneb.verifyBlobSidecarMerkleProof(blobSidecar)) {
return completedFuture(reject("BlobSidecar inclusion proof validation failed"));
}

/*
* [REJECT] The sidecar's blob is valid as verified by
* `verify_blob_kzg_proof(blob_sidecar.blob, blob_sidecar.kzg_commitment, blob_sidecar.kzg_proof)`.
*/
if (!miscHelpersDeneb.verifyBlobKzgProof(kzg, blobSidecar)) {
return completedFuture(reject("BlobSidecar does not pass kzg validation"));
}

// This can be changed between two received BlobSidecars from one block, so checking
/*
* [REJECT] The current finalized_checkpoint is an ancestor of the sidecar's block -- i.e.
* `get_checkpoint_block(store, block_header.parent_root, store.finalized_checkpoint.epoch) == store.finalized_checkpoint.root`.
*/
if (!gossipValidationHelper.currentFinalizedCheckpointIsAncestorOfBlock(
blockHeader.getSlot(), blockHeader.getParentRoot())) {
return completedFuture(
reject("BlobSidecar block header does not descend from finalized checkpoint"));
}

/*
* [IGNORE] The sidecar is the first sidecar for the tuple (block_header.slot, block_header.proposer_index, blob_sidecar.index)
* with valid header signature, sidecar inclusion proof, and kzg proof.
*/
if (!receivedValidBlobSidecarInfoSet.add(
new SlotProposerIndexAndBlobIndex(
blockHeader.getSlot(), blockHeader.getProposerIndex(), blobSidecar.getIndex()))) {
return SafeFuture.completedFuture(
ignore("BlobSidecar is not the first valid for its slot and index. It will be dropped."));
}

return SafeFuture.completedFuture(ACCEPT);
}

private boolean verifyBlockHeaderSignature(
final BeaconState state, final SignedBeaconBlockHeader signedBlockHeader) {
final Bytes32 domain =
Expand Down
168 changes: 167 additions & 1 deletion ...est/java/tech/pegasys/teku/statetransition/validation/BlobSidecarGossipValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.HashMap;
Expand All @@ -30,6 +33,7 @@
import tech.pegasys.teku.kzg.KZG;
import tech.pegasys.teku.spec.Spec;
import tech.pegasys.teku.spec.SpecMilestone;
import tech.pegasys.teku.spec.SpecVersion;
import tech.pegasys.teku.spec.TestSpecContext;
import tech.pegasys.teku.spec.TestSpecInvocationContextProvider.SpecContext;
import tech.pegasys.teku.spec.datastructures.blobs.versions.deneb.BlobSidecar;
Expand All @@ -46,6 +50,7 @@ public class BlobSidecarGossipValidatorTest {
private final GossipValidationHelper gossipValidationHelper = mock(GossipValidationHelper.class);
private final MiscHelpersDeneb miscHelpersDeneb = mock(MiscHelpersDeneb.class);
private final KZG kzg = mock(KZG.class);
private DataStructureUtil dataStructureUtil;
private BlobSidecarGossipValidator blobSidecarValidator;

private UInt64 parentSlot;
Expand All @@ -61,7 +66,7 @@ public class BlobSidecarGossipValidatorTest {

@BeforeEach
void setup(final SpecContext specContext) {
final DataStructureUtil dataStructureUtil = specContext.getDataStructureUtil();
this.dataStructureUtil = specContext.getDataStructureUtil();

blobSidecarValidator =
BlobSidecarGossipValidator.create(
Expand Down Expand Up @@ -135,6 +140,9 @@ void shouldRejectWhenIndexIsTooBig(final SpecContext specContext) {
void shouldRejectWhenSlotIsNotDeneb() {
final Spec mockedSpec = mock(Spec.class);
when(mockedSpec.getMaxBlobsPerBlock(slot)).thenReturn(Optional.empty());
final SpecVersion mockedSpecVersion = mock(SpecVersion.class);
when(mockedSpec.getGenesisSpec()).thenReturn(mockedSpecVersion);
when(mockedSpecVersion.getSlotsPerEpoch()).thenReturn(1);

blobSidecarValidator =
BlobSidecarGossipValidator.create(
Expand Down Expand Up @@ -268,4 +276,162 @@ void shouldTrackValidInfoSet() {
SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar))
.isCompletedWithValueMatching(InternalValidationResult::isIgnore);
}

@TestTemplate
void shouldNotVerifyKnownValidSignedHeader() {
SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar))
.isCompletedWithValueMatching(InternalValidationResult::isAccept);

verify(miscHelpersDeneb).verifyBlobSidecarMerkleProof(blobSidecar);
verify(miscHelpersDeneb).verifyBlobKzgProof(kzg, blobSidecar);
verify(gossipValidationHelper).getParentStateInBlockEpoch(any(), any(), any());
verify(gossipValidationHelper).isProposerTheExpectedProposer(any(), any(), any());
verify(gossipValidationHelper)
.isSignatureValidWithRespectToProposerIndex(any(), any(), any(), any());
clearInvocations(gossipValidationHelper);

// Other BlobSidecar from the same block
final BlobSidecar blobSidecar0 =
dataStructureUtil
.createRandomBlobSidecarBuilder()
.signedBeaconBlockHeader(blobSidecar.getSignedBeaconBlockHeader())
.index(UInt64.ZERO)
.build();

SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar0))
.isCompletedWithValueMatching(InternalValidationResult::isAccept);

verify(miscHelpersDeneb).verifyBlobSidecarMerkleProof(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());
clearInvocations(gossipValidationHelper);

// BlobSidecar from the new block
final BlobSidecar blobSidecarNew =
dataStructureUtil.createRandomBlobSidecarBuilder().index(UInt64.ZERO).build();
final Bytes32 parentRoot =
blobSidecarNew.getSignedBeaconBlockHeader().getMessage().getParentRoot();

when(gossipValidationHelper.isSlotFinalized(blobSidecarNew.getSlot())).thenReturn(false);
when(gossipValidationHelper.isSlotFromFuture(blobSidecarNew.getSlot())).thenReturn(false);
when(gossipValidationHelper.isBlockAvailable(parentRoot)).thenReturn(true);
when(gossipValidationHelper.getSlotForBlockRoot(parentRoot))
.thenReturn(Optional.of(parentSlot));
when(gossipValidationHelper.getParentStateInBlockEpoch(
parentSlot, parentRoot, blobSidecarNew.getSlot()))
.thenReturn(SafeFuture.completedFuture(Optional.empty()));
when(gossipValidationHelper.currentFinalizedCheckpointIsAncestorOfBlock(
blobSidecarNew.getSlot(), parentRoot))
.thenReturn(true);

SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecarNew))
.isCompletedWithValueMatching(InternalValidationResult::isIgnore);

verify(miscHelpersDeneb).verifyBlobSidecarMerkleProof(blobSidecarNew);
verify(miscHelpersDeneb).verifyBlobKzgProof(kzg, blobSidecarNew);
verify(gossipValidationHelper).getParentStateInBlockEpoch(any(), any(), any());
}

@TestTemplate
void shouldVerifySignedHeaderAgainAfterItDroppedFromCache() {
final Spec specMock = mock(Spec.class);
final SpecVersion specVersion = mock(SpecVersion.class);
when(specMock.getMaxBlobsPerBlock(any())).thenReturn(Optional.of(6));
when(specMock.getGenesisSpec()).thenReturn(specVersion);
// This will make cache of size 3
when(specVersion.getSlotsPerEpoch()).thenReturn(1);
this.blobSidecarValidator =
BlobSidecarGossipValidator.create(
specMock, invalidBlocks, gossipValidationHelper, miscHelpersDeneb, kzg);
// Accept everything
when(gossipValidationHelper.isSlotFinalized(any())).thenReturn(false);
when(gossipValidationHelper.isSlotFromFuture(any())).thenReturn(false);
when(gossipValidationHelper.isBlockAvailable(any())).thenReturn(true);
when(gossipValidationHelper.getParentStateInBlockEpoch(any(), any(), any()))
.thenReturn(SafeFuture.completedFuture(Optional.of(postState)));
when(gossipValidationHelper.isProposerTheExpectedProposer(any(), any(), any()))
.thenReturn(true);
when(gossipValidationHelper.currentFinalizedCheckpointIsAncestorOfBlock(any(), any()))
.thenReturn(true);
when(gossipValidationHelper.isSignatureValidWithRespectToProposerIndex(
any(), any(), any(), any()))
.thenReturn(true);

// First blobSidecar
SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar))
.isCompletedWithValueMatching(InternalValidationResult::isAccept);
clearInvocations(gossipValidationHelper);

// Other BlobSidecar from the same block, known valid block header is detected, so short
// validation is used
final BlobSidecar blobSidecar0 =
dataStructureUtil
.createRandomBlobSidecarBuilder()
.signedBeaconBlockHeader(blobSidecar.getSignedBeaconBlockHeader())
.index(UInt64.ZERO)
.build();

SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar0))
.isCompletedWithValueMatching(InternalValidationResult::isAccept);

verify(gossipValidationHelper, never())
.isSignatureValidWithRespectToProposerIndex(any(), any(), any(), any());
clearInvocations(gossipValidationHelper);

// 2nd block BlobSidecar
final BlobSidecar blobSidecar2 = dataStructureUtil.randomBlobSidecar();
when(gossipValidationHelper.getSlotForBlockRoot(any()))
.thenReturn(Optional.of(blobSidecar2.getSlot().decrement()));

SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar2))
.isCompletedWithValueMatching(InternalValidationResult::isAccept);

verify(gossipValidationHelper)
.isSignatureValidWithRespectToProposerIndex(any(), any(), any(), any());
clearInvocations(gossipValidationHelper);

// 3rd block BlobSidecar
final BlobSidecar blobSidecar3 = dataStructureUtil.randomBlobSidecar();
when(gossipValidationHelper.getSlotForBlockRoot(any()))
.thenReturn(Optional.of(blobSidecar3.getSlot().decrement()));

SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar3))
.isCompletedWithValueMatching(InternalValidationResult::isAccept);

verify(gossipValidationHelper)
.isSignatureValidWithRespectToProposerIndex(any(), any(), any(), any());
clearInvocations(gossipValidationHelper);

// 4th block BlobSidecar, erasing block from blobSidecar0 from cache
final BlobSidecar blobSidecar4 = dataStructureUtil.randomBlobSidecar();
when(gossipValidationHelper.getSlotForBlockRoot(any()))
.thenReturn(Optional.of(blobSidecar4.getSlot().decrement()));

SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar4))
.isCompletedWithValueMatching(InternalValidationResult::isAccept);

verify(gossipValidationHelper)
.isSignatureValidWithRespectToProposerIndex(any(), any(), any(), any());
clearInvocations(gossipValidationHelper);

// BlobSidecar from the same block as blobSidecar0 and blobSidecar
final BlobSidecar blobSidecar5 =
dataStructureUtil
.createRandomBlobSidecarBuilder()
.signedBeaconBlockHeader(blobSidecar.getSignedBeaconBlockHeader())
.index(UInt64.valueOf(2))
.build();
when(gossipValidationHelper.getSlotForBlockRoot(any()))
.thenReturn(Optional.of(blobSidecar5.getSlot().decrement()));

SafeFutureAssert.assertThatSafeFuture(blobSidecarValidator.validate(blobSidecar5))
.isCompletedWithValueMatching(InternalValidationResult::isAccept);

// Signature is validating again though header was known valid until dropped from cache
verify(gossipValidationHelper)
.isSignatureValidWithRespectToProposerIndex(any(), any(), any(), any());
}
}

0 comments on commit 82c578d

Please sign in to comment.