Skip to content

Commit

Permalink
avoid block fetcher to fetch blocks tracked in blobs pool (#7134)
Browse files Browse the repository at this point in the history
  • Loading branch information
tbenr authored May 16, 2023
1 parent 1a62e86 commit e0addad
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ public void requestRecentBlock(final Bytes32 blockRoot) {
// We've already got this block
return;
}
if (blobSidecarPool.containsBlock(blockRoot)) {
// We already have this block, waiting for blobs
return;
}
final FetchBlockTask task = createTask(blockRoot);
if (allTasks.putIfAbsent(blockRoot, task) != null) {
// We're already tracking this task
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public void setup() {
fetchTaskFactory,
maxConcurrentRequests);

// blobs pool doesn't contain blocks by default
when(blobSidecarPool.containsBlock(any())).thenReturn(false);
lenient().when(fetchTaskFactory.createFetchBlockTask(any())).thenAnswer(this::createMockTask);
recentBlockFetcher.subscribeBlockFetched(importedBlocks::add);
}
Expand Down Expand Up @@ -238,6 +240,15 @@ void shouldNotFetchBlocksWhileForwardSyncIsInProgress() {
assertTaskCounts(0, 0, 0);
}

@Test
void shouldNotFetchIfBlockIsWaitingForBlobs() {
Bytes32 blockRoot = dataStructureUtil.randomBytes32();
when(blobSidecarPool.containsBlock(blockRoot)).thenReturn(true);

recentBlockFetcher.requestRecentBlock(blockRoot);
assertTaskCounts(0, 0, 0);
}

@Test
void shouldRequestRemainingRequiredBlocksWhenForwardSyncCompletes() {
final Set<Bytes32> requiredRoots =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public boolean containsBlobSidecar(final BlobIdentifier blobIdentifier) {
return false;
}

@Override
public boolean containsBlock(Bytes32 blockRoot) {
return false;
}

@Override
public BlockBlobSidecarsTracker getOrCreateBlockBlobSidecarsTracker(
SignedBeaconBlock block) {
Expand Down Expand Up @@ -93,6 +98,8 @@ public void subscribeRequiredBlockRootDropped(

boolean containsBlobSidecar(BlobIdentifier blobIdentifier);

boolean containsBlock(Bytes32 blockRoot);

Set<BlobIdentifier> getAllRequiredBlobSidecars();

BlockBlobSidecarsTracker getOrCreateBlockBlobSidecarsTracker(SignedBeaconBlock block);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,9 @@ public synchronized boolean containsBlobSidecar(final BlobIdentifier blobIdentif
.orElse(false);
}

public synchronized boolean containsBlock(final SignedBeaconBlock block) {
return Optional.ofNullable(blockBlobSidecarsTrackers.get(block.getRoot()))
@Override
public synchronized boolean containsBlock(final Bytes32 blockRoot) {
return Optional.ofNullable(blockBlobSidecarsTrackers.get(blockRoot))
.map(tracker -> tracker.getBlockBody().isPresent())
.orElse(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void onNewBlock_addTrackerWithBlock() {
dataStructureUtil.randomSignedBeaconBlock(currentSlot.longValue());
blobSidecarPool.onNewBlock(block);

assertThat(blobSidecarPool.containsBlock(block)).isTrue();
assertThat(blobSidecarPool.containsBlock(block.getRoot())).isTrue();
assertThat(requiredBlockRootEvents).isEmpty();
assertThat(requiredBlockRootDroppedEvents).isEmpty();
assertThat(requiredBlobSidecarEvents).isEmpty();
Expand Down Expand Up @@ -147,7 +147,7 @@ public void onNewBlock_shouldIgnorePreDenebBlocks() {
dataStructureUtil.randomSignedBeaconBlock(currentSlot.longValue());
blobSidecarPool.onNewBlock(block);

assertThat(blobSidecarPool.containsBlock(block)).isFalse();
assertThat(blobSidecarPool.containsBlock(block.getRoot())).isFalse();
assertThat(requiredBlockRootEvents).isEmpty();
assertThat(requiredBlockRootDroppedEvents).isEmpty();
assertThat(requiredBlobSidecarEvents).isEmpty();
Expand All @@ -172,7 +172,7 @@ public void onNewBlobSidecarOnNewBlock_addTrackerWithBothBlockAndBlobSidecar() {

assertThat(blobSidecarPool.containsBlobSidecar(blobIdentifierFromBlobSidecar(blobSidecar)))
.isTrue();
assertThat(blobSidecarPool.containsBlock(block)).isTrue();
assertThat(blobSidecarPool.containsBlock(block.getRoot())).isTrue();
assertThat(requiredBlockRootEvents).isEmpty();
assertThat(requiredBlockRootDroppedEvents).isEmpty();
assertThat(requiredBlobSidecarEvents).isEmpty();
Expand Down Expand Up @@ -237,8 +237,8 @@ public void twoOnNewBlock_addTrackerWithBothBlobSidecars() {
blobSidecarPool.onNewBlock(blockAtPreviousSlot);
blobSidecarPool.onNewBlock(block);

assertThat(blobSidecarPool.containsBlock(blockAtPreviousSlot)).isTrue();
assertThat(blobSidecarPool.containsBlock(block)).isTrue();
assertThat(blobSidecarPool.containsBlock(blockAtPreviousSlot.getRoot())).isTrue();
assertThat(blobSidecarPool.containsBlock(block.getRoot())).isTrue();
assertThat(requiredBlockRootEvents).isEmpty();
assertThat(requiredBlockRootDroppedEvents).isEmpty();
assertThat(requiredBlobSidecarEvents).isEmpty();
Expand Down Expand Up @@ -332,7 +332,7 @@ public void shouldApplyIgnoreForBlock() {

blobSidecarPool.onNewBlock(block);

assertThat(blobSidecarPool.containsBlock(block)).isFalse();
assertThat(blobSidecarPool.containsBlock(block.getRoot())).isFalse();
assertThat(requiredBlockRootEvents).isEmpty();
assertThat(requiredBlockRootDroppedEvents).isEmpty();
assertThat(requiredBlobSidecarEvents).isEmpty();
Expand Down Expand Up @@ -369,7 +369,7 @@ public void add_moreThanMaxItems() {
blobSidecarPool.onNewBlock(block);

final int expectedSize = Math.min(maxItems, i + 1);
assertThat(blobSidecarPool.containsBlock(block)).isTrue();
assertThat(blobSidecarPool.containsBlock(block.getRoot())).isTrue();
assertThat(blobSidecarPool.getTotalBlobSidecarsTrackers()).isEqualTo(expectedSize);
assertBlobSidecarsTrackersCount(expectedSize);
}
Expand Down Expand Up @@ -404,7 +404,7 @@ public void prune_finalizedBlocks() {
// Check that all blocks are in the collection
assertBlobSidecarsTrackersCount(finalizedBlocks.size() + nonFinalBlocks.size());
for (SignedBeaconBlock block : allBlocks) {
assertThat(blobSidecarPool.containsBlock(block)).isTrue();
assertThat(blobSidecarPool.containsBlock(block.getRoot())).isTrue();
}

// Update finalized checkpoint and prune
Expand All @@ -414,7 +414,7 @@ public void prune_finalizedBlocks() {
// Check that all final blocks have been pruned
assertBlobSidecarsTrackersCount(nonFinalBlocks.size());
for (SignedBeaconBlock block : nonFinalBlocks) {
assertThat(blobSidecarPool.containsBlock(block)).isTrue();
assertThat(blobSidecarPool.containsBlock(block.getRoot())).isTrue();
}
}

Expand Down

0 comments on commit e0addad

Please sign in to comment.