Skip to content

Commit

Permalink
Shanghai Engine API Updates (#4945)
Browse files Browse the repository at this point in the history
Update engine API to include some new changes introduced in 
- ethereum/execution-apis#338
- ethereum/execution-apis#337

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Zhenyang Shi <wcgcyx@gmail.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
  • Loading branch information
wcgcyx and siladu authored Jan 18, 2023
1 parent 8f32c5d commit aef335c
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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),
Expand All @@ -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()))
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}

Expand All @@ -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();
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -680,7 +675,7 @@ protected EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResu
fcuParam, payloadParam, forkchoiceResult, expectedStatus, Optional.empty());
}

private EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResult(
protected EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResult(
final EngineForkchoiceUpdatedParameter fcuParam,
final Optional<EnginePayloadAttributesParameter> payloadParam,
final ForkchoiceResult forkchoiceResult,
Expand Down Expand Up @@ -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<EnginePayloadAttributesParameter> payloadParam) {
Expand Down
Loading

0 comments on commit aef335c

Please sign in to comment.