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

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.

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.

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)));

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)));

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);

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;

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);
 

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", ""},

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));

src/rpc/quorums.cpp

@@ -205,9 +205,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]));

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()