Merge dashpay#6626: refactor: reduce references to masternode service, cleanup index code, consolidate ProTx versioning and make it friendlier to upgrades

86bec48 refactor: rearrange checks in ProTx RPC wrapper to bail out faster (Kittywhiskers Van Gogh)
14b7cf3 refactor: rename CPro*Tx::GetVersion() to GetMaxVersion(), increase use (Kittywhiskers Van Gogh)
714fb3a refactor: consolidate ProTx versioning, make logic friendly to upgrades (Kittywhiskers Van Gogh)
2f9d65d refactor: cleanup index code, style updates, deduplication (Kittywhiskers Van Gogh)
eaca912 refactor: defer resolving masternode service in CoinJoin code (Kittywhiskers Van Gogh)
e722272 chore: use ProTx hash to identify masternodes in logs when possible (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependency for dashpay#6627

  * As [dash#6627](dashpay#6627) aims to abstract away masternode network information (so far stored as a `CService`) behind an interface in preparation for supporting multiple addresses of different types, it still needs to be exposed to networking logic that actively relies on the "primary" address (i.e. the address used by masternodes to connect to each other). This is done through `MnNetInfo::GetPrimary()`.

    To reduce the `GetPrimary()` invocations needed to a minimum in [dash#6627](dashpay#6627), debug log messages have been changed to identify masternodes using the ProTx hash instead of the primary address and `CPendingDsaRequest` has been refactored to delay resolving the primary address, tracking the ProTx hash instead.

  * Currently, valid versions of `CPro*Tx` are expressed as `CPro*Tx::*_VERSION`. This isn't necessary as the `*_VERSION` (currently `{BASIC,LEGACY}_BLS_VERSION`) are the same for all `CPro*Tx`es, so they have been consolidated into a namespaced enum `ProTxVersion`.

    * Additionally, some checks in the codebase assume that are only two versions ([source](https://github.com/dashpay/dash/blob/bcd14b05cec7d94986f114ca17bbdadbee701d9b/src/evo/providertx.cpp#L20), [source](https://github.com/dashpay/dash/blob/bcd14b05cec7d94986f114ca17bbdadbee701d9b/src/evo/providertx.h#L150)), which isn't conducive to adding new versions. Those checks have been modified to allow higher versions.

    * `CPro*Tx::GetVersion()` has been renamed to `CPro*Tx::GetMaxVersion()` as its actually used to check what is the highest _allowed_ version given a set of consensus rules. `GetMaxVersion()` has not been consolidated like `ProTxVersion` as any changes to `addr` (as intended in [dash#6627](dashpay#6627)) would only affect `CProRegTx` and `CProUpServTx` (i.e. the remaining ProTx transactions would not be getting a version upgrade).

  * In later PRs, the change to a multiple-address capable implementation is hidden away using a `shared_ptr` to the interface class. To decide _what_ implementation to use, we need to set the version of `CPro*Tx`, as early as possible so that the version can be used to determine the applicable implementation. This is easy enough to do in serialization code but requires additional care when constructing a `CPro*Tx` in-place as it will be initialized with an empty `shared_ptr` and the applicable implementation needs to be explicitly set.

    To avoid problems in those later PRs, some instances of  `CPro*Tx` construction have been reshuffled to set `nVersion` as early as possible.

  ## Breaking Changes

  None expected.

  ## Checklist

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

ACKs for top commit:
  UdjinM6:
    utACK 86bec48
  knst:
    utACK 86bec48

Tree-SHA512: 3439ea4c51bed09b6147bb0bc26580cbb960ba1856855827d1db01e5303d6d9dce654e87b2982794aa6349a560e73b4b3fd99ca3bb7402e1b95f24a852657ffb

Author: PastaPastaPasta

src/coinjoin/client.cpp

@@ -106,10 +106,9 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CConnm
         // if the queue is ready, submit if we can
         if (dsq.fReady &&
             m_walletman.ForAnyCJClientMan([this, &connman, &dmn](std::unique_ptr<CCoinJoinClientManager>& clientman) {
-                return clientman->TrySubmitDenominate(dmn->pdmnState->addr, connman);
+                return clientman->TrySubmitDenominate(dmn->proTxHash, connman);
             })) {
-            LogPrint(BCLog::COINJOIN, "DSQUEUE -- CoinJoin queue (%s) is ready on masternode %s\n", dsq.ToString(),
-                     dmn->pdmnState->addr.ToStringAddrPort());
+            LogPrint(BCLog::COINJOIN, "DSQUEUE -- CoinJoin queue is ready, masternode=%s, queue=%s\n", dmn->proTxHash.ToString(), dsq.ToString());
             return {};
         } else {
             int64_t nLastDsq = m_mn_metaman.GetMetaInfo(dmn->proTxHash)->GetLastDsq();
@@ -125,8 +124,7 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CConnm
 
             m_mn_metaman.AllowMixing(dmn->proTxHash);
 
-            LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue (%s) from masternode %s\n", dsq.ToString(),
-                     dmn->pdmnState->addr.ToStringAddrPort());
+            LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue, masternode=%s, queue=%s\n", dmn->proTxHash.ToString(), dsq.ToString());
 
             m_walletman.ForAnyCJClientMan([&dsq](const std::unique_ptr<CCoinJoinClientManager>& clientman) {
                 return clientman->MarkAlreadyJoinedQueueAsTried(dsq);
@@ -186,23 +184,15 @@ void CCoinJoinClientSession::ProcessMessage(CNode& peer, CChainState& active_cha
     if (!CCoinJoinClientOptions::IsEnabled()) return;
     if (!m_mn_sync.IsBlockchainSynced()) return;
 
-    if (msg_type == NetMsgType::DSSTATUSUPDATE) {
-        if (!mixingMasternode) return;
-        if (mixingMasternode->pdmnState->addr != peer.addr) {
-            return;
-        }
+    if (!mixingMasternode) return;
+    if (mixingMasternode->pdmnState->addr != peer.addr) return;
 
+    if (msg_type == NetMsgType::DSSTATUSUPDATE) {
         CCoinJoinStatusUpdate psssup;
         vRecv >> psssup;
 
         ProcessPoolStateUpdate(psssup);
-
     } else if (msg_type == NetMsgType::DSFINALTX) {
-        if (!mixingMasternode) return;
-        if (mixingMasternode->pdmnState->addr != peer.addr) {
-            return;
-        }
-
         int nMsgSessionID;
         vRecv >> nMsgSessionID;
         CTransaction txNew(deserialize, vRecv);
@@ -216,15 +206,7 @@ void CCoinJoinClientSession::ProcessMessage(CNode& peer, CChainState& active_cha
 
         // check to see if input is spent already? (and probably not confirmed)
         SignFinalTransaction(peer, active_chainstate, connman, mempool, txNew);
-
     } else if (msg_type == NetMsgType::DSCOMPLETE) {
-        if (!mixingMasternode) return;
-        if (mixingMasternode->pdmnState->addr != peer.addr) {
-            WalletCJLogPrint(m_wallet, "DSCOMPLETE -- message doesn't match current Masternode: infoMixingMasternode=%s  addr=%s\n",
-                             mixingMasternode->pdmnState->addr.ToStringAddrPort(), peer.addr.ToStringAddrPort());
-            return;
-        }
-
         int nMsgSessionID;
         PoolMessage nMsgMessageID;
         vRecv >> nMsgSessionID >> nMsgMessageID;
@@ -1126,22 +1108,19 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized,
 
         if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->addr)) {
             WalletCJLogPrint(m_wallet, /* Continued */
-                             "CCoinJoinClientSession::JoinExistingQueue -- skipping masternode connection, addr=%s\n",
-                             dmn->pdmnState->addr.ToStringAddrPort());
+                             "CCoinJoinClientSession::JoinExistingQueue -- skipping connection, masternode=%s\n", dmn->proTxHash.ToString());
             continue;
         }
 
         nSessionDenom = dsq.nDenom;
         mixingMasternode = dmn;
-        pendingDsaRequest = CPendingDsaRequest(dmn->pdmnState->addr, CCoinJoinAccept(nSessionDenom, txMyCollateral));
+        pendingDsaRequest = CPendingDsaRequest(dmn->proTxHash, CCoinJoinAccept(nSessionDenom, txMyCollateral));
         connman.AddPendingMasternode(dmn->proTxHash);
         SetState(POOL_STATE_QUEUE);
         nTimeLastSuccessfulStep = GetTime();
         WalletCJLogPrint(m_wallet, /* Continued */
-                         "CCoinJoinClientSession::JoinExistingQueue -- pending connection (from queue): nSessionDenom: "
-                         "%d (%s), addr=%s\n",
-                         nSessionDenom, CoinJoin::DenominationToString(nSessionDenom),
-                         dmn->pdmnState->addr.ToStringAddrPort());
+                         "CCoinJoinClientSession::JoinExistingQueue -- pending connection, masternode=%s, nSessionDenom=%d (%s)\n",
+                         dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom));
         strAutoDenomResult = _("Trying to connect…");
         return true;
     }
@@ -1192,23 +1171,22 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon
         int64_t nDsqThreshold = m_mn_metaman.GetDsqThreshold(dmn->proTxHash, nMnCount);
         if (nLastDsq != 0 && nDsqThreshold > m_mn_metaman.GetDsqCount()) {
             WalletCJLogPrint(m_wallet, /* Continued */
-                             "CCoinJoinClientSession::StartNewQueue -- Too early to mix on this masternode!" /* Continued */
-                             " masternode=%s  addr=%s  nLastDsq=%d  nDsqThreshold=%d  nDsqCount=%d\n",
-                             dmn->proTxHash.ToString(), dmn->pdmnState->addr.ToStringAddrPort(), nLastDsq,
-                             nDsqThreshold, m_mn_metaman.GetDsqCount());
+                             "CCoinJoinClientSession::StartNewQueue -- too early to mix with node," /* Continued */
+                             " masternode=%s, nLastDsq=%d, nDsqThreshold=%d, nDsqCount=%d\n",
+                             dmn->proTxHash.ToString(), nLastDsq, nDsqThreshold, m_mn_metaman.GetDsqCount());
             nTries++;
             continue;
         }
 
         if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->addr)) {
-            WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- skipping masternode connection, addr=%s\n",
-                             dmn->pdmnState->addr.ToStringAddrPort());
+            WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- skipping connection, masternode=%s\n",
+                             dmn->proTxHash.ToString());
             nTries++;
             continue;
         }
 
-        WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- attempt %d connection to Masternode %s\n",
-                         nTries, dmn->pdmnState->addr.ToStringAddrPort());
+        WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- attempting connection, masternode=%s, tries=%s\n",
+                         dmn->proTxHash.ToString(), nTries);
 
         // try to get a single random denom out of setAmounts
         while (nSessionDenom == 0) {
@@ -1221,12 +1199,12 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon
 
         mixingMasternode = dmn;
         connman.AddPendingMasternode(dmn->proTxHash);
-        pendingDsaRequest = CPendingDsaRequest(dmn->pdmnState->addr, CCoinJoinAccept(nSessionDenom, txMyCollateral));
+        pendingDsaRequest = CPendingDsaRequest(dmn->proTxHash, CCoinJoinAccept(nSessionDenom, txMyCollateral));
         SetState(POOL_STATE_QUEUE);
         nTimeLastSuccessfulStep = GetTime();
         WalletCJLogPrint( /* Continued */
-            m_wallet, "CCoinJoinClientSession::StartNewQueue -- pending connection, nSessionDenom: %d (%s), addr=%s\n",
-            nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), dmn->pdmnState->addr.ToStringAddrPort());
+            m_wallet, "CCoinJoinClientSession::StartNewQueue -- pending connection, masternode=%s, nSessionDenom=%d (%s)\n",
+            dmn->proTxHash.ToString(), nSessionDenom, CoinJoin::DenominationToString(nSessionDenom));
         strAutoDenomResult = _("Trying to connect…");
         return true;
     }
