Merge: Backward sync re-org below TTD fix (#3696)

Signed-off-by: garyschulte <garyschulte@gmail.com>

besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java

@@ -55,14 +55,13 @@ public class MergeBesuControllerBuilder extends BesuControllerBuilder {
       final MiningParameters miningParameters,
       final SyncState syncState,
       final EthProtocolManager ethProtocolManager) {
-
-    this.syncState.set(syncState);
-
-    return new MergeCoordinator(
-        protocolContext,
+    return createTransitionMiningCoordinator(
         protocolSchedule,
-        transactionPool.getPendingTransactions(),
+        protocolContext,
+        transactionPool,
         miningParameters,
+        syncState,
+        ethProtocolManager,
         new BackwardSyncContext(
             protocolContext,
             protocolSchedule,
@@ -74,6 +73,25 @@ public class MergeBesuControllerBuilder extends BesuControllerBuilder {
             storageProvider));
   }
 
+  protected MiningCoordinator createTransitionMiningCoordinator(
+      final ProtocolSchedule protocolSchedule,
+      final ProtocolContext protocolContext,
+      final TransactionPool transactionPool,
+      final MiningParameters miningParameters,
+      final SyncState syncState,
+      final EthProtocolManager ethProtocolManager,
+      final BackwardSyncContext backwardSyncContext) {
+
+    this.syncState.set(syncState);
+
+    return new MergeCoordinator(
+        protocolContext,
+        protocolSchedule,
+        transactionPool.getPendingTransactions(),
+        miningParameters,
+        backwardSyncContext);
+  }
+
   @Override
   protected ProtocolSchedule createProtocolSchedule() {
     return MergeProtocolSchedule.create(

besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java

@@ -16,6 +16,7 @@ package org.hyperledger.besu.controller;
 
 import org.hyperledger.besu.config.GenesisConfigFile;
 import org.hyperledger.besu.consensus.merge.PostMergeContext;
+import org.hyperledger.besu.consensus.merge.TransitionBackwardSyncContext;
 import org.hyperledger.besu.consensus.merge.TransitionContext;
 import org.hyperledger.besu.consensus.merge.TransitionProtocolSchedule;
 import org.hyperledger.besu.consensus.merge.blockcreation.TransitionCoordinator;
@@ -32,6 +33,8 @@ import org.hyperledger.besu.ethereum.core.PrivacyParameters;
 import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration;
 import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager;
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
+import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext;
+import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncLookupService;
 import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
@@ -79,29 +82,46 @@ public class TransitionBesuControllerBuilder extends BesuControllerBuilder {
       final EthProtocolManager ethProtocolManager) {
 
     // cast to transition schedule for explicit access to pre and post objects:
-    final TransitionProtocolSchedule tps = (TransitionProtocolSchedule) protocolSchedule;
+    final TransitionProtocolSchedule transitionProtocolSchedule =
+        (TransitionProtocolSchedule) protocolSchedule;
 
     // PoA consensus mines by default, get consensus-specific mining parameters for
     // TransitionCoordinator:
     MiningParameters transitionMiningParameters =
         preMergeBesuControllerBuilder.getMiningParameterOverrides(miningParameters);
 
+    // construct a transition backward sync context
+    BackwardSyncContext transitionBackwardsSyncContext =
+        new TransitionBackwardSyncContext(
+            protocolContext,
+            transitionProtocolSchedule,
+            metricsSystem,
+            ethProtocolManager.ethContext(),
+            syncState,
+            new BackwardSyncLookupService(
+                transitionProtocolSchedule,
+                ethProtocolManager.ethContext(),
+                metricsSystem,
+                protocolContext),
+            storageProvider);
+
     final TransitionCoordinator composedCoordinator =
         new TransitionCoordinator(
             preMergeBesuControllerBuilder.createMiningCoordinator(
-                tps.getPreMergeSchedule(),
+                transitionProtocolSchedule.getPreMergeSchedule(),
                 protocolContext,
                 transactionPool,
                 new MiningParameters.Builder(miningParameters).miningEnabled(false).build(),
                 syncState,
                 ethProtocolManager),
-            mergeBesuControllerBuilder.createMiningCoordinator(
-                tps.getPostMergeSchedule(),
+            mergeBesuControllerBuilder.createTransitionMiningCoordinator(
+                transitionProtocolSchedule,
                 protocolContext,
                 transactionPool,
                 transitionMiningParameters,
                 syncState,
-                ethProtocolManager));
+                ethProtocolManager,
+                transitionBackwardsSyncContext));
     initTransitionWatcher(protocolContext, composedCoordinator);
     return composedCoordinator;
   }
@@ -147,7 +167,7 @@ public class TransitionBesuControllerBuilder extends BesuControllerBuilder {
                 .setBlockChoiceRule((newBlockHeader, currentBlockHeader) -> -1);
 
           } else if (composedCoordinator.isMiningBeforeMerge()) {
-            // if our merge state is set to pre-merge and we are mining, start mining
+            // if our merge state is set to mine pre-merge and we are mining, start mining
             composedCoordinator.getPreMergeObject().enable();
             composedCoordinator.getPreMergeObject().start();
           }

besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java

@@ -15,7 +15,10 @@
 package org.hyperledger.besu.controller;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.consensus.clique.CliqueContext;
@@ -25,11 +28,16 @@ import org.hyperledger.besu.consensus.merge.blockcreation.TransitionCoordinator;
 import org.hyperledger.besu.crypto.NodeKeyUtils;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
+import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
+import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager;
 import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
+import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
+
+import java.util.Optional;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -45,28 +53,35 @@ import org.mockito.junit.MockitoJUnitRunner;
 @RunWith(MockitoJUnitRunner.class)
 public class TransitionControllerBuilderTest {
 
-  @Mock ProtocolSchedule protocolSchedule;
-  @Mock TransitionProtocolSchedule transitionProtocolSchedule;
+  @Mock ProtocolSchedule preMergeProtocolSchedule;
+  @Mock ProtocolSchedule postMergeProtocolSchedule;
   @Mock ProtocolContext protocolContext;
+  @Mock MutableBlockchain mockBlockchain;
   @Mock TransactionPool transactionPool;
   @Mock SyncState syncState;
   @Mock EthProtocolManager ethProtocolManager;
+  @Mock PostMergeContext mergeContext;
 
   @Spy CliqueBesuControllerBuilder cliqueBuilder = new CliqueBesuControllerBuilder();
   @Spy BesuControllerBuilder powBuilder = new MainnetBesuControllerBuilder();
   @Spy MergeBesuControllerBuilder postMergeBuilder = new MergeBesuControllerBuilder();
   @Spy MiningParameters miningParameters = new MiningParameters.Builder().build();
 
+  TransitionProtocolSchedule transitionProtocolSchedule;
+
   @Before
   public void setup() {
+    transitionProtocolSchedule =
+        spy(
+            new TransitionProtocolSchedule(
+                preMergeProtocolSchedule, postMergeProtocolSchedule, mergeContext));
     cliqueBuilder.nodeKey(NodeKeyUtils.generate());
-    when(protocolContext.getBlockchain()).thenReturn(mock(MutableBlockchain.class));
-    when(transitionProtocolSchedule.getPostMergeSchedule()).thenReturn(protocolSchedule);
-    when(transitionProtocolSchedule.getPreMergeSchedule()).thenReturn(protocolSchedule);
+    when(protocolContext.getBlockchain()).thenReturn(mockBlockchain);
+    when(transitionProtocolSchedule.getPostMergeSchedule()).thenReturn(postMergeProtocolSchedule);
+    when(transitionProtocolSchedule.getPreMergeSchedule()).thenReturn(preMergeProtocolSchedule);
     when(protocolContext.getConsensusContext(CliqueContext.class))
         .thenReturn(mock(CliqueContext.class));
-    when(protocolContext.getConsensusContext(PostMergeContext.class))
-        .thenReturn(mock(PostMergeContext.class));
+    when(protocolContext.getConsensusContext(PostMergeContext.class)).thenReturn(mergeContext);
   }
 
   @Test
@@ -90,6 +105,50 @@ public class TransitionControllerBuilderTest {
     assertThat(transCoordinator.isMiningBeforeMerge()).isTrue();
   }
 
+  @Test
+  public void assertPreMergeScheduleForNotPostMerge() {
+    var mockBlock = new BlockHeaderTestFixture().buildHeader();
+    var preMergeProtocolSpec = mock(ProtocolSpec.class);
+    when(mergeContext.isPostMerge()).thenReturn(Boolean.FALSE);
+    when(mergeContext.getFinalized()).thenReturn(Optional.empty());
+    when(preMergeProtocolSchedule.getByBlockNumber(anyLong())).thenReturn(preMergeProtocolSpec);
+    assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock))
+        .isEqualTo(preMergeProtocolSpec);
+  }
+
+  @Test
+  public void assertPostMergeScheduleForAnyBlockWhenPostMergeAndFinalized() {
+    var mockBlock = new BlockHeaderTestFixture().buildHeader();
+    var postMergeProtocolSpec = mock(ProtocolSpec.class);
+    when(mergeContext.getFinalized()).thenReturn(Optional.of(mockBlock));
+    when(postMergeProtocolSchedule.getByBlockNumber(anyLong())).thenReturn(postMergeProtocolSpec);
+    assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock))
+        .isEqualTo(postMergeProtocolSpec);
+  }
+
+  @Test
+  public void assertPreMergeScheduleForBelowTerminalBlockWhenPostMergeIfNotFinalized() {
+    var mockParentBlock = new BlockHeaderTestFixture().number(100).buildHeader();
+    var mockBlock =
+        new BlockHeaderTestFixture()
+            .number(101)
+            .difficulty(Difficulty.of(1L))
+            .parentHash(mockParentBlock.getHash())
+            .buildHeader();
+
+    var preMergeProtocolSpec = mock(ProtocolSpec.class);
+
+    when(mergeContext.getTerminalTotalDifficulty()).thenReturn(Difficulty.of(1337L));
+    when(mergeContext.isPostMerge()).thenReturn(Boolean.TRUE);
+    when(mergeContext.getFinalized()).thenReturn(Optional.empty());
+    when(mockBlockchain.getTotalDifficultyByHash(any()))
+        .thenReturn(Optional.of(Difficulty.of(1335L)));
+
+    when(preMergeProtocolSchedule.getByBlockNumber(anyLong())).thenReturn(preMergeProtocolSpec);
+    assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock))
+        .isEqualTo(preMergeProtocolSpec);
+  }
+
   TransitionCoordinator buildTransitionCoordinator(
       final BesuControllerBuilder preMerge, final MergeBesuControllerBuilder postMerge) {
     var builder = new TransitionBesuControllerBuilder(preMerge, postMerge);

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java

@@ -80,8 +80,9 @@ public class PostMergeContext implements MergeContext {
 
   @Override
   public void setIsPostMerge(final Difficulty totalDifficulty) {
-    if (isPostMerge.get().orElse(Boolean.FALSE)) {
-      // we never switch back to a pre-merge once we have transitioned post-TTD.
+    if (isPostMerge.get().orElse(Boolean.FALSE) && lastFinalized.get() != null) {
+      // if we have finalized, we never switch back to a pre-merge once we have transitioned
+      // post-TTD.
       return;
     }
     final boolean newState = terminalTotalDifficulty.get().lessOrEqualThan(totalDifficulty);

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.consensus.merge;
+
+import org.hyperledger.besu.ethereum.BlockValidator;
+import org.hyperledger.besu.ethereum.ProtocolContext;
+import org.hyperledger.besu.ethereum.core.Block;
+import org.hyperledger.besu.ethereum.eth.manager.EthContext;
+import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext;
+import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncLookupService;
+import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;
+import org.hyperledger.besu.ethereum.storage.StorageProvider;
+import org.hyperledger.besu.plugin.services.MetricsSystem;
+
+public class TransitionBackwardSyncContext extends BackwardSyncContext {
+
+  private final TransitionProtocolSchedule transitionProtocolSchedule;
+
+  public TransitionBackwardSyncContext(
+      final ProtocolContext protocolContext,
+      final TransitionProtocolSchedule transitionProtocolSchedule,
+      final MetricsSystem metricsSystem,
+      final EthContext ethContext,
+      final SyncState syncState,
+      final BackwardSyncLookupService backwardSyncLookupService,
+      final StorageProvider storageProvider) {
+    super(
+        protocolContext,
+        transitionProtocolSchedule,
+        metricsSystem,
+        ethContext,
+        syncState,
+        backwardSyncLookupService,
+        storageProvider);
+    this.transitionProtocolSchedule = transitionProtocolSchedule;
+  }
+
+  /**
+   * Choose the correct protocolSchedule and blockvalidator by block rather than number. This should
+   * be used in the merge transition, specifically when the chain has not yet finalized.
+   */
+  @Override
+  public BlockValidator getBlockValidatorForBlock(final Block block) {
+    return transitionProtocolSchedule
+        .getByBlockHeader(protocolContext, block.getHeader())
+        .getBlockValidator();
+  }
+}

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java

@@ -14,6 +14,10 @@
  */
 package org.hyperledger.besu.consensus.merge;
 
+import org.hyperledger.besu.ethereum.ProtocolContext;
+import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
+import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.core.TransactionFilter;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
@@ -32,6 +36,13 @@ public class TransitionProtocolSchedule extends TransitionUtils<ProtocolSchedule
     super(preMergeProtocolSchedule, postMergeProtocolSchedule);
   }
 
+  public TransitionProtocolSchedule(
+      final ProtocolSchedule preMergeProtocolSchedule,
+      final ProtocolSchedule postMergeProtocolSchedule,
+      final MergeContext mergeContext) {
+    super(preMergeProtocolSchedule, postMergeProtocolSchedule, mergeContext);
+  }
+
   public ProtocolSchedule getPreMergeSchedule() {
     return getPreMergeObject();
   }
@@ -40,6 +51,33 @@ public class TransitionProtocolSchedule extends TransitionUtils<ProtocolSchedule
     return getPostMergeObject();
   }
 
+  public ProtocolSpec getByBlockHeader(
+      final ProtocolContext protocolContext, final BlockHeader blockHeader) {
+    // if we do not have a finalized block we might return pre or post merge protocol schedule:
+    if (mergeContext.getFinalized().isEmpty()) {
+
+      // if head is not post-merge, return pre-merge schedule:
+      if (!mergeContext.isPostMerge()) {
+        return getPreMergeSchedule().getByBlockNumber(blockHeader.getNumber());
+      }
+
+      // otherwise check to see if this block represents a re-org below TTD:
+      MutableBlockchain blockchain = protocolContext.getBlockchain();
+      Difficulty parentDifficulty =
+          blockchain.getTotalDifficultyByHash(blockHeader.getParentHash()).orElseThrow();
+      Difficulty thisDifficulty = parentDifficulty.add(blockHeader.getDifficulty());
+      Difficulty terminalDifficulty = mergeContext.getTerminalTotalDifficulty();
+
+      // if this block is pre-merge
+      if (thisDifficulty.lessOrEqualThan(terminalDifficulty)
+          || TransitionUtils.isTerminalProofOfWorkBlock(blockHeader, protocolContext)) {
+        return getPreMergeSchedule().getByBlockNumber(blockHeader.getNumber());
+      }
+    }
+    // else return post-merge schedule
+    return getPostMergeSchedule().getByBlockNumber(blockHeader.getNumber());
+  }
+
   @Override
   public ProtocolSpec getByBlockNumber(final long number) {
     return dispatchFunctionAccordingToMergeState(

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java

@@ -30,14 +30,22 @@ import org.slf4j.LoggerFactory;
 public abstract class TransitionUtils<SwitchingObject> {
   private static final Logger LOG = LoggerFactory.getLogger(TransitionUtils.class);
 
-  private final MergeContext mergeContext = PostMergeContext.get();
+  protected final MergeContext mergeContext;
   private final SwitchingObject preMergeObject;
   private final SwitchingObject postMergeObject;
 
   public TransitionUtils(
       final SwitchingObject preMergeObject, final SwitchingObject postMergeObject) {
+    this(preMergeObject, postMergeObject, PostMergeContext.get());
+  }
+
+  public TransitionUtils(
+      final SwitchingObject preMergeObject,
+      final SwitchingObject postMergeObject,
+      final MergeContext mergeContext) {
     this.preMergeObject = preMergeObject;
     this.postMergeObject = postMergeObject;
+    this.mergeContext = mergeContext;
   }
 
   protected void dispatchConsumerAccordingToMergeState(final Consumer<SwitchingObject> consumer) {

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java

@@ -46,7 +46,7 @@ public class BackwardSyncContext {
   public static final int BATCH_SIZE = 200;
   private static final int MAX_RETRIES = 100;
 
-  private final ProtocolContext protocolContext;
+  protected final ProtocolContext protocolContext;
   private final ProtocolSchedule protocolSchedule;
   private final EthContext ethContext;
   private final MetricsSystem metricsSystem;
@@ -257,6 +257,10 @@ public class BackwardSyncContext {
     return protocolSchedule.getByBlockNumber(blockNumber).getBlockValidator();
   }
 
+  public BlockValidator getBlockValidatorForBlock(final Block block) {
+    return getBlockValidator(block.getHeader().getNumber());
+  }
+
   public Optional<BackwardChain> findCorrectChainFromPivot(final long number) {
     return Optional.ofNullable(backwardChainMap.get(number));
   }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhase.java

@@ -170,7 +170,7 @@ public class ForwardSyncPhase extends BackwardSyncTask {
     debugLambda(LOG, "Going to validate block {}", () -> block.getHeader().getHash().toHexString());
     var optResult =
         context
-            .getBlockValidator(block.getHeader().getNumber())
+            .getBlockValidatorForBlock(block)
             .validateAndProcessBlock(
                 context.getProtocolContext(),
                 block,

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java

@@ -20,7 +20,6 @@ package org.hyperledger.besu.ethereum.eth.sync.backwardsync;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryBlockchain;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -114,7 +113,9 @@ public class ForwardSyncPhaseTest {
     EthContext ethContext = ethProtocolManager.ethContext();
     when(context.getEthContext()).thenReturn(ethContext);
 
-    when(context.getBlockValidator(anyLong()).validateAndProcessBlock(any(), any(), any(), any()))
+    when(context
+            .getBlockValidatorForBlock(any())
+            .validateAndProcessBlock(any(), any(), any(), any()))
         .thenAnswer(
             invocation -> {
               final Object[] arguments = invocation.getArguments();