Skip to content

Commit

Permalink
Limit number of blocks and headers added in bad block event
Browse files Browse the repository at this point in the history 
Signed-off-by: Jason Frame <jason.frame@consensys.net>
  • Loading branch information
@jframe
jframe committed Nov 7, 2023
1 parent dde273e commit 00b4e16
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 9 deletions.
15 changes: 10 additions & 5 deletions ...rc/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
*/
package org.hyperledger.besu.ethereum.eth.sync.backwardsync;

import static org.hyperledger.besu.ethereum.chain.BadBlockManager.MAX_BAD_BLOCKS_SIZE;
import static org.hyperledger.besu.util.FutureUtils.exceptionallyCompose;

import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.ethereum.BlockValidator;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.chain.BadBlockManager;
import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
import org.hyperledger.besu.ethereum.core.Block;
import org.hyperledger.besu.ethereum.core.BlockHeader;
Expand Down Expand Up @@ -47,6 +47,7 @@ public class BackwardSyncContext {
private static final int DEFAULT_MAX_RETRIES = 20;
private static final long MILLIS_DELAY_BETWEEN_PROGRESS_LOG = 10_000L;
private static final long DEFAULT_MILLIS_BETWEEN_RETRIES = 5000;
private static final int DEFAULT_MAX_CHAIN_EVENT_ENTRIES = BadBlockManager.MAX_BAD_BLOCKS_SIZE;

protected final ProtocolContext protocolContext;
private final ProtocolSchedule protocolSchedule;
Expand All @@ -57,6 +58,7 @@ public class BackwardSyncContext {
private final BackwardChain backwardChain;
private int batchSize = BATCH_SIZE;
private final int maxRetries;
private final int maxBadChainEventEntries;
private final long millisBetweenRetries = DEFAULT_MILLIS_BETWEEN_RETRIES;
private final Subscribers<BadChainListener> badChainListeners = Subscribers.create();

Expand All @@ -74,7 +76,8 @@ public BackwardSyncContext(
ethContext,
syncState,
backwardChain,
DEFAULT_MAX_RETRIES);
DEFAULT_MAX_RETRIES,
DEFAULT_MAX_CHAIN_EVENT_ENTRIES);
}