@@ -1238,7 +1216,17 @@ bool CCoinJoinClientSession::ProcessPendingDsaRequest(CConnman& connman)
 {
     if (!pendingDsaRequest) return false;
 
-    bool fDone = connman.ForNode(pendingDsaRequest.GetAddr(), [this, &connman](CNode* pnode) {
+    CService mn_addr;
+    if (auto dmn = m_dmnman.GetListAtChainTip().GetMN(pendingDsaRequest.GetProTxHash())) {
+        mn_addr = Assert(dmn->pdmnState)->addr;
+    } else {
+        WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- cannot find address to connect, masternode=%s\n", __func__,
+            pendingDsaRequest.GetProTxHash().ToString());
+        WITH_LOCK(cs_coinjoin, SetNull());
+        return false;
+    }
+
+    bool fDone = connman.ForNode(mn_addr, [this, &connman](CNode* pnode) {
         WalletCJLogPrint(m_wallet, "-- processing dsa queue for addr=%s\n", pnode->addr.ToStringAddrPort());
         nTimeLastSuccessfulStep = GetTime();
         CNetMsgMaker msgMaker(pnode->GetCommonVersion());
@@ -1249,8 +1237,8 @@ bool CCoinJoinClientSession::ProcessPendingDsaRequest(CConnman& connman)
     if (fDone) {
         pendingDsaRequest = CPendingDsaRequest();
     } else if (pendingDsaRequest.IsExpired()) {
-        WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- failed to connect to %s\n", __func__,
-                         pendingDsaRequest.GetAddr().ToStringAddrPort());
+        WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- failed to connect, masternode=%s\n", __func__,
+                         pendingDsaRequest.GetProTxHash().ToString());
         WITH_LOCK(cs_coinjoin, SetNull());
     }
 
@@ -1268,13 +1256,13 @@ void CCoinJoinClientManager::ProcessPendingDsaRequest(CConnman& connman)
     }
 }
 
