refactor: multiple fixes, cleanups, improvements and refactorings

src/chainparams.cpp

@@ -12,6 +12,7 @@
 #include <util/ranges.h>
 #include <util/system.h>
 #include <util/underlying.h>
+#include <versionbits.h>
 #include <versionbitsinfo.h>
 
 #include <arith_uint256.h>
@@ -107,6 +108,8 @@ static CBlock FindDevNetGenesisBlock(const CBlock &prevBlock, const CAmount& rew
 
 bool CChainParams::UpdateMNActivationParam(int nBit, int height, int64_t timePast, bool fJustCheck) const
 {
+    assert(nBit < VERSIONBITS_NUM_BITS);
+
     for (int index = 0; index < Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++index) {
         if (consensus.vDeployments[index].bit == nBit) {
             auto& deployment = consensus.vDeployments[index];

src/evo/mnhftx.cpp

@@ -117,7 +117,7 @@ std::optional<uint8_t> extractEHFSignal(const CTransaction& tx)
     return mnhfTx.signal.versionBit;
 }
 
-static bool extractSignals(const CBlock& block, const CBlockIndex* const pindex, std::vector<uint8_t>& signals_to_process, BlockValidationState& state)
+static bool extractSignals(const CBlock& block, const CBlockIndex* const pindex, std::vector<uint8_t>& new_signals, BlockValidationState& state)
 {
     AssertLockHeld(cs_main);
 
@@ -139,14 +139,11 @@ static bool extractSignals(const CBlock& block, const CBlockIndex* const pindex,
         if (!GetTxPayload(tx, mnhfTx)) {
             return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-mnhf-tx-payload");
         }
-        signals_to_process.push_back(mnhfTx.signal.versionBit);
-    }
-
-    // Checking that there's no any duplicates...
-    std::sort(signals_to_process.begin(), signals_to_process.end());
-    const auto it = std::unique(signals_to_process.begin(), signals_to_process.end());
-    if (std::distance(signals_to_process.begin(), it) != signals_to_process.size()) {
-        return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-mnhf-duplicates-in-block");
+        const uint8_t bit = mnhfTx.signal.versionBit;
+        if (std::find(new_signals.begin(), new_signals.end(), bit) != new_signals.end()) {
+            return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-mnhf-duplicates-in-block");
+        }
+        new_signals.push_back(bit);
     }
 
     return true;
@@ -162,7 +159,7 @@ bool CMNHFManager::ProcessBlock(const CBlock& block, const CBlockIndex* const pi
         }
         Signals signals = GetSignalsStage(pindex->pprev);
         if (new_signals.empty()) {
-            if (!fJustCheck) {
+            if (fJustCheck) {
                 AddToCache(signals, pindex);
             }
             return true;
@@ -217,8 +214,6 @@ bool CMNHFManager::UndoBlock(const CBlock& block, const CBlockIndex* const pinde
 
     const Signals signals = GetFromCache(pindex);
     for (const auto& versionBit : excluded_signals) {
-        assert(versionBit < VERSIONBITS_NUM_BITS);
-
         LogPrintf("%s: exclude mnhf bit=%d block:%s number of known signals:%lld\n", __func__, versionBit, pindex->GetBlockHash().ToString(), signals.size());
         assert(signals.find(versionBit) != signals.end());
 
@@ -236,8 +231,6 @@ void CMNHFManager::UpdateChainParams(const CBlockIndex* const pindex, const CBlo
     for (const auto& signal: signals_old) {
         const uint8_t versionBit = signal.first;
 
-        assert(versionBit < VERSIONBITS_NUM_BITS);
-
         LogPrintf("%s: unload mnhf bit=%d block:%s number of known signals:%lld\n", __func__, versionBit, pindex->GetBlockHash().ToString(), signals_old.size());
 
         bool update_ret = Params().UpdateMNActivationParam(versionBit, 0, pindex->GetMedianTimePast(), /* fJustCheck= */ false);
@@ -249,8 +242,6 @@ void CMNHFManager::UpdateChainParams(const CBlockIndex* const pindex, const CBlo
         const uint8_t versionBit = signal.first;
         const int value = signal.second;
 
-        assert(versionBit < VERSIONBITS_NUM_BITS);
-
         LogPrintf("%s: load mnhf bit=%d block:%s number of known signals:%lld\n", __func__, versionBit, pindex->GetBlockHash().ToString(), signals.size());
 
         bool update_ret = Params().UpdateMNActivationParam(versionBit, value, pindex->GetMedianTimePast(), /* fJustCheck= */ false);

src/miner.cpp

@@ -175,7 +175,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
         LogPrintf("%s: CCreditPool is %s\n", __func__, creditPool.ToString());
         creditPoolDiff.emplace(std::move(creditPool), pindexPrev, chainparams.GetConsensus());
     }
-    std::unordered_map<uint8_t, int> signals = m_chainstate.m_mnhfManager.GetSignalsStage(pindexPrev);
+    std::unordered_map<uint8_t, int> signals = m_chainstate.GetMNHFSignalsStage();
     addPackageTxs(nPackagesSelected, nDescendantsUpdated, creditPoolDiff, signals);
 
     int64_t nTime1 = GetTimeMicros();

src/validation.cpp

@@ -2493,6 +2493,13 @@ CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(
     return CoinsCacheSizeState::OK;
 }
 
+std::unordered_map<uint8_t, int> CChainState::GetMNHFSignalsStage()
+{
+    const CBlockIndex* const tip = m_chain.Tip();
+    if (tip == nullptr) return {};
+    return this->m_mnhfManager.GetSignalsStage(tip);
+}
+
 bool CChainState::FlushStateToDisk(
     BlockValidationState &state,
     FlushStateMode mode,

src/validation.h

@@ -585,9 +585,7 @@ private:
     const std::unique_ptr<llmq::CChainLocksHandler>& m_clhandler;
     const std::unique_ptr<llmq::CInstantSendManager>& m_isman;
     const std::unique_ptr<llmq::CQuorumBlockProcessor>& m_quorum_block_processor;
-public:
     CMNHFManager& m_mnhfManager;
-private:
     CEvoDB& m_evoDb;
 
 public:
@@ -780,8 +778,10 @@ public:
         size_t max_coins_cache_size_bytes,
         size_t max_mempool_size_bytes) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
-    std::string ToString() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    /** Return list of MN EHF signals for current Tip() */
+    std::unordered_map<uint8_t, int> GetMNHFSignalsStage() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    std::string ToString() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 private:
     bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
     bool ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);

test/functional/feature_mnehf.py

@@ -248,16 +248,16 @@ class MnehfTest(DashTestFramework):
         ehf_tx_second = self.create_mnehf(28, pubkey)
         assert_equal(get_bip9_details(node, 'testdummy')['status'], 'defined')
 
-        self.log.info("Ehf with same bit signal should fail after 575 blocks but be accepted after 576 on regnet.")
-        self.log.info(f"Current progress is from {ehf_height} to {node.getblockcount()}")
-        self.slowly_generate_batch(576 - (node.getblockcount() - ehf_height))
+        self.log.info("Testing EHF signal with same bit")
+        self.log.info(f"Previous signal at height={ehf_height}, total blocks={node.getblockcount()}, should success at {ehf_hight + 576}")
+        self.slowly_generate_batch(576 - (node.getblockcount() - ehf_height) - 1)
         ehf_tx_sent = self.send_tx(ehf_tx_second)
-        self.log.info(f"ehf tx sent: {ehf_tx_sent}")
-        self.log.info(f"block: {node.getblock(node.getbestblockhash())}")
+        self.log.info("Mine block and ensure not mined yet...")
+        node.generate(1)
         self.ensure_tx_is_not_mined(ehf_tx_sent)
         node.generate(1)
         self.sync_all()
-        self.log.info(f"block: {node.getblock(node.getbestblockhash())}")
+        self.log.info("Mine one more block and ensure it is mined")
         block = node.getblock(node.getbestblockhash())
         assert ehf_tx_sent in block['tx']
 

test/functional/test_runner.py

@@ -121,10 +121,10 @@ BASE_SCRIPTS = [
     'feature_llmq_dkgerrors.py', # NOTE: needs dash_hash to pass
     'feature_dip4_coinbasemerkleroots.py', # NOTE: needs dash_hash to pass
     'feature_asset_locks.py', # NOTE: needs dash_hash to pass
+    'feature_mnehf.py', # NOTE: needs dash_hash to pass
     # vv Tests less than 60s vv
     'p2p_sendheaders.py', # NOTE: needs dash_hash to pass
     'p2p_sendheaders_compressed.py', # NOTE: needs dash_hash to pass
-    'feature_mnehf.py', # NOTE: needs dash_hash to pass
     'wallet_importmulti.py',
     'mempool_limit.py',
     'rpc_txoutproof.py',