fix: apply review suggestions

Author: UdjinM6

src/evo/deterministicmns.cpp

@@ -1342,7 +1342,8 @@ bool CDeterministicMNManager::IsMigrationRequired() const
 {
     // Check if there are any legacy format diffs in the database
     // by looking for DB_LIST_DIFF_LEGACY entries
-    LOCK(::cs_main);
+
+    AssertLockHeld(::cs_main);
 
     std::unique_ptr<CDBIterator> pcursor{m_evoDb.GetRawDB().NewIterator()};
     auto start{std::make_tuple(DB_LIST_DIFF_LEGACY, uint256())};
@@ -1368,8 +1369,9 @@ bool CDeterministicMNManager::MigrateLegacyDiffs()
     // CRITICAL: This migration converts ALL stored CDeterministicMNListDiff entries
     // from legacy database key (DB_LIST_DIFF_LEGACY) to new key (DB_LIST_DIFF) format
 
+    AssertLockHeld(::cs_main);
+
     LogPrintf("CDeterministicMNManager::%s -- Starting migration to nVersion-first format\n", __func__);
-    LOCK(::cs_main);
 
     std::vector<uint256> keys_to_erase;
 

src/evo/deterministicmns.h

@@ -563,7 +563,9 @@ class CDeterministicMNListDiff
 
         for (size_t to_read = ReadCompactSize(s); to_read > 0; --to_read) {
             uint64_t internalId = ReadVarInt<Stream, VarIntMode::DEFAULT, uint64_t>(s);
-
+            // CDeterministicMNState can have newer fields but doesn't need migration logic here as
+            // CDeterministicMNStateDiff is always serialised using a bitmask and new fields have a new bit guide value,
+            // so we are good to continue. We do need migration logic to change field order though.
             if (isLegacyFormat) {
                 // Use legacy deserializer for old format
                 CDeterministicMNStateDiffLegacy legacyDiff(deserialize, s);
@@ -654,8 +656,8 @@ class CDeterministicMNManager
     void DoMaintenance() EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     // Migration support for nVersion-first CDeterministicMNStateDiff format
-    [[nodiscard]] bool IsMigrationRequired() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    [[nodiscard]] bool MigrateLegacyDiffs() EXCLUSIVE_LOCKS_REQUIRED(!cs);
+    [[nodiscard]] bool IsMigrationRequired() const EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main);
+    [[nodiscard]] bool MigrateLegacyDiffs() EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main);
 
 private:
     void CleanupCache(int nHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);

src/evo/dmnstate.h

@@ -246,9 +246,9 @@ class CDeterministicMNStateDiff
     {
         READWRITE(VARINT(obj.fields));
 
-        if ((obj.fields & Field_pubKeyOperator) || (obj.fields & Field_netInfo)) {
+        if (((obj.fields & Field_pubKeyOperator) || (obj.fields & Field_netInfo)) && !(obj.fields & Field_nVersion)) {
             // pubKeyOperator and netInfo need nVersion
-            assert(obj.fields & Field_nVersion);
+            throw std::ios_base::failure("Invalid data, nVersion unset when pubKeyOperator or netInfo set");
         }
 
         boost::hana::for_each(members, [&](auto&& member) {