Merge dashpay#6811: docs(rpc): revert gating of service/address behind -deprecatedrpc=service, mark platform{P2P,HTTP}Port as deprecated

PastaPastaPasta · PastaPastaPasta · commit 3a27f30cc8d8 · 2025-09-25T10:47:24.000-05:00
ed3db7c docs: mark `platformP2PPort` and `platformHTTPPort` as deprecated (Kittywhiskers Van Gogh) ac77d42 docs: clarify that deprecation extends to `address` in `masternodelist` (Kittywhiskers Van Gogh) bbcd9d5 rpc: remove `-deprecatedrpc=service` gating for `service` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * dashpay#6666 * No help text has been updated to mark `platformP2PPort` and `platformHTTPPort` as deprecated outputs as we do not generate help text that mentions what these fields represent. This gap will be addressed in a future PR and the text "(DEPRECATED)" will be appended there. * Like `service`, the field is only marked as deprecated in documentation but is accessible without any additional runtime flags. ## Breaking Changes Refer to release notes. ## 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 - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK ed3db7c Tree-SHA512: 44c524f01b829c3890c88ada19dd5da547a7b0df28f70b750196aabfc61cb0b5598359df649b78147a05fba41bfae474b4411aeae0632b701dde1004576a5396
diff --git a/doc/release-notes-6665.md b/doc/release-notes-6665.md
@@ -6,8 +6,9 @@ Updated RPCs
 
 * The key `service` has been deprecated for some RPCs (`decoderawtransaction`, `decodepsbt`, `getblock`, `getrawtransaction`,
   `gettransaction`, `masternode status` (only for the `dmnState` key), `protx diff`, `protx listdiff`) and has been replaced
-  with the field `addresses`.
-  * The deprecated field can be re-enabled using `-deprecatedrpc=service` but is liable to be removed in future versions
+  with the key `addresses`.
+  * This deprecation also extends to the functionally identical key, `address` in `masternode list` (and its alias, `masternodelist`)
+  * The deprecated key is still available without additional runtime arguments but is liable to be removed in future versions
     of Dash Core.
   * This change does not affect `masternode status` (except for the `dmnState` key) as `service` does not represent a payload
     value but the external address advertised by the active masternode.
diff --git a/doc/release-notes-6811.md b/doc/release-notes-6811.md
@@ -0,0 +1,8 @@
+Updated RPCs
+------------
+
+* The keys `platformP2PPort` and `platformHTTPPort` have been deprecated for the following RPCs, `decoderawtransaction`,
+  `decodepsbt`, `getblock`, `getrawtransaction`, `gettransaction`, `masternode status` (only the `dmnState` key),
+  `protx diff`, `protx listdiff` and has been replaced with the key `addresses`.
+  * The deprecated key is still available without additional runtime arguments but is liable to be removed in future versions
+    of Dash Core.
diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp
@@ -1873,9 +1873,7 @@ void CCoinJoinClientSession::GetJsonInfo(UniValue& obj) const
         assert(mixingMasternode->pdmnState);
         obj.pushKV("protxhash", mixingMasternode->proTxHash.ToString());
         obj.pushKV("outpoint", mixingMasternode->collateralOutpoint.ToStringShort());
