add oldState to MergeStateHandler, move Pandas to Transition watcher

Signed-off-by: garyschulte <garyschulte@gmail.com>
hyperledger · Jul 29, 2022 · f6bc563 · f6bc563
1 parent 6289823
commit f6bc563
Show file tree

Hide file tree

Showing 10 changed files with 24 additions and 13 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.controller;
 
 import org.hyperledger.besu.config.GenesisConfigFile;
+import org.hyperledger.besu.consensus.merge.PandaPrinter;
 import org.hyperledger.besu.consensus.merge.PostMergeContext;
 import org.hyperledger.besu.consensus.merge.TransitionBackwardSyncContext;
 import org.hyperledger.besu.consensus.merge.TransitionContext;
@@ -180,7 +181,7 @@ private void initTransitionWatcher(
 
     PostMergeContext postMergeContext = protocolContext.getConsensusContext(PostMergeContext.class);
     postMergeContext.observeNewIsPostMergeState(
-        (isPoS, difficultyStoppedAt) -> {
+        (isPoS, priorState, difficultyStoppedAt) -> {
           if (isPoS) {
             // if we transitioned to post-merge, stop and disable any mining
             composedCoordinator.getPreMergeObject().disable();
@@ -190,6 +191,11 @@ private void initTransitionWatcher(
                 .getBlockchain()
                 .setBlockChoiceRule((newBlockHeader, currentBlockHeader) -> -1);
 
+            if (priorState.filter(prior -> !prior).isPresent()) {
+              // only print pandas if we had a prior merge state, and it was false
+              PandaPrinter.printOnFirstCrossing();
+            }
+
           } else if (composedCoordinator.isMiningBeforeMerge()) {
             // if our merge state is set to mine pre-merge and we are mining, start mining
             composedCoordinator.getPreMergeObject().enable();

diff --git a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java
@@ -23,6 +23,7 @@
 
 import org.hyperledger.besu.consensus.clique.CliqueContext;
 import org.hyperledger.besu.consensus.merge.MergeContext;
+import org.hyperledger.besu.consensus.merge.PandaPrinter;
 import org.hyperledger.besu.consensus.merge.PostMergeContext;
 import org.hyperledger.besu.consensus.merge.TransitionProtocolSchedule;
 import org.hyperledger.besu.consensus.merge.blockcreation.TransitionCoordinator;

diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java
@@ -105,12 +105,7 @@ public void setIsPostMerge(final Difficulty totalDifficulty) {
     if (oldState.isEmpty() || oldState.get() != newState) {
       newMergeStateCallbackSubscribers.forEach(
           newMergeStateCallback ->
-              newMergeStateCallback.mergeStateChanged(newState, Optional.of(totalDifficulty)));
-    }
-
-    // only print if we changed state from pre-TTD to post-TTD, not at startup/init:
-    if (newState && oldState.filter(old -> old != newState).isPresent()) {
-      PandaPrinter.printOnFirstCrossing();
+              newMergeStateCallback.mergeStateChanged(newState, oldState, Optional.of(totalDifficulty)));
     }
   }
 

diff --git a/...erge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBestPeerComparator.java b/...erge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBestPeerComparator.java
@@ -56,7 +56,7 @@ public TransitionBestPeerComparator(final Difficulty configuredTerminalTotalDiff
 
   @Override
   public void mergeStateChanged(
-      final boolean isPoS, final Optional<Difficulty> difficultyStoppedAt) {
+      final boolean isPoS, final Optional<Boolean> oldState, final Optional<Difficulty> difficultyStoppedAt) {
     if (isPoS && difficultyStoppedAt.isPresent()) {
       terminalTotalDifficulty.set(difficultyStoppedAt.get());
     }

diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PandaPrinterTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PandaPrinterTest.java
@@ -24,6 +24,11 @@
 
 public class PandaPrinterTest {
 
+  final MergeStateHandler fauxTransitionHandler = (isPoS, priorState, ttd) -> {
+    if (isPoS && priorState.filter(prior -> !prior).isPresent())
+      PandaPrinter.printOnFirstCrossing();
+  };
+
   @Test
   public void printsPanda() {
     PandaPrinter.resetForTesting();
@@ -35,6 +40,8 @@ public void printsPanda() {
   public void doesNotPrintAtInit() {
     PandaPrinter.resetForTesting();
     var mergeContext = new PostMergeContext(Difficulty.ONE);
+    mergeContext.observeNewIsPostMergeState(fauxTransitionHandler);
+
     assertThat(PandaPrinter.hasDisplayed()).isFalse();
     mergeContext.setIsPostMerge(Difficulty.ONE);
     assertThat(PandaPrinter.hasDisplayed()).isFalse();
@@ -44,6 +51,8 @@ public void doesNotPrintAtInit() {
   public void printsWhenCrossingOnly() {
     PandaPrinter.resetForTesting();
     var mergeContext = new PostMergeContext(Difficulty.ONE);
+    mergeContext.observeNewIsPostMergeState(fauxTransitionHandler);
+
     assertThat(PandaPrinter.hasDisplayed()).isFalse();
     mergeContext.setIsPostMerge(Difficulty.ZERO);
     assertThat(PandaPrinter.hasDisplayed()).isFalse();

diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java
@@ -173,7 +173,7 @@ private static class MergeStateChangeCollector implements MergeStateHandler {
 
     @Override
     public void mergeStateChanged(
-        final boolean isPoS, final Optional<Difficulty> difficultyStoppedAt) {
+        final boolean isPoS, final Optional<Boolean> oldState, final Optional<Difficulty> difficultyStoppedAt) {
       stateChanges.add(isPoS);
     }
 

diff --git a/.../src/test/java/org/hyperledger/besu/consensus/merge/TransitionBestPeerComparatorTest.java b/.../src/test/java/org/hyperledger/besu/consensus/merge/TransitionBestPeerComparatorTest.java
@@ -60,7 +60,7 @@ public void assertHandlesNewTTD() {
     assertThat(comparator.compare(a, b)).isEqualTo(-1);
 
     // update TTD with actual value
-    comparator.mergeStateChanged(true, Optional.of(Difficulty.of(5002)));
+    comparator.mergeStateChanged(true, Optional.empty(), Optional.of(Difficulty.of(5002)));
     assertThat(comparator.compare(a, b)).isEqualTo(1);
   }
 }
diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/consensus/merge/MergeStateHandler.java b/ethereum/eth/src/main/java/org/hyperledger/besu/consensus/merge/MergeStateHandler.java
@@ -22,5 +22,5 @@
 
 public interface MergeStateHandler {
 
-  void mergeStateChanged(final boolean isPoS, final Optional<Difficulty> difficultyStoppedAt);
+  void mergeStateChanged(final boolean isPoS, final Optional<Boolean> priorState, final Optional<Difficulty> difficultyStoppedAt);
 }
diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/MergePeerFilter.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/MergePeerFilter.java
@@ -88,7 +88,7 @@ public void onNewForkchoiceMessage(
 
   @Override
   public void mergeStateChanged(
-      final boolean isPoS, final Optional<Difficulty> difficultyStoppedAt) {
+      final boolean isPoS, final Optional<Boolean> oldState, final Optional<Difficulty> difficultyStoppedAt) {
     if (isPoS && difficultyStoppedAt.isPresent()) {
       LOG.debug("terminal difficulty set to {}", difficultyStoppedAt.get().getValue());
       long lockStamp = this.powTerminalDifficultyLock.writeLock();

diff --git a/...m/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java b/...m/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java
@@ -235,7 +235,7 @@ public void disconnectNewPoWPeers() {
       ethManager.processMessage(EthProtocol.ETH63, new DefaultMessage(stakePeer, stakePeerStatus));
 
       mergePeerFilter.mergeStateChanged(
-          true, Optional.of(blockchain.getChainHead().getTotalDifficulty()));
+          true, Optional.empty(), Optional.of(blockchain.getChainHead().getTotalDifficulty()));
       mergePeerFilter.onNewForkchoiceMessage(
           Hash.EMPTY, Optional.of(Hash.hash(Bytes.of(1))), Hash.EMPTY);
       mergePeerFilter.onNewForkchoiceMessage(