From 9d579a4f39d159b6e0d72ea276df64d1801890bf Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:36:09 +0200 Subject: [PATCH 1/6] fix: read Core 23 nested platform addresses in rpc-json DMNState Dash Core 23.1 moved the platform P2P/HTTPS ports out of the deprecated top-level platformP2PPort/platformHTTPPort keys into a nested `addresses` object whose `platform_p2p`/`platform_https` sub-keys hold "host:port" arrays. Without reading them, platform_p2p_port deserialized to None on Core-23 entries, dropping or mis-porting validators downstream. Add MasternodeAddresses and backfill the existing port fields from the addresses object when the legacy port is absent or zero, via a private DMNStateIntermediate (mirroring DMNStateDiffIntermediate) and inside the existing TryFrom for DMNStateDiff. Legacy pre-23 JSON is untouched. Co-Authored-By: Claude Opus 4.8 (1M context) --- rpc-json/src/lib.rs | 218 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 217 insertions(+), 1 deletion(-) diff --git a/rpc-json/src/lib.rs b/rpc-json/src/lib.rs index 6416f2857..df06ce533 100644 --- a/rpc-json/src/lib.rs +++ b/rpc-json/src/lib.rs @@ -2052,9 +2052,42 @@ pub struct GetMasternodePaymentsResult { pub masternodes: Vec, } +/// Nested `addresses` object on Core 23+ masternode entries. +/// +/// Each purpose maps to an array of `"host:port"` strings. Dash Core 23 moved the +/// platform ports here, away from the deprecated top-level `platformP2PPort`/ +/// `platformHTTPPort` keys. Unknown purposes are ignored. +#[derive(Clone, PartialEq, Eq, Debug, Default, Deserialize, Serialize)] +pub struct MasternodeAddresses { + #[serde(default)] + pub core_p2p: Vec, + #[serde(default)] + pub platform_p2p: Vec, + #[serde(default)] + pub platform_https: Vec, +} + +/// Extracts the trailing `:port` from a `"host:port"` string as a `u32`. +/// +/// Uses [`str::rsplit_once`] so it works for IPv4 and unbracketed IPv6 hosts — +/// only the final port segment is parsed. Returns `None` when no colon is present +/// or the suffix is not a valid `u32`. +fn parse_port(addr: &str) -> Option { + addr.rsplit_once(':').and_then(|(_, port)| port.parse().ok()) +} + +/// Backfills a legacy port field from the matching `addresses` array. +/// +/// Keeps a non-zero legacy port as-is; otherwise takes the port of the first entry +/// in `addrs`. This preserves pre-23 behavior exactly while reading Core 23 entries. +fn backfill_port(legacy: Option, addrs: &[String]) -> Option { + legacy.filter(|&p| p != 0).or_else(|| addrs.first().and_then(|a| parse_port(a))) +} + #[serde_as] #[derive(Clone, PartialEq, Eq, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] +#[serde(from = "DMNStateIntermediate")] pub struct DMNState { #[serde_as(as = "DisplayFromStr")] pub service: SocketAddr, @@ -2084,6 +2117,90 @@ pub struct DMNState { pub platform_p2p_port: Option, #[serde(default, rename = "platformHTTPPort")] pub platform_http_port: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub addresses: Option, +} + +#[serde_as] +#[derive(Clone, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +struct DMNStateIntermediate { + #[serde_as(as = "DisplayFromStr")] + service: SocketAddr, + registered_height: u32, + #[serde(default, rename = "PoSeRevivedHeight", deserialize_with = "deserialize_u32_opt")] + pose_revived_height: Option, + #[serde(default, rename = "PoSeBanHeight", deserialize_with = "deserialize_u32_opt")] + pose_ban_height: Option, + revocation_reason: u32, + #[serde(deserialize_with = "deserialize_address")] + owner_address: [u8; 20], + #[serde(deserialize_with = "deserialize_address")] + voting_address: [u8; 20], + #[serde(deserialize_with = "deserialize_address")] + payout_address: [u8; 20], + #[serde(with = "hex")] + pub_key_operator: Vec, + #[serde(default, deserialize_with = "deserialize_address_optional")] + operator_payout_address: Option<[u8; 20]>, + #[serde( + default, + deserialize_with = "deserialize_hex_to_address_optional", + rename = "platformNodeID" + )] + platform_node_id: Option<[u8; 20]>, + #[serde(default, rename = "platformP2PPort")] + platform_p2p_port: Option, + #[serde(default, rename = "platformHTTPPort")] + platform_http_port: Option, + #[serde(default)] + addresses: Option, +} + +impl From for DMNState { + fn from(value: DMNStateIntermediate) -> Self { + let DMNStateIntermediate { + service, + registered_height, + pose_revived_height, + pose_ban_height, + revocation_reason, + owner_address, + voting_address, + payout_address, + pub_key_operator, + operator_payout_address, + platform_node_id, + platform_p2p_port, + platform_http_port, + addresses, + } = value; + + let (platform_p2p_port, platform_http_port) = match &addresses { + Some(addr) => ( + backfill_port(platform_p2p_port, &addr.platform_p2p), + backfill_port(platform_http_port, &addr.platform_https), + ), + None => (platform_p2p_port, platform_http_port), + }; + + DMNState { + service, + registered_height, + pose_revived_height, + pose_ban_height, + revocation_reason, + owner_address, + voting_address, + payout_address, + pub_key_operator, + operator_payout_address, + platform_node_id, + platform_p2p_port, + platform_http_port, + addresses, + } + } } #[derive(Clone, PartialEq, Eq, Debug, Deserialize)] @@ -2105,6 +2222,7 @@ pub struct DMNStateDiff { pub platform_node_id: Option<[u8; 20]>, pub platform_p2p_port: Option, pub platform_http_port: Option, + pub addresses: Option, } impl TryFrom for DMNStateDiff { @@ -2127,8 +2245,17 @@ impl TryFrom for DMNStateDiff { platform_http_port, payout_address, pub_key_operator, + addresses, } = value; + let (platform_p2p_port, platform_http_port) = match &addresses { + Some(addr) => ( + backfill_port(platform_p2p_port, &addr.platform_p2p), + backfill_port(platform_http_port, &addr.platform_https), + ), + None => (platform_p2p_port, platform_http_port), + }; + let owner_address = owner_address .map(|address| { let address = Address::from_str(address.as_str())?; @@ -2190,6 +2317,7 @@ impl TryFrom for DMNStateDiff { platform_node_id, platform_p2p_port, platform_http_port, + addresses, }) } } @@ -2284,6 +2412,12 @@ impl DMNState { } else { None }, + addresses: if self.addresses != newer.addresses { + has_diff = true; + newer.addresses.clone() + } else { + None + }, }; if has_diff { Some(diff) @@ -2893,6 +3027,8 @@ pub struct DMNStateDiffIntermediate { pub payout_address: Option, #[serde(default, deserialize_with = "deserialize_hex_opt")] pub pub_key_operator: Option>, + #[serde(default)] + pub addresses: Option, } #[derive(Clone, PartialEq, Debug, Deserialize)] @@ -3258,7 +3394,8 @@ mod tests { use serde_json::json; use crate::{ - ExtendedQuorumListResult, MasternodeListDiff, MnSyncStatus, QuorumType, deserialize_u32_opt, + DMNState, DMNStateDiff, ExtendedQuorumListResult, MasternodeListDiff, MnSyncStatus, + QuorumType, deserialize_u32_opt, }; #[test] @@ -3435,6 +3572,85 @@ mod tests { ); } + #[test] + fn dmn_state_core23_addresses_backfills_platform_ports() { + // Core 23 simplified entry: platformP2PPort/platformHTTPPort removed, + // ports now live in the nested `addresses` object as "host:port" arrays. + let json = r#"{ + "service": "192.0.2.1:9999", + "registeredHeight": 123456, + "revocationReason": 0, + "ownerAddress": "yPBWCdMRY5PsS3hJzs7csbdWQVRR85yxUz", + "votingAddress": "ySM11LUD65Bi4p1gm68XLkdWc65TBKRzvQ", + "payoutAddress": "yX4Ve7Q8Y4jscV4LZJD8HVCHKyePzR3MhA", + "pubKeyOperator": "8ed3f0c208efbcfc815cbfb94490dc68cf2e29d44dd9f8a91e20e06057aa110d7062c8ab7ccc85a9ff0c88760157f563", + "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5", + "addresses": { + "core_p2p": ["192.0.2.1:9999"], + "platform_p2p": ["192.0.2.2:36656"], + "platform_https": ["192.0.2.2:443"] + } + }"#; + let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); + assert_eq!(state.platform_p2p_port, Some(36656), "platform_p2p_port from addresses"); + assert_eq!(state.platform_http_port, Some(443), "platform_http_port from addresses"); + } + + #[test] + fn dmn_state_diff_core23_addresses_backfills_platform_ports() { + // updatedMNs entry carrying only the new `addresses` object. + let json = r#"{ + "addresses": { + "platform_p2p": ["192.0.2.2:36656"], + "platform_https": ["192.0.2.2:443"] + } + }"#; + let diff: DMNStateDiff = serde_json::from_str(json).expect("expected to deserialize json"); + assert_eq!(diff.platform_p2p_port, Some(36656), "diff platform_p2p_port from addresses"); + assert_eq!(diff.platform_http_port, Some(443), "diff platform_http_port from addresses"); + } + + #[test] + fn dmn_state_legacy_platform_ports_preserved() { + // Pre-23 entry: legacy top-level keys, no `addresses`. Behavior must not change. + let json = r#"{ + "service": "192.0.2.1:9999", + "registeredHeight": 123456, + "revocationReason": 0, + "ownerAddress": "yPBWCdMRY5PsS3hJzs7csbdWQVRR85yxUz", + "votingAddress": "ySM11LUD65Bi4p1gm68XLkdWc65TBKRzvQ", + "payoutAddress": "yX4Ve7Q8Y4jscV4LZJD8HVCHKyePzR3MhA", + "pubKeyOperator": "8ed3f0c208efbcfc815cbfb94490dc68cf2e29d44dd9f8a91e20e06057aa110d7062c8ab7ccc85a9ff0c88760157f563", + "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5", + "platformP2PPort": 26656, + "platformHTTPPort": 443 + }"#; + let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); + assert_eq!(state.platform_p2p_port, Some(26656), "legacy platform_p2p_port preserved"); + assert_eq!(state.platform_http_port, Some(443), "legacy platform_http_port preserved"); + } + + #[test] + fn dmn_state_zero_legacy_port_yields_to_addresses() { + // Transitional entry: legacy port present but zero -> addresses wins. + let json = r#"{ + "service": "192.0.2.1:9999", + "registeredHeight": 123456, + "revocationReason": 0, + "ownerAddress": "yPBWCdMRY5PsS3hJzs7csbdWQVRR85yxUz", + "votingAddress": "ySM11LUD65Bi4p1gm68XLkdWc65TBKRzvQ", + "payoutAddress": "yX4Ve7Q8Y4jscV4LZJD8HVCHKyePzR3MhA", + "pubKeyOperator": "8ed3f0c208efbcfc815cbfb94490dc68cf2e29d44dd9f8a91e20e06057aa110d7062c8ab7ccc85a9ff0c88760157f563", + "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5", + "platformP2PPort": 0, + "addresses": { + "platform_p2p": ["192.0.2.2:36656"] + } + }"#; + let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); + assert_eq!(state.platform_p2p_port, Some(36656), "zero legacy port replaced by addresses"); + } + #[test] fn deserialize_mnsync_status() { let json_value = json!({ From ea721ea09cc8b4c7ba1f71698c618815058cd377 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:05:53 +0200 Subject: [PATCH 2/6] refactor(rpc-json): resolve platform ports via accessors; drop in-deserialize backfill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match the plain-derive style of the rest of the crate: deserialize DMNState and DMNStateDiff with no logic in the deserialize path. The nested Core 23 `addresses` object now lands as a plain public field instead of being folded into the legacy port fields during deserialization. Port resolution moves into accessor methods on both DMNState and DMNStateDiff: resolved_platform_p2p_port / resolved_platform_http_port. They check the new `addresses` object first (first non-zero, parseable port) and fall back to the deprecated top-level platformP2PPort/platformHTTPPort — new-first, legacy-fallback. Removed: the DMNStateIntermediate mirror struct and its `#[serde(from)]`, the `backfill_port` helper, and the in-deserialize backfill blocks in both From/TryFrom impls. DMNState regains its plain `#[derive(Deserialize)]`; DMNStateDiff keeps its pre-existing `try_from` with `addresses` flowing through as raw data. apply_diff now propagates `addresses` so a stored DMNState stays resolvable after diffs. Co-Authored-By: Claude Opus 4.8 (1M context) --- rpc-json/src/lib.rs | 221 ++++++++++++++++++++++---------------------- 1 file changed, 111 insertions(+), 110 deletions(-) diff --git a/rpc-json/src/lib.rs b/rpc-json/src/lib.rs index df06ce533..dcc80502e 100644 --- a/rpc-json/src/lib.rs +++ b/rpc-json/src/lib.rs @@ -2067,6 +2067,13 @@ pub struct MasternodeAddresses { pub platform_https: Vec, } +impl MasternodeAddresses { + /// First valid (non-zero, parseable) port from a `"host:port"` array, if any. + fn first_valid_port(addrs: &[String]) -> Option { + addrs.iter().filter_map(|a| parse_port(a)).find(|&p| p != 0) + } +} + /// Extracts the trailing `:port` from a `"host:port"` string as a `u32`. /// /// Uses [`str::rsplit_once`] so it works for IPv4 and unbracketed IPv6 hosts — @@ -2076,18 +2083,9 @@ fn parse_port(addr: &str) -> Option { addr.rsplit_once(':').and_then(|(_, port)| port.parse().ok()) } -/// Backfills a legacy port field from the matching `addresses` array. -/// -/// Keeps a non-zero legacy port as-is; otherwise takes the port of the first entry -/// in `addrs`. This preserves pre-23 behavior exactly while reading Core 23 entries. -fn backfill_port(legacy: Option, addrs: &[String]) -> Option { - legacy.filter(|&p| p != 0).or_else(|| addrs.first().and_then(|a| parse_port(a))) -} - #[serde_as] #[derive(Clone, PartialEq, Eq, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] -#[serde(from = "DMNStateIntermediate")] pub struct DMNState { #[serde_as(as = "DisplayFromStr")] pub service: SocketAddr, @@ -2121,85 +2119,29 @@ pub struct DMNState { pub addresses: Option, } -#[serde_as] -#[derive(Clone, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] -struct DMNStateIntermediate { - #[serde_as(as = "DisplayFromStr")] - service: SocketAddr, - registered_height: u32, - #[serde(default, rename = "PoSeRevivedHeight", deserialize_with = "deserialize_u32_opt")] - pose_revived_height: Option, - #[serde(default, rename = "PoSeBanHeight", deserialize_with = "deserialize_u32_opt")] - pose_ban_height: Option, - revocation_reason: u32, - #[serde(deserialize_with = "deserialize_address")] - owner_address: [u8; 20], - #[serde(deserialize_with = "deserialize_address")] - voting_address: [u8; 20], - #[serde(deserialize_with = "deserialize_address")] - payout_address: [u8; 20], - #[serde(with = "hex")] - pub_key_operator: Vec, - #[serde(default, deserialize_with = "deserialize_address_optional")] - operator_payout_address: Option<[u8; 20]>, - #[serde( - default, - deserialize_with = "deserialize_hex_to_address_optional", - rename = "platformNodeID" - )] - platform_node_id: Option<[u8; 20]>, - #[serde(default, rename = "platformP2PPort")] - platform_p2p_port: Option, - #[serde(default, rename = "platformHTTPPort")] - platform_http_port: Option, - #[serde(default)] - addresses: Option, -} - -impl From for DMNState { - fn from(value: DMNStateIntermediate) -> Self { - let DMNStateIntermediate { - service, - registered_height, - pose_revived_height, - pose_ban_height, - revocation_reason, - owner_address, - voting_address, - payout_address, - pub_key_operator, - operator_payout_address, - platform_node_id, - platform_p2p_port, - platform_http_port, - addresses, - } = value; - - let (platform_p2p_port, platform_http_port) = match &addresses { - Some(addr) => ( - backfill_port(platform_p2p_port, &addr.platform_p2p), - backfill_port(platform_http_port, &addr.platform_https), - ), - None => (platform_p2p_port, platform_http_port), - }; +impl DMNState { + /// Resolved platform P2P port. + /// + /// Prefers the Core 23+ nested `addresses.platform_p2p` port; falls back to the + /// deprecated top-level `platformP2PPort`. Read this instead of the raw + /// `platform_p2p_port` field, which is `0`/absent on Core 23+ entries. + pub fn resolved_platform_p2p_port(&self) -> Option { + self.addresses + .as_ref() + .and_then(|a| MasternodeAddresses::first_valid_port(&a.platform_p2p)) + .or(self.platform_p2p_port) + } - DMNState { - service, - registered_height, - pose_revived_height, - pose_ban_height, - revocation_reason, - owner_address, - voting_address, - payout_address, - pub_key_operator, - operator_payout_address, - platform_node_id, - platform_p2p_port, - platform_http_port, - addresses, - } + /// Resolved platform HTTPS port. + /// + /// Prefers the Core 23+ nested `addresses.platform_https` port; falls back to the + /// deprecated top-level `platformHTTPPort`. Read this instead of the raw + /// `platform_http_port` field, which is `0`/absent on Core 23+ entries. + pub fn resolved_platform_http_port(&self) -> Option { + self.addresses + .as_ref() + .and_then(|a| MasternodeAddresses::first_valid_port(&a.platform_https)) + .or(self.platform_http_port) } } @@ -2248,14 +2190,6 @@ impl TryFrom for DMNStateDiff { addresses, } = value; - let (platform_p2p_port, platform_http_port) = match &addresses { - Some(addr) => ( - backfill_port(platform_p2p_port, &addr.platform_p2p), - backfill_port(platform_http_port, &addr.platform_https), - ), - None => (platform_p2p_port, platform_http_port), - }; - let owner_address = owner_address .map(|address| { let address = Address::from_str(address.as_str())?; @@ -2322,6 +2256,32 @@ impl TryFrom for DMNStateDiff { } } +impl DMNStateDiff { + /// Resolved platform P2P port carried by this diff, if any. + /// + /// Prefers the Core 23+ nested `addresses.platform_p2p` port; falls back to the + /// deprecated top-level `platformP2PPort`. Read this instead of the raw + /// `platform_p2p_port` field, which is `0`/absent on Core 23+ diffs. + pub fn resolved_platform_p2p_port(&self) -> Option { + self.addresses + .as_ref() + .and_then(|a| MasternodeAddresses::first_valid_port(&a.platform_p2p)) + .or(self.platform_p2p_port) + } + + /// Resolved platform HTTPS port carried by this diff, if any. + /// + /// Prefers the Core 23+ nested `addresses.platform_https` port; falls back to the + /// deprecated top-level `platformHTTPPort`. Read this instead of the raw + /// `platform_http_port` field, which is `0`/absent on Core 23+ diffs. + pub fn resolved_platform_http_port(&self) -> Option { + self.addresses + .as_ref() + .and_then(|a| MasternodeAddresses::first_valid_port(&a.platform_https)) + .or(self.platform_http_port) + } +} + impl DMNState { pub fn compare_to_older_dmn_state(&self, older: &DMNState) -> Option { older.compare_to_newer_dmn_state(self) @@ -2440,6 +2400,7 @@ impl DMNState { platform_node_id, platform_p2p_port, platform_http_port, + addresses, .. } = diff; self.pose_revived_height = pose_revived_height; @@ -2479,6 +2440,10 @@ impl DMNState { if let Some(platform_http_port) = platform_http_port { self.platform_http_port = Some(platform_http_port); } + + if addresses.is_some() { + self.addresses = addresses; + } } } @@ -3573,9 +3538,9 @@ mod tests { } #[test] - fn dmn_state_core23_addresses_backfills_platform_ports() { - // Core 23 simplified entry: platformP2PPort/platformHTTPPort removed, - // ports now live in the nested `addresses` object as "host:port" arrays. + fn dmn_state_core23_addresses_resolve_platform_ports() { + // Core 23 entry: legacy platformP2PPort/platformHTTPPort absent, ports live + // in the nested `addresses` object. Raw fields stay None; accessors resolve. let json = r#"{ "service": "192.0.2.1:9999", "registeredHeight": 123456, @@ -3592,12 +3557,14 @@ mod tests { } }"#; let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); - assert_eq!(state.platform_p2p_port, Some(36656), "platform_p2p_port from addresses"); - assert_eq!(state.platform_http_port, Some(443), "platform_http_port from addresses"); + assert_eq!(state.platform_p2p_port, None, "raw legacy field deserialized as-is"); + assert_eq!(state.platform_http_port, None, "raw legacy field deserialized as-is"); + assert_eq!(state.resolved_platform_p2p_port(), Some(36656), "p2p resolved from addresses"); + assert_eq!(state.resolved_platform_http_port(), Some(443), "http resolved from addresses"); } #[test] - fn dmn_state_diff_core23_addresses_backfills_platform_ports() { + fn dmn_state_diff_core23_addresses_resolve_platform_ports() { // updatedMNs entry carrying only the new `addresses` object. let json = r#"{ "addresses": { @@ -3606,13 +3573,23 @@ mod tests { } }"#; let diff: DMNStateDiff = serde_json::from_str(json).expect("expected to deserialize json"); - assert_eq!(diff.platform_p2p_port, Some(36656), "diff platform_p2p_port from addresses"); - assert_eq!(diff.platform_http_port, Some(443), "diff platform_http_port from addresses"); + assert_eq!(diff.platform_p2p_port, None, "raw legacy diff field deserialized as-is"); + assert_eq!(diff.platform_http_port, None, "raw legacy diff field deserialized as-is"); + assert_eq!( + diff.resolved_platform_p2p_port(), + Some(36656), + "diff p2p resolved from addresses" + ); + assert_eq!( + diff.resolved_platform_http_port(), + Some(443), + "diff http resolved from addresses" + ); } #[test] - fn dmn_state_legacy_platform_ports_preserved() { - // Pre-23 entry: legacy top-level keys, no `addresses`. Behavior must not change. + fn dmn_state_legacy_platform_ports_resolve_to_legacy() { + // Pre-23 entry: legacy top-level keys, no `addresses`. Accessors fall back to legacy. let json = r#"{ "service": "192.0.2.1:9999", "registeredHeight": 123456, @@ -3626,13 +3603,14 @@ mod tests { "platformHTTPPort": 443 }"#; let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); - assert_eq!(state.platform_p2p_port, Some(26656), "legacy platform_p2p_port preserved"); - assert_eq!(state.platform_http_port, Some(443), "legacy platform_http_port preserved"); + assert!(state.addresses.is_none(), "no addresses object present"); + assert_eq!(state.resolved_platform_p2p_port(), Some(26656), "p2p resolved from legacy"); + assert_eq!(state.resolved_platform_http_port(), Some(443), "http resolved from legacy"); } #[test] - fn dmn_state_zero_legacy_port_yields_to_addresses() { - // Transitional entry: legacy port present but zero -> addresses wins. + fn dmn_state_zero_legacy_port_resolves_to_addresses() { + // Transitional entry: legacy port present but zero -> addresses wins (new-first). let json = r#"{ "service": "192.0.2.1:9999", "registeredHeight": 123456, @@ -3648,7 +3626,30 @@ mod tests { } }"#; let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); - assert_eq!(state.platform_p2p_port, Some(36656), "zero legacy port replaced by addresses"); + assert_eq!(state.platform_p2p_port, Some(0), "raw legacy zero deserialized as-is"); + assert_eq!( + state.resolved_platform_p2p_port(), + Some(36656), + "zero legacy yields to addresses" + ); + } + + #[test] + fn dmn_state_no_ports_resolve_to_none() { + // No addresses and no legacy ports -> accessors return None. + let json = r#"{ + "service": "192.0.2.1:9999", + "registeredHeight": 123456, + "revocationReason": 0, + "ownerAddress": "yPBWCdMRY5PsS3hJzs7csbdWQVRR85yxUz", + "votingAddress": "ySM11LUD65Bi4p1gm68XLkdWc65TBKRzvQ", + "payoutAddress": "yX4Ve7Q8Y4jscV4LZJD8HVCHKyePzR3MhA", + "pubKeyOperator": "8ed3f0c208efbcfc815cbfb94490dc68cf2e29d44dd9f8a91e20e06057aa110d7062c8ab7ccc85a9ff0c88760157f563", + "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5" + }"#; + let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); + assert_eq!(state.resolved_platform_p2p_port(), None, "no source -> None"); + assert_eq!(state.resolved_platform_http_port(), None, "no source -> None"); } #[test] From e3991aecaf02a1c98d923f21f8bcb8c754bf0ff0 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:42:21 +0200 Subject: [PATCH 3/6] refactor(rpc-json): platform address accessors return (host, port), clearable diff Address self-review on the Core 23 platform-address work. Rename the raw legacy fields on DMNState and DMNStateDiff to legacy_platform_p2p_port / legacy_platform_http_port (serde wire renames intact) so callers reach for the accessors instead of the deprecated fields. The resolver accessors become platform_p2p_address / platform_http_address and return Option<(String, u32)> instead of a bare port. On DMNState the legacy fallback pairs the port with the node IP from `service`, since Dash deploys platform services on the masternode's core IP; on DMNStateDiff there is no node IP to pair with, so the legacy path is not resolved. Zero ports never surface: they are dropped in both the addresses path and the legacy fallback. Ports parse through u16 then widen to u32, rejecting out-of-range values like "host:70000". DMNStateDiff::addresses becomes Option>, mirroring pose_ban_height: None = unchanged, Some(None) = cleared, Some(Some(_)) = set. A custom serde helper distinguishes absent / null / object on the wire. compare_to_newer_dmn_state now encodes a Some->None clear as Some(None) and apply_diff applies the inner value, so the clear survives the round-trip. Tests cover diff addresses propagation through apply_diff, the Some->None clear round-trip, zero-port-to-None, and out-of-range rejection. Co-Authored-By: Claude Opus 4.8 (1M context) --- rpc-json/src/lib.rs | 340 +++++++++++++++++++++++++++++++++----------- 1 file changed, 257 insertions(+), 83 deletions(-) diff --git a/rpc-json/src/lib.rs b/rpc-json/src/lib.rs index dcc80502e..7003d9418 100644 --- a/rpc-json/src/lib.rs +++ b/rpc-json/src/lib.rs @@ -2068,19 +2068,26 @@ pub struct MasternodeAddresses { } impl MasternodeAddresses { - /// First valid (non-zero, parseable) port from a `"host:port"` array, if any. - fn first_valid_port(addrs: &[String]) -> Option { - addrs.iter().filter_map(|a| parse_port(a)).find(|&p| p != 0) + /// First valid `(host, port)` from a `"host:port"` array, if any. + /// + /// "Valid" means parseable and with a non-zero in-range port; zero ports are + /// skipped because Dash Core uses `0` as a "not set" sentinel. + fn first_valid_host_port(addrs: &[String]) -> Option<(String, u32)> { + addrs.iter().filter_map(|a| parse_host_port(a)).find(|(_, p)| *p != 0) } } -/// Extracts the trailing `:port` from a `"host:port"` string as a `u32`. +/// Splits a `"host:port"` string into `(host, port)` with a non-fabricated host. /// -/// Uses [`str::rsplit_once`] so it works for IPv4 and unbracketed IPv6 hosts — -/// only the final port segment is parsed. Returns `None` when no colon is present -/// or the suffix is not a valid `u32`. -fn parse_port(addr: &str) -> Option { - addr.rsplit_once(':').and_then(|(_, port)| port.parse().ok()) +/// Uses [`str::rsplit_once`] so only the final `:port` segment is parsed, which +/// keeps IPv4 and unbracketed IPv6 hosts intact. The port is parsed through +/// [`u16`] then widened to `u32`, rejecting values outside the TCP/UDP port range +/// (e.g. `"host:70000"`). Returns `None` when no colon is present or the suffix is +/// not a valid `u16`. +fn parse_host_port(addr: &str) -> Option<(String, u32)> { + let (host, port) = addr.rsplit_once(':')?; + let port = port.parse::().ok()?; + Some((host.to_string(), u32::from(port))) } #[serde_as] @@ -2112,36 +2119,45 @@ pub struct DMNState { )] pub platform_node_id: Option<[u8; 20]>, #[serde(default, rename = "platformP2PPort")] - pub platform_p2p_port: Option, + pub legacy_platform_p2p_port: Option, #[serde(default, rename = "platformHTTPPort")] - pub platform_http_port: Option, + pub legacy_platform_http_port: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub addresses: Option, } impl DMNState { - /// Resolved platform P2P port. + /// Resolved platform P2P `(host, port)`. /// - /// Prefers the Core 23+ nested `addresses.platform_p2p` port; falls back to the - /// deprecated top-level `platformP2PPort`. Read this instead of the raw - /// `platform_p2p_port` field, which is `0`/absent on Core 23+ entries. - pub fn resolved_platform_p2p_port(&self) -> Option { + /// Prefers the Core 23+ nested `addresses.platform_p2p` entry, returning its + /// host and port verbatim. Falls back to the deprecated top-level + /// `platformP2PPort`, pairing it with the node IP from [`service`](Self::service) + /// because Dash deploys platform services on the masternode's core IP. Returns + /// `None` when no source yields a non-zero in-range port. + pub fn platform_p2p_address(&self) -> Option<(String, u32)> { self.addresses .as_ref() - .and_then(|a| MasternodeAddresses::first_valid_port(&a.platform_p2p)) - .or(self.platform_p2p_port) + .and_then(|a| MasternodeAddresses::first_valid_host_port(&a.platform_p2p)) + .or_else(|| self.legacy_platform_address(self.legacy_platform_p2p_port)) } - /// Resolved platform HTTPS port. + /// Resolved platform HTTPS `(host, port)`. /// - /// Prefers the Core 23+ nested `addresses.platform_https` port; falls back to the - /// deprecated top-level `platformHTTPPort`. Read this instead of the raw - /// `platform_http_port` field, which is `0`/absent on Core 23+ entries. - pub fn resolved_platform_http_port(&self) -> Option { + /// Prefers the Core 23+ nested `addresses.platform_https` entry, returning its + /// host and port verbatim. Falls back to the deprecated top-level + /// `platformHTTPPort`, pairing it with the node IP from [`service`](Self::service) + /// because Dash deploys platform services on the masternode's core IP. Returns + /// `None` when no source yields a non-zero in-range port. + pub fn platform_http_address(&self) -> Option<(String, u32)> { self.addresses .as_ref() - .and_then(|a| MasternodeAddresses::first_valid_port(&a.platform_https)) - .or(self.platform_http_port) + .and_then(|a| MasternodeAddresses::first_valid_host_port(&a.platform_https)) + .or_else(|| self.legacy_platform_address(self.legacy_platform_http_port)) + } + + /// Pairs a legacy platform port with the node IP, dropping zero/absent ports. + fn legacy_platform_address(&self, port: Option) -> Option<(String, u32)> { + port.filter(|&p| p != 0).map(|p| (self.service.ip().to_string(), p)) } } @@ -2162,9 +2178,11 @@ pub struct DMNStateDiff { pub pub_key_operator: Option>, pub operator_payout_address: Option>, pub platform_node_id: Option<[u8; 20]>, - pub platform_p2p_port: Option, - pub platform_http_port: Option, - pub addresses: Option, + pub legacy_platform_p2p_port: Option, + pub legacy_platform_http_port: Option, + /// Three-state nested addresses: `None` = unchanged, `Some(None)` = cleared, + /// `Some(Some(_))` = set. Mirrors [`pose_ban_height`](Self::pose_ban_height). + pub addresses: Option>, } impl TryFrom for DMNStateDiff { @@ -2183,8 +2201,8 @@ impl TryFrom for DMNStateDiff { owner_address, voting_address, platform_node_id, - platform_p2p_port, - platform_http_port, + legacy_platform_p2p_port, + legacy_platform_http_port, payout_address, pub_key_operator, addresses, @@ -2249,36 +2267,38 @@ impl TryFrom for DMNStateDiff { pub_key_operator, operator_payout_address, platform_node_id, - platform_p2p_port, - platform_http_port, + legacy_platform_p2p_port, + legacy_platform_http_port, addresses, }) } } impl DMNStateDiff { - /// Resolved platform P2P port carried by this diff, if any. + /// Resolved platform P2P `(host, port)` carried by this diff, if any. /// - /// Prefers the Core 23+ nested `addresses.platform_p2p` port; falls back to the - /// deprecated top-level `platformP2PPort`. Read this instead of the raw - /// `platform_p2p_port` field, which is `0`/absent on Core 23+ diffs. - pub fn resolved_platform_p2p_port(&self) -> Option { + /// Returns the host and port from the Core 23+ nested `addresses.platform_p2p` + /// entry. The legacy top-level `platformP2PPort` is not resolved here: a diff + /// carries no node IP to pair it with, so a host would have to be fabricated. + /// Read the legacy port via [`legacy_platform_p2p_port`](Self::legacy_platform_p2p_port). + pub fn platform_p2p_address(&self) -> Option<(String, u32)> { self.addresses .as_ref() - .and_then(|a| MasternodeAddresses::first_valid_port(&a.platform_p2p)) - .or(self.platform_p2p_port) + .and_then(|a| a.as_ref()) + .and_then(|a| MasternodeAddresses::first_valid_host_port(&a.platform_p2p)) } - /// Resolved platform HTTPS port carried by this diff, if any. + /// Resolved platform HTTPS `(host, port)` carried by this diff, if any. /// - /// Prefers the Core 23+ nested `addresses.platform_https` port; falls back to the - /// deprecated top-level `platformHTTPPort`. Read this instead of the raw - /// `platform_http_port` field, which is `0`/absent on Core 23+ diffs. - pub fn resolved_platform_http_port(&self) -> Option { + /// Returns the host and port from the Core 23+ nested `addresses.platform_https` + /// entry. The legacy top-level `platformHTTPPort` is not resolved here: a diff + /// carries no node IP to pair it with, so a host would have to be fabricated. + /// Read the legacy port via [`legacy_platform_http_port`](Self::legacy_platform_http_port). + pub fn platform_http_address(&self) -> Option<(String, u32)> { self.addresses .as_ref() - .and_then(|a| MasternodeAddresses::first_valid_port(&a.platform_https)) - .or(self.platform_http_port) + .and_then(|a| a.as_ref()) + .and_then(|a| MasternodeAddresses::first_valid_host_port(&a.platform_https)) } } @@ -2360,21 +2380,25 @@ impl DMNState { } else { None }, - platform_p2p_port: if self.platform_p2p_port != newer.platform_p2p_port { + legacy_platform_p2p_port: if self.legacy_platform_p2p_port + != newer.legacy_platform_p2p_port + { has_diff = true; - newer.platform_p2p_port + newer.legacy_platform_p2p_port } else { None }, - platform_http_port: if self.platform_http_port != newer.platform_http_port { + legacy_platform_http_port: if self.legacy_platform_http_port + != newer.legacy_platform_http_port + { has_diff = true; - newer.platform_http_port + newer.legacy_platform_http_port } else { None }, addresses: if self.addresses != newer.addresses { has_diff = true; - newer.addresses.clone() + Some(newer.addresses.clone()) } else { None }, @@ -2398,8 +2422,8 @@ impl DMNState { pub_key_operator, operator_payout_address, platform_node_id, - platform_p2p_port, - platform_http_port, + legacy_platform_p2p_port, + legacy_platform_http_port, addresses, .. } = diff; @@ -2433,15 +2457,15 @@ impl DMNState { self.platform_node_id = Some(platform_node_id); } - if let Some(platform_p2p_port) = platform_p2p_port { - self.platform_p2p_port = Some(platform_p2p_port); + if let Some(legacy_platform_p2p_port) = legacy_platform_p2p_port { + self.legacy_platform_p2p_port = Some(legacy_platform_p2p_port); } - if let Some(platform_http_port) = platform_http_port { - self.platform_http_port = Some(platform_http_port); + if let Some(legacy_platform_http_port) = legacy_platform_http_port { + self.legacy_platform_http_port = Some(legacy_platform_http_port); } - if addresses.is_some() { + if let Some(addresses) = addresses { self.addresses = addresses; } } @@ -2985,15 +3009,16 @@ pub struct DMNStateDiffIntermediate { #[serde(default, rename = "platformNodeID")] pub platform_node_id: Option, #[serde(default, rename = "platformP2PPort")] - pub platform_p2p_port: Option, + pub legacy_platform_p2p_port: Option, #[serde(default, rename = "platformHTTPPort")] - pub platform_http_port: Option, + pub legacy_platform_http_port: Option, #[serde(default)] pub payout_address: Option, #[serde(default, deserialize_with = "deserialize_hex_opt")] pub pub_key_operator: Option>, - #[serde(default)] - pub addresses: Option, + // Three-state: missing field = None, `null` = Some(None), object = Some(Some(_)). + #[serde(default, deserialize_with = "deserialize_addresses_2opt")] + pub addresses: Option>, } #[derive(Clone, PartialEq, Debug, Deserialize)] @@ -3352,6 +3377,20 @@ where Ok(Some(Some(val as u32))) } +/// Deserializes the present-but-clearable nested `addresses` object. +/// +/// Paired with `#[serde(default)]`, the field absence yields `None` (unchanged), +/// a JSON `null` yields `Some(None)` (cleared), and an object yields +/// `Some(Some(_))` (set) — the canonical serde double-`Option` pattern. +fn deserialize_addresses_2opt<'de, D>( + deserializer: D, +) -> Result>, D::Error> +where + D: Deserializer<'de>, +{ + Ok(Some(Option::::deserialize(deserializer)?)) +} + #[cfg(test)] mod tests { use dashcore::hashes::Hash; @@ -3359,8 +3398,8 @@ mod tests { use serde_json::json; use crate::{ - DMNState, DMNStateDiff, ExtendedQuorumListResult, MasternodeListDiff, MnSyncStatus, - QuorumType, deserialize_u32_opt, + DMNState, DMNStateDiff, ExtendedQuorumListResult, MasternodeAddresses, MasternodeListDiff, + MnSyncStatus, QuorumType, deserialize_u32_opt, }; #[test] @@ -3557,10 +3596,18 @@ mod tests { } }"#; let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); - assert_eq!(state.platform_p2p_port, None, "raw legacy field deserialized as-is"); - assert_eq!(state.platform_http_port, None, "raw legacy field deserialized as-is"); - assert_eq!(state.resolved_platform_p2p_port(), Some(36656), "p2p resolved from addresses"); - assert_eq!(state.resolved_platform_http_port(), Some(443), "http resolved from addresses"); + assert_eq!(state.legacy_platform_p2p_port, None, "raw legacy field deserialized as-is"); + assert_eq!(state.legacy_platform_http_port, None, "raw legacy field deserialized as-is"); + assert_eq!( + state.platform_p2p_address(), + Some(("192.0.2.2".to_string(), 36656)), + "p2p resolved from addresses" + ); + assert_eq!( + state.platform_http_address(), + Some(("192.0.2.2".to_string(), 443)), + "http resolved from addresses" + ); } #[test] @@ -3573,23 +3620,27 @@ mod tests { } }"#; let diff: DMNStateDiff = serde_json::from_str(json).expect("expected to deserialize json"); - assert_eq!(diff.platform_p2p_port, None, "raw legacy diff field deserialized as-is"); - assert_eq!(diff.platform_http_port, None, "raw legacy diff field deserialized as-is"); + assert_eq!(diff.legacy_platform_p2p_port, None, "raw legacy diff field deserialized as-is"); assert_eq!( - diff.resolved_platform_p2p_port(), - Some(36656), + diff.legacy_platform_http_port, None, + "raw legacy diff field deserialized as-is" + ); + assert_eq!( + diff.platform_p2p_address(), + Some(("192.0.2.2".to_string(), 36656)), "diff p2p resolved from addresses" ); assert_eq!( - diff.resolved_platform_http_port(), - Some(443), + diff.platform_http_address(), + Some(("192.0.2.2".to_string(), 443)), "diff http resolved from addresses" ); } #[test] - fn dmn_state_legacy_platform_ports_resolve_to_legacy() { - // Pre-23 entry: legacy top-level keys, no `addresses`. Accessors fall back to legacy. + fn dmn_state_legacy_platform_ports_resolve_to_node_ip() { + // Pre-23 entry: legacy top-level keys, no `addresses`. Accessors fall back to + // the legacy port paired with the node IP from `service`. let json = r#"{ "service": "192.0.2.1:9999", "registeredHeight": 123456, @@ -3604,8 +3655,16 @@ mod tests { }"#; let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); assert!(state.addresses.is_none(), "no addresses object present"); - assert_eq!(state.resolved_platform_p2p_port(), Some(26656), "p2p resolved from legacy"); - assert_eq!(state.resolved_platform_http_port(), Some(443), "http resolved from legacy"); + assert_eq!( + state.platform_p2p_address(), + Some(("192.0.2.1".to_string(), 26656)), + "p2p resolved from legacy paired with node IP" + ); + assert_eq!( + state.platform_http_address(), + Some(("192.0.2.1".to_string(), 443)), + "http resolved from legacy paired with node IP" + ); } #[test] @@ -3626,14 +3685,54 @@ mod tests { } }"#; let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); - assert_eq!(state.platform_p2p_port, Some(0), "raw legacy zero deserialized as-is"); + assert_eq!(state.legacy_platform_p2p_port, Some(0), "raw legacy zero deserialized as-is"); assert_eq!( - state.resolved_platform_p2p_port(), - Some(36656), + state.platform_p2p_address(), + Some(("192.0.2.2".to_string(), 36656)), "zero legacy yields to addresses" ); } + #[test] + fn dmn_state_zero_legacy_port_no_addresses_resolves_to_none() { + // Legacy port present but zero and no `addresses` -> accessor returns None, + // never `(host, 0)`. + let json = r#"{ + "service": "192.0.2.1:9999", + "registeredHeight": 123456, + "revocationReason": 0, + "ownerAddress": "yPBWCdMRY5PsS3hJzs7csbdWQVRR85yxUz", + "votingAddress": "ySM11LUD65Bi4p1gm68XLkdWc65TBKRzvQ", + "payoutAddress": "yX4Ve7Q8Y4jscV4LZJD8HVCHKyePzR3MhA", + "pubKeyOperator": "8ed3f0c208efbcfc815cbfb94490dc68cf2e29d44dd9f8a91e20e06057aa110d7062c8ab7ccc85a9ff0c88760157f563", + "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5", + "platformP2PPort": 0 + }"#; + let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); + assert_eq!(state.legacy_platform_p2p_port, Some(0), "raw legacy zero deserialized as-is"); + assert_eq!(state.platform_p2p_address(), None, "zero legacy with no addresses -> None"); + } + + #[test] + fn dmn_state_out_of_range_port_rejected() { + // A port above the u16 range must be rejected rather than truncated/accepted. + let json = r#"{ + "service": "192.0.2.1:9999", + "registeredHeight": 123456, + "revocationReason": 0, + "ownerAddress": "yPBWCdMRY5PsS3hJzs7csbdWQVRR85yxUz", + "votingAddress": "ySM11LUD65Bi4p1gm68XLkdWc65TBKRzvQ", + "payoutAddress": "yX4Ve7Q8Y4jscV4LZJD8HVCHKyePzR3MhA", + "pubKeyOperator": "8ed3f0c208efbcfc815cbfb94490dc68cf2e29d44dd9f8a91e20e06057aa110d7062c8ab7ccc85a9ff0c88760157f563", + "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5", + "addresses": { + "platform_p2p": ["192.0.2.2:70000"] + } + }"#; + let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); + assert_eq!(state.platform_p2p_address(), None, "out-of-range port rejected"); + } + #[test] fn dmn_state_no_ports_resolve_to_none() { // No addresses and no legacy ports -> accessors return None. @@ -3648,8 +3747,83 @@ mod tests { "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5" }"#; let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); - assert_eq!(state.resolved_platform_p2p_port(), None, "no source -> None"); - assert_eq!(state.resolved_platform_http_port(), None, "no source -> None"); + assert_eq!(state.platform_p2p_address(), None, "no source -> None"); + assert_eq!(state.platform_http_address(), None, "no source -> None"); + } + + fn dmn_state_with_legacy_p2p_zero() -> DMNState { + let json = r#"{ + "service": "192.0.2.1:9999", + "registeredHeight": 123456, + "revocationReason": 0, + "ownerAddress": "yPBWCdMRY5PsS3hJzs7csbdWQVRR85yxUz", + "votingAddress": "ySM11LUD65Bi4p1gm68XLkdWc65TBKRzvQ", + "payoutAddress": "yX4Ve7Q8Y4jscV4LZJD8HVCHKyePzR3MhA", + "pubKeyOperator": "8ed3f0c208efbcfc815cbfb94490dc68cf2e29d44dd9f8a91e20e06057aa110d7062c8ab7ccc85a9ff0c88760157f563", + "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5", + "platformP2PPort": 0 + }"#; + serde_json::from_str(json).expect("expected to deserialize json") + } + + #[test] + fn dmn_state_apply_diff_propagates_addresses() { + // Stored entry has a zero legacy port and no addresses; a diff carrying a + // nested `addresses` object must make the merged state resolvable. + let mut state = dmn_state_with_legacy_p2p_zero(); + assert_eq!(state.platform_p2p_address(), None, "unresolvable before diff"); + + let diff = DMNStateDiff { + service: None, + registered_height: None, + last_paid_height: None, + consecutive_payments: None, + pose_penalty: None, + pose_revived_height: None, + pose_ban_height: None, + revocation_reason: None, + owner_address: None, + voting_address: None, + payout_address: None, + pub_key_operator: None, + operator_payout_address: None, + platform_node_id: None, + legacy_platform_p2p_port: None, + legacy_platform_http_port: None, + addresses: Some(Some(MasternodeAddresses { + core_p2p: vec![], + platform_p2p: vec!["192.0.2.2:36656".to_string()], + platform_https: vec![], + })), + }; + + state.apply_diff(diff); + assert_eq!( + state.platform_p2p_address(), + Some(("192.0.2.2".to_string(), 36656)), + "diff addresses propagated and resolvable" + ); + } + + #[test] + fn dmn_state_diff_clears_addresses() { + // A Some -> None transition must survive the compare/apply round-trip: the + // diff carries `Some(None)` and applying it clears the stored addresses. + let mut newer = dmn_state_with_legacy_p2p_zero(); + newer.addresses = Some(MasternodeAddresses { + core_p2p: vec![], + platform_p2p: vec!["192.0.2.2:36656".to_string()], + platform_https: vec![], + }); + let older = dmn_state_with_legacy_p2p_zero(); + + let diff = + newer.compare_to_newer_dmn_state(&older).expect("addresses change yields a diff"); + assert_eq!(diff.addresses, Some(None), "clear is encoded as Some(None)"); + + let mut applied = newer; + applied.apply_diff(diff); + assert!(applied.addresses.is_none(), "Some(None) diff clears stored addresses"); } #[test] From c42768483cf5e6e0361d385b792f26318f8dfd86 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 12 Jun 2026 13:28:55 +0200 Subject: [PATCH 4/6] chore: add deprecated annotation to legacy fields --- rpc-json/src/lib.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/rpc-json/src/lib.rs b/rpc-json/src/lib.rs index 7003d9418..5cb2b0fb9 100644 --- a/rpc-json/src/lib.rs +++ b/rpc-json/src/lib.rs @@ -2178,7 +2178,9 @@ pub struct DMNStateDiff { pub pub_key_operator: Option>, pub operator_payout_address: Option>, pub platform_node_id: Option<[u8; 20]>, + #[deprecated(note = "Core 23+ nested addresses.platform_p2p should be used instead")] pub legacy_platform_p2p_port: Option, + #[deprecated(note = "Core 23+ nested addresses.platform_https should be used instead")] pub legacy_platform_http_port: Option, /// Three-state nested addresses: `None` = unchanged, `Some(None)` = cleared, /// `Some(Some(_))` = set. Mirrors [`pose_ban_height`](Self::pose_ban_height). @@ -2267,7 +2269,9 @@ impl TryFrom for DMNStateDiff { pub_key_operator, operator_payout_address, platform_node_id, + #[allow(deprecated)] legacy_platform_p2p_port, + #[allow(deprecated)] legacy_platform_http_port, addresses, }) @@ -2380,6 +2384,7 @@ impl DMNState { } else { None }, + #[allow(deprecated)] legacy_platform_p2p_port: if self.legacy_platform_p2p_port != newer.legacy_platform_p2p_port { @@ -2388,6 +2393,7 @@ impl DMNState { } else { None }, + #[allow(deprecated)] legacy_platform_http_port: if self.legacy_platform_http_port != newer.legacy_platform_http_port { @@ -2422,7 +2428,9 @@ impl DMNState { pub_key_operator, operator_payout_address, platform_node_id, + #[allow(deprecated)] legacy_platform_p2p_port, + #[allow(deprecated)] legacy_platform_http_port, addresses, .. @@ -3611,6 +3619,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn dmn_state_diff_core23_addresses_resolve_platform_ports() { // updatedMNs entry carrying only the new `addresses` object. let json = r#"{ @@ -3788,7 +3797,9 @@ mod tests { pub_key_operator: None, operator_payout_address: None, platform_node_id: None, + #[allow(deprecated)] legacy_platform_p2p_port: None, + #[allow(deprecated)] legacy_platform_http_port: None, addresses: Some(Some(MasternodeAddresses { core_p2p: vec![], From b4d77bd95fdff0683550c2a533ff87c90465c56b Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 12 Jun 2026 15:28:12 +0200 Subject: [PATCH 5/6] fix(rpc-json): tighten legacy platform-port resolution and deprecate DMNState fields Address a fresh review pass on the Core 23 platform-address work. - Mirror the `#[deprecated]` annotation onto DMNState::legacy_platform_p2p_port / legacy_platform_http_port so both DMNState and DMNStateDiff signal the same deprecation; add `#[allow(deprecated)]` to the two accessor methods, the apply_diff write sites, and the tests that read the fields to keep `clippy -D warnings` green. - legacy_platform_address now validates through `u16::try_from`, so an out-of-range legacy port (e.g. platformP2PPort: 70000) yields None instead of Some((ip, 70000)), matching the addresses path and the accessor's documented in-range contract. - parse_host_port rejects unbracketed multi-colon (bare IPv6) hosts rather than silently mangling them via rsplit_once; bracketed IPv6 and IPv4 still parse. The doc now states the supported forms accurately. Tests: wire-level `addresses: null` -> Some(None) (and absent -> None) round trip, out-of-range legacy port rejection, and parse_host_port IPv6 handling. Co-Authored-By: Claude Opus 4.8 (1M context) --- rpc-json/src/lib.rs | 80 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 72 insertions(+), 8 deletions(-) diff --git a/rpc-json/src/lib.rs b/rpc-json/src/lib.rs index 5cb2b0fb9..9885c7ff7 100644 --- a/rpc-json/src/lib.rs +++ b/rpc-json/src/lib.rs @@ -2079,13 +2079,17 @@ impl MasternodeAddresses { /// Splits a `"host:port"` string into `(host, port)` with a non-fabricated host. /// -/// Uses [`str::rsplit_once`] so only the final `:port` segment is parsed, which -/// keeps IPv4 and unbracketed IPv6 hosts intact. The port is parsed through -/// [`u16`] then widened to `u32`, rejecting values outside the TCP/UDP port range -/// (e.g. `"host:70000"`). Returns `None` when no colon is present or the suffix is -/// not a valid `u16`. +/// Supports IPv4 (`1.2.3.4:9999`) and bracketed IPv6 (`[2001:db8::1]:9999`). The +/// final `:port` segment is parsed through [`u16`] then widened to `u32`, rejecting +/// values outside the TCP/UDP port range (e.g. `"host:70000"`). Unbracketed +/// multi-colon hosts (bare IPv6) are rejected rather than silently mangled. Returns +/// `None` when no colon is present, the host is ambiguous, or the suffix is not a +/// valid `u16`. fn parse_host_port(addr: &str) -> Option<(String, u32)> { let (host, port) = addr.rsplit_once(':')?; + if host.contains(':') && !(host.starts_with('[') && host.ends_with(']')) { + return None; + } let port = port.parse::().ok()?; Some((host.to_string(), u32::from(port))) } @@ -2118,8 +2122,10 @@ pub struct DMNState { rename = "platformNodeID" )] pub platform_node_id: Option<[u8; 20]>, + #[deprecated(note = "Core 23+ nested addresses.platform_p2p should be used instead")] #[serde(default, rename = "platformP2PPort")] pub legacy_platform_p2p_port: Option, + #[deprecated(note = "Core 23+ nested addresses.platform_https should be used instead")] #[serde(default, rename = "platformHTTPPort")] pub legacy_platform_http_port: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -2134,6 +2140,7 @@ impl DMNState { /// `platformP2PPort`, pairing it with the node IP from [`service`](Self::service) /// because Dash deploys platform services on the masternode's core IP. Returns /// `None` when no source yields a non-zero in-range port. + #[allow(deprecated)] pub fn platform_p2p_address(&self) -> Option<(String, u32)> { self.addresses .as_ref() @@ -2148,6 +2155,7 @@ impl DMNState { /// `platformHTTPPort`, pairing it with the node IP from [`service`](Self::service) /// because Dash deploys platform services on the masternode's core IP. Returns /// `None` when no source yields a non-zero in-range port. + #[allow(deprecated)] pub fn platform_http_address(&self) -> Option<(String, u32)> { self.addresses .as_ref() @@ -2155,9 +2163,12 @@ impl DMNState { .or_else(|| self.legacy_platform_address(self.legacy_platform_http_port)) } - /// Pairs a legacy platform port with the node IP, dropping zero/absent ports. + /// Pairs a legacy platform port with the node IP, dropping zero, absent, and + /// out-of-`u16`-range ports so the result honors the TCP/UDP port range. fn legacy_platform_address(&self, port: Option) -> Option<(String, u32)> { - port.filter(|&p| p != 0).map(|p| (self.service.ip().to_string(), p)) + port.and_then(|p| u16::try_from(p).ok()) + .filter(|&p| p != 0) + .map(|p| (self.service.ip().to_string(), u32::from(p))) } } @@ -2465,10 +2476,12 @@ impl DMNState { self.platform_node_id = Some(platform_node_id); } + #[allow(deprecated)] if let Some(legacy_platform_p2p_port) = legacy_platform_p2p_port { self.legacy_platform_p2p_port = Some(legacy_platform_p2p_port); } + #[allow(deprecated)] if let Some(legacy_platform_http_port) = legacy_platform_http_port { self.legacy_platform_http_port = Some(legacy_platform_http_port); } @@ -3407,7 +3420,7 @@ mod tests { use crate::{ DMNState, DMNStateDiff, ExtendedQuorumListResult, MasternodeAddresses, MasternodeListDiff, - MnSyncStatus, QuorumType, deserialize_u32_opt, + MnSyncStatus, QuorumType, deserialize_u32_opt, parse_host_port, }; #[test] @@ -3585,6 +3598,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn dmn_state_core23_addresses_resolve_platform_ports() { // Core 23 entry: legacy platformP2PPort/platformHTTPPort absent, ports live // in the nested `addresses` object. Raw fields stay None; accessors resolve. @@ -3677,6 +3691,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn dmn_state_zero_legacy_port_resolves_to_addresses() { // Transitional entry: legacy port present but zero -> addresses wins (new-first). let json = r#"{ @@ -3703,6 +3718,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn dmn_state_zero_legacy_port_no_addresses_resolves_to_none() { // Legacy port present but zero and no `addresses` -> accessor returns None, // never `(host, 0)`. @@ -3742,6 +3758,25 @@ mod tests { assert_eq!(state.platform_p2p_address(), None, "out-of-range port rejected"); } + #[test] + fn dmn_state_legacy_out_of_range_port_rejected() { + // A legacy port above the u16 range must be rejected, matching the addresses + // path, so the accessor honors its documented in-range invariant. + let json = r#"{ + "service": "192.0.2.1:9999", + "registeredHeight": 123456, + "revocationReason": 0, + "ownerAddress": "yPBWCdMRY5PsS3hJzs7csbdWQVRR85yxUz", + "votingAddress": "ySM11LUD65Bi4p1gm68XLkdWc65TBKRzvQ", + "payoutAddress": "yX4Ve7Q8Y4jscV4LZJD8HVCHKyePzR3MhA", + "pubKeyOperator": "8ed3f0c208efbcfc815cbfb94490dc68cf2e29d44dd9f8a91e20e06057aa110d7062c8ab7ccc85a9ff0c88760157f563", + "platformNodeID": "f2dbd9b0a1f541a7c44d34a58674d0262f5feca5", + "platformP2PPort": 70000 + }"#; + let state: DMNState = serde_json::from_str(json).expect("expected to deserialize json"); + assert_eq!(state.platform_p2p_address(), None, "out-of-range legacy port rejected"); + } + #[test] fn dmn_state_no_ports_resolve_to_none() { // No addresses and no legacy ports -> accessors return None. @@ -3837,6 +3872,35 @@ mod tests { assert!(applied.addresses.is_none(), "Some(None) diff clears stored addresses"); } + #[test] + fn dmn_state_diff_addresses_null_wire_clears() { + // Wire-level three-state: `null` -> Some(None) (clear), absent -> None + // (unchanged). Exercises `deserialize_addresses_2opt` through the intermediate. + let diff: DMNStateDiff = + serde_json::from_str(r#"{"addresses": null}"#).expect("expected to deserialize json"); + assert_eq!(diff.addresses, Some(None), "null wire -> Some(None) (clear)"); + + let diff: DMNStateDiff = + serde_json::from_str(r#"{}"#).expect("expected to deserialize json"); + assert_eq!(diff.addresses, None, "absent wire -> None (unchanged)"); + } + + #[test] + fn parse_host_port_ipv6() { + // Bracketed IPv6 keeps host intact; unbracketed (ambiguous) is rejected. + assert_eq!( + parse_host_port("[2001:db8::1]:9999"), + Some(("[2001:db8::1]".to_string(), 9999)), + "bracketed IPv6 parses host + port" + ); + assert_eq!(parse_host_port("2001:db8::1"), None, "unbracketed IPv6 rejected"); + assert_eq!( + parse_host_port("192.0.2.1:9999"), + Some(("192.0.2.1".to_string(), 9999)), + "IPv4 still parses" + ); + } + #[test] fn deserialize_mnsync_status() { let json_value = json!({ From 01ddd0ef849544cd22df5fad503f25c9b3739cee Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:27:35 +0200 Subject: [PATCH 6/6] fix(rpc-json): reject empty host in parse_host_port; deprecate intermediate legacy ports Addresses CodeRabbit review on PR #808: - parse_host_port now rejects empty hosts (":36656" -> None) - DMNStateDiffIntermediate legacy_platform_*_port mirror the #[deprecated] of DMNState/DMNStateDiff Co-Authored-By: Claude Opus 4.8 (1M context) --- rpc-json/src/lib.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/rpc-json/src/lib.rs b/rpc-json/src/lib.rs index 9885c7ff7..5d4e7015e 100644 --- a/rpc-json/src/lib.rs +++ b/rpc-json/src/lib.rs @@ -2082,11 +2082,14 @@ impl MasternodeAddresses { /// Supports IPv4 (`1.2.3.4:9999`) and bracketed IPv6 (`[2001:db8::1]:9999`). The /// final `:port` segment is parsed through [`u16`] then widened to `u32`, rejecting /// values outside the TCP/UDP port range (e.g. `"host:70000"`). Unbracketed -/// multi-colon hosts (bare IPv6) are rejected rather than silently mangled. Returns -/// `None` when no colon is present, the host is ambiguous, or the suffix is not a -/// valid `u16`. +/// multi-colon hosts (bare IPv6) are rejected rather than silently mangled. An empty +/// host (e.g. `":36656"`) is also rejected. Returns `None` when no colon is present, +/// the host is empty or ambiguous, or the suffix is not a valid `u16`. fn parse_host_port(addr: &str) -> Option<(String, u32)> { let (host, port) = addr.rsplit_once(':')?; + if host.is_empty() { + return None; + } if host.contains(':') && !(host.starts_with('[') && host.ends_with(']')) { return None; } @@ -2201,6 +2204,7 @@ pub struct DMNStateDiff { impl TryFrom for DMNStateDiff { type Error = encode::Error; + #[allow(deprecated)] fn try_from(value: DMNStateDiffIntermediate) -> Result { let DMNStateDiffIntermediate { service, @@ -3029,8 +3033,10 @@ pub struct DMNStateDiffIntermediate { pub voting_address: Option, #[serde(default, rename = "platformNodeID")] pub platform_node_id: Option, + #[deprecated(note = "Core 23+ nested addresses.platform_p2p should be used instead")] #[serde(default, rename = "platformP2PPort")] pub legacy_platform_p2p_port: Option, + #[deprecated(note = "Core 23+ nested addresses.platform_https should be used instead")] #[serde(default, rename = "platformHTTPPort")] pub legacy_platform_http_port: Option, #[serde(default)] @@ -3899,6 +3905,9 @@ mod tests { Some(("192.0.2.1".to_string(), 9999)), "IPv4 still parses" ); + // Empty host must be rejected. + assert_eq!(parse_host_port(":36656"), None, "empty host rejected"); + assert_eq!(parse_host_port(":443"), None, "empty host rejected"); } #[test]