From 6830f4d761a03d04fe3a75ee5e78d1920eed7e8c Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Fri, 22 Sep 2023 20:51:07 -0400 Subject: [PATCH 1/6] payload validation moved earlier, fcu v2 checks for cancun timestamps Signed-off-by: Justin Florentine --- .../AbstractEngineForkchoiceUpdated.java | 56 +++++++++---------- .../engine/EngineForkchoiceUpdatedV2.java | 12 ++-- .../AbstractEngineForkchoiceUpdatedTest.java | 3 +- 3 files changed, 36 insertions(+), 35 deletions(-) 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 aca6ee9dc6b..c189bd63828 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 @@ -87,15 +87,15 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) LOG.debug("Forkchoice parameters {}", forkChoice); mergeContext - .get() - .fireNewUnverifiedForkchoiceEvent( - forkChoice.getHeadBlockHash(), - forkChoice.getSafeBlockHash(), - forkChoice.getFinalizedBlockHash()); + .get() + .fireNewUnverifiedForkchoiceEvent( + forkChoice.getHeadBlockHash(), + forkChoice.getSafeBlockHash(), + forkChoice.getFinalizedBlockHash()); final Optional maybeNewHead = - mergeCoordinator.getOrSyncHeadByHash( - forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash()); + mergeCoordinator.getOrSyncHeadByHash( + forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash()); if (maybeNewHead.isEmpty()) { return syncingResponse(requestId, forkChoice); @@ -104,24 +104,22 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) final BlockHeader newHead = maybeNewHead.get(); if (maybePayloadAttributes.isPresent()) { final EnginePayloadAttributesParameter payloadAttributes = maybePayloadAttributes.get(); - withdrawals = - maybePayloadAttributes.flatMap( - pa -> - Optional.ofNullable(pa.getWithdrawals()) - .map( - ws -> - ws.stream() - .map(WithdrawalParameter::toWithdrawal) - .collect(toList()))); + withdrawals = + maybePayloadAttributes.flatMap( + pa -> + Optional.ofNullable(pa.getWithdrawals()) + .map( + ws -> + ws.stream().map(WithdrawalParameter::toWithdrawal).collect(toList()))); if (!isPayloadAttributesValid(maybePayloadAttributes.get(), withdrawals, newHead)) { LOG.atWarn() - .setMessage("Invalid payload attributes: {}") - .addArgument( - () -> - maybePayloadAttributes - .map(EnginePayloadAttributesParameter::serialize) - .orElse(null)) - .log(); + .setMessage("Invalid payload attributes: {}") + .addArgument( + () -> + maybePayloadAttributes + .map(EnginePayloadAttributesParameter::serialize) + .orElse(null)) + .log(); return new JsonRpcErrorResponse(requestId, getInvalidPayloadError()); } ValidationResult forkValidationResult = @@ -131,11 +129,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } } - ValidationResult parameterValidationResult = - validateParameter(forkChoice, maybePayloadAttributes); - if (!parameterValidationResult.isValid()) { - return new JsonRpcSuccessResponse(requestId, parameterValidationResult); - } + if (mergeContext.get().isSyncing()) { return syncingResponse(requestId, forkChoice); @@ -154,6 +148,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block"))); } + + if (!isValidForkchoiceState( forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) { logForkchoiceUpdatedCall(INVALID, forkChoice); @@ -163,10 +159,14 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) maybePayloadAttributes.ifPresentOrElse( this::logPayload, () -> LOG.debug("Payload attributes are null")); + + ForkchoiceResult result = mergeCoordinator.updateForkChoice( newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash()); + + if (result.shouldNotProceedToPayloadBuildProcess()) { if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) { logForkchoiceUpdatedCall(VALID, forkChoice); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java index d8e0fd715a3..808c2f6c9f0 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java @@ -22,11 +22,11 @@ import org.hyperledger.besu.ethereum.core.Withdrawal; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import io.vertx.core.Vertx; + import java.util.List; import java.util.Optional; -import io.vertx.core.Vertx; - // TODO Withdrawals use composition instead? Want to make it more obvious that there is no // difference between V1/V2 code other than the method name public class EngineForkchoiceUpdatedV2 extends AbstractEngineForkchoiceUpdated { @@ -47,10 +47,10 @@ public String getName() { @Override protected boolean isPayloadAttributesValid( - final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals, - final BlockHeader headBlockHeader) { - if (payloadAttributes.getTimestamp() >= cancunTimestamp) { + final EnginePayloadAttributesParameter payloadAttributes, + final Optional> maybeWithdrawals, + final BlockHeader headBlockHeader) { + if(payloadAttributes.getTimestamp() >= cancunTimestamp) { return false; } else { return super.isPayloadAttributesValid(payloadAttributes, maybeWithdrawals, headBlockHeader); 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 693271bf689..ac62aa26ff1 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 @@ -143,7 +143,8 @@ public void shouldReturnSyncingIfMissingNewHead() { public void shouldReturnInvalidWithLatestValidHashOnBadBlock() { BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); Hash latestValidHash = Hash.hash(Bytes32.fromHexStringLenient("0xcafebabe")); - when(mergeCoordinator.getOrSyncHeadByHash(any(), any())).thenReturn(Optional.of(mockHeader)); + when(mergeCoordinator.getOrSyncHeadByHash(any(), any())) + .thenReturn(Optional.of(mockHeader)); when(mergeCoordinator.isBadBlock(mockHeader.getHash())).thenReturn(true); when(mergeCoordinator.getLatestValidHashOfBadBlock(mockHeader.getHash())) .thenReturn(Optional.of(latestValidHash)); From 5228c8fe6ba06550a49ae82532a75588253798ac Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Fri, 22 Sep 2023 21:29:17 -0400 Subject: [PATCH 2/6] payload validation moved earlier, fcu v2 checks for cancun timestamps Signed-off-by: Justin Florentine --- .../AbstractEngineForkchoiceUpdated.java | 55 +++++++++---------- .../engine/EngineForkchoiceUpdatedV2.java | 12 ++-- .../AbstractEngineForkchoiceUpdatedTest.java | 3 +- 3 files changed, 34 insertions(+), 36 deletions(-) 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 c189bd63828..cd10acac7e4 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 @@ -87,15 +87,15 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) LOG.debug("Forkchoice parameters {}", forkChoice); mergeContext - .get() - .fireNewUnverifiedForkchoiceEvent( - forkChoice.getHeadBlockHash(), - forkChoice.getSafeBlockHash(), - forkChoice.getFinalizedBlockHash()); + .get() + .fireNewUnverifiedForkchoiceEvent( + forkChoice.getHeadBlockHash(), + forkChoice.getSafeBlockHash(), + forkChoice.getFinalizedBlockHash()); final Optional maybeNewHead = - mergeCoordinator.getOrSyncHeadByHash( - forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash()); + mergeCoordinator.getOrSyncHeadByHash( + forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash()); if (maybeNewHead.isEmpty()) { return syncingResponse(requestId, forkChoice); @@ -104,22 +104,24 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) final BlockHeader newHead = maybeNewHead.get(); if (maybePayloadAttributes.isPresent()) { final EnginePayloadAttributesParameter payloadAttributes = maybePayloadAttributes.get(); - withdrawals = - maybePayloadAttributes.flatMap( - pa -> - Optional.ofNullable(pa.getWithdrawals()) - .map( - ws -> - ws.stream().map(WithdrawalParameter::toWithdrawal).collect(toList()))); + withdrawals = + maybePayloadAttributes.flatMap( + pa -> + Optional.ofNullable(pa.getWithdrawals()) + .map( + ws -> + ws.stream() + .map(WithdrawalParameter::toWithdrawal) + .collect(toList()))); if (!isPayloadAttributesValid(maybePayloadAttributes.get(), withdrawals, newHead)) { LOG.atWarn() - .setMessage("Invalid payload attributes: {}") - .addArgument( - () -> - maybePayloadAttributes - .map(EnginePayloadAttributesParameter::serialize) - .orElse(null)) - .log(); + .setMessage("Invalid payload attributes: {}") + .addArgument( + () -> + maybePayloadAttributes + .map(EnginePayloadAttributesParameter::serialize) + .orElse(null)) + .log(); return new JsonRpcErrorResponse(requestId, getInvalidPayloadError()); } ValidationResult forkValidationResult = @@ -129,7 +131,10 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } } - + ValidationResult parameterValidationResult = validateParameter(forkChoice); + if (!parameterValidationResult.isValid()) { + return new JsonRpcSuccessResponse(requestId, parameterValidationResult); + } if (mergeContext.get().isSyncing()) { return syncingResponse(requestId, forkChoice); @@ -148,8 +153,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block"))); } - - if (!isValidForkchoiceState( forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) { logForkchoiceUpdatedCall(INVALID, forkChoice); @@ -159,14 +162,10 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) maybePayloadAttributes.ifPresentOrElse( this::logPayload, () -> LOG.debug("Payload attributes are null")); - - ForkchoiceResult result = mergeCoordinator.updateForkChoice( newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash()); - - if (result.shouldNotProceedToPayloadBuildProcess()) { if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) { logForkchoiceUpdatedCall(VALID, forkChoice); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java index 808c2f6c9f0..d8e0fd715a3 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java @@ -22,11 +22,11 @@ import org.hyperledger.besu.ethereum.core.Withdrawal; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; -import io.vertx.core.Vertx; - import java.util.List; import java.util.Optional; +import io.vertx.core.Vertx; + // TODO Withdrawals use composition instead? Want to make it more obvious that there is no // difference between V1/V2 code other than the method name public class EngineForkchoiceUpdatedV2 extends AbstractEngineForkchoiceUpdated { @@ -47,10 +47,10 @@ public String getName() { @Override protected boolean isPayloadAttributesValid( - final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals, - final BlockHeader headBlockHeader) { - if(payloadAttributes.getTimestamp() >= cancunTimestamp) { + final EnginePayloadAttributesParameter payloadAttributes, + final Optional> maybeWithdrawals, + final BlockHeader headBlockHeader) { + if (payloadAttributes.getTimestamp() >= cancunTimestamp) { return false; } else { return super.isPayloadAttributesValid(payloadAttributes, maybeWithdrawals, headBlockHeader); 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 ac62aa26ff1..693271bf689 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 @@ -143,8 +143,7 @@ public void shouldReturnSyncingIfMissingNewHead() { public void shouldReturnInvalidWithLatestValidHashOnBadBlock() { BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); Hash latestValidHash = Hash.hash(Bytes32.fromHexStringLenient("0xcafebabe")); - when(mergeCoordinator.getOrSyncHeadByHash(any(), any())) - .thenReturn(Optional.of(mockHeader)); + when(mergeCoordinator.getOrSyncHeadByHash(any(), any())).thenReturn(Optional.of(mockHeader)); when(mergeCoordinator.isBadBlock(mockHeader.getHash())).thenReturn(true); when(mergeCoordinator.getLatestValidHashOfBadBlock(mockHeader.getHash())) .thenReturn(Optional.of(latestValidHash)); From 21ebc522c09cc6bb34f6ff0d1b21c1b3149e4a8f Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Tue, 26 Sep 2023 13:57:52 -0400 Subject: [PATCH 3/6] build passes Signed-off-by: Justin Florentine --- .../methods/engine/AbstractEngineForkchoiceUpdated.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 cd10acac7e4..aca6ee9dc6b 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 @@ -131,7 +131,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } } - ValidationResult parameterValidationResult = validateParameter(forkChoice); + ValidationResult parameterValidationResult = + validateParameter(forkChoice, maybePayloadAttributes); if (!parameterValidationResult.isValid()) { return new JsonRpcSuccessResponse(requestId, parameterValidationResult); } From c7b4d37112cb0eefc367a0ac77b28233ad155494 Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Wed, 27 Sep 2023 09:21:48 -0400 Subject: [PATCH 4/6] allow fcu when payload invalid Signed-off-by: Justin Florentine --- .../AbstractEngineForkchoiceUpdated.java | 19 +++++++++++-------- .../AbstractEngineForkchoiceUpdatedTest.java | 2 ++ 2 files changed, 13 insertions(+), 8 deletions(-) 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 aca6ee9dc6b..7fddc1ccfbc 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 @@ -102,6 +102,16 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } Optional> withdrawals = Optional.empty(); final BlockHeader newHead = maybeNewHead.get(); + if (!isValidForkchoiceState( + forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) { + logForkchoiceUpdatedCall(INVALID, forkChoice); + return new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_FORKCHOICE_STATE); + } + ForkchoiceResult result = + mergeCoordinator.updateForkChoice( + newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash()); + + if (maybePayloadAttributes.isPresent()) { final EnginePayloadAttributesParameter payloadAttributes = maybePayloadAttributes.get(); withdrawals = @@ -154,18 +164,11 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block"))); } - if (!isValidForkchoiceState( - forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) { - logForkchoiceUpdatedCall(INVALID, forkChoice); - return new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_FORKCHOICE_STATE); - } maybePayloadAttributes.ifPresentOrElse( this::logPayload, () -> LOG.debug("Payload attributes are null")); - ForkchoiceResult result = - mergeCoordinator.updateForkChoice( - newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash()); + if (result.shouldNotProceedToPayloadBuildProcess()) { if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) { 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 693271bf689..daad391cbb0 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 @@ -143,6 +143,8 @@ public void shouldReturnSyncingIfMissingNewHead() { public void shouldReturnInvalidWithLatestValidHashOnBadBlock() { BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); Hash latestValidHash = Hash.hash(Bytes32.fromHexStringLenient("0xcafebabe")); + when(blockchain.getBlockHeader(mockHeader.getHash())).thenReturn(Optional.of(mockHeader)); + when(blockchain.getBlockHeader(mockHeader.getParentHash())).thenReturn(Optional.of(mockHeader)); when(mergeCoordinator.getOrSyncHeadByHash(any(), any())).thenReturn(Optional.of(mockHeader)); when(mergeCoordinator.isBadBlock(mockHeader.getHash())).thenReturn(true); when(mergeCoordinator.getLatestValidHashOfBadBlock(mockHeader.getHash())) From 11839d24c00a99ce0406b1d86e7de24ad01cf8fe Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Wed, 27 Sep 2023 17:39:19 -0400 Subject: [PATCH 5/6] new order requires more mocking Signed-off-by: Justin Florentine --- .../engine/AbstractEngineForkchoiceUpdatedTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 daad391cbb0..249a12d9f0b 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 @@ -141,18 +141,21 @@ public void shouldReturnSyncingIfMissingNewHead() { @Test public void shouldReturnInvalidWithLatestValidHashOnBadBlock() { + BlockHeader mockParent = blockHeaderBuilder.buildHeader(); + blockHeaderBuilder.parentHash(mockParent.getHash()); BlockHeader mockHeader = blockHeaderBuilder.buildHeader(); Hash latestValidHash = Hash.hash(Bytes32.fromHexStringLenient("0xcafebabe")); when(blockchain.getBlockHeader(mockHeader.getHash())).thenReturn(Optional.of(mockHeader)); - when(blockchain.getBlockHeader(mockHeader.getParentHash())).thenReturn(Optional.of(mockHeader)); + when(blockchain.getBlockHeader(mockHeader.getParentHash())).thenReturn(Optional.of(mockParent)); when(mergeCoordinator.getOrSyncHeadByHash(any(), any())).thenReturn(Optional.of(mockHeader)); when(mergeCoordinator.isBadBlock(mockHeader.getHash())).thenReturn(true); + when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true); when(mergeCoordinator.getLatestValidHashOfBadBlock(mockHeader.getHash())) .thenReturn(Optional.of(latestValidHash)); assertSuccessWithPayloadForForkchoiceResult( new EngineForkchoiceUpdatedParameter( - mockHeader.getHash(), Hash.ZERO, mockHeader.getParentHash()), + mockHeader.getHash(), mockHeader.getParentHash(), mockHeader.getParentHash()), Optional.empty(), mock(ForkchoiceResult.class), INVALID, From c1107e4b1ae8c7b67e94289055e10b8c4a5d93e6 Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Wed, 27 Sep 2023 22:33:03 -0400 Subject: [PATCH 6/6] spotless Signed-off-by: Justin Florentine --- .../engine/AbstractEngineForkchoiceUpdated.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) 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 7fddc1ccfbc..8e471e2a113 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 @@ -103,14 +103,13 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) Optional> withdrawals = Optional.empty(); final BlockHeader newHead = maybeNewHead.get(); if (!isValidForkchoiceState( - forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) { + forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) { logForkchoiceUpdatedCall(INVALID, forkChoice); return new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_FORKCHOICE_STATE); } ForkchoiceResult result = - mergeCoordinator.updateForkChoice( - newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash()); - + mergeCoordinator.updateForkChoice( + newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash()); if (maybePayloadAttributes.isPresent()) { final EnginePayloadAttributesParameter payloadAttributes = maybePayloadAttributes.get(); @@ -164,12 +163,9 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block"))); } - maybePayloadAttributes.ifPresentOrElse( this::logPayload, () -> LOG.debug("Payload attributes are null")); - - if (result.shouldNotProceedToPayloadBuildProcess()) { if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) { logForkchoiceUpdatedCall(VALID, forkChoice);