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

[kwvg]

Motivation

Currently, we store an address and port pair for all masternodes and two port numbers for Evonodes. The first pair is used by Dash Core and the latter two port numbers are paired with the address from the first pair and are used by Dash Platform.

This arrangement has allowed the network to grow and sustain its current operations but proves to be rigid as it imposes the constraint that all advertised activity (Core P2P, Platform P2P and the Platform HTTPS API) happen only on one network (IPv4), from the same public IP (as we can only register one address).

This prevents specifying different networks (like IPv6), alternate addresses (e.g. on privacy-oriented networks), expanding to advertise other purposes or deferring resolution of the underlying address (e.g. specifying domain names). To allow for these use cases, the changes made to transaction, storage and state formats alongside changes made to RPC input and output fields are collectively referred to as "extended addresses".

This pull request includes the following:

• A basic extended addresses implementation that allows storing 4 addresses per purpose code, recognizing the following purpose codes, CORE_P2P, PLATFORM_P2P and PLATFORM_HTTPS.

• Support for specifying (arrays of) addr:port pairs to platformP2PAddrs (formerly known as platformP2PPort) and platformHTTPSAddrs (formerly known as platformHTTPPort).

• Compatibility code to allow

  • Reporting platformP2PPort and platformHTTPPort for extended address payloads even though they have been subsumed into netInfo

  • Reporting platformP2PPort and platformHTTPPort data from legacy address payloads through addresses even though they aren't stored in netInfo

  • Specifying only ports in platformP2PAddrs and platformHTTPSAddrs when using protx register{,_evo}, protx register_fund{,_evo} and protx update_service{,_evo} to create/update an extended addresses eligible masternode by reading the address of the first coreP2PAddrs entry and pairing it with the supplied port.

This pull request does not include the the full set of validation rules applicable on extended addresses as they have been reserved for a subsequent pull request. This pull request's scope is to lay down the base implementation, its surrounding compatibility code and tests to ensure its sanity.

Additional Information

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

• Depends on feat: Migrate CDeterministicMNStateDiff to a new format with nVersion being the first bit to read #6813

