Merge #6674: feat(rpc): allow reporting multiple addresses with new addresses field, deprecate service field, allow submitting multiple addresses

7fdd642 docs: add documentation for deprecation, new field and renamed input (Kittywhiskers Van Gogh)
5c564c2 test: check pre-fork conditions for ProRegTx and ProUpServTx preparation (Kittywhiskers Van Gogh)
859ce5c test: validate common conditions for input validation (Kittywhiskers Van Gogh)
a60c39a test: add functional test for `addresses` and deprecated `service` field (Kittywhiskers Van Gogh)
e0d2a81 rpc: deprecate key "service" replaced by "addresses" (Kittywhiskers Van Gogh)
d9247ad rpc: add new key "addresses" to allow reporting multiple entries (Kittywhiskers Van Gogh)
33e5f6a refactor: s/ipAndPort/coreP2PAddrs/g (Kittywhiskers Van Gogh)
4fd4e0e rpc: allow `ipAndPort` to accept multiple entries with arrays (Kittywhiskers Van Gogh)
8c28f30 rpc: spin-off `ipAndPort` and `platform{HTTP,P2P}Port` setters to util (Kittywhiskers Van Gogh)

Pull request description:

  ## Motivation

  To enable including functional tests with the extended addresses (ExtAddr) implementation (i.e. [dash#6666](#6666)), we need the ability to _specify_ and _retrieve_ multiple addresses. The current RPCs do not allow for that and this pull request aims to remedy it.

  This _particular_ pull request does not include:
  * Compatibility logic to allow reporting Platform-specific addresses using legacy fields (will be submitted after ExtAddr impl)
  * Compatibility logic to allow deriving Platform ports from extended address entries to populate deprecated fields (will be submitted after ExtAddr impl)
  * The ability to submit multiple Platform addresses (done in conjunction with ExtAddr impl)

  ## Additional Information

  * Depends on #6665

  * Depends on #6720

  * Dependency for #6666

  * ⚠️ The format for the field replacing `service`, `addresses` is not stable and is changed in [dash#6666](#6666) to allow reporting Platform-specific addresses. This should be acceptable as the whole extended addresses changeset across multiple PRs is expected to be part of a major release but is mentioned here for the sake of posterity.

  * As `IsDeprecatedRPCEnabled()` is defined in `libbitcoin_node` (see `rpc/server.cpp`, [source](https://github.com/dashpay/dash/blob/da8a475dfac9ad80d566eef8cf119c8036d6ebcc/src/rpc/server.cpp#L370-L375)), it is not available to `libbitcoin_common`, where special transaction and network information source files are included. To get around this, a practically identical function, `IsServiceDeprecatedRPCEnabled()` is defined and used in non-RPC code.
    * It is located in `evo/netinfo.cpp` instead of a more natural `rpc/evo_util.cpp` to avoid unnecessary circular dependencies.

  ## Breaking Changes

  * The input field `ipAndPort` has been renamed to `coreP2PAddrs`.
    * `coreP2PAddrs` can now, in addition to accepting a string, accept an array of strings, subject to validation rules.

  * 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
      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.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [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:
  PastaPastaPasta:
    utACK 7fdd642
  UdjinM6:
    utACK 7fdd642

Tree-SHA512: 75331a467e7170542349c94408b71f5d25ebd2bf418cb6bdf77866a342b9079ec795638535f67c6487739b37651bf8265dce66ef3b06e0732748a173441695ef

Author: PastaPastaPasta

contrib/seeds/makeseeds.py

@@ -164,7 +164,7 @@ def main():
     mns = filtermulticollateraladdress(mns)
     mns = filtermultipayoutaddress(mns)
     # Extract IPs
-    ips = [parseip(mn['state']['service']) for mn in mns]
+    ips = [parseip(mn['state']['addresses'][0]) for mn in mns]
     for onion in onions:
         parsed = parseip(onion)
         if parsed is not None:

doc/release-notes-6665.md

@@ -0,0 +1,13 @@
+Updated RPCs
+------------
+
+* The input field `ipAndPort` has been renamed to `coreP2PAddrs`.
+  * `coreP2PAddrs` can now, in addition to accepting a string, accept an array of strings, subject to validation rules.
+
+* 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
+    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.

src/Makefile.am

@@ -300,6 +300,7 @@ BITCOIN_CORE_H = \
   randomenv.h \
   rpc/blockchain.h \
   rpc/client.h \
+  rpc/evo_util.h \
   rpc/index_util.h \
   rpc/mempool.h \
   rpc/mining.h \
@@ -801,6 +802,7 @@ libbitcoin_common_a_SOURCES = \
   policy/policy.cpp \
   protocol.cpp \
   psbt.cpp \
+  rpc/evo_util.cpp \
   rpc/rawtransaction_util.cpp \
   rpc/util.cpp \
   saltedhasher.cpp \

src/coinjoin/client.cpp

@@ -1879,7 +1879,10 @@ void CCoinJoinClientSession::GetJsonInfo(UniValue& obj) const
         assert(mixingMasternode->pdmnState);
         obj.pushKV("protxhash", mixingMasternode->proTxHash.ToString());
         obj.pushKV("outpoint", mixingMasternode->collateralOutpoint.ToStringShort());
-        obj.pushKV("service", mixingMasternode->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
+        if (m_wallet->chain().rpcEnableDeprecated("service")) {
+            obj.pushKV("service", mixingMasternode->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
+        }
+        obj.pushKV("addresses", mixingMasternode->pdmnState->netInfo->ToJson());
     }
     obj.pushKV("denomination", ValueFromAmount(CoinJoin::DenominationToAmount(nSessionDenom)));
     obj.pushKV("state", GetStateString());

src/evo/core_write.cpp

@@ -67,7 +67,10 @@
     ret.pushKV("type", ToUnderlying(nType));
     ret.pushKV("collateralHash", collateralOutpoint.hash.ToString());
     ret.pushKV("collateralIndex", (int)collateralOutpoint.n);
-    ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
+    if (IsServiceDeprecatedRPCEnabled()) {
+        ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
+    }
+    ret.pushKV("addresses", netInfo->ToJson());
     ret.pushKV("ownerAddress", EncodeDestination(PKHash(keyIDOwner)));
     ret.pushKV("votingAddress", EncodeDestination(PKHash(keyIDVoting)));
     if (CTxDestination dest; ExtractDestination(scriptPayout, dest)) {
@@ -114,7 +117,10 @@
     ret.pushKV("version", nVersion);
     ret.pushKV("type", ToUnderlying(nType));
     ret.pushKV("proTxHash", proTxHash.ToString());
-    ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
+    if (IsServiceDeprecatedRPCEnabled()) {
+        ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
+    }
+    ret.pushKV("addresses", netInfo->ToJson());
     if (CTxDestination dest; ExtractDestination(scriptOperatorPayout, dest)) {
         ret.pushKV("operatorPayoutAddress", EncodeDestination(dest));
     }

src/evo/dmnstate.cpp

@@ -38,7 +38,10 @@ UniValue CDeterministicMNState::ToJson(MnType nType) const
 {
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("version", nVersion);
-    obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
+    if (IsServiceDeprecatedRPCEnabled()) {
+        obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
+    }
+    obj.pushKV("addresses", netInfo->ToJson());
     obj.pushKV("registeredHeight", nRegisteredHeight);
     obj.pushKV("lastPaidHeight", nLastPaidHeight);
     obj.pushKV("consecutivePayments", nConsecutivePayments);
@@ -72,7 +75,10 @@ UniValue CDeterministicMNStateDiff::ToJson(MnType nType) const
         obj.pushKV("version", state.nVersion);
     }
     if (fields & Field_netInfo) {
-        obj.pushKV("service", state.netInfo->GetPrimary().ToStringAddrPort());
+        if (IsServiceDeprecatedRPCEnabled()) {
+            obj.pushKV("service", state.netInfo->GetPrimary().ToStringAddrPort());
+        }
+        obj.pushKV("addresses", state.netInfo->ToJson());
     }
     if (fields & Field_nRegisteredHeight) {
         obj.pushKV("registeredHeight", state.nRegisteredHeight);

src/evo/netinfo.cpp

@@ -11,6 +11,8 @@
 #include <util/check.h>
 #include <util/system.h>
 
+#include <univalue.h>
+
 namespace {
 static std::unique_ptr<const CChainParams> g_main_params{nullptr};
 static std::once_flag g_main_params_flag;
@@ -33,6 +35,19 @@ bool MatchCharsFilter(std::string_view input, std::string_view filter)
 }
 } // anonymous namespace
 
+UniValue ArrFromService(const CService& addr)
+{
+    UniValue obj(UniValue::VARR);
+    obj.push_back(addr.ToStringAddrPort());
+    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;
@@ -227,6 +242,11 @@ NetInfoStatus MnNetInfo::Validate() const
     return ValidateService(GetPrimary());
 }
 
+UniValue MnNetInfo::ToJson() const
+{
+    return ArrFromService(GetPrimary());
+}
+
 std::string MnNetInfo::ToString() const
 {
     // Extra padding to account for padding done by the calling function.

src/evo/netinfo.h

@@ -13,6 +13,8 @@
 
 class CService;
 
+class UniValue;
+
 enum class NetInfoStatus : uint8_t {
     // Managing entries
     BadInput,
@@ -51,6 +53,9 @@ constexpr std::string_view NISToString(const NetInfoStatus code)
     assert(false);
 }
 
+/* Identical to IsDeprecatedRPCEnabled("service"). For use outside of RPC code. */
+bool IsServiceDeprecatedRPCEnabled();
+
 class NetInfoEntry
 {
 public:
@@ -141,6 +146,7 @@ class NetInfoInterface
     virtual bool CanStorePlatform() const = 0;
     virtual bool IsEmpty() const = 0;
     virtual NetInfoStatus Validate() const = 0;
+    virtual UniValue ToJson() const = 0;
     virtual std::string ToString() const = 0;
 
     virtual void Clear() = 0;
@@ -197,6 +203,7 @@ class MnNetInfo final : public NetInfoInterface
     bool IsEmpty() const override { return m_addr.IsEmpty(); }
     bool CanStorePlatform() const override { return false; }
     NetInfoStatus Validate() const override;
+    UniValue ToJson() const override;
     std::string ToString() const override;
 
     void Clear() override { m_addr.Clear(); }

src/evo/simplifiedmns.cpp

@@ -4,15 +4,16 @@
 
 #include <evo/simplifiedmns.h>
 
-#include <evo/cbtx.h>
 #include <core_io.h>
 #include <deploymentstatus.h>
+#include <evo/cbtx.h>
 #include <evo/deterministicmns.h>
+#include <evo/netinfo.h>
+#include <evo/specialtx.h>
 #include <llmq/blockprocessor.h>
 #include <llmq/commitment.h>
 #include <llmq/quorums.h>
 #include <node/blockstorage.h>
-#include <evo/specialtx.h>
 
 #include <pubkey.h>
 #include <serialize.h>
@@ -80,7 +81,10 @@ UniValue CSimplifiedMNListEntry::ToJson(bool extended) const
     obj.pushKV("nType", ToUnderlying(nType));
     obj.pushKV("proRegTxHash", proRegTxHash.ToString());
     obj.pushKV("confirmedHash", confirmedHash.ToString());
-    obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
+    if (IsServiceDeprecatedRPCEnabled()) {
+        obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
+    }
+    obj.pushKV("addresses", netInfo->ToJson());
     obj.pushKV("pubKeyOperator", pubKeyOperator.ToString());
     obj.pushKV("votingAddress", EncodeDestination(PKHash(keyIDVoting)));
     obj.pushKV("isValid", isValid);

src/rpc/coinjoin.cpp

@@ -435,7 +435,13 @@ 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"},
+                                    {RPCResult::Type::STR, "service", "The IP address and port of the masternode (DEPRECATED, returned only if config option -deprecatedrpc=service is passed)"},
+                                    {RPCResult::Type::ARR, "addresses", "Network addresses of the masternode",
+                                    {
+                                        {
+                                            {RPCResult::Type::STR, "address", ""},
+                                        }
+                                    }},
                                     {RPCResult::Type::NUM, "denomination", "The denomination of the mixing session in " + CURRENCY_UNIT + ""},
                                     {RPCResult::Type::STR_HEX, "state", "Current state of the mixing session"},
                                     {RPCResult::Type::NUM, "entries_count", "The number of entries in the mixing session"},