fix(zebrad): keep active mempool through sync noise (#10929)

* fix(mempool): keep active mempool through sync noise

Keep an already-active mempool running when the legacy sync-status heuristic temporarily reports that Zebra is far from the tip.

The initial activation gate still waits for near-tip sync status, but once active, transient sync noise no longer drops mempool storage or cancels queued transaction verification. Chain tip resets reinitialize the active mempool directly so pending transactions can be requeued and reverified.

Also raise the close-to-tip threshold to accept recent sync batches averaging up to 100 blocks, matching the intended near-tip behavior covered by the updated tests.

(cherry picked from commit 78d1a5e)

* test(mempool): rename legacy sync wording

* docs(zebrad): clarify mempool activation state update

* fix(zebrad): revert global close-to-tip threshold; correct changelog

The mempool latch (keeping an already-active mempool alive through transient
far-from-tip sync status) is what closes the peer-toggle issue. The
MIN_DIST_FROM_TIP 20->101 raise additionally loosened every other consumer of
`is_close_to_tip()` — health readiness, sync gossip, block notify, and the
getblocktemplate mining RPC — which was unjustified, so revert it.

Also rewrite the CHANGELOG entry to describe only the latch (getblocktemplate is
not latched; it is still bounded by the network-tip-distance estimate), and drop
leftover "legacy" wording missed by the rename commit.

* refactor(zebrad): tidy mempool activation log and reset tip-action binding

Address review on #10929:

- Move the "activating mempool: Zebra is close to the tip" log out of
  `enable_at_tip` into the initial-activation arm. The helper is also called from
  the chain-tip reset path, where sync status may report far-from-tip, so the log
  was misleading there (Copilot).
- Bind the `Reset` action via a pattern match instead of `matches!`, dropping the
  `expect` and the `unwrap` in the reset branch (jvff).

* fix(zebrad): address mempool PR review comments

- Point the CHANGELOG entry at #10929 instead of the closed, unmerged #10926
  (gustavovalverde).
- Evaluate `is_caught_up_to_start()` once per `poll_ready` and pass it into
  `update_state()` instead of recomputing it there (also avoids a duplicate
  debug-activation log) (gustavovalverde).
- Clarify the `fully_notified` assertion message: it is a placeholder that is
  always None pending the regtest network-info TODO, not active-mempool-specific
  (jvff).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Dev Ojha <dojha@berkeley.edu>

Author: upbqdn

CHANGELOG.md

@@ -20,6 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ### Fixed
 
+- Keep the mempool active through transient sync-status noise. Once started, the
+  mempool is no longer cleared and its queued transaction verification is no longer
+  cancelled when a temporary signal (which lower-work forks or stale peers can
+  trigger) reports Zebra is far from the tip; initial activation still waits until
+  Zebra is near the chain tip
+  ([#10929](https://github.com/ZcashFoundation/zebra/pull/10929)).
 - Don't disconnect from peers that return empty `FindBlocks` or `FindHeaders`
   responses when the local node is at or near the chain tip
   ([#10732](https://github.com/ZcashFoundation/zebra/pull/10732))

zebrad/src/components/mempool.rs

@@ -308,7 +308,8 @@ impl Mempool {
 
         // Make sure `is_enabled` is accurate.
         // Otherwise, it is only updated in `poll_ready`, right before each service call.
-        service.update_state(None);
+        let is_caught_up_to_start = service.is_caught_up_to_start();
+        service.update_state(None, is_caught_up_to_start);
 
         (service, transaction_subscriber)
     }
@@ -341,48 +342,70 @@ impl Mempool {
         is_debug_enabled
     }
 
-    /// Update the mempool state (enabled / disabled) depending on how close to
-    /// the tip is the synchronization, including side effects to state changes.
+    /// Returns `true` if Zebra is caught up enough to start the mempool.
+    fn is_caught_up_to_start(&self) -> bool {
+        self.sync_status.is_close_to_tip() || self.is_enabled_by_debug()
+    }
+
+    /// Replaces the active state with a freshly-initialised [`ActiveState::Enabled`],
+    /// using `tip_action`'s best tip hash as the `last_seen_tip_hash`.
+    fn enable_at_tip(&mut self, tip_action: &TipAction) {
+        let (last_seen_tip_hash, _) = tip_action.best_tip_hash_and_height();
+
+        let tx_downloads = Box::pin(TxDownloads::new(
+            Timeout::new(self.outbound.clone(), TRANSACTION_DOWNLOAD_TIMEOUT),
+            Timeout::new(self.tx_verifier.clone(), TRANSACTION_VERIFY_TIMEOUT),
+            self.state.clone(),
+        ));
+        self.active_state = ActiveState::Enabled {
+            storage: storage::Storage::new(&self.config),
+            tx_downloads,
+            last_seen_tip_hash,
+        };
+    }
+
+    /// Activate the mempool once Zebra is close enough to the tip.
+    ///
+    /// Sync status only gates initial activation. Once the mempool is active,
+    /// this method does not disable it.
     ///
-    /// Accepts an optional [`TipAction`] for setting the `last_seen_tip_hash` field
-    /// when enabling the mempool state, it will not enable the mempool if this is None.
+    /// Accepts an optional [`TipAction`] for setting the `last_seen_tip_hash`
+    /// field when enabling the mempool state. It will not enable the mempool if
+    /// this is [`None`]. `is_caught_up_to_start` is supplied by the caller, which
+    /// already computes it, to avoid evaluating the sync-status predicate twice.
     ///
     /// Returns `true` if the state changed.
-    fn update_state(&mut self, tip_action: Option<&TipAction>) -> bool {
-        let is_close_to_tip = self.sync_status.is_close_to_tip() || self.is_enabled_by_debug();
-
-        match (is_close_to_tip, self.is_enabled(), tip_action) {
+    fn update_state(
+        &mut self,
+        tip_action: Option<&TipAction>,
+        is_caught_up_to_start: bool,
+    ) -> bool {
+        // TODO: revisit these state transitions when sync status can prove
+        // whether Zebra is behind the network tip.
+        match (is_caught_up_to_start, self.is_enabled(), tip_action) {
             // the active state is up to date, or there is no tip action to activate the mempool
             (false, false, _) | (true, true, _) | (true, false, None) => return false,
 
             // Enable state - there should be a chain tip when Zebra is close to the network tip
             (true, false, Some(tip_action)) => {
-                let (last_seen_tip_hash, tip_height) = tip_action.best_tip_hash_and_height();
-
-                info!(?tip_height, "activating mempool: Zebra is close to the tip");
-
-                let tx_downloads = Box::pin(TxDownloads::new(
-                    Timeout::new(self.outbound.clone(), TRANSACTION_DOWNLOAD_TIMEOUT),
-                    Timeout::new(self.tx_verifier.clone(), TRANSACTION_VERIFY_TIMEOUT),
-                    self.state.clone(),
-                ));
-                self.active_state = ActiveState::Enabled {
-                    storage: storage::Storage::new(&self.config),
-                    tx_downloads,
-                    last_seen_tip_hash,
-                };
-            }
-
-            // Disable state
-            (false, true, _) => {
                 info!(
-                    tip_height = ?self.latest_chain_tip.best_tip_height(),
-                    "deactivating mempool: Zebra is syncing lots of blocks"
+                    tip_height = ?tip_action.best_tip_height(),
+                    "activating mempool: Zebra is close to the tip"
                 );
 
-                // This drops the previous ActiveState::Enabled, cancelling its download tasks.
-                // We don't preserve the previous transactions, because we are syncing lots of blocks.
-                self.active_state = ActiveState::Disabled;
+                self.enable_at_tip(tip_action);
+            }
+
+            // TODO: only disable an already-active mempool when validated sync
+            // state proves Zebra is behind a higher-work chain that follows
+            // this node's consensus rules.
+            //
+            // The sync status can be triggered by lower-work forks,
+            // stale peers, or peers on incompatible consensus rules, so
+            // it is strong enough to delay initial activation but not to shut
+            // down a working mempool.
+            (false, true, _) => {
+                return false;
             }
         };
 
@@ -526,10 +549,14 @@ impl Service<Request> for Mempool {
         Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static>>;
 
     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
-        let tip_action = self.chain_tip_change.last_tip_change();
+        let is_caught_up_to_start = self.is_caught_up_to_start();
+        let should_check_tip = self.is_enabled() || is_caught_up_to_start;
+        let tip_action = should_check_tip
+            .then(|| self.chain_tip_change.last_tip_change())
+            .flatten();
 
         // TODO: Consider broadcasting a `MempoolChange` when the mempool is disabled.
-        let is_state_changed = self.update_state(tip_action.as_ref());
+        let is_state_changed = self.update_state(tip_action.as_ref(), is_caught_up_to_start);
 
         tracing::trace!(is_enabled = ?self.is_enabled(), ?is_state_changed, "started polling the mempool...");
 
@@ -545,9 +572,16 @@ impl Service<Request> for Mempool {
         //
         // But if the mempool was just freshly enabled,
         // skip resetting and removing mined transactions for this tip.
-        if !is_state_changed && matches!(tip_action, Some(TipAction::Reset { .. })) {
+        let reset_tip_action = match tip_action.as_ref() {
+            Some(reset_tip_action @ TipAction::Reset { .. }) if !is_state_changed => {
+                Some(reset_tip_action)
+            }
+            _ => None,
+        };
+
+        if let Some(reset_tip_action) = reset_tip_action {
             info!(
-                tip_height = ?tip_action.as_ref().unwrap().best_tip_height(),
+                tip_height = ?reset_tip_action.best_tip_height(),
                 "resetting mempool: switched best chain, skipped blocks, or activated network upgrade"
             );
 
@@ -563,7 +597,13 @@ impl Service<Request> for Mempool {
             std::mem::drop(previous_state);
 
             // Re-initialise an empty state.
-            self.update_state(tip_action.as_ref());
+            //
+            // This deliberately bypasses the initial-activation gate in `update_state()`:
+            // the mempool was already active when the reset arrived, and a
+            // far-from-tip sync status must not disable an already-active mempool
+            // (it can be triggered by lower-work forks, stale peers, or peers on
+            // incompatible consensus rules).
+            self.enable_at_tip(reset_tip_action);
 
             // Re-verify the transactions that were pending or valid at the previous tip.
             // This saves us the time and data needed to re-download them.

zebrad/src/components/mempool/tests.rs

@@ -47,11 +47,11 @@ impl Mempool {
         self.dummy_call().await;
     }
 
-    /// Disable the mempool by pretending the synchronization is far from the tip.
-    pub async fn disable(&mut self, recent_syncs: &mut RecentSyncLengths) {
+    /// Pretend the synchronization is far from the tip and poll the mempool.
+    async fn sync_far_from_tip(&mut self, recent_syncs: &mut RecentSyncLengths) {
         // Pretend we're far from the tip
         SyncStatus::sync_far_from_tip(recent_syncs);
-        // Make a dummy request to poll the mempool and make it disable itself
+        // Make a dummy request to poll the mempool.
         self.dummy_call().await;
     }
 

zebrad/src/components/mempool/tests/prop.rs

@@ -190,9 +190,9 @@ proptest! {
         })?;
     }
 
-    /// Test if the mempool storage is cleared if the syncer falls behind and starts to catch up.
+    /// Test if the mempool storage is kept if sync status falls behind.
     #[test]
-    fn storage_is_cleared_if_syncer_falls_behind(
+    fn storage_is_kept_if_sync_status_falls_behind(
         network in any::<Network>(),
         transaction in standard_verified_unmined_tx_strategy(),
     ) {
@@ -205,7 +205,7 @@ proptest! {
                 mut state_service,
                 mut tx_verifier,
                 mut recent_syncs,
-                mut chain_tip_sender,
+                _chain_tip_sender,
             ) = setup(&network);
 
             time::pause();
@@ -223,19 +223,15 @@ proptest! {
 
             prop_assert_eq!(mempool.storage().transaction_count(), 1);
 
-            // Simulate the synchronizer catching up to the network chain tip.
-            mempool.disable(&mut recent_syncs).await;
+            // Simulate sync status reporting a large gap. That signal
+            // can be caused by lower-work forks or incompatible peers, so it
+            // should not shut down an already-active mempool.
+            mempool.sync_far_from_tip(&mut recent_syncs).await;
 
-            // This time a call to `poll_ready` should clear the storage.
+            // This time a call to `poll_ready` should keep the storage.
             mempool.dummy_call().await;
 
-            // sends a new fake chain tip so that the mempool can be enabled
-            chain_tip_sender.set_finalized_tip(block1_chain_tip());
-
-            // Enable the mempool again so the storage can be accessed.
-            mempool.enable(&mut recent_syncs).await;
-
-            prop_assert_eq!(mempool.storage().transaction_count(), 0);
+            prop_assert_eq!(mempool.storage().transaction_count(), 1);
 
             peer_set.expect_no_requests().await?;
             state_service.expect_no_requests().await?;
@@ -254,14 +250,6 @@ fn genesis_chain_tip() -> Option<ChainTipBlock> {
         .ok()
 }
 
-fn block1_chain_tip() -> Option<ChainTipBlock> {
-    zebra_test::vectors::BLOCK_MAINNET_1_BYTES
-        .zcash_deserialize_into::<Arc<Block>>()
-        .map(CheckpointVerifiedBlock::from)
-        .map(ChainTipBlock::from)
-        .ok()
-}
-
 /// Create a new [`Mempool`] instance using mocked services.
 fn setup(
     network: &Network,