-        if (m_wallet->chain().rpcEnableDeprecated("service")) {
-            obj.pushKV("service", mixingMasternode->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
-        }
+        obj.pushKV("service", mixingMasternode->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
         obj.pushKV("addrs_core_p2p", mixingMasternode->pdmnState->netInfo->ToJson(NetInfoPurpose::CORE_P2P));
     }
     obj.pushKV("denomination", ValueFromAmount(CoinJoin::DenominationToAmount(nSessionDenom)));
diff --git a/src/evo/core_write.cpp b/src/evo/core_write.cpp
@@ -72,9 +72,7 @@
     ret.pushKV("type", ToUnderlying(nType));
     ret.pushKV("collateralHash", collateralOutpoint.hash.ToString());
     ret.pushKV("collateralIndex", collateralOutpoint.n);
-    if (IsServiceDeprecatedRPCEnabled()) {
-        ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
-    }
+    ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
     ret.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
     ret.pushKV("ownerAddress", EncodeDestination(PKHash(keyIDOwner)));
     ret.pushKV("votingAddress", EncodeDestination(PKHash(keyIDVoting)));
@@ -122,9 +120,7 @@
     ret.pushKV("version", nVersion);
     ret.pushKV("type", ToUnderlying(nType));
     ret.pushKV("proTxHash", proTxHash.ToString());
-    if (IsServiceDeprecatedRPCEnabled()) {
-        ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
-    }
+    ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
     ret.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
     if (CTxDestination dest; ExtractDestination(scriptOperatorPayout, dest)) {
         ret.pushKV("operatorPayoutAddress", EncodeDestination(dest));
@@ -162,9 +158,7 @@
     obj.pushKV("nType", ToUnderlying(nType));
     obj.pushKV("proRegTxHash", proRegTxHash.ToString());
     obj.pushKV("confirmedHash", confirmedHash.ToString());
-    if (IsServiceDeprecatedRPCEnabled()) {
-        obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
-    }
+    obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
     obj.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
     obj.pushKV("pubKeyOperator", pubKeyOperator.ToString());
     obj.pushKV("votingAddress", EncodeDestination(PKHash(keyIDVoting)));
diff --git a/src/evo/dmnstate.cpp b/src/evo/dmnstate.cpp
@@ -36,9 +36,7 @@ UniValue CDeterministicMNState::ToJson(MnType nType) const
 {
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("version", nVersion);
-    if (IsServiceDeprecatedRPCEnabled()) {
-        obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
-    }
+    obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
     obj.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
     obj.pushKV("registeredHeight", nRegisteredHeight);
     obj.pushKV("lastPaidHeight", nLastPaidHeight);
@@ -73,9 +71,7 @@ UniValue CDeterministicMNStateDiff::ToJson(MnType nType) const
         obj.pushKV("version", state.nVersion);
     }
     if (fields & Field_netInfo) {
-        if (IsServiceDeprecatedRPCEnabled()) {
-            obj.pushKV("service", state.netInfo->GetPrimary().ToStringAddrPort());
-        }
+        obj.pushKV("service", state.netInfo->GetPrimary().ToStringAddrPort());
     }
     if (fields & Field_nRegisteredHeight) {
         obj.pushKV("registeredHeight", state.nRegisteredHeight);
diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp
@@ -42,12 +42,6 @@ UniValue ArrFromService(const CService& addr)
     return obj;
 }
 
-bool IsServiceDeprecatedRPCEnabled()
-{
-    const auto args = gArgs.GetArgs("-deprecatedrpc");
-    return std::find(args.begin(), args.end(), "service") != args.end();
-}
-
 bool NetInfoEntry::operator==(const NetInfoEntry& rhs) const
 {
     if (m_type != rhs.m_type) return false;
diff --git a/src/evo/netinfo.h b/src/evo/netinfo.h
@@ -100,9 +100,6 @@ constexpr std::string_view PurposeToString(const NetInfoPurpose purpose)
 /** Will return true if node is running on mainnet */
 bool IsNodeOnMainnet();
 
-/** Identical to IsDeprecatedRPCEnabled("service"). For use outside of RPC code */
-bool IsServiceDeprecatedRPCEnabled();
-
 /** Creates a one-element array using CService::ToStringPortAddr() output */
 UniValue ArrFromService(const CService& addr);
 
diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp
@@ -433,7 +433,7 @@ static RPCHelpMan getcoinjoininfo()
                                 {
                                     {RPCResult::Type::STR_HEX, "protxhash", "The ProTxHash of the masternode"},
                                     {RPCResult::Type::STR_HEX, "outpoint", "The outpoint of the masternode"},
-                                    {RPCResult::Type::STR, "service", "The IP address and port of the masternode (DEPRECATED, returned only if config option -deprecatedrpc=service is passed)"},
+                                    {RPCResult::Type::STR, "service", "(DEPRECATED) The IP address and port of the masternode"},
                                     {RPCResult::Type::ARR, "addrs_core_p2p", "Network addresses of the masternode used for protocol P2P",
                                         {
                                             {RPCResult::Type::STR, "address", ""},
diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp
@@ -614,9 +614,7 @@ static RPCHelpMan masternodelist_helper(bool is_composite)
                 strOutpoint.find(strFilter) == std::string::npos) return;
             UniValue objMN(UniValue::VOBJ);
             objMN.pushKV("proTxHash", dmn.proTxHash.ToString());
-            if (IsDeprecatedRPCEnabled("service")) {
-                objMN.pushKV("address", dmn.pdmnState->netInfo->GetPrimary().ToStringAddrPort());
-            }
+            objMN.pushKV("address", dmn.pdmnState->netInfo->GetPrimary().ToStringAddrPort());
             objMN.pushKV("addresses", GetNetInfoWithLegacyFields(*dmn.pdmnState, dmn.nType));
             objMN.pushKV("payee", payeeStr);
             objMN.pushKV("status", dmnToStatus(dmn));
diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp
@@ -206,9 +206,7 @@ static UniValue BuildQuorumInfo(const llmq::CQuorumBlockProcessor& quorum_block_
             const auto& dmn = quorum->members[i];
             UniValue mo(UniValue::VOBJ);
             mo.pushKV("proTxHash", dmn->proTxHash.ToString());
-            if (IsDeprecatedRPCEnabled("service")) {
-                mo.pushKV("service", dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
-            }
+            mo.pushKV("service", dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
             mo.pushKV("addresses", GetNetInfoWithLegacyFields(*dmn->pdmnState, dmn->nType));
             mo.pushKV("pubKeyOperator", dmn->pdmnState->pubKeyOperator.ToString());
             mo.pushKV("valid", static_cast<bool>(quorum->qc->validMembers[i]));
diff --git a/test/functional/rpc_netinfo.py b/test/functional/rpc_netinfo.py
@@ -135,11 +135,10 @@ def destroy_mn(self, test: BitcoinTestFramework):
 
 class NetInfoTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.num_nodes = 3
+        self.num_nodes = 2
         self.extra_args = [[
             "-dip3params=2:2", f"-vbparams=v23:{self.mocktime}:999999999999:{V23_ACTIVATION_THRESHOLD}:10:8:6:5:0"
         ] for _ in range(self.num_nodes)]
-        self.extra_args[2] += ["-deprecatedrpc=service"]
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -177,14 +176,12 @@ def run_test(self):
         self.node_two: EvoNode = EvoNode(self.nodes[1])
         self.node_two.generate_collateral(self)
         self.node_two.register_mn(self, True, "", DEFAULT_PORT_PLATFORM_P2P, DEFAULT_PORT_PLATFORM_HTTP)
-        # Used to check deprecated field behavior
-        self.node_simple: TestNode = self.nodes[2]
         # Test routines
         self.log.info("Test input validation for masternode address fields (pre-fork)")
         self.test_validation_common()
         self.test_validation_legacy()
         self.log.info("Test output masternode address fields for consistency (pre-fork)")
-        self.test_deprecation()
+        self.test_fields()
         self.log.info("Mine blocks to activate DEPLOYMENT_V23")
         self.activate_v23()
         self.log.info("Test input validation for masternode address fields (post-fork)")
@@ -361,7 +358,7 @@ def test_validation_extended(self):
         self.node_evo.register_mn(self, False, "127.0.0.1", DEFAULT_PORT_PLATFORM_P2P, DEFAULT_PORT_PLATFORM_HTTP,
                                   -8, "Error setting coreP2PAddrs[0] to '127.0.0.1' (invalid port)")
 
-    def test_deprecation(self):
+    def test_fields(self):
         # netInfo is represented with JSON in CProRegTx, CProUpServTx, CDeterministicMNState and CSimplifiedMNListEntry,
         # so we need to test calls that rely on these underlying implementations. Start by collecting RPC responses.
         self.log.info("Collect JSON RPC responses from node")
@@ -374,12 +371,7 @@ def test_deprecation(self):
         self.node_evo.set_active_state(self, True)
         masternode_status = self.node_evo.node.masternode('status')
 
-        # Generate deprecation-disabled response to avoid having to re-create a masternode again later on
-        self.node_evo.set_active_state(self, True, ['-deprecatedrpc=service'])
-        self.reconnect_nodes()
-        masternode_status_depr = self.node_evo.node.masternode('status')
-
-        # Stop actively running the masternode so we can issue a CProUpServTx (and enable the deprecation)
+        # Stop actively running the masternode so we can issue a CProUpServTx
         self.node_evo.set_active_state(self, False)
         self.reconnect_nodes()
 
@@ -411,13 +403,6 @@ def test_deprecation(self):
         protx_listdiff_rpc_pl = self.node_evo.node.protx('listdiff', proupservtx_height_pl - 1, proupservtx_height_pl)
         protx_listdiff_rpc_pl = self.extract_from_listdiff(protx_listdiff_rpc_pl, proregtx_hash)
 
-        self.log.info("Test RPCs return an 'addresses' field")
-        assert "addresses" in proregtx_rpc['proRegTx'].keys()
-        assert "addresses" in masternode_status['dmnState'].keys()
-        assert "addresses" in proupservtx_rpc['proUpServTx'].keys()
-        assert "addresses" in protx_diff_rpc['mnList'][0].keys()
-        assert "addresses" in protx_listdiff_rpc.keys()
-
         self.log.info("Test 'addresses' report correctly")
         self.check_netinfo_fields(proregtx_rpc['proRegTx']['addresses'], self.node_evo.mn.nodePort, DEFAULT_PORT_PLATFORM_HTTP, DEFAULT_PORT_PLATFORM_P2P)
         self.check_netinfo_fields(masternode_status['dmnState']['addresses'], self.node_evo.mn.nodePort, DEFAULT_PORT_PLATFORM_HTTP, DEFAULT_PORT_PLATFORM_P2P)
@@ -431,37 +416,10 @@ def test_deprecation(self):
         assert_equal(protx_listdiff_rpc_pl['addresses']['platform_https'][0], f"{DMNSTATE_DIFF_DUMMY_ADDR}:{DEFAULT_PORT_PLATFORM_HTTP + 10}")
         assert_equal(protx_listdiff_rpc_pl['addresses']['platform_p2p'][0], f"{DMNSTATE_DIFF_DUMMY_ADDR}:{DEFAULT_PORT_PLATFORM_P2P + 10}")
 
-        self.log.info("Test RPCs by default no longer return a 'service' field")
-        assert "service" not in proregtx_rpc['proRegTx'].keys()
-        assert "service" not in masternode_status['dmnState'].keys()
-        assert "service" not in proupservtx_rpc['proUpServTx'].keys()
-        assert "service" not in protx_diff_rpc['mnList'][0].keys()
-        assert "service" not in protx_listdiff_rpc.keys()
-        # "service" in "masternode status" is exempt from the deprecation as the primary address is
-        # relevant on the host node as opposed to expressing payload information in most other RPCs.
-        assert "service" in masternode_status.keys()
-
         # Shut down masternode
         self.node_evo.destroy_mn(self)
         self.reconnect_nodes()
 
-        self.log.info("Collect RPC responses from node with -deprecatedrpc=service")
-
-        # Re-use chain activity from earlier
-        proregtx_rpc = self.node_simple.getrawtransaction(proregtx_hash, True)
-        proupservtx_rpc = self.node_simple.getrawtransaction(proupservtx_hash, True)
-        protx_diff_rpc = self.node_simple.protx('diff', masternode_active_height - 1, masternode_active_height)
-        masternode_status = masternode_status_depr # Pull in response generated from earlier
-        protx_listdiff_rpc = self.node_simple.protx('listdiff', proupservtx_height - 1, proupservtx_height)
-        protx_listdiff_rpc = self.extract_from_listdiff(protx_listdiff_rpc, proregtx_hash)
-
-        self.log.info("Test RPCs return 'service' with -deprecatedrpc=service")
-        assert "service" in proregtx_rpc['proRegTx'].keys()
-        assert "service" in masternode_status['dmnState'].keys()
-        assert "service" in proupservtx_rpc['proUpServTx'].keys()
-        assert "service" in protx_diff_rpc['mnList'][0].keys()
-        assert "service" in protx_listdiff_rpc.keys()
-
     def test_empty_fields(self):
         def empty_common(grt_dict):
             # Even if '[::]:0' is reported by 'service', 'addresses' should always be blank if we have nothing to report
@@ -471,7 +429,7 @@ def empty_common(grt_dict):
             assert_equal(grt_dict['service'], "[::]:0")
 
         # Validate that our masternode was registered before the fork
-        grt_proregtx = self.node_simple.getrawtransaction(self.node_two.mn.proTxHash, True)['proRegTx']
+        grt_proregtx = self.node_two.node.getrawtransaction(self.node_two.mn.proTxHash, True)['proRegTx']
         assert_equal(grt_proregtx['version'], PROTXVER_BASIC)
 
         # Test reporting on legacy address (i.e. basic BLS and earlier) nodes
@@ -487,7 +445,7 @@ def empty_common(grt_dict):
         self.node_two.register_mn(self, True, "", "", "")
 
         # Validate that masternode uses extended addresses
-        grt_proregtx = self.node_simple.getrawtransaction(self.node_two.mn.proTxHash, True)['proRegTx']
+        grt_proregtx = self.node_two.node.getrawtransaction(self.node_two.mn.proTxHash, True)['proRegTx']
         assert_equal(grt_proregtx['version'], PROTXVER_EXTADDR)
 
         # Test reporting for extended addresses
@@ -507,7 +465,7 @@ def empty_common(grt_dict):
     def test_shims(self):
         # There are two shims there to help with migrating between legacy and extended addresses, one reads from legacy platform
         # fields to ensure 'addresses' is adequately populated. The other, reads from netInfo to populate platform{HTTP,P2P}Port.
-        # As the fork is now active, we can now evaluate the test cases that couldn't be evaluated in test_deprecation().
+        # As the fork is now active, we can now evaluate the test cases that couldn't be evaluated in test_fields().
         self.log.info("Collect JSON RPC responses from node")
 
         # Create an masternode that clearly uses extended addresses (hello IPv6!)
@@ -545,16 +503,13 @@ def test_shims(self):
         # platform{HTTP,P2P}Port *won't* be populated by listdiff as that *field* is now unused and it's dangerous to report "updates" to disused fields
         assert "platformP2PPort" not in protx_listdiff_rpc.keys()
         assert "platformHTTPPort" not in protx_listdiff_rpc.keys()
+        # Check that 'service' correctly reports as coreP2PAddrs[0]
+        assert_equal(proregtx_rpc['proRegTx']['service'], f"127.0.0.1:{self.node_evo.mn.nodePort}")
+        assert_equal(proupservtx_rpc['proUpServTx']['service'], f"127.0.0.1:{self.node_evo.mn.nodePort}")
 
         # Restart the client to see if (de)ser works as intended (CDeterministicMNStateDiff is a special case and we just made an update)
         self.node_evo.set_active_state(self, False)
         self.reconnect_nodes()
 
-        # Check that 'service' correctly reports as coreP2PAddrs[0]
-        proregtx_rpc = self.node_simple.getrawtransaction(proregtx_hash, True)
-        proupservtx_rpc = self.node_simple.getrawtransaction(proupservtx_hash, True)
-        assert_equal(proregtx_rpc['proRegTx']['service'], f"127.0.0.1:{self.node_evo.mn.nodePort}")
-        assert_equal(proupservtx_rpc['proUpServTx']['service'], f"127.0.0.1:{self.node_evo.mn.nodePort}")
-
 if __name__ == "__main__":
     NetInfoTest().main()

Original file line number	Diff line number	Diff line change
`@@ -42,12 +42,6 @@ UniValue ArrFromService(const CService& addr)`
`42`	`42`	`return obj;`
`43`	`43`	`}`
`44`	`44`
`45`		`-bool IsServiceDeprecatedRPCEnabled()`
`46`		`-{`
`47`		`- const auto args = gArgs.GetArgs("-deprecatedrpc");`
`48`		`- return std::find(args.begin(), args.end(), "service") != args.end();`
`49`		`-}`
`50`		`-`
`51`	`45`	`bool NetInfoEntry::operator==(const NetInfoEntry& rhs) const`
`52`	`46`	`{`
`53`	`47`	`if (m_type != rhs.m_type) return false;`
Original file line number	Diff line number	Diff line change
`@@ -433,7 +433,7 @@ static RPCHelpMan getcoinjoininfo()`
`433`	`433`	`{`
`434`	`434`	`{RPCResult::Type::STR_HEX, "protxhash", "The ProTxHash of the masternode"},`
`435`	`435`	`{RPCResult::Type::STR_HEX, "outpoint", "The outpoint of the masternode"},`
`436`		`- {RPCResult::Type::STR, "service", "The IP address and port of the masternode (DEPRECATED, returned only if config option -deprecatedrpc=service is passed)"},`
	`436`	`+ {RPCResult::Type::STR, "service", "(DEPRECATED) The IP address and port of the masternode"},`
`437`	`437`	`{RPCResult::Type::ARR, "addrs_core_p2p", "Network addresses of the masternode used for protocol P2P",`
`438`	`438`	`{`
`439`	`439`	`{RPCResult::Type::STR, "address", ""},`