Merge dashpay#6434: fix: early EHF and buried EHF are indistinguish

4629bb9 fix: add missing cs_main annotation for ForceSignalDBUpdate (Konstantin Akimov)
05041a4 fix: force ehf signal db update (UdjinM6)
94d8032 fix: typo name of key (Konstantin Akimov)
9ceba88 style: clang suggestion (Konstantin Akimov)
c6bb9a5 perf: re-use evo data about signals between v20 and mn_rr as non-corrupted (Konstantin Akimov)
7a7c9f1 fix: early EHF and buried EHF are indistinguish (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  It seems as EHF signal will be mined before node is updated, this signal is lost and node can't activate hard-fork anymore.

  ## What was done?
  EHF signals doesn't expire anymore.
  To avoid full re-index key in database is changed.
  Client with enabled "pruned mode" will be required to do re-index.

  Alternate solution - revert this commit 4b046bb and introduce time-out for expiring EHF signals.

  ## How Has This Been Tested?
  Test on my local instance with testnet and mainnet.

  Testing on miner-1 on testnet is done.
  First start of miner took 50 seconds, 29 of them the node was re-scanning blockchain and looking for EHF transaction

  ## Breaking Changes
  It requires re-index for nodes with enabled pruning of blocks.

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK [4629bb9](dashpay@4629bb9)
  UdjinM6:
    utACK 4629bb9

Tree-SHA512: 189533da5726edbcf2d9cf0e9a3957a10ebc223c25fd88aec3aa9095ae2e7d955ea1f7a1384bc2c97a0cc06110c9e38845a8cafdbd56ff9637bb907ddc639850

Author: PastaPastaPasta

src/evo/mnhftx.cpp

@@ -24,6 +24,7 @@
 
 static const std::string MNEHF_REQUESTID_PREFIX = "mnhf";
 static const std::string DB_SIGNALS = "mnhf_s";
+static const std::string DB_SIGNALS_v2 = "mnhf_s2";
 
 uint256 MNHFTxPayload::GetRequestId() const
 {
@@ -57,34 +58,33 @@ CMNHFManager::Signals CMNHFManager::GetSignalsStage(const CBlockIndex* const pin
 {
     if (!DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) return {};
 
-    Signals signals = GetForBlock(pindexPrev);
+    Signals signals_tmp = GetForBlock(pindexPrev);
+
     if (pindexPrev == nullptr) return {};
     const int height = pindexPrev->nHeight + 1;
-    for (auto it = signals.begin(); it != signals.end(); ) {
-        bool found{false};
-        const auto signal_pindex = pindexPrev->GetAncestor(it->second);
+
+    Signals signals_ret;
+
+    for (auto signal : signals_tmp) {
+        bool expired{false};
+        const auto signal_pindex = pindexPrev->GetAncestor(signal.second);
         assert(signal_pindex != nullptr);
         const int64_t signal_time = signal_pindex->GetMedianTimePast();
         for (int index = 0; index < Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++index) {
             const auto& deployment = Params().GetConsensus().vDeployments[index];
-            if (deployment.bit != it->first) continue;
+            if (deployment.bit != signal.first) continue;
             if (signal_time < deployment.nStartTime) {
                 // new deployment is using the same bit as the old one
-                LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is expired at height=%d\n", it->first, it->second, height);
-                it = signals.erase(it);
-            } else {
-                ++it;
+                LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is expired at height=%d\n",
+                          signal.first, signal.second, height);
+                expired = true;
             }
-            found = true;
-            break;
         }
-        if (!found) {
-            // no deployment means we buried it and aren't using the same bit (yet)
-            LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is not known at height=%d\n", it->first, it->second, height);
-            it = signals.erase(it);
+        if (!expired) {
+            signals_ret.insert(signal);
         }
     }
-    return signals;
+    return signals_ret;
 }
 
 bool MNHFTx::Verify(const llmq::CQuorumManager& qman, const uint256& quorumHash, const uint256& requestId, const uint256& msgHash, TxValidationState& state) const
@@ -287,6 +287,9 @@ CMNHFManager::Signals CMNHFManager::GetForBlock(const CBlockIndex* pindex)
     const Consensus::Params& consensusParams{Params().GetConsensus()};
     while (!to_calculate.empty()) {
         const CBlockIndex* pindex_top{to_calculate.top()};
+        if (pindex_top->nHeight % 1000 == 0) {
+            LogPrintf("re-index EHF signals at block %d\n", pindex_top->nHeight);
+        }
         CBlock block;
         if (!ReadBlockFromDisk(block, pindex_top, consensusParams)) {
             throw std::runtime_error("failed-getehfforblock-read");
@@ -328,11 +331,19 @@ std::optional<CMNHFManager::Signals> CMNHFManager::GetFromCache(const CBlockInde
             return signals;
         }
     }
-    if (m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals)) {
+    if (m_evoDb.Read(std::make_pair(DB_SIGNALS_v2, blockHash), signals)) {
         LOCK(cs_cache);
         mnhfCache.insert(blockHash, signals);
         return signals;
     }
+    if (!DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR)) {
+        // before mn_rr activation we are safe
+        if (m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals)) {
+            LOCK(cs_cache);
+            mnhfCache.insert(blockHash, signals);
+            return signals;
+        }
+    }
     return std::nullopt;
 }
 
@@ -346,7 +357,7 @@ void CMNHFManager::AddToCache(const Signals& signals, const CBlockIndex* const p
     }
     if (!DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) return;
 
-    m_evoDb.Write(std::make_pair(DB_SIGNALS, blockHash), signals);
+    m_evoDb.Write(std::make_pair(DB_SIGNALS_v2, blockHash), signals);
 }
 
 void CMNHFManager::AddSignal(const CBlockIndex* const pindex, int bit)
@@ -364,6 +375,25 @@ void CMNHFManager::ConnectManagers(gsl::not_null<ChainstateManager*> chainman, g
     m_qman = qman;
 }
 
+bool CMNHFManager::ForceSignalDBUpdate()
+{
+    // force ehf signals db update
+    auto dbTx = m_evoDb.BeginTransaction();
+
+    const bool last_legacy = bls::bls_legacy_scheme.load();
+    bls::bls_legacy_scheme.store(false);
+    GetSignalsStage(m_chainman->ActiveChainstate().m_chain.Tip());
+    bls::bls_legacy_scheme.store(last_legacy);
+
+    dbTx->Commit();
+    // flush it to disk
+    if (!m_evoDb.CommitRootTransaction()) {
+        LogPrintf("CMNHFManager::%s -- failed to commit to evoDB\n", __func__);
+        return false;
+    }
+    return true;
+}
+
 std::string MNHFTx::ToString() const
 {
     return strprintf("MNHFTx(versionBit=%d, quorumHash=%s, sig=%s)",

src/evo/mnhftx.h

@@ -155,6 +155,8 @@ class CMNHFManager : public AbstractEHFManager
      */
     void DisconnectManagers() { m_chainman = nullptr; m_qman = nullptr; };
 
+    bool ForceSignalDBUpdate() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+
 private:
     void AddToCache(const Signals& signals, const CBlockIndex* const pindex);
 

src/init.cpp

@@ -2064,6 +2064,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                     strLoadError = _("Error upgrading evo database");
                     break;
                 }
+                if (!node.mnhf_manager->ForceSignalDBUpdate()) {
+                    strLoadError = _("Error upgrading evo database for EHF");
+                    break;
+                }
 
                 for (CChainState* chainstate : chainman.GetAll()) {
                     if (!is_coinsview_empty(chainstate)) {