Limit memory used in handling invalid blocks (#6138)

Signed-off-by: Jason Frame <jason.frame@consensys.net>
This commit is contained in:
Jason Frame
2023-11-09 09:23:53 +10:00
committed by GitHub
parent 8319fae725
commit 701cbb000f
4 changed files with 90 additions and 10 deletions

View File

@@ -24,6 +24,7 @@
- Upgrade netty to address CVE-2023-44487, CVE-2023-34462 [#6100](https://github.com/hyperledger/besu/pull/6100)
- Upgrade grpc to address CVE-2023-32731, CVE-2023-33953, CVE-2023-44487, CVE-2023-4785 [#6100](https://github.com/hyperledger/besu/pull/6100)
- Fix blob gas calculation in reference tests [#6107](https://github.com/hyperledger/besu/pull/6107)
- Limit memory used in handling invalid blocks [#6138](https://github.com/hyperledger/besu/pull/6138)
---

View File

@@ -28,12 +28,13 @@ import com.google.common.cache.CacheBuilder;
public class BadBlockManager {
public static final int MAX_BAD_BLOCKS_SIZE = 100;
private final Cache<Hash, Block> badBlocks =
CacheBuilder.newBuilder().maximumSize(100).concurrencyLevel(1).build();
CacheBuilder.newBuilder().maximumSize(MAX_BAD_BLOCKS_SIZE).concurrencyLevel(1).build();
private final Cache<Hash, BlockHeader> badHeaders =
CacheBuilder.newBuilder().maximumSize(100).concurrencyLevel(1).build();
CacheBuilder.newBuilder().maximumSize(MAX_BAD_BLOCKS_SIZE).concurrencyLevel(1).build();
private final Cache<Hash, Hash> latestValidHashes =
CacheBuilder.newBuilder().maximumSize(100).concurrencyLevel(1).build();
CacheBuilder.newBuilder().maximumSize(MAX_BAD_BLOCKS_SIZE).concurrencyLevel(1).build();
/**
* Add a new invalid block.

View File

@@ -19,6 +19,7 @@ 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;
@@ -46,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;
@@ -56,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();
@@ -73,7 +76,8 @@ public class BackwardSyncContext {
ethContext,
syncState,
backwardChain,
DEFAULT_MAX_RETRIES);
DEFAULT_MAX_RETRIES,
DEFAULT_MAX_CHAIN_EVENT_ENTRIES);
}
public BackwardSyncContext(
@@ -83,7 +87,8 @@ public class 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;
@@ -92,6 +97,7 @@ public class BackwardSyncContext {
this.syncState = syncState;
this.backwardChain = backwardChain;
this.maxRetries = maxRetries;
this.maxBadChainEventEntries = maxBadChainEventEntries;
}
public synchronized boolean isSyncing() {
@@ -368,7 +374,9 @@ public class BackwardSyncContext {
Optional<Hash> descendant = backwardChain.getDescendant(badBlock.getHash());
while (descendant.isPresent()) {
while (descendant.isPresent()
&& badBlockDescendants.size() < maxBadChainEventEntries
&& badBlockHeaderDescendants.size() < maxBadChainEventEntries) {
final Optional<Block> block = backwardChain.getBlock(descendant.get());
if (block.isPresent()) {
badBlockDescendants.add(block.get());

View File

@@ -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;
@@ -64,6 +65,7 @@ import org.junit.jupiter.api.BeforeEach;
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;
@@ -75,11 +77,12 @@ import org.mockito.quality.Strictness;
@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;
@@ -173,7 +176,8 @@ public class BackwardSyncContextTest {
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();
}
@@ -347,6 +351,72 @@ public class BackwardSyncContextTest {
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()))