From 6bd31357133c8798bf53045f6fb0ceaa336fdb17 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Thu, 24 Aug 2023 03:09:41 +0200 Subject: [PATCH] Post Merge cleanup: remove latestValidAncestorDescendsFromTerminal (#4703) * Post Merge cleanup: remove latestValidAncestorDescendsFromTerminal Signed-off-by: Fabio Di Fabio * fixed tests Signed-off-by: Sally MacFarlane --------- Signed-off-by: Fabio Di Fabio Signed-off-by: Sally MacFarlane Co-authored-by: Sally MacFarlane --- .../merge/blockcreation/MergeCoordinator.java | 91 ------- .../blockcreation/MergeMiningCoordinator.java | 14 +- .../blockcreation/TransitionCoordinator.java | 6 - .../blockcreation/MergeCoordinatorTest.java | 229 ------------------ .../AbstractEngineForkchoiceUpdated.java | 16 -- .../engine/AbstractEngineNewPayload.java | 15 -- .../AbstractEngineForkchoiceUpdatedTest.java | 19 -- .../engine/AbstractEngineNewPayloadTest.java | 22 -- .../engine/EngineForkchoiceUpdatedV1Test.java | 29 --- .../engine/EngineNewPayloadEIP6110Test.java | 2 - .../engine/EngineNewPayloadV1Test.java | 36 --- .../engine/EngineNewPayloadV2Test.java | 5 - 12 files changed, 3 insertions(+), 481 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index bbe22f19b1e..7b394b5b952 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -15,7 +15,6 @@ package org.hyperledger.besu.consensus.merge.blockcreation; import static java.util.stream.Collectors.joining; -import static org.hyperledger.besu.consensus.merge.TransitionUtils.isTerminalProofOfWorkBlock; import static org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator.ForkchoiceResult.Status.INVALID; import org.hyperledger.besu.consensus.merge.MergeContext; @@ -667,96 +666,6 @@ private boolean moveWorldStateTo(final BlockHeader newHead) { return newWorldState.isPresent(); } - @Override - public boolean latestValidAncestorDescendsFromTerminal(final BlockHeader blockHeader) { - if (blockHeader.getNumber() <= 1L) { - // parent is a genesis block, check for merge-at-genesis - var blockchain = protocolContext.getBlockchain(); - - return blockchain - .getTotalDifficultyByHash(blockHeader.getBlockHash()) - .map(Optional::of) - .orElse(blockchain.getTotalDifficultyByHash(blockHeader.getParentHash())) - .filter( - currDiff -> currDiff.greaterOrEqualThan(mergeContext.getTerminalTotalDifficulty())) - .isPresent(); - } - - Optional validAncestorHash = this.getLatestValidAncestor(blockHeader); - if (validAncestorHash.isPresent()) { - final Optional maybeFinalized = mergeContext.getFinalized(); - if (maybeFinalized.isPresent()) { - return isDescendantOf(maybeFinalized.get(), blockHeader); - } else { - Optional terminalBlockHeader = mergeContext.getTerminalPoWBlock(); - if (terminalBlockHeader.isPresent()) { - return isDescendantOf(terminalBlockHeader.get(), blockHeader); - } else { - if (isTerminalProofOfWorkBlock(blockHeader, protocolContext) - || ancestorIsValidTerminalProofOfWork(blockHeader)) { - return true; - } else { - LOG.warn("Couldn't find terminal block, no blocks will be valid"); - return false; - } - } - } - } else { - return false; - } - } - - /** - * Ancestor is valid terminal proof of work boolean. - * - * @param blockheader the blockheader - * @return the boolean - */ - // package visibility for testing - boolean ancestorIsValidTerminalProofOfWork(final BlockHeader blockheader) { - // this should only happen very close to the transition from PoW to PoS, prior to a finalized - // block. For example, after a full sync of an already-merged chain which does not have - // terminal block info in the genesis config. - - // check a 'cached' block which was determined to descend from terminal to short circuit - // in the case of a long period of non-finality - if (Optional.ofNullable(latestDescendsFromTerminal.get()) - .map(latestDescendant -> isDescendantOf(latestDescendant, blockheader)) - .orElse(Boolean.FALSE)) { - latestDescendsFromTerminal.set(blockheader); - return true; - } - - var blockchain = protocolContext.getBlockchain(); - Optional parent = blockchain.getBlockHeader(blockheader.getParentHash()); - do { - LOG.debug( - "checking ancestor {} is valid terminal PoW for {}", - parent.map(BlockHeader::toLogString).orElse("empty"), - blockheader.toLogString()); - - if (parent.isPresent()) { - if (!parent.get().getDifficulty().equals(Difficulty.ZERO)) { - break; - } - parent = blockchain.getBlockHeader(parent.get().getParentHash()); - } - - } while (parent.isPresent()); - - boolean resp = - parent.filter(header -> isTerminalProofOfWorkBlock(header, protocolContext)).isPresent(); - LOG.debug( - "checking ancestor {} is valid terminal PoW for {}\n {}", - parent.map(BlockHeader::toLogString).orElse("empty"), - blockheader.toLogString(), - resp); - if (resp) { - latestDescendsFromTerminal.set(blockheader); - } - return resp; - } - @Override public Optional getLatestValidAncestor(final Hash blockHash) { final var chain = protocolContext.getBlockchain(); diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java index 6e486bddd70..887fef66299 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java @@ -101,19 +101,11 @@ ForkchoiceResult updateForkChoice( Optional getLatestValidAncestor(BlockHeader blockheader); /** - * Check if latest valid ancestor descends from terminal. - * - * @param blockHeader the block header - * @return the boolean - */ - boolean latestValidAncestorDescendsFromTerminal(final BlockHeader blockHeader); - - /** - * Is descendant of. + * Checks if a block descends from another * * @param ancestorBlock the ancestor block - * @param newBlock the new block - * @return the boolean + * @param newBlock the block we want to check if it is descendant + * @return true if newBlock is a descendant of ancestorBlock */ boolean isDescendantOf(final BlockHeader ancestorBlock, final BlockHeader newBlock); diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java index 056f654da47..4044ac3d34b 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java @@ -179,12 +179,6 @@ public Optional getLatestValidAncestor(final BlockHeader blockHeader) { return mergeCoordinator.getLatestValidAncestor(blockHeader); } - @Override - public boolean latestValidAncestorDescendsFromTerminal(final BlockHeader blockHeader) { - // this is nonsensical pre-merge, but should be fine to delegate - return mergeCoordinator.latestValidAncestorDescendsFromTerminal(blockHeader); - } - @Override public boolean isBackwardSyncing() { return mergeCoordinator.isBackwardSyncing(); diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java index 049d87c4a36..95f1d24f220 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java @@ -91,7 +91,6 @@ import com.google.common.base.Suppliers; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; -import org.apache.tuweni.units.bigints.UInt256; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -182,7 +181,6 @@ public void setUp() { when(mergeContext.as(MergeContext.class)).thenReturn(mergeContext); when(mergeContext.getTerminalTotalDifficulty()) .thenReturn(genesisState.getBlock().getHeader().getDifficulty().plus(1L)); - when(mergeContext.getTerminalPoWBlock()).thenReturn(Optional.of(terminalPowBlock())); doAnswer( getSpecInvocation -> { ProtocolSpec spec = (ProtocolSpec) spy(getSpecInvocation.callRealMethod()); @@ -746,34 +744,6 @@ public void childTimestampExceedsParentsFails() { verify(mergeContext, never()).setFinalized(childHeader); verify(blockchain, never()).setSafeBlock(childHeader.getHash()); verify(mergeContext, never()).setSafeBlock(childHeader); - - assertThat(this.coordinator.latestValidAncestorDescendsFromTerminal(child.getHeader())) - .isTrue(); - } - - @Test - public void latestValidAncestorDescendsFromTerminal() { - BlockHeader terminalHeader = terminalPowBlock(); - sendNewPayloadAndForkchoiceUpdate( - new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO); - - BlockHeader parentHeader = nextBlockHeader(terminalHeader); - Block parent = new Block(parentHeader, BlockBody.empty()); - - // if latest valid ancestor is PoW, then latest valid hash should be Hash.ZERO - var lvh = this.coordinator.getLatestValidAncestor(parentHeader); - assertThat(lvh).isPresent(); - assertThat(lvh.get()).isEqualTo(Hash.ZERO); - - sendNewPayloadAndForkchoiceUpdate(parent, Optional.empty(), terminalHeader.getHash()); - BlockHeader childHeader = nextBlockHeader(parentHeader); - Block child = new Block(childHeader, BlockBody.empty()); - coordinator.validateBlock(child); - assertThat(this.coordinator.latestValidAncestorDescendsFromTerminal(child.getHeader())) - .isTrue(); - var nextLvh = this.coordinator.getLatestValidAncestor(childHeader); - assertThat(nextLvh).isPresent(); - assertThat(nextLvh.get()).isEqualTo(parentHeader.getHash()); } @Test @@ -800,9 +770,6 @@ public void latestValidAncestorDescendsFromFinalizedBlock() { Block child = new Block(childHeader, BlockBody.empty()); coordinator.validateBlock(child); - assertThat(this.coordinator.latestValidAncestorDescendsFromTerminal(child.getHeader())) - .isTrue(); - var nextLvh = this.coordinator.getLatestValidAncestor(childHeader); assertThat(nextLvh).isPresent(); assertThat(nextLvh.get()).isEqualTo(parentHeader.getHash()); @@ -924,152 +891,6 @@ public void assertGetOrSyncForBlockNotPresent() { assertThat(res).isNotPresent(); } - @Test - public void ancestorIsValidTerminalProofOfWork() { - final long howDeep = 100L; - assertThat( - terminalAncestorMock(howDeep, true) - .ancestorIsValidTerminalProofOfWork( - new BlockHeaderTestFixture().number(howDeep).buildHeader())) - .isTrue(); - } - - @Test - public void assertCachedUnfinalizedAncestorDescendsFromTTD() { - final long howDeep = 10; - var mockBlockHeader = new BlockHeaderTestFixture().number(howDeep).buildHeader(); - var mockCoordinator = terminalAncestorMock(howDeep, true); - assertThat( - mockCoordinator.ancestorIsValidTerminalProofOfWork( - new BlockHeaderTestFixture().number(howDeep).buildHeader())) - .isTrue(); - - // assert that parent block was cached as descending from TTD - assertThat( - mockCoordinator.ancestorIsValidTerminalProofOfWork( - new BlockHeaderTestFixture() - .number(howDeep + 1) - .parentHash(mockBlockHeader.getHash()) - .buildHeader())) - .isTrue(); - } - - @Test - public void ancestorNotFoundValidTerminalProofOfWork() { - final long howDeep = 10L; - assertThat( - terminalAncestorMock(howDeep, false) - .ancestorIsValidTerminalProofOfWork( - new BlockHeaderTestFixture().number(howDeep).buildHeader())) - .isFalse(); - } - - @Test - public void assertNonGenesisTerminalBlockSatisfiesDescendsFromTerminal() { - - var mockConsensusContext = mock(MergeContext.class); - when(mockConsensusContext.getTerminalTotalDifficulty()).thenReturn(Difficulty.of(1337L)); - var mockBlockchain = mock(MutableBlockchain.class); - var mockProtocolContext = mock(ProtocolContext.class); - when(mockProtocolContext.getBlockchain()).thenReturn(mockBlockchain); - when(mockProtocolContext.getConsensusContext(MergeContext.class)) - .thenReturn(mockConsensusContext); - - var mockHeaderBuilder = new BlockHeaderTestFixture(); - - MergeCoordinator mockCoordinator = - new MergeCoordinator( - mockProtocolContext, - protocolSchedule, - CompletableFuture::runAsync, - transactionPool, - new MiningParameters.Builder().coinbase(coinbase).build(), - mock(BackwardSyncContext.class), - Optional.empty()); - - var blockZero = mockHeaderBuilder.number(0L).difficulty(Difficulty.of(1336L)).buildHeader(); - var blockOne = - mockHeaderBuilder - .number(1L) - .difficulty(Difficulty.ONE) - .parentHash(blockZero.getHash()) - .buildHeader(); - var blockTwo = - mockHeaderBuilder - .number(2L) - .difficulty(Difficulty.ZERO) - .parentHash(blockOne.getHash()) - .buildHeader(); - var blockThree = mockHeaderBuilder.number(3L).parentHash(blockTwo.getHash()).buildHeader(); - - when(mockBlockchain.getTotalDifficultyByHash(any())) - .thenReturn(Optional.of(Difficulty.of(1337L))); - when(mockBlockchain.getTotalDifficultyByHash(blockZero.getHash())) - .thenReturn(Optional.of(Difficulty.of(1336L))); - - when(mockBlockchain.getBlockHeader(blockOne.getHash())).thenReturn(Optional.of(blockOne)); - when(mockBlockchain.getBlockHeader(blockTwo.getHash())).thenReturn(Optional.of(blockTwo)); - when(mockBlockchain.getBlockHeader(blockThree.getHash())).thenReturn(Optional.of(blockThree)); - - // assert pre-merge genesis block does not descend from terminal - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockZero)).isFalse(); - assertThat(mockCoordinator.latestDescendsFromTerminal.get()).isNull(); - // assert TTD merge block (1) descends from terminal returns true - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockOne)).isTrue(); - assertThat(mockCoordinator.latestDescendsFromTerminal.get()).isNull(); - // assert post-merge block (2) descends from terminal returns true - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockTwo)).isTrue(); - assertThat(mockCoordinator.latestDescendsFromTerminal.get()).isEqualTo(blockTwo); - // assert post-merge block (3) descends from terminal returns true - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockThree)).isTrue(); - assertThat(mockCoordinator.latestDescendsFromTerminal.get()).isEqualTo(blockThree); - } - - @Test - public void assertMergeAtGenesisSatisifiesTerminalPoW() { - var mockConsensusContext = mock(MergeContext.class); - when(mockConsensusContext.getTerminalTotalDifficulty()).thenReturn(Difficulty.of(1337L)); - var mockBlockchain = mock(MutableBlockchain.class); - when(mockBlockchain.getTotalDifficultyByHash(any(Hash.class))) - .thenReturn(Optional.of(Difficulty.of(1337L))); - var mockProtocolContext = mock(ProtocolContext.class); - when(mockProtocolContext.getBlockchain()).thenReturn(mockBlockchain); - when(mockProtocolContext.getConsensusContext(MergeContext.class)) - .thenReturn(mockConsensusContext); - - var mockHeaderBuilder = new BlockHeaderTestFixture(); - - MergeCoordinator mockCoordinator = - new MergeCoordinator( - mockProtocolContext, - protocolSchedule, - CompletableFuture::runAsync, - transactionPool, - new MiningParameters.Builder().coinbase(coinbase).build(), - mock(BackwardSyncContext.class), - Optional.empty()); - - var blockZero = mockHeaderBuilder.number(0L).buildHeader(); - var blockOne = mockHeaderBuilder.number(1L).parentHash(blockZero.getHash()).buildHeader(); - - // assert total difficulty found for block 1 return true if post-merge - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockOne)).isTrue(); - // change mock behavior to not find TTD for block 1 and defer to parent - when(mockBlockchain.getTotalDifficultyByHash(blockOne.getBlockHash())) - .thenReturn(Optional.empty()); - // assert total difficulty NOT found for block 1 returns true if parent is post-merge - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockOne)).isTrue(); - // assert true if we send in a merge-at-genesis block - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockZero)).isTrue(); - - // change mock TTD so that neither block satisfies TTD condition: - when(mockConsensusContext.getTerminalTotalDifficulty()) - .thenReturn(Difficulty.of(UInt256.fromHexString("0xdeadbeef"))); - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockOne)).isFalse(); - // assert true if we send in a merge-at-genesis block - assertThat(mockCoordinator.latestValidAncestorDescendsFromTerminal(blockZero)).isFalse(); - } - @Test public void forkchoiceUpdateShouldIgnoreAncestorOfChainHead() { BlockHeader terminalHeader = terminalPowBlock(); @@ -1150,56 +971,6 @@ private BlockHeader nextBlockHeader( .buildHeader(); } - MergeCoordinator terminalAncestorMock(final long chainDepth, final boolean hasTerminalPoW) { - final Difficulty mockTTD = Difficulty.of(1000); - BlockHeaderTestFixture builder = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE); - MutableBlockchain mockBlockchain = mock(MutableBlockchain.class); - - BlockHeader terminal = - spy( - builder - .number(0L) - .difficulty(hasTerminalPoW ? mockTTD : Difficulty.ZERO) - .buildHeader()); - when(terminal.getParentHash()).thenReturn(Hash.ZERO); - - // return decreasing numbered blocks: - final var invocations = new AtomicLong(chainDepth); - when(mockBlockchain.getBlockHeader(any())) - .thenAnswer( - z -> - Optional.of( - (invocations.decrementAndGet() < 1) - ? terminal - : builder - .difficulty(Difficulty.ZERO) - .number(invocations.get()) - .buildHeader())); - - // mock total difficulty for isTerminalProofOfWorkBlock invocation: - when(mockBlockchain.getTotalDifficultyByHash(any())).thenReturn(Optional.of(Difficulty.ZERO)); - when(mockBlockchain.getBlockHeader(Hash.ZERO)).thenReturn(Optional.empty()); - - var mockContext = mock(MergeContext.class); - when(mockContext.getTerminalTotalDifficulty()).thenReturn(mockTTD); - ProtocolContext mockProtocolContext = mock(ProtocolContext.class); - when(mockProtocolContext.getBlockchain()).thenReturn(mockBlockchain); - when(mockProtocolContext.getConsensusContext(any())).thenReturn(mockContext); - - MergeCoordinator mockCoordinator = - spy( - new MergeCoordinator( - mockProtocolContext, - protocolSchedule, - CompletableFuture::runAsync, - transactionPool, - new MiningParameters.Builder().coinbase(coinbase).build(), - mock(BackwardSyncContext.class), - Optional.empty())); - - return mockCoordinator; - } - private Transaction createTransaction(final long transactionNumber) { return new TransactionTestFixture() .value(Wei.of(transactionNumber + 1)) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java index 817dd4540dc..cfb28bcc9ef 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java @@ -140,22 +140,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) return new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_FORKCHOICE_STATE); } - // TODO: post-merge cleanup, this should be unnecessary after merge - if (requireTerminalPoWBlockValidation() - && !mergeContext.get().isCheckpointPostMergeSync() - && !mergeContext.get().isPostMergeAtGenesis() - && !mergeCoordinator.latestValidAncestorDescendsFromTerminal(newHead) - && !mergeContext.get().isChainPruningEnabled()) { - logForkchoiceUpdatedCall(INVALID, forkChoice); - return new JsonRpcSuccessResponse( - requestId, - new EngineUpdateForkchoiceResult( - INVALID, - Hash.ZERO, - null, - Optional.of(newHead + " did not descend from terminal block"))); - } - maybePayloadAttributes.ifPresentOrElse( this::logPayload, () -> LOG.debug("Payload attributes are null")); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java index e314807f1a6..ad237d3590e 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java @@ -289,21 +289,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) return respondWith(reqId, blockParam, null, SYNCING); } - // TODO: post-merge cleanup - if (requireTerminalPoWBlockValidation() - && !mergeContext.get().isCheckpointPostMergeSync() - && !mergeContext.get().isPostMergeAtGenesis() - && !mergeCoordinator.latestValidAncestorDescendsFromTerminal(newBlockHeader) - && !mergeContext.get().isChainPruningEnabled()) { - mergeCoordinator.addBadBlock(block, Optional.empty()); - return respondWithInvalid( - reqId, - blockParam, - Hash.ZERO, - INVALID, - newBlockHeader.getHash() + " did not descend from terminal block"); - } - final var latestValidAncestor = mergeCoordinator.getLatestValidAncestor(newBlockHeader); if (latestValidAncestor.isEmpty()) { diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java index 859021d6327..32dc15ffc57 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java @@ -167,9 +167,6 @@ public void shouldReturnValidWithoutFinalizedOrPayload() { BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO)) .thenReturn(Optional.of(mockHeader)); - if (validateTerminalPoWBlock()) { - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); - } assertSuccessWithPayloadForForkchoiceResult( new EngineForkchoiceUpdatedParameter(mockHeader.getHash(), Hash.ZERO, Hash.ZERO), @@ -187,9 +184,6 @@ public void shouldReturnInvalidOnOldTimestamp() { .timestamp(parent.getTimestamp()) .buildHeader(); when(blockchain.getBlockHeader(parent.getHash())).thenReturn(Optional.of(parent)); - if (validateTerminalPoWBlock()) { - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); - } when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true); when(mergeContext.isSyncing()).thenReturn(false); when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), parent.getHash())) @@ -234,9 +228,6 @@ public void shouldReturnValidWithoutFinalizedWithPayload() { BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO)) .thenReturn(Optional.of(mockHeader)); - if (validateTerminalPoWBlock()) { - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); - } var payloadParams = new EnginePayloadAttributesParameter( @@ -422,9 +413,6 @@ public void shouldIgnoreUpdateToOldHeadAndNotPreparePayload() { when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO)) .thenReturn(Optional.of(mockHeader)); - if (validateTerminalPoWBlock()) { - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); - } var ignoreOldHeadUpdateRes = ForkchoiceResult.withIgnoreUpdateToOldHead(mockHeader); when(mergeCoordinator.updateForkChoice(any(), any(), any())).thenReturn(ignoreOldHeadUpdateRes); @@ -680,9 +668,6 @@ private void setupValidForkchoiceUpdate(final BlockHeader mockHeader) { when(blockchain.getBlockHeader(any())).thenReturn(Optional.of(mockHeader)); when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO)) .thenReturn(Optional.of(mockHeader)); - if (validateTerminalPoWBlock()) { - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); - } when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true); } @@ -739,10 +724,6 @@ protected EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResu return res; } - protected boolean validateTerminalPoWBlock() { - return false; - } - protected RpcErrorType expectedInvalidPayloadError() { return RpcErrorType.INVALID_PARAMS; } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java index 9f4d45d7a62..97976f32087 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java @@ -175,11 +175,6 @@ public void shouldReturnAcceptedOnLatestValidAncestorEmpty() { .thenReturn(Optional.of(mock(BlockHeader.class))); when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class))) .thenReturn(Optional.empty()); - if (validateTerminalPoWBlock()) { - lenient() - .when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) - .thenReturn(true); - } var resp = resp(mockEnginePayload(mockHeader, Collections.emptyList())); @@ -266,11 +261,6 @@ public void shouldNotReturnInvalidOnThrownMerkleTrieException() { .thenReturn(Optional.of(mock(BlockHeader.class))); when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class))) .thenReturn(Optional.of(mockHash)); - if (validateTerminalPoWBlock()) { - lenient() - .when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) - .thenReturn(true); - } when(mergeCoordinator.rememberBlock(any())).thenThrow(new MerkleTrieException("missing leaf")); var resp = resp(mockEnginePayload(mockHeader, Collections.emptyList())); @@ -290,9 +280,6 @@ public void shouldReturnInvalidBlockHashOnBadHashParameter() { lenient() .when(blockchain.getBlockHeader(mockHeader.getParentHash())) .thenReturn(Optional.of(mock(BlockHeader.class))); - lenient() - .when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) - .thenReturn(true); lenient().when(mockHeader.getHash()).thenReturn(Hash.fromHexStringLenient("0x1337")); var resp = resp(mockEnginePayload(mockHeader, Collections.emptyList())); @@ -469,19 +456,10 @@ protected BlockHeader setupValidPayload( // .thenReturn(Optional.of(mock(BlockHeader.class))); when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class))) .thenReturn(Optional.of(mockHash)); - if (validateTerminalPoWBlock()) { - lenient() - .when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) - .thenReturn(true); - } when(mergeCoordinator.rememberBlock(any())).thenReturn(value); return mockHeader; } - protected boolean validateTerminalPoWBlock() { - return false; - } - protected ExecutionEngineJsonRpcMethod.EngineStatus getExpectedInvalidBlockHashStatus() { return INVALID; } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1Test.java index 37e84ea1582..a6f78d145c8 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1Test.java @@ -15,18 +15,9 @@ package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine; import static org.assertj.core.api.Assertions.assertThat; -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.blockcreation.MergeMiningCoordinator; -import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod; -import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineForkchoiceUpdatedParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; -import org.hyperledger.besu.ethereum.core.BlockHeader; - -import java.util.Optional; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -48,31 +39,11 @@ public void shouldReturnExpectedMethodName() { assertThat(method.getName()).isEqualTo("engine_forkchoiceUpdatedV1"); } - @Test - public void shouldReturnInvalidOnBadTerminalBlock() { - BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); - - when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO)) - .thenReturn(Optional.of(mockHeader)); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(false); - assertSuccessWithPayloadForForkchoiceResult( - new EngineForkchoiceUpdatedParameter(mockHeader.getHash(), Hash.ZERO, Hash.ZERO), - Optional.empty(), - mock(MergeMiningCoordinator.ForkchoiceResult.class), - INVALID, - Optional.of(Hash.ZERO)); - } - @Override protected String getMethodName() { return RpcMethod.ENGINE_FORKCHOICE_UPDATED_V1.getMethodName(); } - @Override - protected boolean validateTerminalPoWBlock() { - return true; - } - @Override protected RpcErrorType expectedInvalidPayloadError() { return RpcErrorType.INVALID_PAYLOAD_ATTRIBUTES; diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadEIP6110Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadEIP6110Test.java index 577825ffb49..639be286034 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadEIP6110Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadEIP6110Test.java @@ -96,7 +96,6 @@ public void shouldReturnValidIfDepositsIsNull_WhenDepositsProhibited() { .thenReturn(Optional.of(mock(BlockHeader.class))); when(mergeCoordinator.getLatestValidAncestor(mockHeader)) .thenReturn(Optional.of(mockHeader.getHash())); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); var resp = resp(mockEnginePayload(mockHeader, Collections.emptyList(), null, deposits, null)); @@ -137,7 +136,6 @@ public void shouldReturnValidIfDepositsIsNotNull_WhenDepositsAllowed() { .thenReturn(Optional.of(mock(BlockHeader.class))); when(mergeCoordinator.getLatestValidAncestor(mockHeader)) .thenReturn(Optional.of(mockHeader.getHash())); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); var resp = resp(mockEnginePayload(mockHeader, Collections.emptyList(), null, depositsParam)); assertValidResponse(mockHeader, resp); diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1Test.java index 9a98635117a..0ad39def5ac 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1Test.java @@ -15,22 +15,9 @@ package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine; import static org.assertj.core.api.Assertions.assertThat; -import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID_BLOCK_HASH; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; -import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod; -import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EnginePayloadStatusResult; -import org.hyperledger.besu.ethereum.core.BlockHeader; - -import java.util.Collections; -import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -65,29 +52,6 @@ public void shouldReturnExpectedMethodName() { assertThat(method.getName()).isEqualTo("engine_newPayloadV1"); } - @Test - public void shouldReturnInvalidOnBadTerminalBlock() { - BlockHeader mockHeader = createBlockHeader(Optional.empty(), Optional.empty()); - when(blockchain.getBlockByHash(mockHeader.getHash())).thenReturn(Optional.empty()); - when(blockchain.getBlockHeader(mockHeader.getParentHash())) - .thenReturn(Optional.of(mock(BlockHeader.class))); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) - .thenReturn(false); - - var resp = resp(mockEnginePayload(mockHeader, Collections.emptyList())); - - EnginePayloadStatusResult res = fromSuccessResp(resp); - assertThat(res.getLatestValidHash()).isEqualTo(Optional.of(Hash.ZERO)); - assertThat(res.getStatusAsString()).isEqualTo(INVALID.name()); - verify(mergeCoordinator, atLeastOnce()).addBadBlock(any(), any()); - verify(engineCallListener, times(1)).executionEngineCalled(); - } - - @Override - protected boolean validateTerminalPoWBlock() { - return true; - } - @Override protected ExecutionEngineJsonRpcMethod.EngineStatus getExpectedInvalidBlockHashStatus() { return INVALID_BLOCK_HASH; diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java index 8605ad10890..7b01766f263 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java @@ -147,11 +147,6 @@ public void shouldReturnInvalidIfWithdrawalsIsNull_WhenWithdrawalsAllowed() { verify(engineCallListener, times(1)).executionEngineCalled(); } - @Override - protected boolean validateTerminalPoWBlock() { - return false; - } - @Override protected ExecutionEngineJsonRpcMethod.EngineStatus getExpectedInvalidBlockHashStatus() { return INVALID;