When on PoS the head can be only be updated by ForkchoiceUpdate (hype…

…rledger#4013) * When executing a newPayload do not move the chain head or update the world state * When proposing a block, use a lightweight validation, without storing * forwardToBlock moves head to the block and triggers advanced head event * Do not persist prepared blocks Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Co-authored-by: garyschulte <garyschulte@gmail.com> Co-authored-by: Jiri Peinlich <jiri.peinlich@gmail.com> Co-authored-by: Karim TAAM <karim.t2am@gmail.com>
noirgif · Jul 1, 2022 · 90f891b · 90f891b
1 parent b5fa62c
commit 90f891b
Show file tree

Hide file tree

Showing 22 changed files with 422 additions and 183 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,8 @@
 ## 22.7.1
 
 ### Additions and Improvements
+- Do not require a minimum block height when downloading headers or blocks [#3911](https://github.com/hyperledger/besu/pull/3911)
+- When on PoS the head can be only be updated by ForkchoiceUpdate [#3994](https://github.com/hyperledger/besu/pull/3994)
 - Version information available in metrics [#3997](https://github.com/hyperledger/besu/pull/3997)
 - Add TTD and DNS to Sepolia config [#4024](https://github.com/hyperledger/besu/pull/4024)
 - Return `type` with value `0x0` when serializing legacy transactions [#4027](https://github.com/hyperledger/besu/pull/4027)
@@ -18,7 +20,6 @@
 - Support `finalized` and `safe` as tags for the block parameter in RPC APIs [#3950](https://github.com/hyperledger/besu/pull/3950)
 - Added verification of payload attributes in ForkchoiceUpdated [#3837](https://github.com/hyperledger/besu/pull/3837)
 - Add support for Gray Glacier hardfork [#3961](https://github.com/hyperledger/besu/issues/3961)
-- Do not require a minimum block height when downloading headers or blocks [#3911](https://github.com/hyperledger/besu/pull/3911)
 
 ### Bug Fixes
 - alias engine-rpc-port parameter with the former rpc param name [#3958](https://github.com/hyperledger/besu/pull/3958)

diff --git a/...ge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/...ge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java
@@ -174,7 +174,7 @@ public PayloadIdentifier preparePayload(
     final Block emptyBlock =
         mergeBlockCreator.createBlock(Optional.of(Collections.emptyList()), random, timestamp);
 
-    Result result = executeBlock(emptyBlock);
+    Result result = validateBlock(emptyBlock);
     if (result.blockProcessingOutputs.isPresent()) {
       mergeContext.putPayloadById(payloadIdentifier, emptyBlock);
     } else {
@@ -193,7 +193,7 @@ public PayloadIdentifier preparePayload(
               if (throwable != null) {
                 LOG.warn("something went wrong creating block", throwable);
               } else {
-                final var resultBest = executeBlock(bestBlock);
+                final var resultBest = validateBlock(bestBlock);
                 if (resultBest.blockProcessingOutputs.isPresent()) {
                   mergeContext.putPayloadById(payloadIdentifier, bestBlock);
                 } else {
@@ -239,7 +239,7 @@ public Optional<BlockHeader> getOrSyncHeaderByHash(
   }
 
   @Override
-  public Result executeBlock(final Block block) {
+  public Result validateBlock(final Block block) {
 
     final var chain = protocolContext.getBlockchain();
     chain
@@ -254,11 +254,14 @@ public Result executeBlock(final Block block) {
             .getByBlockNumber(block.getHeader().getNumber())
             .getBlockValidator()
             .validateAndProcessBlock(
-                protocolContext, block, HeaderValidationMode.FULL, HeaderValidationMode.NONE);
+                protocolContext,
+                block,
+                HeaderValidationMode.FULL,
+                HeaderValidationMode.NONE,
+                false);
 
-    validationResult.blockProcessingOutputs.ifPresentOrElse(
-        result -> chain.appendBlock(block, result.receipts),
-        () ->
+    validationResult.errorMessage.ifPresent(
+        errMsg ->
             protocolSchedule
                 .getByBlockNumber(chain.getChainHeadBlockNumber())
                 .getBadBlocksManager()
@@ -267,6 +270,16 @@ public Result executeBlock(final Block block) {
     return validationResult;
   }
 
+  @Override
+  public Result rememberBlock(final Block block) {
+    debugLambda(LOG, "Remember block {}", block::toLogString);
+    final var chain = protocolContext.getBlockchain();
+    final var validationResult = validateBlock(block);
+    validationResult.blockProcessingOutputs.ifPresent(
+        result -> chain.storeBlock(block, result.receipts));
+    return validationResult;
+  }
+
   @Override
   public ForkchoiceResult updateForkChoice(
       final BlockHeader newHead,
@@ -284,8 +297,8 @@ public ForkchoiceResult updateForkChoice(
       return ForkchoiceResult.withFailure(
           INVALID, "new head timestamp not greater than parent", latestValid);
     }
-    // set the new head
-    blockchain.rewindToBlock(newHead.getHash());
+
+    setNewHead(blockchain, newHead);
 
     // set and persist the new finalized block if it is present
     newFinalized.ifPresent(
@@ -310,6 +323,44 @@ public ForkchoiceResult updateForkChoice(
     return ForkchoiceResult.withResult(newFinalized, Optional.of(newHead));
   }
 
+  private boolean setNewHead(final MutableBlockchain blockchain, final BlockHeader newHead) {
+
+    if (newHead.getHash().equals(blockchain.getChainHeadHash())) {
+      debugLambda(LOG, "Nothing to do new head {} is already chain head", newHead::toLogString);
+      return true;
+    }
+
+    if (newHead.getParentHash().equals(blockchain.getChainHeadHash())) {
+      debugLambda(
+          LOG,
+          "Forwarding chain head to the block {} saved from a previous newPayload invocation",
+          newHead::toLogString);
+      forwardWorldStateTo(newHead);
+      return blockchain.forwardToBlock(newHead);
+    }
+
+    debugLambda(LOG, "New head {} is a chain reorg, rewind chain head to it", newHead::toLogString);
+    return blockchain.rewindToBlock(newHead.getHash());
+  }
+
+  private void forwardWorldStateTo(final BlockHeader newHead) {
+    protocolContext
+        .getWorldStateArchive()
+        .getMutable(newHead.getStateRoot(), newHead.getHash())
+        .ifPresentOrElse(
+            mutableWorldState ->
+                debugLambda(
+                    LOG,
+                    "World state for state root hash {} and block hash {} persisted successfully",
+                    mutableWorldState::rootHash,
+                    newHead::getHash),
+            () ->
+                LOG.error(
+                    "Could not persist world for root hash {} and block hash {}",
+                    newHead.getStateRoot(),
+                    newHead.getHash()));
+  }
+
   @Override
   public boolean latestValidAncestorDescendsFromTerminal(final BlockHeader blockHeader) {
     if (blockHeader.getNumber() <= 1L) {

diff --git a/.../main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java b/.../main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java
@@ -35,7 +35,9 @@ PayloadIdentifier preparePayload(
       final Bytes32 random,
       final Address feeRecipient);
 
-  Result executeBlock(final Block block);
+  Result rememberBlock(final Block block);
+
+  Result validateBlock(final Block block);
 
   ForkchoiceResult updateForkChoice(
       final BlockHeader newHead,

diff --git a/...c/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java b/...c/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java
@@ -133,8 +133,13 @@ public PayloadIdentifier preparePayload(
   }
 
   @Override
-  public Result executeBlock(final Block block) {
-    return mergeCoordinator.executeBlock(block);
+  public Result rememberBlock(final Block block) {
+    return mergeCoordinator.rememberBlock(block);
+  }
+
+  @Override
+  public Result validateBlock(final Block block) {
+    return mergeCoordinator.validateBlock(block);
   }
 
   @Override

diff --git a/...rc/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/...rc/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java
@@ -128,18 +128,17 @@ public void coinbaseShouldMatchSuggestedFeeRecipient() {
 
   @Test
   public void childTimestampExceedsParentsFails() {
-
     BlockHeader terminalHeader = terminalPowBlock();
-    coordinator.executeBlock(new Block(terminalHeader, BlockBody.empty()));
+    sendNewPayloadAndForkchoiceUpdate(
+        new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO);
 
     BlockHeader parentHeader = nextBlockHeader(terminalHeader);
-
     Block parent = new Block(parentHeader, BlockBody.empty());
-    coordinator.executeBlock(parent);
+    sendNewPayloadAndForkchoiceUpdate(parent, Optional.empty(), terminalHeader.getHash());
 
     BlockHeader childHeader = nextBlockHeader(parentHeader, parentHeader.getTimestamp());
     Block child = new Block(childHeader, BlockBody.empty());
-    coordinator.executeBlock(child);
+    coordinator.rememberBlock(child);
 
     ForkchoiceResult result =
         coordinator.updateForkChoice(
@@ -161,42 +160,39 @@ public void childTimestampExceedsParentsFails() {
 
   @Test
   public void latestValidAncestorDescendsFromTerminal() {
-
     BlockHeader terminalHeader = terminalPowBlock();
-    coordinator.executeBlock(new Block(terminalHeader, BlockBody.empty()));
+    sendNewPayloadAndForkchoiceUpdate(
+        new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO);
 
     BlockHeader parentHeader = nextBlockHeader(terminalHeader);
-
     Block parent = new Block(parentHeader, BlockBody.empty());
-    coordinator.executeBlock(parent);
+    sendNewPayloadAndForkchoiceUpdate(parent, Optional.empty(), terminalHeader.getHash());
 
     BlockHeader childHeader = nextBlockHeader(parentHeader);
     Block child = new Block(childHeader, BlockBody.empty());
-    coordinator.executeBlock(child);
+    coordinator.validateBlock(child);
     assertThat(this.coordinator.latestValidAncestorDescendsFromTerminal(child.getHeader()))
         .isTrue();
   }
 
   @Test
   public void latestValidAncestorDescendsFromFinalizedBlock() {
-
     BlockHeader terminalHeader = terminalPowBlock();
-    coordinator.executeBlock(new Block(terminalHeader, BlockBody.empty()));
+    sendNewPayloadAndForkchoiceUpdate(
+        new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO);
 
     BlockHeader grandParentHeader = nextBlockHeader(terminalHeader);
-
     Block grandParent = new Block(grandParentHeader, BlockBody.empty());
-    coordinator.executeBlock(grandParent);
-    when(mergeContext.getFinalized()).thenReturn(Optional.of(grandParentHeader));
+    sendNewPayloadAndForkchoiceUpdate(grandParent, Optional.empty(), terminalHeader.getHash());
 
     BlockHeader parentHeader = nextBlockHeader(grandParentHeader);
-
     Block parent = new Block(parentHeader, BlockBody.empty());
-    coordinator.executeBlock(parent);
+    sendNewPayloadAndForkchoiceUpdate(
+        parent, Optional.of(grandParentHeader), grandParentHeader.getHash());
 
     BlockHeader childHeader = nextBlockHeader(parentHeader);
     Block child = new Block(childHeader, BlockBody.empty());
-    coordinator.executeBlock(child);
+    coordinator.validateBlock(child);
 
     assertThat(this.coordinator.latestValidAncestorDescendsFromTerminal(child.getHeader()))
         .isTrue();
@@ -205,25 +201,19 @@ public void latestValidAncestorDescendsFromFinalizedBlock() {
 
   @Test
   public void updateForkChoiceShouldPersistFirstFinalizedBlockHash() {
-
-    when(mergeContext.getFinalized()).thenReturn(Optional.empty());
-
     BlockHeader terminalHeader = terminalPowBlock();
-    coordinator.executeBlock(new Block(terminalHeader, BlockBody.empty()));
+    sendNewPayloadAndForkchoiceUpdate(
+        new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO);
 
     BlockHeader firstFinalizedHeader = nextBlockHeader(terminalHeader);
     Block firstFinalizedBlock = new Block(firstFinalizedHeader, BlockBody.empty());
-    coordinator.executeBlock(firstFinalizedBlock);
+    sendNewPayloadAndForkchoiceUpdate(
+        firstFinalizedBlock, Optional.empty(), terminalHeader.getHash());
 
     BlockHeader headBlockHeader = nextBlockHeader(firstFinalizedHeader);
     Block headBlock = new Block(headBlockHeader, BlockBody.empty());
-    coordinator.executeBlock(headBlock);
-
-    coordinator.updateForkChoice(
-        headBlockHeader,
-        firstFinalizedBlock.getHash(),
-        firstFinalizedBlock.getHash(),
-        Optional.empty());
+    sendNewPayloadAndForkchoiceUpdate(
+        headBlock, Optional.of(firstFinalizedHeader), firstFinalizedHeader.getHash());
 
     verify(blockchain).setFinalized(firstFinalizedBlock.getHash());
     verify(mergeContext).setFinalized(firstFinalizedHeader);
@@ -234,27 +224,23 @@ public void updateForkChoiceShouldPersistFirstFinalizedBlockHash() {
   @Test
   public void updateForkChoiceShouldPersistLastFinalizedBlockHash() {
     BlockHeader terminalHeader = terminalPowBlock();
-    coordinator.executeBlock(new Block(terminalHeader, BlockBody.empty()));
+    sendNewPayloadAndForkchoiceUpdate(
+        new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO);
 
     BlockHeader prevFinalizedHeader = nextBlockHeader(terminalHeader);
     Block prevFinalizedBlock = new Block(prevFinalizedHeader, BlockBody.empty());
-    coordinator.executeBlock(prevFinalizedBlock);
-
-    when(mergeContext.getFinalized()).thenReturn(Optional.of(prevFinalizedHeader));
+    sendNewPayloadAndForkchoiceUpdate(
+        prevFinalizedBlock, Optional.empty(), terminalHeader.getHash());
 
     BlockHeader lastFinalizedHeader = nextBlockHeader(prevFinalizedHeader);
     Block lastFinalizedBlock = new Block(lastFinalizedHeader, BlockBody.empty());
-    coordinator.executeBlock(lastFinalizedBlock);
+    sendNewPayloadAndForkchoiceUpdate(
+        lastFinalizedBlock, Optional.of(prevFinalizedHeader), prevFinalizedHeader.getHash());
 
     BlockHeader headBlockHeader = nextBlockHeader(lastFinalizedHeader);
     Block headBlock = new Block(headBlockHeader, BlockBody.empty());
-    coordinator.executeBlock(headBlock);
-
-    coordinator.updateForkChoice(
-        headBlockHeader,
-        lastFinalizedBlock.getHash(),
-        lastFinalizedBlock.getHash(),
-        Optional.empty());
+    sendNewPayloadAndForkchoiceUpdate(
+        headBlock, Optional.of(lastFinalizedHeader), lastFinalizedHeader.getHash());
 
     verify(blockchain).setFinalized(lastFinalizedBlock.getHash());
     verify(mergeContext).setFinalized(lastFinalizedHeader);
@@ -427,21 +413,23 @@ public void assertMergeAtGenesisSatisifiesTerminalPoW() {
   @Test
   public void invalidPayloadShouldReturnErrorAndUpdateForkchoiceState() {
     BlockHeader terminalHeader = terminalPowBlock();
-    coordinator.executeBlock(new Block(terminalHeader, BlockBody.empty()));
+    sendNewPayloadAndForkchoiceUpdate(
+        new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO);
 
     BlockHeader prevFinalizedHeader = nextBlockHeader(terminalHeader);
     Block prevFinalizedBlock = new Block(prevFinalizedHeader, BlockBody.empty());
-    coordinator.executeBlock(prevFinalizedBlock);
-
-    when(mergeContext.getFinalized()).thenReturn(Optional.of(prevFinalizedHeader));
+    sendNewPayloadAndForkchoiceUpdate(
+        prevFinalizedBlock, Optional.empty(), terminalHeader.getHash());
 
     BlockHeader lastFinalizedHeader = nextBlockHeader(prevFinalizedHeader);
     Block lastFinalizedBlock = new Block(lastFinalizedHeader, BlockBody.empty());
-    coordinator.executeBlock(lastFinalizedBlock);
+
+    sendNewPayloadAndForkchoiceUpdate(
+        lastFinalizedBlock, Optional.of(prevFinalizedHeader), prevFinalizedHeader.getHash());
 
     BlockHeader headBlockHeader = nextBlockHeader(lastFinalizedHeader);
     Block headBlock = new Block(headBlockHeader, BlockBody.empty());
-    coordinator.executeBlock(headBlock);
+    assertThat(coordinator.rememberBlock(headBlock).blockProcessingOutputs).isPresent();
 
     var res =
         coordinator.updateForkChoice(
@@ -461,6 +449,23 @@ public void invalidPayloadShouldReturnErrorAndUpdateForkchoiceState() {
     verify(mergeContext).setSafeBlock(lastFinalizedHeader);
   }
 
+  private void sendNewPayloadAndForkchoiceUpdate(
+      final Block block, final Optional<BlockHeader> finalizedHeader, final Hash safeHash) {
+
+    assertThat(coordinator.rememberBlock(block).blockProcessingOutputs).isPresent();
+    assertThat(
+            coordinator
+                .updateForkChoice(
+                    block.getHeader(),
+                    finalizedHeader.map(BlockHeader::getHash).orElse(Hash.ZERO),
+                    safeHash,
+                    Optional.empty())
+                .isValid())
+        .isTrue();
+
+    when(mergeContext.getFinalized()).thenReturn(finalizedHeader);
+  }
+
   private BlockHeader terminalPowBlock() {
     return headerGenerator
         .difficulty(Difficulty.MAX_VALUE)
@@ -472,6 +477,7 @@ private BlockHeader terminalPowBlock() {
                 genesisState.getBlock().getHeader().getBaseFee().orElse(Wei.of(0x3b9aca00)),
                 0,
                 15000000l))
+        .timestamp(1)
         .gasLimit(genesisState.getBlock().getHeader().getGasLimit())
         .stateRoot(genesisState.getBlock().getHeader().getStateRoot())
         .buildHeader();