diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedBadAncestorIntegrationTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedBadAncestorIntegrationTest.java new file mode 100644 index 00000000000..b702a58ae6f --- /dev/null +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedBadAncestorIntegrationTest.java @@ -0,0 +1,235 @@ +/* + * Copyright contributors to Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine; + +import static java.util.Collections.emptyList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.AMSTERDAM; +import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.CANCUN; +import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.hyperledger.besu.consensus.merge.MergeContext; +import org.hyperledger.besu.consensus.merge.blockcreation.MergeCoordinator; +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineForkchoiceUpdatedParameter; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EngineUpdateForkchoiceResult; +import org.hyperledger.besu.ethereum.chain.BadBlockCause; +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.BlockBody; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; +import org.hyperledger.besu.ethereum.core.MiningConfiguration; +import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext; +import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; +import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive; + +import java.util.List; +import java.util.Optional; + +import io.vertx.core.Vertx; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Integration test for the engine_forkchoiceUpdated "invalid block as ancestor" flow. + * + *
    + *
  1. A bad block B is registered in {@link BadBlockManager} (simulating what {@link + * org.hyperledger.besu.ethereum.MainnetBlockValidator} does on a failed import). + *
  2. {@link MergeCoordinator#onBadChain(Block, List, List)} is called with B and a descendant D + * simulating the backward-sync code path that fires on {@code + * BackwardSyncContext.emitBadChainEvent}. This should propagate B's "bad" marking to every + * descendant, and record each descendant's {@code latestValidHash} as B's (valid) parent + * hash. + *
  3. A JSON-RPC {@code engine_forkchoiceUpdated} call is invoked with {@code headBlockHash = D}. + * The bad-block short-circuit in {@link + * org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.AbstractEngineForkchoiceUpdated} + * should fire, returning {@code INVALID} with {@code latestValidHash} equal to B's valid + * parent hash. + *
+ */ +public class EngineForkchoiceUpdatedBadAncestorIntegrationTest { + + private static final Vertx vertx = Vertx.vertx(); + + @AfterAll + public static void tearDown() { + vertx.close(); + } + + private final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); + + private BadBlockManager badBlockManager; + private MutableBlockchain blockchain; + private MergeCoordinator mergeCoordinator; + private EngineForkchoiceUpdatedV3 method; + + @BeforeEach + public void setUp() { + badBlockManager = new BadBlockManager(); + + // Blockchain only needs to serve the bad block's parent header lookup for + // MergeCoordinator.onBadChain; everything else on the fcU short-circuit path avoids it. + blockchain = mock(MutableBlockchain.class); + + final MergeContext mergeContext = mock(MergeContext.class); + when(mergeContext.as(MergeContext.class)).thenReturn(mergeContext); + + final WorldStateArchive worldStateArchive = mock(WorldStateArchive.class); + + final ProtocolContext protocolContext = + new ProtocolContext.Builder() + .withBlockchain(blockchain) + .withWorldStateArchive(worldStateArchive) + .withConsensusContext(mergeContext) + .withBadBlockManager(badBlockManager) + .build(); + + final ProtocolSchedule protocolSchedule = mock(ProtocolSchedule.class); + + when(protocolSchedule.getChainId()).thenReturn(Optional.empty()); + when(protocolSchedule.milestoneFor(CANCUN)).thenReturn(Optional.empty()); + when(protocolSchedule.milestoneFor(AMSTERDAM)).thenReturn(Optional.empty()); + + final EthScheduler ethScheduler = mock(EthScheduler.class); + final TransactionPool transactionPool = mock(TransactionPool.class); + final BackwardSyncContext backwardSyncContext = mock(BackwardSyncContext.class); + final MiningConfiguration miningConfiguration = MiningConfiguration.newDefault(); + + mergeCoordinator = + new MergeCoordinator( + protocolContext, + protocolSchedule, + ethScheduler, + transactionPool, + miningConfiguration, + backwardSyncContext); + + method = + new EngineForkchoiceUpdatedV3( + vertx, + protocolSchedule, + protocolContext, + mergeCoordinator, + mock(EngineCallListener.class)); + } + + @Test + public void shouldReturnInvalidForForkchoiceUpdateWhenHeadHasBadAncestor() { + // Chain shape: + // validParent (PoS, difficulty=0, known to the blockchain) + // └── badBlock B (rejected by validator, in BadBlockManager) + // └── descendant D (CL's head — only in BadBlockManager via onBadChain) + final BlockHeader validParent = headerBuilder.number(100L).buildHeader(); + final BlockHeader badHeader = + headerBuilder.number(101L).parentHash(validParent.getHash()).buildHeader(); + final Block badBlock = new Block(badHeader, BlockBody.empty()); + final BlockHeader descendantHeader = + headerBuilder.number(102L).parentHash(badHeader.getHash()).buildHeader(); + + // onBadChain looks up the bad block's parent in the blockchain to compute latestValidHash. + when(blockchain.getBlockHeader(validParent.getHash())).thenReturn(Optional.of(validParent)); + + // Simulate MainnetBlockValidator.handleFailedBlockProcessing — add the bad block + // to the manager directly. (The direct add path intentionally does NOT record a + // latestValidHash for B itself; only the descendants in stage 2 get one.) + badBlockManager.addBadBlock(badBlock, BadBlockCause.fromValidationFailure("BAL mismatch")); + + // Simulate BackwardSyncContext.emitBadChainEvent firing — MergeCoordinator is + // registered as a BadChainListener in its constructor and receives the descendant list. + mergeCoordinator.onBadChain(badBlock, emptyList(), List.of(descendantHeader)); + + // onBadChain propagated "bad" status and latestValidHash to the descendant. + assertThat(mergeCoordinator.isBadBlock(descendantHeader.getHash())).isTrue(); + assertThat(mergeCoordinator.getLatestValidHashOfBadBlock(descendantHeader.getHash())) + .contains(validParent.getHash()); + + // CL sends engine_forkchoiceUpdated with the descendant as the head, simulating the + // case where the CL reuses a head whose ancestry was poisoned while we were backward-syncing. + final JsonRpcResponse response = + invokeForkchoiceUpdated( + new EngineForkchoiceUpdatedParameter( + descendantHeader.getHash(), validParent.getHash(), validParent.getHash())); + + // Must return INVALID with + // latestValidHash pointing at the last valid ancestor (B's parent), not SYNCING and not + // VALID, without ever touching the blockchain state. + final EngineUpdateForkchoiceResult forkchoiceResult = + (EngineUpdateForkchoiceResult) ((JsonRpcSuccessResponse) response).getResult(); + assertThat(forkchoiceResult.getPayloadStatus().getStatus()).isEqualTo(INVALID); + assertThat(forkchoiceResult.getPayloadStatus().getLatestValidHashAsString()) + .isEqualTo(validParent.getHash().toHexString()); + final String error = forkchoiceResult.getPayloadStatus().getError(); + assertThat(error).contains(descendantHeader.getHash().toString()); + assertThat(error).containsIgnoringCase("invalid"); + assertThat(forkchoiceResult.getPayloadId()).isNull(); + } + + @Test + public void shouldReturnInvalidWithZeroHashWhenBadBlockParentIsUnknown() { + // Same shape as the previous test, except the bad block's parent is NOT in the blockchain. + // This models a deep backward-sync failure where we never resolved the ancestor — onBadChain + // cannot compute a latestValidHash, so the fcU short-circuit must fall back to Hash.ZERO. + final BlockHeader unknownParent = headerBuilder.number(100L).buildHeader(); + final BlockHeader badHeader = + headerBuilder.number(101L).parentHash(unknownParent.getHash()).buildHeader(); + final Block badBlock = new Block(badHeader, BlockBody.empty()); + final BlockHeader descendantHeader = + headerBuilder.number(102L).parentHash(badHeader.getHash()).buildHeader(); + + // Deliberately leave blockchain.getBlockHeader(unknownParent.getHash()) unstubbed so the + // mock returns Optional.empty() — that's the path that drives maybeLatestValidHash empty. + + badBlockManager.addBadBlock(badBlock, BadBlockCause.fromValidationFailure("BAL mismatch")); + mergeCoordinator.onBadChain(badBlock, emptyList(), List.of(descendantHeader)); + + // Descendant is still marked bad, but no latestValidHash was recorded for it. + assertThat(mergeCoordinator.isBadBlock(descendantHeader.getHash())).isTrue(); + assertThat(mergeCoordinator.getLatestValidHashOfBadBlock(descendantHeader.getHash())).isEmpty(); + + final JsonRpcResponse response = + invokeForkchoiceUpdated( + new EngineForkchoiceUpdatedParameter( + descendantHeader.getHash(), unknownParent.getHash(), unknownParent.getHash())); + + final EngineUpdateForkchoiceResult forkchoiceResult = + (EngineUpdateForkchoiceResult) ((JsonRpcSuccessResponse) response).getResult(); + assertThat(forkchoiceResult.getPayloadStatus().getStatus()).isEqualTo(INVALID); + assertThat(forkchoiceResult.getPayloadStatus().getLatestValidHashAsString()) + .isEqualTo(Hash.ZERO.toHexString()); + final String error = forkchoiceResult.getPayloadStatus().getError(); + assertThat(error).contains(descendantHeader.getHash().toString()); + assertThat(error).containsIgnoringCase("invalid"); + assertThat(forkchoiceResult.getPayloadId()).isNull(); + } + + private JsonRpcResponse invokeForkchoiceUpdated(final EngineForkchoiceUpdatedParameter fcu) { + return method.response( + new JsonRpcRequestContext( + new JsonRpcRequest("2.0", "engine_forkchoiceUpdatedV3", new Object[] {fcu}))); + } +} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java index 7fde3a17c7f..f63bfa71f4b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java @@ -337,6 +337,14 @@ private void handleNewBlockFromNetwork(final EthMessage message) { .log(); return; } + if (protocolContext.getBadBlockManager().isBadBlock(block.getHash())) { + LOG.atDebug() + .setMessage("Ignoring known-bad block {} from peer {}") + .addArgument(block::toLogString) + .addArgument(message::getPeer) + .log(); + return; + } importOrSavePendingBlock(block, message.getPeer().nodeId()); } catch (final RLPException e) { @@ -391,6 +399,10 @@ private void handleNewBlockHashesFromNetwork(final EthMessage message) { LOG.trace("New block hash from network {} was already imported", announcedBlock); continue; } + if (protocolContext.getBadBlockManager().isBadBlock(announcedBlock.hash())) { + LOG.debug("New block hash from network {} is a known bad block", announcedBlock); + continue; + } if (processingBlocksManager.addRequestedBlock(announcedBlock.hash())) { newBlocks.add(announcedBlock); } else { @@ -625,6 +637,14 @@ CompletableFuture importOrSavePendingBlock(final Block block, final Bytes .addArgument(block::toLogString) .log(); + if (protocolContext.getBadBlockManager().isBadBlock(block.getHash())) { + LOG.atTrace() + .setMessage("Skipping known-bad block {} in importOrSavePendingBlock") + .addArgument(block::toLogString) + .log(); + return CompletableFuture.completedFuture(block); + } + if (!protocolContext.getBlockchain().contains(block.getHeader().getParentHash())) { // Block isn't connected to local chain, save it to pending blocks collection if (savePendingBlock(block, nodeId)) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java index b493f0534dd..8cef5af7642 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java @@ -19,6 +19,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -29,6 +30,7 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.ConsensusContext; import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.chain.BadBlockCause; import org.hyperledger.besu.ethereum.chain.BadBlockManager; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; @@ -861,6 +863,138 @@ public Object answer(final InvocationOnMock invocation) throws Throwable { verify(ethScheduler, times(1)).scheduleSyncWorkerTask(any(Supplier.class)); } + @Test + public void shouldSkipKnownBadBlockOnNewBlockMessage() { + blockchainUtil.importFirstBlocks(2); + final Block firstBlock = blockchainUtil.getBlock(1); + final BadBlockManager badBlocksManager = protocolContext.getBadBlockManager(); + final Block badBlock = + new BlockDataGenerator() + .block( + BlockDataGenerator.BlockOptions.create() + .setBlockNumber(2) + .setParentHash(firstBlock.getHash()) + .setBlockHeaderFunctions(new MainnetBlockHeaderFunctions())); + + // Pre-populate BadBlockManager, simulating an earlier validation failure. + badBlocksManager.addBadBlock(badBlock, BadBlockCause.fromValidationFailure("test")); + assertThat(badBlocksManager.getBadBlocks().size()).isEqualTo(1); + + final BlockPropagationManager propManager = spy(blockPropagationManager); + propManager.start(); + + final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 0); + final NewBlockMessage newBlockMessage = + NewBlockMessage.create(badBlock, Difficulty.ONE, maxMessageSize); + + EthProtocolManagerTestUtil.broadcastMessage(ethProtocolManager, peer, newBlockMessage); + + // handleNewBlockFromNetwork must short-circuit before dispatching to importOrSavePendingBlock. + // Checking addImportingBlock alone would also pass via the defensive second check inside + // importOrSavePendingBlock, so verify the dispatch itself never happened. + verify(propManager, never()).importOrSavePendingBlock(any(), any(Bytes.class)); + // BadBlockManager should still contain exactly one entry — we didn't re-add it. + assertThat(badBlocksManager.getBadBlocks().size()).isEqualTo(1); + } + + @Test + public void shouldSkipKnownBadBlockOnNewBlockHashesMessage() { + blockchainUtil.importFirstBlocks(2); + final Block firstBlock = blockchainUtil.getBlock(1); + final BadBlockManager badBlocksManager = protocolContext.getBadBlockManager(); + final Block badBlock = + new BlockDataGenerator() + .block( + BlockDataGenerator.BlockOptions.create() + .setBlockNumber(2) + .setParentHash(firstBlock.getHash()) + .setBlockHeaderFunctions(new MainnetBlockHeaderFunctions())); + + // Pre-populate BadBlockManager, simulating an earlier validation failure. + badBlocksManager.addBadBlock(badBlock, BadBlockCause.fromValidationFailure("test")); + + blockPropagationManager.start(); + + final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 0); + final NewBlockHashesMessage newBlockHashesMessage = + NewBlockHashesMessage.create( + Collections.singletonList( + new NewBlockHashesMessage.NewBlockHash( + badBlock.getHash(), badBlock.getHeader().getNumber()))); + + EthProtocolManagerTestUtil.broadcastMessage(ethProtocolManager, peer, newBlockHashesMessage); + + // The hash announcement should have been filtered out before requesting the body. + verify(processingBlocksManager, never()).addRequestedBlock(badBlock.getHash()); + // No body request should have been issued to the peer. + assertThat(peer.hasOutstandingRequests()).isFalse(); + } + + @SuppressWarnings("unchecked") + @Test + public void importOrSavePendingBlockShouldSkipKnownBadBlock() { + // Mirrors shouldDetectAndCacheInvalidBlocks but pre-populates BadBlockManager to + // exercise the defensive short-circuit at the top of importOrSavePendingBlock. + final EthScheduler ethScheduler = mock(EthScheduler.class); + when(ethScheduler.scheduleSyncWorkerTask(any(Supplier.class))) + .thenAnswer( + new Answer() { + @Override + public Object answer(final InvocationOnMock invocation) throws Throwable { + return invocation.getArgument(0, Supplier.class).get(); + } + }); + + final EthContext ethContext = + new EthContext( + new EthPeers( + () -> protocolSchedule.getByBlockHeader(blockchain.getChainHeadHeader()), + TestClock.fixed(), + metricsSystem, + EthProtocolConfiguration.DEFAULT_MAX_MESSAGE_SIZE, + Collections.emptyList(), + Bytes.random(64), + 25, + 25, + false, + SyncMode.SNAP, + new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList())), + new EthMessages(), + ethScheduler, + null); + final BlockPropagationManager blockPropagationManager = + new BlockPropagationManager( + syncConfig, + protocolSchedule, + protocolContext, + ethContext, + syncState, + pendingBlocksManager, + metricsSystem, + blockBroadcaster); + + blockchainUtil.importFirstBlocks(2); + final Block firstBlock = blockchainUtil.getBlock(1); + final BadBlockManager badBlocksManager = protocolContext.getBadBlockManager(); + final Block badBlock = + new BlockDataGenerator() + .block( + BlockDataGenerator.BlockOptions.create() + .setBlockNumber(2) + .setParentHash(firstBlock.getHash()) + .setBlockHeaderFunctions(new MainnetBlockHeaderFunctions())); + + // Pre-populate BadBlockManager, simulating an earlier validation failure. + badBlocksManager.addBadBlock(badBlock, BadBlockCause.fromValidationFailure("test")); + assertThat(badBlocksManager.getBadBlocks().size()).isEqualTo(1); + + blockPropagationManager.importOrSavePendingBlock(badBlock, NODE_ID_1); + + // The defensive check should have short-circuited before scheduling validation. + verify(ethScheduler, never()).scheduleSyncWorkerTask(any(Supplier.class)); + assertThat(badBlocksManager.getBadBlocks().size()).isEqualTo(1); + } + @Test public void shouldTryWithAnotherPeerWhenFailedDownloadingBlock() { blockchainUtil.importFirstBlocks(2);