fix(rpc): use deployment state for BLS scheme in protx revoke/update_service, improve feature_dip3_v19.py

src/rpc/evo.cpp

@@ -1009,9 +1009,16 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques
         throw std::runtime_error(strprintf("masternode with proTxHash %s is not a %s", ptx.proTxHash.ToString(), GetMnType(mnType).description));
     }
 
-    ptx.nVersion = ProTxVersion::GetMaxFromDeployment<CProUpServTx>(
-        WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()), chainman,
-        /*is_basic_override=*/dmn->pdmnState->nVersion > ProTxVersion::LegacyBLS);
+    ptx.nVersion = ProTxVersion::GetMaxFromDeployment<CProUpServTx>(WITH_LOCK(::cs_main,
+                                                                              return chainman.ActiveChain().Tip()),
+                                                                    chainman);
+
+    // Legacy masternodes must upgrade to BasicBLS before using higher versions.
+    // Clamp to BasicBLS to avoid "bad-protx-version-upgrade" validation failure.
+    if (dmn->pdmnState->nVersion == ProTxVersion::LegacyBLS && ptx.nVersion > ProTxVersion::BasicBLS) {
+        ptx.nVersion = ProTxVersion::BasicBLS;
+    }
+
     ptx.netInfo = NetInfoInterface::MakeNetInfo(ptx.nVersion);
 
     ProcessNetInfoCore(ptx, request.params[1], /*optional=*/false);
@@ -1077,9 +1084,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques
 
     FundSpecialTx(*wallet, tx, ptx, feeSource);
 
-    const bool isV19active = DeploymentActiveAfter(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip();),
-                                                   chainman.GetConsensus(), Consensus::DEPLOYMENT_V19);
-    SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active);
+    SignSpecialTxPayloadByHash(tx, ptx, keyOperator, /*use_legacy=*/ptx.nVersion == ProTxVersion::LegacyBLS);
     SetTxPayload(tx, ptx);
 
     return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit);
@@ -1260,9 +1265,14 @@ static RPCHelpMan protx_revoke()
         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()), chainman,
-        /*is_basic_override=*/dmn->pdmnState->nVersion >= ProTxVersion::BasicBLS);
+    ptx.nVersion = ProTxVersion::GetMaxFromDeployment<CProUpRevTx>(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()),
+                                                                   chainman);
+
+    // Legacy masternodes must upgrade to BasicBLS before using higher versions.
+    // Clamp to BasicBLS to avoid "bad-protx-version-upgrade" validation failure.
+    if (dmn->pdmnState->nVersion == ProTxVersion::LegacyBLS && ptx.nVersion > ProTxVersion::BasicBLS) {
+        ptx.nVersion = ProTxVersion::BasicBLS;
+    }
 
     CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[1].get_str(), "operatorKey");
 

test/functional/feature_dip3_v19.py

@@ -68,6 +68,9 @@ def run_test(self):
         b_0 = self.nodes[0].getbestblockhash()
         self.test_getmnlistdiff(null_hash, b_0, {}, [], expected_updated)
 
+        extra_legacy_mn: MasternodeInfo = self.dynamically_add_masternode()
+        assert extra_legacy_mn is not None
+
         mn_list_before = self.nodes[0].masternodelist()
         pubkeyoperator_list_before = set([mn_list_before[e]["pubkeyoperator"] for e in mn_list_before])
 
@@ -95,9 +98,19 @@ def run_test(self):
         assert evo_info_3 is not None
         self.dynamically_evo_update_service(evo_info_0, 9, should_be_rejected=True)
 
-        self.log.info(f"Trying to revoke proTx:{self.mninfo[-1].proTxHash}")
+        # Test revoking a post-V19 masternode (basic BLS scheme)
+        self.log.info(f"Trying to revoke post-V19 proTx:{evo_info_3.proTxHash}")
         self.test_revoke_protx(evo_info_3.nodeIdx, self.mninfo[-1])
 
+        # Test updating and revoking a pre-V19 masternode (legacy BLS scheme) after V19 activation
+        self.log.info(f"Trying to update pre-V19 proTx:{extra_legacy_mn.proTxHash}")
+        self.test_update_service_protx(extra_legacy_mn)
+        self.log.info(f"Trying to revoke pre-V19 (legacy) proTx:{extra_legacy_mn.proTxHash}")
+        self.test_revoke_protx(extra_legacy_mn.nodeIdx, extra_legacy_mn)
+
+        # Avoid including these masternodes in next dkg to improve test stability
+        self.move_blocks(self.nodes, 24)
+
         self.mine_quorum(llmq_type_name='llmq_test', llmq_type=100)
 
         self.log.info("Checking that adding more regular MNs after v19 doesn't break DKGs and IS/CLs")
@@ -136,6 +149,24 @@ def test_revoke_protx(self, node_idx, revoke_mn: MasternodeInfo):
                 self.mninfo.remove(mn)
                 return
 
+    def test_update_service_protx(self, mn: MasternodeInfo):
+        fund_txid = self.nodes[0].sendtoaddress(mn.fundsAddr, 1)
+        self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block
+        tip = self.generate(self.nodes[0], 1)[0]
+        assert_equal(self.nodes[0].getrawtransaction(fund_txid, 1, tip)['confirmations'], 1)
+
+        protx_result = mn.update_service(self.nodes[0], submit=True, addrs_core_p2p=[f'127.0.0.2:{mn.nodePort}'])
+        self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block
+        tip = self.generate(self.nodes[0], 1)[0]
+        assert_equal(self.nodes[0].getrawtransaction(protx_result, 1, tip)['confirmations'], 1)
+
+        for node in self.nodes:
+            protx_info = node.protx('info', mn.proTxHash)
+            mn_list = node.masternode('list')
+            assert_equal(protx_info['state']['addresses']['core_p2p'][0], '127.0.0.2:%d' % mn.nodePort)
+            assert_equal(mn_list['%s-%d' % (mn.collateral_txid, mn.collateral_vout)]['addresses']['core_p2p'][0], '127.0.0.2:%d' % mn.nodePort)
+        self.log.info(f"Successfully updated={mn.proTxHash}")
+
     def test_getmnlistdiff(self, base_block_hash, block_hash, base_mn_list, expected_deleted, expected_updated):
         d = self.test_getmnlistdiff_base(base_block_hash, block_hash)