-bool CCoinJoinClientManager::TrySubmitDenominate(const CService& mnAddr, CConnman& connman)
+bool CCoinJoinClientManager::TrySubmitDenominate(const uint256& proTxHash, CConnman& connman)
 {
     AssertLockNotHeld(cs_deqsessions);
     LOCK(cs_deqsessions);
     for (auto& session : deqSessions) {
         CDeterministicMNCPtr mnMixing;
-        if (session.GetMixingMasternodeInfo(mnMixing) && mnMixing->pdmnState->addr == mnAddr && session.GetState() == POOL_STATE_QUEUE) {
+        if (session.GetMixingMasternodeInfo(mnMixing) && mnMixing->proTxHash == proTxHash && session.GetState() == POOL_STATE_QUEUE) {
             session.SubmitDenominate(connman);
             return true;
         }

src/coinjoin/client.h

@@ -39,27 +39,27 @@ class CPendingDsaRequest
 private:
     static constexpr int TIMEOUT = 15;
 
-    CService addr;
+    uint256 proTxHash;
     CCoinJoinAccept dsa;
     int64_t nTimeCreated{0};
 
 public:
     CPendingDsaRequest() = default;
 
-    CPendingDsaRequest(CService addr_, CCoinJoinAccept dsa_) :
-        addr(std::move(addr_)),
+    CPendingDsaRequest(uint256 proTxHash_, CCoinJoinAccept dsa_) :
+        proTxHash(std::move(proTxHash_)),
         dsa(std::move(dsa_)),
         nTimeCreated(GetTime())
     {
     }
 
-    [[nodiscard]] CService GetAddr() const { return addr; }
+    [[nodiscard]] uint256 GetProTxHash() const { return proTxHash; }
     [[nodiscard]] CCoinJoinAccept GetDSA() const { return dsa; }
     [[nodiscard]] bool IsExpired() const { return GetTime() - nTimeCreated > TIMEOUT; }
 
     friend bool operator==(const CPendingDsaRequest& a, const CPendingDsaRequest& b)
     {
-        return a.addr == b.addr && a.dsa == b.dsa;
+        return a.proTxHash == b.proTxHash && a.dsa == b.dsa;
     }
     friend bool operator!=(const CPendingDsaRequest& a, const CPendingDsaRequest& b)
     {
@@ -336,7 +336,7 @@ class CCoinJoinClientManager
     bool DoAutomaticDenominating(ChainstateManager& chainman, CConnman& connman, const CTxMemPool& mempool,
                                  bool fDryRun = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
 
-    bool TrySubmitDenominate(const CService& mnAddr, CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
+    bool TrySubmitDenominate(const uint256& proTxHash, CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
     bool MarkAlreadyJoinedQueueAsTried(CCoinJoinQueue& dsq) const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
 
     void CheckTimeout() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);

src/coinjoin/server.cpp

@@ -174,14 +174,12 @@ PeerMsgRet CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv
         LogPrint(BCLog::COINJOIN, "DSQUEUE -- nLastDsq: %d  nDsqThreshold: %d  nDsqCount: %d\n", nLastDsq, nDsqThreshold, m_mn_metaman.GetDsqCount());
         //don't allow a few nodes to dominate the queuing process
         if (nLastDsq != 0 && nDsqThreshold > m_mn_metaman.GetDsqCount()) {
-            LogPrint(BCLog::COINJOIN, "DSQUEUE -- Masternode %s is sending too many dsq messages\n",
-                     dmn->pdmnState->addr.ToStringAddrPort());
+            LogPrint(BCLog::COINJOIN, "DSQUEUE -- node sending too many dsq messages, masternode=%s\n", dmn->proTxHash.ToString());
             return {};
         }
         m_mn_metaman.AllowMixing(dmn->proTxHash);
 
-        LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue (%s) from masternode %s\n", dsq.ToString(),
-                 dmn->pdmnState->addr.ToStringAddrPort());
+        LogPrint(BCLog::COINJOIN, "DSQUEUE -- new CoinJoin queue, masternode=%s, queue=%s\n", dmn->proTxHash.ToString(), dsq.ToString());
 
         TRY_LOCK(cs_vecqueue, lockRecv);
         if (!lockRecv) return {};

src/evo/dmnstate.h

@@ -33,7 +33,7 @@ class CDeterministicMNState
     friend class CDeterministicMNStateDiff;
 
 public:
-    int nVersion{CProRegTx::LEGACY_BLS_VERSION};
+    int nVersion{ProTxVersion::LegacyBLS};
 
     int nRegisteredHeight{-1};
     int nLastPaidHeight{0};
@@ -94,7 +94,7 @@ class CDeterministicMNState
             obj.confirmedHash,
             obj.confirmedHashWithProRegTxHash,
             obj.keyIDOwner);
-        READWRITE(CBLSLazyPublicKeyVersionWrapper(const_cast<CBLSLazyPublicKey&>(obj.pubKeyOperator), obj.nVersion == CProRegTx::LEGACY_BLS_VERSION));
+        READWRITE(CBLSLazyPublicKeyVersionWrapper(const_cast<CBLSLazyPublicKey&>(obj.pubKeyOperator), obj.nVersion == ProTxVersion::LegacyBLS));
         READWRITE(
             obj.keyIDVoting,
             obj.addr,
@@ -107,7 +107,7 @@ class CDeterministicMNState
 
     void ResetOperatorFields()
     {
-        nVersion = CProRegTx::LEGACY_BLS_VERSION;
+        nVersion = ProTxVersion::LegacyBLS;
         pubKeyOperator = CBLSLazyPublicKey();
         addr = CService();
         scriptOperatorPayout = CScript();
@@ -219,14 +219,14 @@ class CDeterministicMNStateDiff
 #define DMN_STATE_DIFF_LINE(f) \
         if (strcmp(#f, "pubKeyOperator") == 0 && (obj.fields & Field_pubKeyOperator)) {\
             SER_READ(obj, read_pubkey = true); \
-            READWRITE(CBLSLazyPublicKeyVersionWrapper(const_cast<CBLSLazyPublicKey&>(obj.state.pubKeyOperator), obj.state.nVersion == CProRegTx::LEGACY_BLS_VERSION)); \
+            READWRITE(CBLSLazyPublicKeyVersionWrapper(const_cast<CBLSLazyPublicKey&>(obj.state.pubKeyOperator), obj.state.nVersion == ProTxVersion::LegacyBLS)); \
         } else if (obj.fields & Field_##f) READWRITE(obj.state.f);
 
         DMN_STATE_DIFF_ALL_FIELDS
 #undef DMN_STATE_DIFF_LINE
         if (read_pubkey) {
             SER_READ(obj, obj.fields |= Field_nVersion);
-            SER_READ(obj, obj.state.pubKeyOperator.SetLegacy(obj.state.nVersion == CProRegTx::LEGACY_BLS_VERSION));
+            SER_READ(obj, obj.state.pubKeyOperator.SetLegacy(obj.state.nVersion == ProTxVersion::LegacyBLS));
         }
     }
 

src/evo/providertx.cpp

@@ -14,10 +14,10 @@
 
 bool CProRegTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const
 {
-    if (nVersion == 0 || nVersion > GetVersion(is_basic_scheme_active)) {
+    if (nVersion == 0 || nVersion > GetMaxVersion(is_basic_scheme_active)) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version");
     }
-    if (nVersion != BASIC_BLS_VERSION && nType == MnType::Evo) {
+    if (nVersion < ProTxVersion::BasicBLS && nType == MnType::Evo) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-evo-version");
     }
     if (!IsValidMnType(nType)) {
@@ -30,7 +30,7 @@ bool CProRegTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationState&
     if (keyIDOwner.IsNull() || !pubKeyOperator.Get().IsValid() || keyIDVoting.IsNull()) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-null");
     }
-    if (pubKeyOperator.IsLegacy() != (nVersion == LEGACY_BLS_VERSION)) {
+    if (pubKeyOperator.IsLegacy() != (nVersion == ProTxVersion::LegacyBLS)) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-pubkey");
     }
     if (!scriptPayout.IsPayToPublicKeyHash() && !scriptPayout.IsPayToScriptHash()) {
@@ -98,10 +98,10 @@ std::string CProRegTx::ToString() const
 
 bool CProUpServTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const
 {
-    if (nVersion == 0 || nVersion > GetVersion(is_basic_scheme_active)) {
+    if (nVersion == 0 || nVersion > GetMaxVersion(is_basic_scheme_active)) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version");
     }
-    if (nVersion != BASIC_BLS_VERSION && nType == MnType::Evo) {
+    if (nVersion < ProTxVersion::BasicBLS && nType == MnType::Evo) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-evo-version");
     }
 
@@ -124,7 +124,7 @@ std::string CProUpServTx::ToString() const
 
 bool CProUpRegTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const
 {
-    if (nVersion == 0 || nVersion > GetVersion(is_basic_scheme_active)) {
+    if (nVersion == 0 || nVersion > GetMaxVersion(is_basic_scheme_active)) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version");
     }
     if (nMode != 0) {
@@ -134,7 +134,7 @@ bool CProUpRegTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationStat
     if (!pubKeyOperator.Get().IsValid() || keyIDVoting.IsNull()) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-key-null");
     }
-    if (pubKeyOperator.IsLegacy() != (nVersion == LEGACY_BLS_VERSION)) {
+    if (pubKeyOperator.IsLegacy() != (nVersion == ProTxVersion::LegacyBLS)) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-pubkey");
     }
     if (!scriptPayout.IsPayToPublicKeyHash() && !scriptPayout.IsPayToScriptHash()) {
@@ -157,7 +157,7 @@ std::string CProUpRegTx::ToString() const
 
 bool CProUpRevTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const
 {
-    if (nVersion == 0 || nVersion > GetVersion(is_basic_scheme_active)) {
+    if (nVersion == 0 || nVersion > GetMaxVersion(is_basic_scheme_active)) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version");
     }