fix: register NetGovernance unconditionally and tie SuperblockManager lifetime to govman

Two issues on top of the governance refactor:

1. With -disablegovernance, NetGovernance was no longer registered, so
   AlreadyHave/ProcessGetData stopped suppressing governance inv items.
   Always register the handler; AlreadyHave short-circuits to true when
   govman isn't loaded so peers can't grow m_requested_hash_time
   without a cleanup task.

2. SuperblockManager's m_loaded was sticky and the chain_helper-outlives-
   govman contract was undocumented and violated by the test teardown.
   SuperblockManager::Clear() now resets m_loaded; CGovernanceManager's
   destructor calls m_superblocks.Clear(); ~TestingSetup resets govman
   before DashChainstateSetupClose to match PrepareShutdown ordering.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: 2 people

src/governance/governance.cpp

@@ -79,6 +79,10 @@ CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, const Cha
 
 CGovernanceManager::~CGovernanceManager()
 {
+    // Reset the paired SuperblockManager so its loaded-flag and trigger map don't
+    // outlive us. chain_helper (its owner) must outlive govman -- see PrepareShutdown
+    // in init.cpp and the test teardown ordering.
+    m_superblocks.Clear();
     if (!is_loaded) return;
     m_db->Store(*this);
 }

src/governance/net_governance.cpp

@@ -205,6 +205,10 @@ bool NetGovernance::AlreadyHave(const CInv& inv)
     if (inv.type != MSG_GOVERNANCE_OBJECT && inv.type != MSG_GOVERNANCE_OBJECT_VOTE) {
         return false;
     }
+    // When governance isn't loaded (e.g. -disablegovernance), claim we already have
+    // the item so we don't fetch or track it. ConfirmInventoryRequest would otherwise
+    // grow m_requested_hash_time unbounded since CheckAndRemove never runs in that mode.
+    if (!m_gov_manager.IsValid()) return true;
     return !m_gov_manager.ConfirmInventoryRequest(inv);
 }
 

src/governance/superblock.cpp

@@ -521,6 +521,7 @@ void SuperblockManager::Clear()
 {
     LOCK(cs_sb);
     m_triggers.clear();
+    m_loaded = false;
 }
 
 std::vector<CSuperblock_sptr> SuperblockManager::GetActiveTriggers() const

src/governance/superblock.h

@@ -123,6 +123,11 @@ namespace governance {
  * AddTrigger() and notifies us via RemoveTrigger() when it erases the
  * underlying object from its store. Each entry holds a strong reference
  * to the underlying CGovernanceObject so we never look it up by hash.
+ *
+ * Lifetime: typically owned by CChainstateHelper, which outlives the
+ * paired CGovernanceManager. The govman drives m_loaded via SetLoaded()
+ * from LoadCache() and Clear()s us in its destructor, so IsValid() and
+ * the trigger map track the live govman's state.
  */
 class SuperblockManager
 {

src/init.cpp

@@ -2270,8 +2270,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             }
             return InitError(strprintf(_("Failed to clear governance cache at %s"), file_path));
         }
-        node.peerman->AddExtraHandler(std::make_unique<NetGovernance>(node.peerman.get(), *node.govman, *node.mn_sync, *node.netfulfilledman, *node.connman));
     }
+    // Always register NetGovernance so it can suppress governance inv items in AlreadyHave()
+    // even when -disablegovernance is set. The handler's ProcessMessage/Schedule paths
+    // early-return on !IsValid(), and AlreadyHave() short-circuits to true so we don't grow
+    // m_requested_hash_time without a cleanup task.
+    node.peerman->AddExtraHandler(std::make_unique<NetGovernance>(node.peerman.get(), *node.govman, *node.mn_sync, *node.netfulfilledman, *node.connman));
     node.peerman->AddExtraHandler(std::make_unique<SyncManager>(node.peerman.get(), *node.govman, *node.mn_sync, *node.connman, *node.netfulfilledman));
 
     // ********************************************************* Step 8: start indexers

src/test/util/setup_common.cpp

@@ -303,7 +303,6 @@ ChainTestingSetup::~ChainTestingSetup()
     StopScriptCheckWorkerThreads();
     GetMainSignals().FlushBackgroundCallbacks();
     GetMainSignals().UnregisterBackgroundSignalScheduler();
-    m_node.govman.reset();
     m_node.mn_sync.reset();
     m_node.chainman.reset();
     m_node.mempool.reset();
@@ -432,6 +431,11 @@ TestingSetup::~TestingSetup()
         m_node.connman->Stop();
     }
 
+    // govman holds a reference to chain_helper->superblocks, so it must be reset
+    // before DashChainstateSetupClose() destroys chain_helper (matches PrepareShutdown
+    // ordering in init.cpp).
+    m_node.govman.reset();
+
     // DashChainstateSetup() is called by LoadChainstate() internally but
     // winding them down is our responsibility
     DashChainstateSetupClose(m_node);