feat: introduce basic extended addresses implementation, stop using platform{HTTP,P2P}Port

contrib/seeds/makeseeds.py

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

doc/release-notes-6666.md

@@ -0,0 +1,65 @@
+Notable Changes
+---------------
+
+* Dash Core has added support for the ability to register multiple addr:port pairs to a masternode and for specifying
+  distinct addresses for platform P2P and platform HTTPS endpoints. The consensus and format changes needed to enable
+  this capability is referred to as "extended addresses" and is enabled by the deployment of the v23 fork, affecting
+  new masternode registrations and service updates to basic BLS masternodes.
+  * Operators must upgrade from legacy BLS scheme to basic BLS scheme before utilizing extended address capabilities
+
+Additional Notes
+----------------
+
+* While the field `service` is deprecated (see dash#6665), its effective value can be obtained by querying
+  `addresses['core_p2p'][0]`.
+
+* If the masternode is eligible for extended addresses, operators may register non-IPv4 addresses, subject to validation
+  and consensus rules.
+
+Updated RPCs
+------------
+
+* The input field `platformP2PPort` has been renamed to `platformP2PAddrs`. In addition to numeric inputs (i.e. ports),
+  the field can now accept a string (i.e. an addr:port pair) and arrays of strings (i.e. multiple addr:port pairs),
+  subject to validation rules.
+
+* The input field `platformHTTPPort` has been renamed to `platformHTTPSAddrs`. In addition to numeric inputs (i.e. ports),
+  the field can now accept a string (i.e. an addr:port pair) and arrays of strings (i.e. multiple addr:port pairs),
+  subject to validation rules.
+
+* The field `addresses` will now also report on platform P2P and platform HTTPS endpoints as `addresses['platform_p2p']`
+  and `addresses['platform_https']` respectively.
+  * On payloads before extended addresses, if a masternode update affects `platformP2PPort` and/or `platformHTTPPort`
+    but does not affect `netInfo`, `protx listdiff` does not contain enough information to report on the masternode's
+    address and will report the changed port paired with the dummy address `255.255.255.255`.
+
+    This does not affect `protx listdiff` queries where `netInfo` was updated or diffs relating to masternodes that
+    have upgraded to extended addresses.
+
+* If the masternode is eligible for extended addresses, `protx register{,_evo}` and `register_fund{,_evo}` will continue
+  allowing `coreP2PAddrs` to be left blank, as long as `platformP2PAddrs` and `platformHTTPSAddrs` are _also_ left blank.
+  * Attempting to populate any three address fields will make populating all fields mandatory.
+  * This does not affect nodes ineligible for extended addresses (i.e. all nodes before fork activation or legacy BLS nodes)
+    and they will have to continue specifying `platformP2PAddrs` and `platformHTTPSAddrs` even if they wish to keep
+    `coreP2PAddrs` blank.
+
+* If the masternode is eligible for extended addresses, `protx register{,_evo}` and `register_fund{,_evo}` will no longer
+  default to the core P2P port if a port is not specified in the addr:port pair. All ports must be specified explicitly.
+  * This does not affect nodes ineligible for extended addresses, continuing to default to the core P2P port if provided an
+    addr without a port.
+
+* `protx register{,_evo}` and `register_fund{,_evo}` will continue to allow specifying only the port number for `platformP2PAddrs`
+  and `platformHTTPSAddrs`, pairing it with the address from the first `coreP2PAddrs` entry. This mirrors existing behavior.
+  * This method of entry may not be available in future releases of Dash Core and operators are recommended to switch over to
+    explicitly specifying (arrays of) addr:port strings for all address fields.
+
+* When reporting on extended address payloads, `platformP2PPort` and `platformHTTPPort` will read the port value from
+  `netInfo[PLATFORM_P2P][0]` and `netInfo[PLATFORM_HTTPS][0]` respectively as both fields are subsumed into `netInfo`.
+  * If `netInfo` is blank (which is allowed by ProRegTx), `platformP2PPort` and `platformHTTPPort` will report `-1` to indicate
+    that the port number cannot be determined.
+  * `protx listdiff` will not report `platformP2PPort` or `platformHTTPPort` if the legacy fields were not updated (i.e.
+    changes to `netInfo` will not translate into reporting). This is because `platformP2PPort` or `platformHTTPPort` have
+    dedicated diff flags and post-consolidation, all changes are now affected by `netInfo`'s diff flag.
+
+    To avoid the perception of changes to fields that not serialized by extended address payloads, data from `netInfo` will
+    not be translated for this RPC call.

src/coinjoin/client.cpp

@@ -14,6 +14,7 @@
 #include <net.h>
 #include <net_processing.h>
 #include <netmessagemaker.h>
+#include <rpc/evo_util.h>
 #include <shutdown.h>
 #include <util/check.h>
 #include <util/irange.h>
@@ -1875,7 +1876,7 @@ void CCoinJoinClientSession::GetJsonInfo(UniValue& obj) const
         if (m_wallet->chain().rpcEnableDeprecated("service")) {
             obj.pushKV("service", mixingMasternode->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
         }
-        obj.pushKV("addresses", mixingMasternode->pdmnState->netInfo->ToJson());
+        obj.pushKV("addrs_core_p2p", mixingMasternode->pdmnState->netInfo->ToJson(NetInfoPurpose::CORE_P2P));
     }
     obj.pushKV("denomination", ValueFromAmount(CoinJoin::DenominationToAmount(nSessionDenom)));
     obj.pushKV("state", GetStateString());

src/evo/core_write.cpp

@@ -13,6 +13,7 @@
 #include <evo/simplifiedmns.h>
 #include <evo/smldiff.h>
 #include <llmq/commitment.h>
+#include <rpc/evo_util.h>
 
 #include <univalue.h>
 
@@ -74,7 +75,7 @@
     if (IsServiceDeprecatedRPCEnabled()) {
         ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
     }
-    ret.pushKV("addresses", netInfo->ToJson());
+    ret.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
     ret.pushKV("ownerAddress", EncodeDestination(PKHash(keyIDOwner)));
     ret.pushKV("votingAddress", EncodeDestination(PKHash(keyIDVoting)));
     if (CTxDestination dest; ExtractDestination(scriptPayout, dest)) {
@@ -84,8 +85,8 @@
     ret.pushKV("operatorReward", (double)nOperatorReward / 100);
     if (nType == MnType::Evo) {
         ret.pushKV("platformNodeID", platformNodeID.ToString());
-        ret.pushKV("platformP2PPort", platformP2PPort);
-        ret.pushKV("platformHTTPPort", platformHTTPPort);
+        ret.pushKV("platformP2PPort", GetPlatformPort</*is_p2p=*/true>(*this));
+        ret.pushKV("platformHTTPPort", GetPlatformPort</*is_p2p=*/false>(*this));
     }
     ret.pushKV("inputsHash", inputsHash.ToString());
     return ret;
@@ -124,14 +125,14 @@
     if (IsServiceDeprecatedRPCEnabled()) {
         ret.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
     }
-    ret.pushKV("addresses", netInfo->ToJson());
+    ret.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
     if (CTxDestination dest; ExtractDestination(scriptOperatorPayout, dest)) {
         ret.pushKV("operatorPayoutAddress", EncodeDestination(dest));
     }
     if (nType == MnType::Evo) {
         ret.pushKV("platformNodeID", platformNodeID.ToString());
-        ret.pushKV("platformP2PPort", platformP2PPort);
-        ret.pushKV("platformHTTPPort", platformHTTPPort);
+        ret.pushKV("platformP2PPort", GetPlatformPort</*is_p2p=*/true>(*this));
+        ret.pushKV("platformHTTPPort", GetPlatformPort</*is_p2p=*/false>(*this));
     }
     ret.pushKV("inputsHash", inputsHash.ToString());
     return ret;
@@ -164,12 +165,12 @@
     if (IsServiceDeprecatedRPCEnabled()) {
         obj.pushKV("service", netInfo->GetPrimary().ToStringAddrPort());
     }
-    obj.pushKV("addresses", netInfo->ToJson());
+    obj.pushKV("addresses", GetNetInfoWithLegacyFields(*this, nType));
     obj.pushKV("pubKeyOperator", pubKeyOperator.ToString());
     obj.pushKV("votingAddress", EncodeDestination(PKHash(keyIDVoting)));
     obj.pushKV("isValid", isValid);
     if (nType == MnType::Evo) {
-        obj.pushKV("platformHTTPPort", platformHTTPPort);
+        obj.pushKV("platformHTTPPort", GetPlatformPort</*is_p2p=*/false>(*this));
         obj.pushKV("platformNodeID", platformNodeID.ToString());
     }
 

src/evo/deterministicmns.cpp

@@ -432,13 +432,12 @@ void CDeterministicMNList::AddMN(const CDeterministicMNCPtr& dmn, bool fBumpTota
         throw(std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate collateralOutpoint=%s", __func__,
                 dmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort())));
     }
