docs: document the extended addresses format in Appendix C of DIP-0003, add new {ProTx, SML} version to support it, add deprecation notices #164
Conversation
Type 0 masternodes are referred to as "default" and "regular" in different documents, let's harmonize them.
| | platformP2PPort | 26656 | | ||
| | platformHTTPPort | 443 | | ||
|
|
||
| ## Extended Format |
There was a problem hiding this comment.
maybe reorder Extended Format and Legacy Format to put "Extenden" on the top; at the beginning add a small disclaimer, something like "for legacy format used by versions 1&2 see below"
There was a problem hiding this comment.
I placed the legacy format description first as it is shorter and is referenced by other documents but I'm not opposed to the idea. @thephez, thoughts?
There was a problem hiding this comment.
I like the idea of a disclaimer just under the "Legacy Format" heading with a link to the extended format section. I think the ordering is okay as-is imo.
|
|
||
| | Field | Type | Size | Description | | ||
| | --------- | ------- | -------- | ---------------------------------------------------------------------------------------------- | | ||
| | p_count | uint_8 | 1 | Number of purposes for which addresses are defined | |
There was a problem hiding this comment.
maybe better to use "compact length" for this type? It takes 1 byte for small numbers, but is not limited by 255.
Same for purpose and count below
There was a problem hiding this comment.
A purpose represents a set of up to 32 entries. Currently we have three, CORE_P2P, PLATFORM_HTTPS and PLATFORM_P2P. If we need more than 255 purposes (i.e. storing up to 8,160 unique addresses per masternode), a revisiting of the spec overall seems to be more in order.
Allowing 32 entries may also be too much and I'm looking for feedback to gauge if 32 is adequate, insufficient or excessive.
| | platformNodeID | byte[] | 0 or 20 bytes | Dash Platform node ID, derived from P2P public key. Only present for masternode type 1. | Only when `version` is 2 and `type` is 1. | | ||
| | platformP2PPort | uint_16 | 0 or 2 bytes | TCP port of Dash Platform peer-to-peer communication between nodes (network byte order). Only present for masternode type 1. | Only when `version` is 2 and `type` is 1. | | ||
| | platformHTTPPort | uint_16 | 0 or 2 bytes | TCP port of Platform HTTP/API interface (network byte order). Only present for masternode type 1. | Only when `version` is 2 and `type` is 1. | | ||
| | platformInfo | byte[] | variable | Platform network information (see [appendix](dip-0003.md#appendix-c-network-information)). Only present for masternode type 1. | Only when `version` is 2 and `type` is 1. | |
There was a problem hiding this comment.
nit: s/platformInfo/PlatformNetwork/ or something similar.
Because platformNodeID is also some kind of info, but here meant only network information.
There was a problem hiding this comment.
Wouldn't be opposed to renaming it to platformNetInfo, thoughts?
|
|
||
| | Field | Type | Size | Description | Serialized | | ||
| | --- | --- | --- | --- | --- | | ||
| | type | uint_16 | 0 or 2 bytes | Masternode type. 0 for regular masternode, 1 for evonode. | Upon activation of this DIP. Only when `version` is 2. | |
There was a problem hiding this comment.
| | type | uint_16 | 0 or 2 bytes | Masternode type. 0 for regular masternode, 1 for evonode. | Upon activation of this DIP. Only when `version` is 2. | | |
| | type | uint_16 | 0 or 2 bytes | Masternode type. 0 for regular masternode, 1 for evonode. | Upon activation of this DIP. Only when `version` is greater than 2. | |
| | Field | Type | Size | Description | Serialized | | ||
| | --- | --- | --- | --- | --- | | ||
| | type | uint_16 | 0 or 2 bytes | Masternode type. 0 for regular masternode, 1 for evonode. | Upon activation of this DIP. Only when `version` is 2. | | ||
| | platformHTTPPort | uint_16 | 0 or 2 bytes | TCP port of Platform HTTP/API interface (network byte order). Only present for masternode type 1. | Only when `version` is 2 and `type` is 1. | |
There was a problem hiding this comment.
| | platformHTTPPort | uint_16 | 0 or 2 bytes | TCP port of Platform HTTP/API interface (network byte order). Only present for masternode type 1. | Only when `version` is 2 and `type` is 1. | | |
| | platformHTTPPort | uint_16 | 0 or 2 bytes | TCP port of Platform HTTP/API interface (network byte order). Only present for masternode type 1. | Only when `version` is greater than 2 and `type` is 1. | |
There was a problem hiding this comment.
platformHTTPPort is replaced by netInfo[PLATFORM_HTTPS] so the field will be removed effective version 3
| | ------- | ------- | -------- | --------------------------------------------------------------- | | ||
| | type | uint_8 | 1 | Network identifier | | ||
| | address | byte[] | variable | Address of `type` that can be used to connect to the masternode | | ||
| | port | uint_16 | 2 | Port (network byte order) | |
There was a problem hiding this comment.
It was surprising for me, that port is part of ID for i2p network.
Strangely, but port is used by all types of network: ipv4, ipv6, tor v2&3, i2p, cjdns and yggdrasil, and what is more surprising it's also 2 bytes. I guess it because most protocols use udp as transport level for delivery.
So, I guess it is fine, that port is always exist and always 2 bytes and no need to make it conditional on type.
|
#162 has been merged now |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe documentation for masternode network information and transaction formats was overhauled to unify and extend address handling. IP address and port fields were replaced by a generalized Changes
Sequence Diagram(s)sequenceDiagram
participant Operator
participant ProRegTx/ProUpServTx
participant MasternodeList
participant Validator
Operator->>ProRegTx/ProUpServTx: Submit registration/update with networkInfo
ProRegTx/ProUpServTx->>Validator: Validate networkInfo/platformInfo fields (per Appendix C)
Validator-->>ProRegTx/ProUpServTx: Accept or reject based on validation
ProRegTx/ProUpServTx->>MasternodeList: Update SML entry with networkInfo
MasternodeList-->>Operator: Reflects new unified address format
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
dip-0028.md (1)
232-246: Avoid repeating the same deprecation block three timesThe identical
[!WARNING] platformInfo is deprecated …call-out is inserted for ProRegTx, ProUpServTx, and validation-rules sections. Consider extracting it once (e.g., right after the “Platform Information” subsection heading) to reduce noise and future maintenance overhead.dip-0004.md (2)
99-111: “HTTP/API interface” tautologyLanguage-tool correctly points out that “API” already contains “interface”. Suggest trimming to “Platform HTTP API” for brevity.
-| platformHTTPPort | uint_16 | 0 or 2 | TCP port of Platform HTTP/API interface (network byte order). Only present for masternode type 1. | +| platformHTTPPort | uint_16 | 0 or 2 | TCP port of the Platform HTTP API (network byte order). Only present for masternode type 1. |
112-119: Duplicate word “indicates”Minor style nit: the sentence starting “The
nVersionfield indicates indicates …” contains a duplicated word.dip-0003/network-info.md (1)
239-244: Upper-case letters prohibition may be too strictDNS is case-insensitive; forbidding upper-case makes validation stricter than RFC 1035, and may reject otherwise valid hostnames supplied by operators. Consider allowing mixed-case but normalise to lower-case during validation/comparison.
dip-0003.md (2)
270-286: Validation rule numbering skips condition foroperatorReward > 10000Rules 5–10 use bullet “1.” etc.; after adding the new rules, numbering is off (Markdown auto-numbering handles this, but the spec mixes manual and automatic). Renumber for readability.
327-331: Successive sentences start with “If the last EvoNode …”Minor style: avoid repetitive sentence starts; combine or rephrase for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dip-0003.md(11 hunks)dip-0003/masternode-types.md(1 hunks)dip-0003/network-info.md(1 hunks)dip-0004.md(1 hunks)dip-0028.md(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
dip-0003.md
271-271: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
dip-0003/network-info.md
66-66: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
67-67: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
68-68: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
69-69: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
70-70: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
71-71: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
77-77: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
78-78: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
79-79: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
80-80: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
82-82: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
83-83: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
84-84: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
85-85: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
86-86: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
87-87: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
88-88: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
89-89: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
90-90: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
92-92: Unordered list indentation
Expected: 6; Actual: 8
(MD007, ul-indent)
116-116: Link fragments should be valid
(MD051, link-fragments)
252-252: Multiple top-level headings in the same document
(MD025, single-title, single-h1)
🪛 LanguageTool
dip-0003.md
[style] ~329-~329: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...aid 4 blocks in a row, go to step 5. 3. If the last EvoNode is not present in the ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
dip-0003/network-info.md
[style] ~34-~34: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “API”.
Context: ...16 | 0 or 2 | TCP port of Platform HTTP/API interface. | #### <a ...
(ACRONYM_TAUTOLOGY)
[style] ~74-~74: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rpose-field) code PLATFORM_P2P. * MUST be registered for EvoNodes (type 1). ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...registered for EvoNodes (type 1). * MUST NOT be registered for regular masternod...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~82-~82: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...(domain name). *p_entries[2] * MUST have [purpose`](#p_entrypurpose-field)...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~83-~83: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ose-field) code PLATFORM_HTTPS. * MUST be registered for EvoNodes (type 1). ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~84-~84: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...registered for EvoNodes (type 1). * MUST NOT be registered for regular masternod...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~235-~235: The phrase “taking into consideration” may be wordy. To make your writing clearer, consider replacing it.
Context: ....org/doc/html/rfc1035#section-2.3.4), taking into consideration the leading and trailing bytes as speci...
(TAKE_INTO_CONSIDERATION)
dip-0004.md
[style] ~109-~109: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “API”.
Context: ... | 0 or 2 | TCP port of Platform HTTP/API interface (network byte order). Only present for ...
(ACRONYM_TAUTOLOGY)
🔇 Additional comments (3)
dip-0003/masternode-types.md (1)
5-6: Ensure consistent EvoNode casing across docsThis file uses “Evolution Masternode (EvoNode)”, whereas other updated DIPs mix
evonode,EvoNode, andevonode. Please settle on one canonical spelling/casing (the spec seems to lean toward “EvoNode”) and sweep the repo to keep terminology uniform.dip-0028.md (1)
328-338: Table anchor mismatch for platformNodeID presence ruleThe table row says “Only when
versionis equal to or greater than 2 …”, while earlier prose states it is serialized only forversion2. Align the conditions or clarify that version ≥ 2 is intended.dip-0003.md (1)
122-132:platformInfomarked “Absent from version 3 onwards” but still listedIf version 3 removes
platformInfo, consider moving the field into a separate “only versions 1-2” subsection or tagging the row accordingly to avoid implementer confusion.
| | ipAddress | byte[] | 16 | IPv6 address in network byte order. Only IPv4 mapped addresses are allowed | | ||
| | port | uint_16 | 2 | Port (network byte order) | | ||
|
|
||
| #### <a name="mninfo_rules">Validation Rules</a> | ||
|
|
||
| * `ipAddress` MUST be a valid IPv4 address that is routable on the global internet | ||
| * `ipAddress` MUST NOT be already used in the registered masternodes set | ||
| * `port` MUST be within the valid port range [1, 65535] |
There was a problem hiding this comment.
IPv4-mapped IPv6 vs “valid IPv4” wording is ambiguous
The field is 16 bytes (IPv6) but validation rule says it “MUST be a valid IPv4 address”. Clarify that it must be an IPv4-mapped IPv6 address (i.e., ::ffff:IPv4). Otherwise implementers could mis-encode plain 4-byte IPv4.
🤖 Prompt for AI Agents
In dip-0003/network-info.md around lines 12 to 19, clarify the validation rule
for the ipAddress field to specify that it must be an IPv4-mapped IPv6 address
(i.e., ::ffff:IPv4) rather than just a valid IPv4 address. Update the wording to
explicitly state this requirement to avoid confusion and ensure implementers
encode the address correctly as a 16-byte IPv6 format with the IPv4 mapping.
| * On mainnet, MUST use [`port`](#entryport-field) of `9999`. | ||
| * [`entries`](#p_entryentries-field) MUST NOT have a [`type`](#entrytype-field) of `0xD0` (domain name). | ||
| * `p_entries[1]` | ||
| * MUST have [`purpose`](#p_entrypurpose-field) code `PLATFORM_P2P`. | ||
| * MUST be registered for EvoNodes (type 1). | ||
| * MUST NOT be registered for regular masternodes (type 0). | ||
| * [`entries[0]`](#p_entryentries-field): | ||
| * MUST have a [`type`](#entrytype-field) of `0x01` (IPv4 address). | ||
| * MUST have the same [`address`](#entryaddress-field) as `p_entries[0][0]` | ||
| * On mainnet, MUST use [`port`](#entryport-field) of `26656`. | ||
| * [`entries`](#p_entryentries-field) MUST NOT have a [`type`](#entrytype-field) of `0xD0` (domain name). | ||
| * `p_entries[2]` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coding entry ordering may break forward-compatibility
Mandating that p_entries[1]/[2] correspond to PLATFORM_* purposes ties semantics to position rather than purpose code. Future extensions (e.g., a fourth purpose) would force a hard break. Instead specify only that an entry with purpose code CORE_P2P MUST be present, etc., without positional constraints.
🧰 Tools
🪛 LanguageTool
[style] ~74-~74: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rpose-field) code PLATFORM_P2P. * MUST be registered for EvoNodes (type 1). ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...registered for EvoNodes (type 1). * MUST NOT be registered for regular masternod...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.17.2)
70-70: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
71-71: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
77-77: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
78-78: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
79-79: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
80-80: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In dip-0003/network-info.md around lines 70 to 81, the current specification
hard-codes the ordering of p_entries by requiring p_entries[1] and p_entries[2]
to correspond to specific purpose codes, which risks breaking forward
compatibility. To fix this, remove positional constraints and instead specify
that entries with the required purpose codes (e.g., PLATFORM_P2P, CORE_P2P) must
be present regardless of their position in the p_entries array.
4 participants
Additional Information
Summary by CodeRabbit
networkInfoformat, replacing separate IP address and port fields.platformInfoas deprecated and updated related validation and serialization rules.