public BackwardSyncContext(
Expand All @@ -84,7 +87,8 @@ public BackwardSyncContext(
final EthContext ethContext,
final SyncState syncState,
final BackwardChain backwardChain,
final int maxRetries) {
final int maxRetries,
final int maxBadChainEventEntries) {

this.protocolContext = protocolContext;
this.protocolSchedule = protocolSchedule;
Expand All @@ -93,6 +97,7 @@ public BackwardSyncContext(
this.syncState = syncState;
this.backwardChain = backwardChain;
this.maxRetries = maxRetries;
this.maxBadChainEventEntries = maxBadChainEventEntries;
}

public synchronized boolean isSyncing() {
Expand Down Expand Up @@ -370,8 +375,8 @@ private void emitBadChainEvent(final Block badBlock) {
Optional<Hash> descendant = backwardChain.getDescendant(badBlock.getHash());

while (descendant.isPresent()
&& badBlockDescendants.size() <= MAX_BAD_BLOCKS_SIZE
&& badBlockHeaderDescendants.size() <= MAX_BAD_BLOCKS_SIZE) {
&& badBlockDescendants.size() < maxBadChainEventEntries
&& badBlockHeaderDescendants.size() < maxBadChainEventEntries) {
final Optional<Block> block = backwardChain.getBlock(descendant.get());
if (block.isPresent()) {
badBlockDescendants.add(block.get());
Expand Down
78 changes: 74 additions & 4 deletions ...est/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@

package org.hyperledger.besu.ethereum.eth.sync.backwardsync;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryBlockchain;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -64,6 +65,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
Expand All @@ -75,11 +77,12 @@
@MockitoSettings(strictness = Strictness.LENIENT)
public class BackwardSyncContextTest {

public static final int REMOTE_HEIGHT = 50;
public static final int LOCAL_HEIGHT = 25;
public static final int REMOTE_HEIGHT = 50;
public static final int UNCLE_HEIGHT = 25 - 3;

public static final int NUM_OF_RETRIES = 100;
public static final int TEST_MAX_BAD_CHAIN_EVENT_ENTRIES = 25;

private BackwardSyncContext context;

Expand Down Expand Up @@ -173,7 +176,8 @@ public void setup() {
ethContext,
syncState,
backwardChain,
NUM_OF_RETRIES));
NUM_OF_RETRIES,
TEST_MAX_BAD_CHAIN_EVENT_ENTRIES));
doReturn(true).when(context).isReady();
doReturn(2).when(context).getBatchSize();
}
Expand Down Expand Up @@ -347,6 +351,72 @@ public void shouldEmitBadChainEvent() {
block, Collections.emptyList(), List.of(childBlockHeader, grandChildBlockHeader));
}

@Test
@SuppressWarnings("unchecked")
public void shouldEmitBadChainEventWithIncludedBlockHeadersLimitedToMaxBadChainEventsSize() {
Block block = Mockito.mock(Block.class);
BlockHeader blockHeader = Mockito.mock(BlockHeader.class);
when(block.getHash()).thenReturn(Hash.fromHexStringLenient("0x42"));
when(blockHeader.getHash()).thenReturn(Hash.fromHexStringLenient("0x42"));
BadChainListener badChainListener = Mockito.mock(BadChainListener.class);
context.subscribeBadChainListener(badChainListener);

backwardChain.clear();

for (int i = REMOTE_HEIGHT; i >= 0; i--) {
backwardChain.prependAncestorsHeader(remoteBlockchain.getBlockByNumber(i).get().getHeader());
}
backwardChain.prependAncestorsHeader(blockHeader);

doReturn(blockValidator).when(context).getBlockValidatorForBlock(any());
BlockProcessingResult result = new BlockProcessingResult("custom error");
doReturn(result).when(blockValidator).validateAndProcessBlock(any(), any(), any(), any());

assertThatThrownBy(() -> context.saveBlock(block))
.isInstanceOf(BackwardSyncException.class)
.hasMessageContaining("custom error");

final ArgumentCaptor<List<BlockHeader>> badBlockHeaderDescendants =
ArgumentCaptor.forClass(List.class);
verify(badChainListener)
.onBadChain(eq(block), eq(Collections.emptyList()), badBlockHeaderDescendants.capture());
assertThat(badBlockHeaderDescendants.getValue()).hasSize(TEST_MAX_BAD_CHAIN_EVENT_ENTRIES);
}

@SuppressWarnings("unchecked")
@Test
public void shouldEmitBadChainEventWithIncludedBlocksLimitedToMaxBadChainEventsSize() {
Block block = Mockito.mock(Block.class);
BlockHeader blockHeader = Mockito.mock(BlockHeader.class);
when(block.getHash()).thenReturn(Hash.fromHexStringLenient("0x42"));
when(blockHeader.getHash()).thenReturn(Hash.fromHexStringLenient("0x42"));
BadChainListener badChainListener = Mockito.mock(BadChainListener.class);
context.subscribeBadChainListener(badChainListener);

backwardChain.clear();
for (int i = REMOTE_HEIGHT; i >= 0; i--) {
backwardChain.prependAncestorsHeader(remoteBlockchain.getBlockByNumber(i).get().getHeader());
}
backwardChain.prependAncestorsHeader(blockHeader);

for (int i = REMOTE_HEIGHT; i >= 0; i--) {
backwardChain.appendTrustedBlock(remoteBlockchain.getBlockByNumber(i).get());
}

doReturn(blockValidator).when(context).getBlockValidatorForBlock(any());
BlockProcessingResult result = new BlockProcessingResult("custom error");
doReturn(result).when(blockValidator).validateAndProcessBlock(any(), any(), any(), any());

assertThatThrownBy(() -> context.saveBlock(block))
.isInstanceOf(BackwardSyncException.class)
.hasMessageContaining("custom error");

final ArgumentCaptor<List<Block>> badBlockDescendants = ArgumentCaptor.forClass(List.class);
verify(badChainListener)
.onBadChain(eq(block), badBlockDescendants.capture(), eq(Collections.emptyList()));
assertThat(badBlockDescendants.getValue()).hasSize(TEST_MAX_BAD_CHAIN_EVENT_ENTRIES);
}

@Test
public void shouldFailAfterMaxNumberOfRetries() {
doReturn(CompletableFuture.failedFuture(new Exception()))
Expand Down

0 comments on commit 00b4e16

Please sign in to comment.