-    for (const NetInfoEntry& entry : dmn->pdmnState->netInfo->GetEntries()) {
-        if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
-            const CService& service{service_opt.value()};
-            if (!AddUniqueProperty(*dmn, service)) {
+    for (const auto& entry : dmn->pdmnState->netInfo->GetEntries()) {
+        if (const auto service_opt{entry.GetAddrPort()}) {
+            if (!AddUniqueProperty(*dmn, *service_opt)) {
                 mnUniquePropertyMap = mnUniquePropertyMapSaved;
-                throw std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate address=%s",
-                                                   __func__, dmn->proTxHash.ToString(), service.ToStringAddrPort()));
+                throw std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate address=%s", __func__,
+                                                   dmn->proTxHash.ToString(), service_opt->ToStringAddrPort()));
             }
         } else {
             mnUniquePropertyMap = mnUniquePropertyMapSaved;
@@ -489,21 +488,19 @@ void CDeterministicMNList::UpdateMN(const CDeterministicMN& oldDmn, const std::s
             // We track each individual entry in netInfo as opposed to netInfo itself (preventing us from
             // using UpdateUniqueProperty()), so we need to successfully purge all old entries and insert
             // new entries to successfully update.
-            for (const NetInfoEntry& old_entry : oldInfo->GetEntries()) {
-                if (const auto& service_opt{old_entry.GetAddrPort()}) {
-                    const CService& service{service_opt.value()};
-                    if (!DeleteUniqueProperty(dmn, service)) {
+            for (const auto& old_entry : oldInfo->GetEntries()) {
+                if (const auto service_opt{old_entry.GetAddrPort()}) {
+                    if (!DeleteUniqueProperty(dmn, *service_opt)) {
                         return "internal error"; // This shouldn't be possible
                     }
                 } else {
                     return "invalid address";
                 }
             }
-            for (const NetInfoEntry& new_entry : newInfo->GetEntries()) {
-                if (const auto& service_opt{new_entry.GetAddrPort()}) {
-                    const CService& service{service_opt.value()};
-                    if (!AddUniqueProperty(dmn, service)) {
-                        return strprintf("duplicate (%s)", service.ToStringAddrPort());
+            for (const auto& new_entry : newInfo->GetEntries()) {
+                if (const auto service_opt{new_entry.GetAddrPort()}) {
+                    if (!AddUniqueProperty(dmn, *service_opt)) {
+                        return strprintf("duplicate (%s)", service_opt->ToStringAddrPort());
                     }
                 } else {
                     return "invalid address";
@@ -578,13 +575,12 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash)
         throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with a collateralOutpoint=%s", __func__,
                 proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort())));
     }
-    for (const NetInfoEntry& entry : dmn->pdmnState->netInfo->GetEntries()) {
-        if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
-            const CService& service{service_opt.value()};
-            if (!DeleteUniqueProperty(*dmn, service)) {
+    for (const auto& entry : dmn->pdmnState->netInfo->GetEntries()) {
+        if (const auto service_opt{entry.GetAddrPort()}) {
+            if (!DeleteUniqueProperty(*dmn, *service_opt)) {
                 mnUniquePropertyMap = mnUniquePropertyMapSaved;
                 throw std::runtime_error(strprintf("%s: Can't delete a masternode %s with an address=%s", __func__,
-                                                   proTxHash.ToString(), service.ToStringAddrPort()));
+                                                   proTxHash.ToString(), service_opt->ToStringAddrPort()));
             }
         } else {
             mnUniquePropertyMap = mnUniquePropertyMapSaved;
@@ -914,46 +910,49 @@ static bool CheckService(const ProTx& proTx, TxValidationState& state)
         return true;
     // Shouldn't be possible during self-checks
     case NetInfoStatus::BadInput:
+    case NetInfoStatus::Duplicate:
     case NetInfoStatus::MaxLimit:
         assert(false);
     } // no default case, so the compiler can warn about missing cases
     assert(false);
 }
 
 template <typename ProTx>
-static bool CheckPlatformFields(const ProTx& proTx, TxValidationState& state)
+static bool CheckPlatformFields(const ProTx& proTx, bool is_extended_addr, TxValidationState& state)
 {
     if (proTx.platformNodeID.IsNull()) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-platform-nodeid");
     }
 
-    // TODO: use real args here
-    static int mainnetPlatformP2PPort = CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN)->GetDefaultPlatformP2PPort();
-    if (Params().NetworkIDString() == CBaseChainParams::MAIN) {
-        if (proTx.platformP2PPort != mainnetPlatformP2PPort) {
+    if (is_extended_addr) {
+        // platformHTTPPort and platformP2PPort have been subsumed by netInfo. They should always be zero.
+        if (proTx.platformP2PPort != 0) {
             return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-platform-p2p-port");
         }
+        if (proTx.platformHTTPPort != 0) {
+            return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-platform-http-port");
+        }
+        return true;
     }
 
-    // TODO: use real args here
-    static int mainnetPlatformHTTPPort = CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN)->GetDefaultPlatformHTTPPort();
-    if (Params().NetworkIDString() == CBaseChainParams::MAIN) {
-        if (proTx.platformHTTPPort != mainnetPlatformHTTPPort) {
+    if (::IsNodeOnMainnet()) {
+        if (proTx.platformP2PPort != ::MainParams().GetDefaultPlatformP2PPort()) {
+            return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-platform-p2p-port");
+        }
+        if (proTx.platformHTTPPort != ::MainParams().GetDefaultPlatformHTTPPort()) {
             return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-platform-http-port");
         }
     }
-
-    // TODO: use real args here
-    static int mainnetDefaultP2PPort = CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN)->GetDefaultPort();
-    if (proTx.platformP2PPort == mainnetDefaultP2PPort) {
+    if (proTx.platformP2PPort == ::MainParams().GetDefaultPort()) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-platform-p2p-port");
     }
-    if (proTx.platformHTTPPort == mainnetDefaultP2PPort) {
+    if (proTx.platformHTTPPort == ::MainParams().GetDefaultPort()) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-platform-http-port");
     }
 
-    if (proTx.platformP2PPort == proTx.platformHTTPPort || proTx.platformP2PPort == proTx.netInfo->GetPrimary().GetPort() ||
-        proTx.platformHTTPPort == proTx.netInfo->GetPrimary().GetPort()) {
+    const uint16_t core_port{proTx.netInfo->GetPrimary().GetPort()};
+    if (proTx.platformP2PPort == proTx.platformHTTPPort || proTx.platformP2PPort == core_port ||
+        proTx.platformHTTPPort == core_port) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-platform-dup-ports");
     }
 
@@ -1065,7 +1064,7 @@ bool CheckProRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl:
     }
 
     if (opt_ptx->nType == MnType::Evo) {
-        if (!CheckPlatformFields(*opt_ptx, state)) {
+        if (!CheckPlatformFields(*opt_ptx, opt_ptx->nVersion >= ProTxVersion::ExtAddr, state)) {
             return false;
         }
     }
@@ -1119,11 +1118,10 @@ bool CheckProRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl:
         auto mnList = dmnman.GetListForBlock(pindexPrev);
 
         // only allow reusing of addresses when it's for the same collateral (which replaces the old MN)
-        for (const NetInfoEntry& entry : opt_ptx->netInfo->GetEntries()) {
-            if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
-                const CService& service{service_opt.value()};
-                if (mnList.HasUniqueProperty(service) &&
-                    mnList.GetUniquePropertyMN(service)->collateralOutpoint != collateralOutpoint) {
+        for (const auto& entry : opt_ptx->netInfo->GetEntries()) {
+            if (const auto service_opt{entry.GetAddrPort()}) {
+                if (mnList.HasUniqueProperty(*service_opt) &&
+                    mnList.GetUniquePropertyMN(*service_opt)->collateralOutpoint != collateralOutpoint) {
                     return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry");
                 }
             } else {
@@ -1185,7 +1183,7 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g
     }
 
     if (opt_ptx->nType == MnType::Evo) {
-        if (!CheckPlatformFields(*opt_ptx, state)) {
+        if (!CheckPlatformFields(*opt_ptx, opt_ptx->nVersion >= ProTxVersion::ExtAddr, state)) {
             return false;
         }
     }
@@ -1202,10 +1200,10 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g
     }
 
     // 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()) {
-            const CService& service{service_opt.value()};
-            if (mnList.HasUniqueProperty(service) && mnList.GetUniquePropertyMN(service)->proTxHash != opt_ptx->proTxHash) {
+    for (const auto& entry : opt_ptx->netInfo->GetEntries()) {
+        if (const auto service_opt{entry.GetAddrPort()}) {
+            if (mnList.HasUniqueProperty(*service_opt) &&
+                mnList.GetUniquePropertyMN(*service_opt)->proTxHash != opt_ptx->proTxHash) {
                 return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry");
             }
         } else {

src/evo/deterministicmns.h

@@ -434,6 +434,7 @@ class CDeterministicMNList
 #define DMNL_NO_TEMPLATE(name) \
     static_assert(!std::is_same_v<std::decay_t<T>, name>, "GetUniquePropertyHash cannot be templated against " #name)
         DMNL_NO_TEMPLATE(CBLSPublicKey);
+        DMNL_NO_TEMPLATE(ExtNetInfo);
         DMNL_NO_TEMPLATE(MnNetInfo);
         DMNL_NO_TEMPLATE(NetInfoEntry);
         DMNL_NO_TEMPLATE(NetInfoInterface);