feat: prohibit new legacy scheme masternodes, restrict ProTx version changes with DEPLOYMENT_V23

doc/release-notes-6729.md

@@ -0,0 +1,12 @@
+Notable Changes
+---------------
+
+* Dash Core will no longer permit the registration of new legacy scheme masternodes after the deployment of the v23
+  fork. Existing basic scheme masternodes will also be prohibited from downgrading to the legacy scheme after the
+  deployment is active.
+
+Updated RPCs
+----------------
+
+* `protx revoke` will now use the legacy scheme version for legacy masternodes instead of the defaulting to the
+   highest `ProUpRevTx` version.

src/evo/deterministicmns.cpp

@@ -1292,6 +1292,41 @@ static std::optional<ProTx> GetValidatedPayload(const CTransaction& tx, gsl::not
     return opt_ptx;
 }
 
+/**
+ * Validates potential changes to masternode state version by ProTx transaction version
+ * @param[in]  pindexPrev    Previous block index to validate DEPLOYMENT_V23 activation
+ * @param[in]  tx_type       Special transaction type
+ * @param[in]  state_version Current masternode state version
+ * @param[in]  tx_version    Proposed transaction version
+ * @param[out] state         This may be set to an Error state if any error occurred processing them
+ * @returns                  true if version change is valid or DEPLOYMENT_V23 is not active
+ */
+bool IsVersionChangeValid(gsl::not_null<const CBlockIndex*> pindexPrev, const uint16_t tx_type,
+                          const uint16_t state_version, const uint16_t tx_version, TxValidationState& state)
+{
+    if (!DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V23)) {
+        // New restrictions only apply after v23 deployment
+        return true;
+    }
+
+    if (state_version >= ProTxVersion::BasicBLS && tx_version == ProTxVersion::LegacyBLS) {
+        // Don't allow legacy scheme versioned transactions after upgrading to basic scheme
+        return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version-downgrade");
+    }
+
+    if (state_version == ProTxVersion::LegacyBLS && tx_version > ProTxVersion::BasicBLS) {
+        // Nodes using the legacy scheme must first upgrade to the basic scheme before upgrading further
+        return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version-upgrade");
+    }
+
+    if (tx_type != TRANSACTION_PROVIDER_UPDATE_SERVICE && tx_version == ProTxVersion::ExtAddr) {
+        // Only new entries (ProRegTx) and service updates (ProUpServTx) can use ExtAddr versioning
+        return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version-tx-type");
+    }
+
+    return true;
+}
+
 bool CheckProRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs)
 {
     const auto opt_ptx = GetValidatedPayload<CProRegTx>(tx, pindexPrev, state);
@@ -1300,6 +1335,13 @@ bool CheckProRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl:
         return false;
     }
 
+    const bool is_v23_active{DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V23)};
+
+    // No longer allow legacy scheme masternode registration
+    if (is_v23_active && opt_ptx->nVersion < ProTxVersion::BasicBLS) {
+        return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-version-disallowed");
+    }
+
     // It's allowed to set addr to 0, which will put the MN into PoSe-banned state and require a ProUpServTx to be issues later
     // If any of both is set, it must be valid however
     if (!opt_ptx->netInfo->IsEmpty() && !CheckService(*opt_ptx, state)) {
@@ -1434,11 +1476,16 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g
     }
 
     auto mnList = dmnman.GetListForBlock(pindexPrev);
-    auto mn = mnList.GetMN(opt_ptx->proTxHash);
-    if (!mn) {
+    auto dmn = mnList.GetMN(opt_ptx->proTxHash);
+    if (!dmn) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
     }
 
+    if (!IsVersionChangeValid(pindexPrev, tx.nType, dmn->pdmnState->nVersion, opt_ptx->nVersion, state)) {
+        // pass the state returned by the function above
+        return false;
+    }
+
     // don't allow updating to addresses already used by other MNs
     for (const NetInfoEntry& entry : opt_ptx->netInfo->GetEntries()) {
         if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
@@ -1459,7 +1506,7 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g
     }
 
     if (opt_ptx->scriptOperatorPayout != CScript()) {
-        if (mn->nOperatorReward == 0) {
+        if (dmn->nOperatorReward == 0) {
             // don't allow setting operator reward payee in case no operatorReward was set
             return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-payee");
         }
@@ -1473,7 +1520,7 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g
         // pass the state returned by the function above
         return false;
     }
-    if (check_sigs && !CheckHashSig(*opt_ptx, mn->pdmnState->pubKeyOperator.Get(), state)) {
+    if (check_sigs && !CheckHashSig(*opt_ptx, dmn->pdmnState->pubKeyOperator.Get(), state)) {
         // pass the state returned by the function above
         return false;
     }
@@ -1501,6 +1548,11 @@ bool CheckProUpRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gs
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
     }
 
+    if (!IsVersionChangeValid(pindexPrev, tx.nType, dmn->pdmnState->nVersion, opt_ptx->nVersion, state)) {
+        // pass the state returned by the function above
+        return false;
+    }
+
     // don't allow reuse of payee key for other keys (don't allow people to put the payee key onto an online server)
     if (payoutDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || payoutDest == CTxDestination(PKHash(opt_ptx->keyIDVoting))) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-reuse");
@@ -1556,8 +1608,14 @@ bool CheckProUpRevTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gs
 
     auto mnList = dmnman.GetListForBlock(pindexPrev);
     auto dmn = mnList.GetMN(opt_ptx->proTxHash);
-    if (!dmn)
+    if (!dmn) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash");
+    }
+
+    if (!IsVersionChangeValid(pindexPrev, tx.nType, dmn->pdmnState->nVersion, opt_ptx->nVersion, state)) {
+        // pass the state returned by the function above
+        return false;
+    }
 
     if (!CheckInputsHash(tx, *opt_ptx, state)) {
         // pass the state returned by the function above

src/rpc/evo.cpp

@@ -1269,14 +1269,16 @@ static RPCHelpMan protx_revoke()
     EnsureWalletIsUnlocked(*pwallet);
 
     CProUpRevTx ptx;
-    ptx.nVersion = ProTxVersion::GetMaxFromDeployment<CProUpRevTx>(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()));
-
     ptx.proTxHash = ParseHashV(request.params[0], "proTxHash");
+
     auto dmn = dmnman.GetListAtChainTip().GetMN(ptx.proTxHash);
     if (!dmn) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("masternode %s not found", ptx.proTxHash.ToString()));
     }
 
+    ptx.nVersion = ProTxVersion::GetMaxFromDeployment<CProUpRevTx>(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()),
+                                                                   /*is_basic_override=*/dmn->pdmnState->nVersion >= ProTxVersion::BasicBLS);
+
     CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[1].get_str(), "operatorKey");
 
     if (!request.params[2].isNull()) {