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 9f151dc6324..2adff6c49b3 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 @@ -119,7 +119,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } // TODO: post-merge cleanup, this should be unnecessary after merge - if (!mergeContext.get().isCheckpointPostMergeSync() + if (requireTerminalPoWBlockValidation() + && !mergeContext.get().isCheckpointPostMergeSync() && !mergeCoordinator.latestValidAncestorDescendsFromTerminal(newHead) && !mergeContext.get().isChainPruningEnabled()) { logForkchoiceUpdatedCall(INVALID, forkChoice); @@ -154,7 +155,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) "Invalid payload attributes: {}", () -> maybePayloadAttributes.map(EnginePayloadAttributesParameter::serialize).orElse(null)); - return new JsonRpcErrorResponse(requestId, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES); + return new JsonRpcErrorResponse(requestId, getInvalidPayloadError()); } if (!result.isValid()) { @@ -217,9 +218,6 @@ private JsonRpcResponse handleNonValidForkchoiceUpdate( new EngineUpdateForkchoiceResult( INVALID, latestValid.orElse(null), null, result.getErrorMessage())); break; - case INVALID_PAYLOAD_ATTRIBUTES: - response = new JsonRpcErrorResponse(requestId, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES); - break; case IGNORE_UPDATE_TO_OLD_HEAD: response = new JsonRpcSuccessResponse( @@ -300,6 +298,14 @@ private JsonRpcResponse syncingResponse( requestId, new EngineUpdateForkchoiceResult(SYNCING, null, null, Optional.empty())); } + protected boolean requireTerminalPoWBlockValidation() { + return false; + } + + protected JsonRpcError getInvalidPayloadError() { + return JsonRpcError.INVALID_PARAMS; + } + // fcU calls are synchronous, no need to make volatile private long lastFcuInfoLog = System.currentTimeMillis(); private static final String logMessage = 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 24ec99b34a7..12a84e69189 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 @@ -165,7 +165,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) "Computed block hash %s does not match block hash parameter %s", newBlockHeader.getBlockHash(), blockParam.getBlockHash()); LOG.debug(errorMessage); - return respondWithInvalid(reqId, blockParam, null, INVALID_BLOCK_HASH, errorMessage); + return respondWithInvalid(reqId, blockParam, null, getInvalidBlockHashStatus(), errorMessage); } // do we already have this payload if (protocolContext.getBlockchain().getBlockByHash(newBlockHeader.getBlockHash()).isPresent()) { @@ -208,7 +208,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } // TODO: post-merge cleanup - if (!mergeContext.get().isCheckpointPostMergeSync() + if (requireTerminalPoWBlockValidation() + && !mergeContext.get().isCheckpointPostMergeSync() && !mergeCoordinator.latestValidAncestorDescendsFromTerminal(newBlockHeader) && !mergeContext.get().isChainPruningEnabled()) { mergeCoordinator.addBadBlock(block, Optional.empty()); @@ -308,6 +309,14 @@ JsonRpcResponse respondWithInvalid( invalidStatus, latestValidHash, Optional.of(validationError))); } + protected boolean requireTerminalPoWBlockValidation() { + return false; + } + + protected EngineStatus getInvalidBlockHashStatus() { + return INVALID; + } + private void logImportedBlockInfo(final Block block, final double timeInS) { LOG.info( String.format( diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java index b43052b1554..dd965f20bcb 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator; import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; import org.hyperledger.besu.ethereum.mainnet.TimestampSchedule; import io.vertx.core.Vertx; @@ -36,4 +37,14 @@ public EngineForkchoiceUpdatedV1( public String getName() { return RpcMethod.ENGINE_FORKCHOICE_UPDATED_V1.getMethodName(); } + + @Override + protected boolean requireTerminalPoWBlockValidation() { + return true; + } + + @Override + protected JsonRpcError getInvalidPayloadError() { + return JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES; + } } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1.java index c9b5c6d060c..d3b023076e8 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1.java @@ -14,6 +14,8 @@ */ package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine; +import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID_BLOCK_HASH; + import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator; import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod; @@ -39,4 +41,14 @@ public EngineNewPayloadV1( public String getName() { return RpcMethod.ENGINE_NEW_PAYLOAD_V1.getMethodName(); } + + @Override + protected boolean requireTerminalPoWBlockValidation() { + return true; + } + + @Override + protected EngineStatus getInvalidBlockHashStatus() { + return INVALID_BLOCK_HASH; + } } 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 8486b80b869..09dd87042e6 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 @@ -95,7 +95,7 @@ public AbstractEngineForkchoiceUpdatedTest(final MethodFactory methodFactory) { private static final EngineForkchoiceUpdatedParameter mockFcuParam = new EngineForkchoiceUpdatedParameter(mockHash, mockHash, mockHash); - private static final BlockHeaderTestFixture blockHeaderBuilder = + protected static final BlockHeaderTestFixture blockHeaderBuilder = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE); @Mock private ProtocolSpec protocolSpec; @@ -104,7 +104,7 @@ public AbstractEngineForkchoiceUpdatedTest(final MethodFactory methodFactory) { @Mock private MergeContext mergeContext; - @Mock private MergeMiningCoordinator mergeCoordinator; + @Mock protected MergeMiningCoordinator mergeCoordinator; @Mock private MutableBlockchain blockchain; @@ -138,21 +138,6 @@ public void shouldReturnSyncingIfMissingNewHead() { mockFcuParam, Optional.empty(), mock(ForkchoiceResult.class), SYNCING); } - @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(ForkchoiceResult.class), - INVALID, - Optional.of(Hash.ZERO)); - } - @Test public void shouldReturnInvalidWithLatestValidHashOnBadBlock() { BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); @@ -181,7 +166,9 @@ public void shouldReturnValidWithoutFinalizedOrPayload() { BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO)) .thenReturn(Optional.of(mockHeader)); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + if (validateTerminalPoWBlock()) { + when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + } assertSuccessWithPayloadForForkchoiceResult( new EngineForkchoiceUpdatedParameter(mockHeader.getHash(), Hash.ZERO, Hash.ZERO), @@ -199,7 +186,9 @@ public void shouldReturnInvalidOnOldTimestamp() { .timestamp(parent.getTimestamp()) .buildHeader(); when(blockchain.getBlockHeader(parent.getHash())).thenReturn(Optional.of(parent)); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + 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())) @@ -244,7 +233,9 @@ public void shouldReturnValidWithoutFinalizedWithPayload() { BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO)) .thenReturn(Optional.of(mockHeader)); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + if (validateTerminalPoWBlock()) { + when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + } var payloadParams = new EnginePayloadAttributesParameter( @@ -427,7 +418,9 @@ public void shouldIgnoreUpdateToOldHeadAndNotPreparePayload() { when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO)) .thenReturn(Optional.of(mockHeader)); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + if (validateTerminalPoWBlock()) { + when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + } var ignoreOldHeadUpdateRes = ForkchoiceResult.withIgnoreUpdateToOldHead(mockHeader); when(mergeCoordinator.updateForkChoice(any(), any(), any())).thenReturn(ignoreOldHeadUpdateRes); @@ -481,7 +474,7 @@ public void shouldReturnInvalidIfPayloadTimestampNotGreaterThanHead() { mockHeader.getHash(), Hash.ZERO, mockParent.getHash()), Optional.of(payloadParams)); - assertInvalidForkchoiceState(resp, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES); + assertInvalidForkchoiceState(resp, expectedInvalidPayloadError()); verify(engineCallListener, times(1)).executionEngineCalled(); } @@ -505,7 +498,7 @@ public void shouldReturnInvalidIfWithdrawalsIsNotNull_WhenWithdrawalsProhibited( mockHeader.getHash(), Hash.ZERO, mockParent.getHash()), Optional.of(payloadParams)); - assertInvalidForkchoiceState(resp, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES); + assertInvalidForkchoiceState(resp, expectedInvalidPayloadError()); verify(engineCallListener, times(1)).executionEngineCalled(); } @@ -569,7 +562,7 @@ public void shouldReturnInvalidIfWithdrawalsIsNull_WhenWithdrawalsAllowed() { mockHeader.getHash(), Hash.ZERO, mockParent.getHash()), Optional.of(payloadParams)); - assertInvalidForkchoiceState(resp, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES); + assertInvalidForkchoiceState(resp, expectedInvalidPayloadError()); verify(engineCallListener, times(1)).executionEngineCalled(); } @@ -667,7 +660,9 @@ 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)); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + if (validateTerminalPoWBlock()) { + when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true); + } when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true); } @@ -680,7 +675,7 @@ protected EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResu fcuParam, payloadParam, forkchoiceResult, expectedStatus, Optional.empty()); } - private EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResult( + protected EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResult( final EngineForkchoiceUpdatedParameter fcuParam, final Optional payloadParam, final ForkchoiceResult forkchoiceResult, @@ -724,6 +719,14 @@ private EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResult return res; } + protected boolean validateTerminalPoWBlock() { + return false; + } + + protected JsonRpcError expectedInvalidPayloadError() { + return JsonRpcError.INVALID_PARAMS; + } + private JsonRpcResponse resp( final EngineForkchoiceUpdatedParameter forkchoiceParam, final Optional payloadParam) { 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 f252fe1a922..58cb578ece4 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 @@ -17,14 +17,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.ACCEPTED; 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.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.SYNCING; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.VALID; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.WithdrawalParameterTestFixture.WITHDRAWAL_PARAM_1; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.INVALID_PARAMS; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -42,6 +40,7 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod; 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.methods.ExecutionEngineJsonRpcMethod; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.UnsignedLongParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.WithdrawalParameter; @@ -108,13 +107,13 @@ public AbstractEngineNewPayloadTest(final MethodFactory methodFactory) { @Mock private MergeContext mergeContext; - @Mock private MergeMiningCoordinator mergeCoordinator; + @Mock protected MergeMiningCoordinator mergeCoordinator; - @Mock private MutableBlockchain blockchain; + @Mock protected MutableBlockchain blockchain; @Mock private EthPeers ethPeers; - @Mock private EngineCallListener engineCallListener; + @Mock protected EngineCallListener engineCallListener; @Before public void before() { @@ -169,8 +168,10 @@ public void shouldReturnAcceptedOnLatestValidAncestorEmpty() { .thenReturn(Optional.of(mock(BlockHeader.class))); when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class))) .thenReturn(Optional.empty()); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) - .thenReturn(true); + if (validateTerminalPoWBlock()) { + when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) + .thenReturn(true); + } var resp = resp(mockPayload(mockHeader, Collections.emptyList())); @@ -194,24 +195,6 @@ public void shouldReturnSuccessOnAlreadyPresent() { assertValidResponse(mockHeader, resp); } - @Test - public void shouldReturnInvalidOnBadTerminalBlock() { - BlockHeader mockHeader = createBlockHeader(); - 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(mockPayload(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(); - } - @Test public void shouldReturnInvalidWithLatestValidHashIsABadBlock() { BlockHeader mockHeader = createBlockHeader(); @@ -266,8 +249,10 @@ public void shouldNotReturnInvalidOnThrownMerkleTrieException() { .thenReturn(Optional.of(mock(BlockHeader.class))); when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class))) .thenReturn(Optional.of(mockHash)); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) - .thenReturn(true); + if (validateTerminalPoWBlock()) { + when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) + .thenReturn(true); + } when(mergeCoordinator.rememberBlock(any())).thenThrow(new MerkleTrieException("missing leaf")); var resp = resp(mockPayload(mockHeader, Collections.emptyList())); @@ -286,7 +271,7 @@ public void shouldReturnInvalidBlockHashOnBadHashParameter() { EnginePayloadStatusResult res = fromSuccessResp(resp); assertThat(res.getLatestValidHash()).isEmpty(); - assertThat(res.getStatusAsString()).isEqualTo(INVALID_BLOCK_HASH.name()); + assertThat(res.getStatusAsString()).isEqualTo(getExpectedInvalidBlockHashStatus().name()); verify(engineCallListener, times(1)).executionEngineCalled(); } @@ -300,7 +285,7 @@ public void shouldCheckBlockValidityBeforeCheckingByHashForExisting() { EnginePayloadStatusResult res = fromSuccessResp(resp); assertThat(res.getLatestValidHash()).isEmpty(); - assertThat(res.getStatusAsString()).isEqualTo(INVALID_BLOCK_HASH.name()); + assertThat(res.getStatusAsString()).isEqualTo(getExpectedInvalidBlockHashStatus().name()); verify(engineCallListener, times(1)).executionEngineCalled(); } @@ -468,14 +453,14 @@ public void shouldReturnValidIfTimestampScheduleIsEmpty() { assertValidResponse(mockHeader, resp); } - private JsonRpcResponse resp(final EnginePayloadParameter payload) { + protected JsonRpcResponse resp(final EnginePayloadParameter payload) { return method.response( new JsonRpcRequestContext( new JsonRpcRequest( "2.0", RpcMethod.ENGINE_EXECUTE_PAYLOAD.getMethodName(), new Object[] {payload}))); } - private EnginePayloadParameter mockPayload(final BlockHeader header, final List txs) { + protected EnginePayloadParameter mockPayload(final BlockHeader header, final List txs) { return new EnginePayloadParameter( header.getHash(), header.getParentHash(), @@ -524,13 +509,23 @@ private BlockHeader setupValidPayload(final BlockProcessingResult value) { .thenReturn(Optional.of(mock(BlockHeader.class))); when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class))) .thenReturn(Optional.of(mockHash)); - when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) - .thenReturn(true); + if (validateTerminalPoWBlock()) { + when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class))) + .thenReturn(true); + } when(mergeCoordinator.rememberBlock(any())).thenReturn(value); return mockHeader; } - private EnginePayloadStatusResult fromSuccessResp(final JsonRpcResponse resp) { + protected boolean validateTerminalPoWBlock() { + return false; + } + + protected ExecutionEngineJsonRpcMethod.EngineStatus getExpectedInvalidBlockHashStatus() { + return INVALID; + } + + protected EnginePayloadStatusResult fromSuccessResp(final JsonRpcResponse resp) { assertThat(resp.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); return Optional.of(resp) .map(JsonRpcSuccessResponse.class::cast) @@ -547,7 +542,7 @@ private JsonRpcError fromErrorResp(final JsonRpcResponse resp) { .get(); } - private BlockHeader createBlockHeader() { + protected BlockHeader createBlockHeader() { BlockHeader parentBlockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader(); BlockHeader mockHeader = 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 5c60d95f57a..0433b55bae1 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,8 +15,18 @@ 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.JsonRpcError; +import org.hyperledger.besu.ethereum.core.BlockHeader; + +import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -35,8 +45,33 @@ 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 JsonRpcError expectedInvalidPayloadError() { + return JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES; + } } 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 fd739a7b89b..06c7b8a1830 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,6 +15,22 @@ 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.Test; import org.junit.runner.RunWith; @@ -32,4 +48,32 @@ public EngineNewPayloadV1Test() { public void shouldReturnExpectedMethodName() { assertThat(method.getName()).isEqualTo("engine_newPayloadV1"); } + + @Test + public void shouldReturnInvalidOnBadTerminalBlock() { + BlockHeader mockHeader = createBlockHeader(); + 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(mockPayload(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; + } }