• The adoption of std::reference_wrapper (starting with dash#6627) has been reverted as while it there were performance considerations that led to its adoption, the risk of dangling references due to a race condition (e.g. iterating through GetEntries() while Clear() is called) are more pronounced for extended addresses.

  The underlying structures (NetInfoEntry, which will predominantly hold a CService) are not heavy enough to justify the usage of locking (i.e. mutexes). Making copies are deemed affordable enough for the safety that it provides.

• CDeterministicMNStateDiff is an append-only structure populated based on flags, which has made it a source of attention throughout work on extended addresses (see dash#6636). It is the reason NetInfoSerWrapper was introduced, as nVersion is placed after netInfo (source), which means, we cannot determine what format to use when deserializing based on the version of the format.

  To work around this, extended address payloads are prepended with the magic 0x23232323 (source) when serializing and deserialization will read the first four bytes to determine if the payload is extended or legacy. No longer true after dash#6813, thanks Udjin!

• As we require a flattened list of all addresses associated with a masternode in order to check it against the mempool or unique properties map (example), it would be inefficient to regenerate that list every time GetEntries() is called. To get around that, we use a memory-only cache, m_all_entries is used.

  It is populated when deserialized or added to when new entries are successful. This proves to be sufficient as ExtNetInfo is an append-only structure (i.e. they can either be added to with AddEntry() or Clear()ed).

  • This cache is also used for addr:port duplicates checking (while purpose-specific lists are used for addr duplicates checking)

• As rpc_netinfo.py is unlikely to use regular masternodes (as Platform-related fields can only be tested with Evonodes), non-Evonode code paths were removed and the following additional changes were made

  • Implementing the helper functions reconnect_nodes() and set_active_state(), the former to reconnect restarted nodes to their peers (which is not done automatically by the test framework) and the latter to restart the node to enable it in active masternode state (and/or optionally define extra arguments).

  • Fix a minor bug where destroy_mn() overwrote the ProTx hash of the destroyed masternode before checking for removal from the masternode list and logging it.

Breaking Changes

Refer to release notes.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/evo/netinfo.h (3)

8-13: Add required standard headers used in this header (fix ODR/fragile transitive includes).

This file uses std::map, std::vector, std::optional, std::shared_ptr, std::string_view, std::string, typeid, and assert but doesn't include their headers. Include what you use to prevent build fragility.

Apply:

 #include <netaddress.h>
 #include <serialize.h>
 #include <streams.h>

 #include <variant>
+#include <map>
+#include <memory>
+#include <optional>
+#include <string>
+#include <string_view>
+#include <typeinfo>
+#include <vector>
+#include <cassert>

---

120-131: Use the enum type for m_type (type-safety + clearer serialization).

m_type is uint8_t but compared against NetInfoEntry::NetInfoType. Make it NetInfoType to avoid accidental mismatches and improve readability.

-    uint8_t m_type{NetInfoType::Invalid};
+    NetInfoType m_type{NetInfoType::Invalid};

No other changes needed; (de)serialization already specializes NetInfoType.

Also applies to: 168-173

---

387-407: Enforce format/version match in NetInfoSerWrapper::Serialize to avoid wire‑format mismatches.

Currently serialization chooses by dynamic type, while deserialization uses m_is_extended. If m_is_extended=true but m_data holds MnNetInfo, you'll produce MnNetInfo bytes and later try to parse as ExtNetInfo → malformed payload risk.

Apply:

-        if (const auto ptr{std::dynamic_pointer_cast<ExtNetInfo>(m_data)}) {
-            s << *ptr;
-        } else if (const auto ptr{std::dynamic_pointer_cast<MnNetInfo>(m_data)}) {
-            s << *ptr;
-        } else {
-            assert(false);
-        }
+        if (m_is_extended) {
+            const auto ptr = std::dynamic_pointer_cast<ExtNetInfo>(m_data);
+            assert(ptr && "Expected ExtNetInfo when m_is_extended=true");
+            s << *ptr;
+        } else {
+            const auto ptr = std::dynamic_pointer_cast<MnNetInfo>(m_data);
+            assert(ptr && "Expected MnNetInfo when m_is_extended=false");
+            s << *ptr;
+        }

If you intentionally allow mismatch for some path, please justify and add a comment explaining why it’s safe.

♻️ Duplicate comments (1)

test/functional/rpc_netinfo.py (1)

73-81: BLS key toggle logic is correct (clean slate on restarts).

Copying self.node.extra_args is safe since it’s the original startup args and doesn’t accumulate across restarts.

🧹 Nitpick comments (20)

src/evo/netinfo.h (3)

100-111: Header comment nits: fix function name.

The comment references ToStringPortAddr but implementation uses ToStringAddrPort.

-/** Creates a one-element array using CService::ToStringPortAddr() output */
+/** Creates a one-element array using CService::ToStringAddrPort() output */

---

86-99: Return key material directly to avoid .data() ambiguity.

PurposeToString returns string_view but call sites pass .data() to UniValue::pushKV. Because these are string literals, it’s safe, but returning const char* removes the need for .data() everywhere.

Consider:

-constexpr std::string_view PurposeToString(const NetInfoPurpose purpose)
+constexpr const char* PurposeToString(const NetInfoPurpose purpose)

No behavior change; reduces chances of misuse.

---

344-348: IsEmpty() semantics on unknown versions.

IsEmpty() returns false when m_version != CURRENT_VERSION even if m_data is empty, which can be surprising. Callers typically care about presence of entries, not version. Consider basing IsEmpty solely on m_data.empty().

-    bool IsEmpty() const override { return m_version == CURRENT_VERSION && m_data.empty(); }
+    bool IsEmpty() const override { return m_data.empty(); }

If you keep current behavior, add a brief comment explaining the intent.

src/rpc/masternode.cpp (1)

570-574: Confirm “addr”/“full”/“info” modes should list all purposes, not just CORE_P2P.

Previously these modes exposed a single service address. Now they include all netInfo entries (including platform). If that’s intentional, great; otherwise restrict to CORE_P2P.

-            for (const auto& entry : dmn.pdmnState->netInfo->GetEntries()) {
+            for (const auto& entry : dmn.pdmnState->netInfo->GetEntries(NetInfoPurpose::CORE_P2P)) {
                 strAddress += entry.ToStringAddrPort() + " ";
             }

src/evo/dmnstate.cpp (1)

133-164: Diff JSON: fallback “addresses” construction is sensible; tiny robustness nits.

• Using 255.255.255.255 as unknown host is fine; a brief comment explains it.
• Consider using PurposeToString(...) directly (returning const char* as suggested) to avoid .data() churn.

-                netInfoObj.pushKV(PurposeToString(NetInfoPurpose::PLATFORM_P2P).data(),
+                netInfoObj.pushKV(PurposeToString(NetInfoPurpose::PLATFORM_P2P),
                                   (has_netinfo)
                                       ? ArrFromService(CService(state.netInfo->GetPrimary(), state.platformP2PPort))
                                       : unknownAddr(state.platformP2PPort));

Apply similarly for PLATFORM_HTTPS.

src/rpc/evo_util.h (1)

29-47: Guard against zero platform ports when augmenting legacy fields

If legacy ports are zero, pushing PLATFORM_HTTPS/PLATFORM_P2P here creates addr:0 JSON. Add a sanity check (early-return or skip) to avoid emitting invalid ports.

-    ret.pushKV(PurposeToString(NetInfoPurpose::PLATFORM_HTTPS).data(),
-               ArrFromService(CService(addr, obj.platformHTTPPort)));
+    if (obj.platformHTTPPort != 0) {
+        ret.pushKV(PurposeToString(NetInfoPurpose::PLATFORM_HTTPS).data(),
+                   ArrFromService(CService(addr, obj.platformHTTPPort)));
+    }
@@
-        ret.pushKV(PurposeToString(NetInfoPurpose::PLATFORM_P2P).data(),
-                   ArrFromService(CService(addr, obj.platformP2PPort)));
+        if (obj.platformP2PPort != 0) {
+            ret.pushKV(PurposeToString(NetInfoPurpose::PLATFORM_P2P).data(),
+                       ArrFromService(CService(addr, obj.platformP2PPort)));
+        }

src/rpc/evo_util.cpp (2)

13-14: Make IsNumeric reject empty strings

Empty strings currently return true, which skews downstream error messaging. Treat empty as non‑numeric.

-bool IsNumeric(std::string_view input) { return input.find_first_not_of("0123456789") == std::string::npos; }
+bool IsNumeric(std::string_view input) {
+    return !input.empty() && input.find_first_not_of("0123456789") == std::string::npos;
+}

---

67-107: Unify array element validation errors

The “must be string” error also triggers for empty strings due to IsNumeric("") quirk and lack of per‑entry emptiness check. After fixing IsNumeric, also reject empty per‑entry with a clearer message.

-                    if (!entry.isStr() || IsNumeric(entry.get_str())) {
+                    if (!entry.isStr() || entry.get_str().empty() || IsNumeric(entry.get_str())) {
                         throw JSONRPCError(RPC_INVALID_PARAMETER,
-                                           strprintf("Invalid param for %s[%zu], must be string", field_name, idx));
+                                           strprintf("Invalid param for %s[%zu], must be non-empty string", field_name, idx));
                     }

src/evo/deterministicmns.cpp (1)

921-960: Version‑aware platform checks: add brief comment on -1 primary port

For empty netInfo, core_port is 0; your logic is safe because platform ports can’t be 0. A short comment would help future readers.

-    const uint16_t core_port{proTx.netInfo->GetPrimary().GetPort()};
+    // For empty netInfo, GetPrimary().GetPort() is 0. Platform ports must be non-zero.
+    const uint16_t core_port{proTx.netInfo->GetPrimary().GetPort()};

src/rpc/evo.cpp (2)

186-201: Help text vs. type: say “empty array” (not “empty string”)

Args are documented as arrays; suggest updating wording to avoid confusion.

-                "Must be unique on the network. Can be set to an empty string, which will require a ProUpServTx afterwards.",
+                "Must be unique on the network. Can be set to an empty array or empty string, which will require a ProUpServTx afterwards.",

Apply similarly to platformHTTPSAddrs(_update).

---

589-596: Examples still show numeric ports for platform fields

Examples pass raw numbers; now fields are “Addrs”. Keep numbers supported, but add an example with "ADDR:PORT" (and/or JSON array) to align with help.

src/evo/netinfo.cpp (1)

196-219: MnNetInfo: reject non‑CORE_P2P/overflow cleanly

Returning MaxLimit for wrong purpose works but is semantically odd; consider a distinct status like BadInput for wrong purpose vs MaxLimit for count overflow.

test/functional/rpc_netinfo.py (4)

82-90: Use non-predictable randomness for platform_nodeid (nit).

Replace randint(...) with secrets to avoid predictable values in tests.

-from random import randint
+import secrets
...
-        self.platform_nodeid = hash160(b'%d' % randint(1, 65535)).hex()
+        self.platform_nodeid = hash160(secrets.token_bytes(2)).hex()

---

154-160: Avoid truthiness for port checks; use explicit None.

-1 would pass the truthiness test accidentally. Check “is not None”.

-        if plat_https_port:
+        if plat_https_port is not None:
             assert_equal(val['platform_https'][0], f"127.0.0.1:{plat_https_port}")
-        if plat_p2p_port:
+        if plat_p2p_port is not None:
             assert_equal(val['platform_p2p'][0], f"127.0.0.1:{plat_p2p_port}")

---

161-166: Harden listdiff parsing.

Guard against missing keys or unexpected shapes to improve failure messages.

-        for arr in rpc_output['updatedMNs']:
+        for arr in rpc_output.get('updatedMNs', []):
             if proregtx_hash in arr.keys():
                 return arr[proregtx_hash]
-        raise AssertionError(f"Unable to find {proregtx_hash} in array")
+        raise AssertionError(f"Unable to find proTxHash={proregtx_hash} in updatedMNs")

---

114-124: Handle empty UTXO edge-case when destroying MN.

listunspent(...)[0] can IndexError if fees were spent; add a check for clearer failure.

-        address_funds_unspent = self.node.listunspent(0, 99999, [self.mn.fundsAddr])[0]
+        utxos = self.node.listunspent(0, 99999, [self.mn.fundsAddr])
+        assert utxos, "No UTXOs found for fundsAddr; cannot destroy MN"
+        address_funds_unspent = utxos[0]

test/functional/test_framework/test_framework.py (4)

1223-1235: Centralized Evo input validation is good; consider leaner messages.

Messages are long; tests only need concise context for assert_raises checks.

---

1248-1254: DRY: factor core P2P defaulting into a helper.

Same “[f'127.0.0.1:{self.nodePort}'] if addrs_core_p2p is None else addrs_core_p2p” appears in three methods.

+    def _core_p2p_or_default(self, addrs_core_p2p):
+        return [f'127.0.0.1:{self.nodePort}'] if addrs_core_p2p is None else addrs_core_p2p
...
-            [f'127.0.0.1:{self.nodePort}'] if addrs_core_p2p is None else addrs_core_p2p,
+            self._core_p2p_or_default(addrs_core_p2p),
...
-            [f'127.0.0.1:{self.nodePort}'] if addrs_core_p2p is None else addrs_core_p2p,
+            self._core_p2p_or_default(addrs_core_p2p),
...
-            [f'127.0.0.1:{self.nodePort}'] if addrs_core_p2p is None else addrs_core_p2p,
+            self._core_p2p_or_default(addrs_core_p2p),

Also applies to: 1301-1308, 1435-1438

---

1266-1272: Minor style: prefer iterable unpacking for list assembly.

Reduces list copies and silences RUF005.

-            args = args + [platform_node_id, addrs_platform_p2p, addrs_platform_https, address_funds, submit]
+            args += [platform_node_id, addrs_platform_p2p, addrs_platform_https, address_funds, submit]

Also applies to: 1313-1318, 1443-1449

---

1631-1632: Naming: “addrs_platform_p2p/https” hold ints in some tests.

Fine per Union type, but consider “platform_p2p_param/platform_https_param” in helpers to reflect “int | str | List[str]”.

Also applies to: 1665-1666

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c26937d and 90713a2.

📒 Files selected for processing (29)

• contrib/seeds/makeseeds.py (1 hunks)
• doc/release-notes-6666.md (1 hunks)
• src/coinjoin/client.cpp (2 hunks)
• src/evo/core_write.cpp (5 hunks)
• src/evo/deterministicmns.cpp (8 hunks)
• src/evo/deterministicmns.h (1 hunks)
• src/evo/dmnstate.cpp (5 hunks)
• src/evo/dmnstate.h (1 hunks)
• src/evo/netinfo.cpp (6 hunks)
• src/evo/netinfo.h (11 hunks)
• src/evo/providertx.cpp (5 hunks)
• src/evo/providertx.h (2 hunks)
• src/evo/simplifiedmns.cpp (1 hunks)
• src/evo/simplifiedmns.h (2 hunks)
• src/evo/specialtxman.cpp (3 hunks)
• src/rpc/coinjoin.cpp (1 hunks)
• src/rpc/evo.cpp (7 hunks)
• src/rpc/evo_util.cpp (1 hunks)
• src/rpc/evo_util.h (1 hunks)
• src/rpc/masternode.cpp (3 hunks)
• src/rpc/quorums.cpp (2 hunks)
• src/test/block_reward_reallocation_tests.cpp (1 hunks)
• src/test/evo_deterministicmns_tests.cpp (6 hunks)
• src/test/evo_netinfo_tests.cpp (4 hunks)
• src/test/evo_simplifiedmns_tests.cpp (1 hunks)
• test/functional/feature_dip3_deterministicmns.py (1 hunks)
• test/functional/rpc_netinfo.py (8 hunks)
• test/functional/rpc_quorum.py (1 hunks)
• test/functional/test_framework/test_framework.py (12 hunks)

🚧 Files skipped from review as they are similar to previous changes (17)

• src/test/evo_deterministicmns_tests.cpp
• src/evo/deterministicmns.h
• test/functional/rpc_quorum.py
• contrib/seeds/makeseeds.py
• src/test/evo_simplifiedmns_tests.cpp
• src/test/block_reward_reallocation_tests.cpp
• src/rpc/quorums.cpp
• src/evo/providertx.h
• src/evo/dmnstate.h
• src/evo/specialtxman.cpp
• doc/release-notes-6666.md
• src/evo/simplifiedmns.h
• src/coinjoin/client.cpp
• src/rpc/coinjoin.cpp
• src/evo/simplifiedmns.cpp
• src/test/evo_netinfo_tests.cpp
• test/functional/feature_dip3_deterministicmns.py

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

• src/evo/providertx.cpp
• src/evo/netinfo.h
• src/rpc/evo.cpp
• src/rpc/masternode.cpp
• src/rpc/evo_util.h
• src/evo/dmnstate.cpp
• src/evo/deterministicmns.cpp
• src/evo/netinfo.cpp
• src/evo/core_write.cpp
• src/rpc/evo_util.cpp

src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

Masternode lists must use immutable data structures (Immer library) for thread safety

Files:

• src/evo/providertx.cpp
• src/evo/netinfo.h
• src/evo/dmnstate.cpp
• src/evo/deterministicmns.cpp
• src/evo/netinfo.cpp
• src/evo/core_write.cpp

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests should be written in Python and placed in test/functional/

Files:

• test/functional/test_framework/test_framework.py
• test/functional/rpc_netinfo.py

🧠 Learnings (11)

📓 Common learnings

Learnt from: kwvg
PR: dashpay/dash#6665
File: src/evo/deterministicmns.cpp:1284-1287
Timestamp: 2025-06-16T17:59:55.669Z
Learning: In Dash masternode ProRegTx validation, platform ports (platformHTTPPort and platformP2PPort) are mandatory and must be non-zero, while netInfo (ipAndPort) is optional. This means that even if an empty netInfo returns 0 from GetPrimary().GetPort(), it won't cause false positives in port duplication checks since platform ports cannot be 0.

📚 Learning: 2025-06-06T11:53:09.094Z

Learnt from: kwvg
PR: dashpay/dash#6665
File: src/evo/providertx.h:82-82
Timestamp: 2025-06-06T11:53:09.094Z
Learning: In ProTx serialization code (SERIALIZE_METHODS), version checks should use hardcoded maximum flags (/*is_basic_scheme_active=*/true, /*is_extended_addr=*/true) rather than deployment-based flags. This is because serialization code should be able to deserialize any structurally valid ProTx up to the maximum version the code knows how to handle, regardless of current consensus validity. Validation code, not serialization code, is responsible for checking whether a ProTx version is consensus-valid based on deployment status.

Applied to files:

• src/evo/providertx.cpp
• src/evo/deterministicmns.cpp

📚 Learning: 2025-07-09T15:02:26.899Z

Learnt from: kwvg
PR: dashpay/dash#6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, `IsVersionChangeValid()` is only called by transaction types that update existing masternode entries (like `ProUpServTx`, `ProUpRegTx`, `ProUpRevTx`), not by `ProRegTx` which creates new entries. This means validation logic in `IsVersionChangeValid()` only applies to the subset of transaction types that actually call it, not all masternode transaction types.

Applied to files:

• src/evo/providertx.cpp

📚 Learning: 2025-05-10T00:54:15.597Z

Learnt from: kwvg
PR: dashpay/dash#6629
File: src/evo/deterministicmns.cpp:473-485
Timestamp: 2025-05-10T00:54:15.597Z
Learning: In the current implementation of `NetInfoEntry`, only `CService` is a valid type and `std::monostate` represents an invalid state. Entries that don't provide a valid `CService` through `GetAddrPort()` should be rejected with an exception, not silently skipped.

Applied to files:

• src/evo/providertx.cpp
• src/evo/netinfo.h
• src/evo/deterministicmns.cpp
• src/evo/netinfo.cpp

📚 Learning: 2025-05-10T00:55:16.184Z

Learnt from: kwvg
PR: dashpay/dash#6629
File: src/test/evo_netinfo_tests.cpp:39-45
Timestamp: 2025-05-10T00:55:16.184Z
Learning: In Dash's validation testing for network entries, checking `IsTriviallyValid()` is specifically used to ensure all three validation methods (`AddEntry()`, `Validate()`, and `IsTriviallyValid()`) consistently work together rather than replacing one check with another. `AddEntry()` and `Validate()` handle the consensus rule checks, while `IsTriviallyValid()` confirms the basic validation steps.

Applied to files:

• src/evo/providertx.cpp

📚 Learning: 2025-06-09T16:43:20.996Z

Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Applied to files:

• test/functional/test_framework/test_framework.py
• src/rpc/masternode.cpp
• src/rpc/evo_util.cpp
• test/functional/rpc_netinfo.py

📚 Learning: 2025-06-16T17:59:55.669Z

Learnt from: kwvg
PR: dashpay/dash#6665
File: src/evo/deterministicmns.cpp:1284-1287
Timestamp: 2025-06-16T17:59:55.669Z
Learning: In Dash masternode ProRegTx validation, platform ports (platformHTTPPort and platformP2PPort) are mandatory and must be non-zero, while netInfo (ipAndPort) is optional. This means that even if an empty netInfo returns 0 from GetPrimary().GetPort(), it won't cause false positives in port duplication checks since platform ports cannot be 0.

Applied to files:

• test/functional/test_framework/test_framework.py
• src/rpc/evo.cpp
• src/evo/deterministicmns.cpp
• src/rpc/evo_util.cpp
• test/functional/rpc_netinfo.py

📚 Learning: 2025-07-20T18:42:49.794Z

Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety

Applied to files:

• src/rpc/masternode.cpp

📚 Learning: 2025-07-20T18:42:49.794Z

Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h

Applied to files:

• src/rpc/masternode.cpp

📚 Learning: 2025-08-08T04:30:37.971Z

Learnt from: PastaPastaPasta
PR: dashpay/dash#6804
File: src/qt/proposalwizard.cpp:0-0
Timestamp: 2025-08-08T04:30:37.971Z
Learning: In the Dash codebase, direct RPC calling has been removed in recent commits, making suggestions about RPC command validation and error handling for direct RPC calls obsolete. The ProposalWizard and related components no longer use direct RPC calling patterns.

Applied to files:

• src/rpc/evo_util.cpp
• test/functional/rpc_netinfo.py

📚 Learning: 2025-08-10T13:52:46.289Z

Learnt from: kwvg
PR: dashpay/dash#6666
File: test/functional/rpc_netinfo.py:73-81
Timestamp: 2025-08-10T13:52:46.289Z
Learning: In the Bitcoin/Dash test framework, `self.node.extra_args` contains the original startup arguments for a TestNode and is not modified by `test.restart_node()`. Each restart with new `extra_args` doesn't update the stored `self.node.extra_args`, so copying from it always provides a clean slate without accumulated modifications from previous restarts.

Applied to files:

• test/functional/rpc_netinfo.py

🧬 Code graph analysis (9)

src/evo/providertx.cpp (3)

src/evo/deterministicmns.h (2)

• nType (51-61)
• nOperatorReward (50-50)

src/evo/providertx.h (7)

• nOperatorReward (74-74)
• platformNodeID (68-68)
• platformNodeID (137-137)
• platformP2PPort (69-69)
• platformP2PPort (138-138)
• platformHTTPPort (70-70)
• platformHTTPPort (139-139)

src/evo/dmnstate.h (3)

• platformNodeID (63-63)
• platformP2PPort (64-64)
• platformHTTPPort (65-65)

test/functional/test_framework/test_framework.py (1)

test/functional/test_framework/test_node.py (2)

• TestNode (58-795)
• generate (347-349)

src/evo/netinfo.h (3)

src/chainparams.h (1)

• CChainParams (71-193)

src/univalue/include/univalue.h (1)

• UniValue (19-23)

src/evo/netinfo.cpp (52)

• IsNodeOnMainnet (30-30)
• IsNodeOnMainnet (30-30)
• IsServiceDeprecatedRPCEnabled (45-49)
• IsServiceDeprecatedRPCEnabled (45-45)
• ArrFromService (38-43)
• ArrFromService (38-38)
• MainParams (32-36)
• MainParams (32-32)
• GetAddrPort (81-88)
• GetAddrPort (81-81)
• AddEntry (196-219)
• AddEntry (196-196)
• AddEntry (355-379)
• AddEntry (355-355)
• GetEntries (221-230)
• GetEntries (221-221)
• GetEntries (381-392)
• GetEntries (381-381)
• GetPrimary (232-238)
• GetPrimary (232-232)
• GetPrimary (394-407)
• GetPrimary (394-394)
• HasEntries (409-414)
• HasEntries (409-409)
• Validate (240-246)
• Validate (240-240)
• Validate (416-452)
• Validate (416-416)
• ToJson (248-263)
• ToJson (248-248)
• ToJson (454-474)
• ToJson (454-454)
• HasAddrPortDuplicates (272-282)
• HasAddrPortDuplicates (272-272)
• IsAddrPortDuplicate (284-288)
• IsAddrPortDuplicate (284-284)
• HasAddrDuplicates (290-300)
• HasAddrDuplicates (290-290)
• IsAddrDuplicate (302-307)
• IsAddrDuplicate (302-302)
• ProcessCandidate (309-335)
• ProcessCandidate (309-309)
• ValidateService (175-194)
• ValidateService (175-175)
• ValidateService (337-353)
• ValidateService (337-337)
• ToString (127-138)
• ToString (127-127)
• ToString (265-270)
• ToString (265-265)
• ToString (476-492)
• ToString (476-476)

src/rpc/evo.cpp (1)

src/rpc/evo_util.cpp (4)

• ProcessNetInfoPlatform (63-136)
• ProcessNetInfoPlatform (63-63)
• ProcessNetInfoPlatform (137-137)
• ProcessNetInfoPlatform (138-138)

src/rpc/evo_util.h (4)

src/rpc/evo_util.cpp (8)

• ProcessNetInfoCore (35-58)
• ProcessNetInfoCore (35-35)
• ProcessNetInfoCore (59-59)
• ProcessNetInfoCore (60-60)
• ProcessNetInfoPlatform (63-136)
• ProcessNetInfoPlatform (63-63)
• ProcessNetInfoPlatform (137-137)
• ProcessNetInfoPlatform (138-138)

src/univalue/include/univalue.h (1)

• UniValue (19-23)

src/evo/netinfo.cpp (4)

• obj (40-40)
• ret (465-465)
• ArrFromService (38-43)
• ArrFromService (38-38)

src/evo/netinfo.h (1)

• CService (198-208)

src/evo/dmnstate.cpp (2)

src/evo/netinfo.cpp (3)

• obj (40-40)
• ArrFromService (38-43)
• ArrFromService (38-38)

src/evo/netinfo.h (1)

• CService (198-208)

src/evo/deterministicmns.cpp (1)

src/evo/netinfo.cpp (4)

• IsNodeOnMainnet (30-30)
• IsNodeOnMainnet (30-30)
• MainParams (32-36)
• MainParams (32-32)

src/evo/netinfo.cpp (1)

src/evo/netinfo.h (8)

• GetPrimary (256-257)
• GetPrimary (344-346)
• HasAddrPortDuplicates (284-323)
• m_all_entries (306-306)
• MAX_ENTRIES_EXTNETINFO (20-20)
• IsValidPurpose (75-84)
• Validate (260-275)
• Validate (348-368)

test/functional/rpc_netinfo.py (3)

test/functional/test_framework/util.py (3)

• assert_equal (69-74)
• softfork_active (509-511)
• p2p_port (347-349)

test/functional/test_framework/test_framework.py (16)

• MasternodeInfo (1142-1458)
• generate_addresses (1164-1178)
• set_node (1211-1214)
• set_params (1198-1209)
• test (2384-2390)
• BitcoinTestFramework (101-1137)
• get_collateral_value (1180-1181)
• bump_mocktime (877-894)
• generate (803-806)
• no_op (800-801)
• restart_node (691-694)
• register (1236-1281)
• bury_tx (1160-1162)
• update_service (1413-1458)
• connect_nodes (699-739)
• connect_nodes (1485-1492)

test/functional/test_framework/test_node.py (1)

• TestNode (58-795)

🪛 Ruff (0.12.2)

test/functional/test_framework/test_framework.py

1227-1227: Avoid specifying long messages outside the exception class

(TRY003)

---

1230-1230: Avoid specifying long messages outside the exception class

(TRY003)

---

1232-1232: Avoid specifying long messages outside the exception class

(TRY003)

---

1234-1234: Avoid specifying long messages outside the exception class

(TRY003)

---

1268-1268: Consider iterable unpacking instead of concatenation

(RUF005)

---

1314-1314: Consider iterable unpacking instead of concatenation

(RUF005)

---

1445-1445: Consider iterable unpacking instead of concatenation

(RUF005)

test/functional/rpc_netinfo.py

84-84: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

---

165-165: Avoid specifying long messages outside the exception class

(TRY003)

🪛 GitHub Actions: Clang Diff Format Check

src/evo/netinfo.h

[error] 1-1: Clang format differences found in src/evo/netinfo.h. Run clang-format to fix code formatting.

src/rpc/evo.cpp

[error] 1-1: Clang format differences found in src/rpc/evo.cpp. Run clang-format to fix code formatting.

src/evo/netinfo.cpp

[error] 1-1: Clang format differences found in src/evo/netinfo.cpp. Run clang-format to fix code formatting.

src/rpc/evo_util.cpp

[error] 1-1: Clang format differences found in src/rpc/evo_util.cpp. Run clang-format to fix code formatting.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Lint / Run linters
• GitHub Check: x86_64-apple-darwin / Build depends
• GitHub Check: x86_64-w64-mingw32 / Build depends
• GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
• GitHub Check: arm-linux-gnueabihf / Build depends
• GitHub Check: x86_64-pc-linux-gnu / Build depends
• GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends

🔇 Additional comments (28)

src/rpc/masternode.cpp (2)

617-621: Good: keep legacy “address” gated behind -deprecatedrpc=service, migrate new “addresses”.

Looks consistent with evo_util helpers and PR goals.

---

626-628: OK to expose platform ports via helper.

GetPlatformPort<> centralizes legacy vs extended mapping here.

src/evo/core_write.cpp (3)

68-93: ProRegTx JSON migration to addresses helper looks correct.

Legacy “service” kept under deprecation flag; new “addresses” via GetNetInfoWithLegacyFields is aligned with the new model.

---

119-139: ProUpServTx JSON: same migration pattern LGTM.

Centralizing port exposure via GetPlatformPort reduces drift.

---

158-176: CSimplifiedMNListEntry JSON: minimal platform exposure is appropriate.

Only HTTP port is serialized (as before). Using helpers maintains consistency.

src/evo/dmnstate.cpp (2)

26-33: Include netInfo in ToString: LGTM.

Concise and places netInfo centrally; helps debugging.

---

39-56: JSON migration: legacy “service” + new “addresses” via helper.

Matches RPC changes elsewhere.

src/evo/providertx.cpp (4)

35-57: Centralized netInfo “trivial” validation is a good addition.

• Enforces CORE_P2P presence
• Guards regular vs Evo purpose usage
• Requires both platform purposes when using extended addresses on Evo

Please confirm functional tests cover:

• Regular MN with accidental platform entries → rejected
• Evo MN (ExtAddr) missing one of platform purposes → rejected
• Evo MN legacy (pre-ExtAddr) path remains accepted with legacy ports

---

86-94: CProRegTx: Only validate netInfo when non-empty.

Matches PR scope (full validation deferred). Ensure consensus-layer validation elsewhere enforces legacy platform port constraints for pre-ExtAddr versions.

If not already covered, add a functional test asserting ProRegTx (pre-ExtAddr) with zero platform ports is rejected.

---

146-154: ToString: version-aware formatting looks correct.

Shows netInfo always, and legacy ports only below ExtAddr.

---

170-178: CProUpServTx: netInfo presence enforced + per-entry trivial checks.

Good balance for this PR.

src/evo/netinfo.h (1)

1-1: Fix clang-format issues flagged by CI — run clang-format on src/evo/netinfo.h

Verification failed in the sandbox: clang-format not found (/bin/bash: clang-format: command not found). Run clang-format -i src/evo/netinfo.h locally and commit the formatted file (or provide the formatted diff).

src/rpc/evo_util.h (1)

52-86: Clarify sentinel return and avoid potential UB on empty entries

GetPlatformPort returns -1 as “blank”. Ensure all call sites treat -1 as “absent” and never index [0] when HasEntries() might be false during refactors.

Would you like a quick grep script to assert no callers assume non‑negative or index without checking HasEntries()?

src/rpc/evo_util.cpp (1)

134-135: Double-check: platform arrays accepted on legacy ProTx versions

process_field correctly bails out with “only supports ports” when CanStorePlatform()==false. Confirm tests cover: array passed against legacy version → RPC_INVALID_PARAMETER.

src/evo/deterministicmns.cpp (4)

435-447: Address validation path looks solid

Using entry.GetAddrPort() with explicit invalid-address rejection matches NetInfoEntry contract. Good.

---

491-509: Update path maintains uniqueness atomically

Delete old, add new with rollback on failure is correct for mnUniquePropertyMap.

---

913-916: Include Duplicate in “shouldn’t happen” bucket is fine

NetInfo self‑checks should prevent Duplicate here; assert path is appropriate.

---

1121-1129: Dup‑address check uses per‑entry services correctly

Checks both existence and ownership mismatch; good.

src/rpc/evo.cpp (2)

761-763: Optionality matches registration flow

Passing optional=true for Evo register is correct; arrays may be empty only if netInfo is empty. Good.

---

1031-1033: Update path requires non-empty: correct

optional=false aligns with semantics for updates. Good.

src/evo/netinfo.cpp (4)

30-36: Public helpers should be declared in a header

If IsNodeOnMainnet/MainParams are used cross‑TU, ensure their declarations live in a header (e.g., netinfo.h) to avoid ODR/visibility issues.

Would you like a quick repo scan to confirm declarations exist and there are no duplicate definitions?

---

272-335: ExtNetInfo duplicate rules look correct

• Global addr:port dedupe across all purposes (m_all_entries)
• Per‑purpose addr dedupe
  Append-only cache (m_all_entries) is safe.

---

355-379: Disallow assumed ports: good

Defaulting port=0 to force BadPort is the right call. Nice fast‑fail on invalid characters.

---

416-452: Validate() covers malformed, duplicates, and per‑entry service validation

Comprehensive and in the right order. LGTM.

test/functional/rpc_netinfo.py (3)

389-399: Good three-step update sequence.

Covers both normal update, revert, and platform-only change to exercise dummy address behavior. Nicely scoped.

---

421-433: Assertions match intended shim behavior.

Checks for presence/absence and dummy address are precise and future-proof.

---

513-538: Solid shim validation for cross-field propagation.

IPv6 + mixed inputs ensure the extended-address path is actually exercised.

test/functional/test_framework/test_framework.py (1)

1711-1716: Legacy MN flows still work with new signatures.

Coverage for register_fund/register with addrs_core_p2p is preserved and clear.

Also applies to: 1724-1727

[knst]

LGTM 90713a2