From 610927511ee307c75a08380c1d6c9a71fa28076b Mon Sep 17 00:00:00 2001
From: garyschulte <garyschulte@gmail.com>
Date: Thu, 7 Mar 2024 13:59:18 -0800
Subject: [PATCH] fix for txpool inSync listener to honor initial sync state
 (#6696)

Signed-off-by: garyschulte <garyschulte@gmail.com>
---
 .../transactions/TransactionPoolFactory.java  | 10 +++---
 .../TransactionPoolFactoryTest.java           | 36 +++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java
index 3a36cedfdae..8f35f81756f 100644
--- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java
+++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java
@@ -174,7 +174,7 @@ public void onInitialSyncRestart() {
     syncState.subscribeInSync(
         isInSync -> {
           if (isInSync != transactionPool.isEnabled()) {
-            if (isInSync) {
+            if (isInSync && syncState.isInitialSyncPhaseDone()) {
               LOG.info("Node is in sync, enabling transaction handling");
               enableTransactionHandling(
                   transactionTracker,
@@ -182,9 +182,11 @@ public void onInitialSyncRestart() {
                   transactionsMessageHandler,
                   pooledTransactionsMessageHandler);
             } else {
-              LOG.info("Node out of sync, disabling transaction handling");
-              disableTransactionHandling(
-                  transactionPool, transactionsMessageHandler, pooledTransactionsMessageHandler);
+              if (transactionPool.isEnabled()) {
+                LOG.info("Node out of sync, disabling transaction handling");
+                disableTransactionHandling(
+                    transactionPool, transactionsMessageHandler, pooledTransactionsMessageHandler);
+              }
             }
           }
         });
diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java
index 95d7f389f55..422f21a8705 100644
--- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java
+++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java
@@ -22,6 +22,7 @@
 import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -34,6 +35,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.core.PrivacyParameters;
+import org.hyperledger.besu.ethereum.core.Synchronizer;
 import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration;
 import org.hyperledger.besu.ethereum.eth.manager.EthContext;
 import org.hyperledger.besu.ethereum.eth.manager.EthMessages;
@@ -129,6 +131,37 @@ public void notRegisteredToBlockAddedEventBeforeInitialSyncIsDone() {
     assertThat(pool.isEnabled()).isFalse();
   }
 
+  @Test
+  public void assertPoolDisabledIfChainInSyncWithoutInitialSync() {
+    SyncState syncSpy = spy(new SyncState(blockchain, ethPeers, true, Optional.empty()));
+    ArgumentCaptor<Synchronizer.InSyncListener> chainSyncCaptor =
+        ArgumentCaptor.forClass(Synchronizer.InSyncListener.class);
+
+    setupInitialSyncPhase(syncSpy);
+    // verify that we are registered to the sync state
+    verify(syncSpy).subscribeInSync(chainSyncCaptor.capture());
+    // Retrieve the captured InSyncListener
+    Synchronizer.InSyncListener chainSyncListener = chainSyncCaptor.getValue();
+
+    // mock chain being in sync:
+    chainSyncListener.onInSyncStatusChange(true);
+
+    // assert pool is disabled if chain in sync and initial sync not done
+    assertThat(pool.isEnabled()).isFalse();
+
+    // mock initial sync done (avoid triggering initial sync listener)
+    when(syncSpy.isInitialSyncPhaseDone()).thenReturn(true);
+
+    // assert pool is enabled when chain in sync and initial sync done
+    chainSyncListener.onInSyncStatusChange(true);
+    assertThat(pool.isEnabled()).isTrue();
+
+    // assert pool is re-disabled when initial sync is incomplete but chain reaches head:
+    when(syncSpy.isInitialSyncPhaseDone()).thenCallRealMethod();
+    chainSyncListener.onInSyncStatusChange(false);
+    assertThat(pool.isEnabled()).isFalse();
+  }
+
   @Test
   public void registeredToBlockAddedEventAfterInitialSyncIsDone() {
     setupInitialSyncPhase(true);
@@ -231,7 +264,10 @@ public void incomingTransactionMessageHandlersRegisteredIfNoInitialSync() {
 
   private void setupInitialSyncPhase(final boolean hasInitialSyncPhase) {
     syncState = new SyncState(blockchain, ethPeers, hasInitialSyncPhase, Optional.empty());
+    setupInitialSyncPhase(syncState);
+  }
 
+  private void setupInitialSyncPhase(final SyncState syncState) {
     pool =
         TransactionPoolFactory.createTransactionPool(
             schedule,