Repository navigation
revert(rpc-json)!: resolve Core 23 nested platform addresses (breaking field/accessor renames) - #814
Merged
Conversation
…eaking f…" This reverts commit 0b056c2.
Contributor
|
Warning Review limit reached
More reviews will be available in 58 minutes and 36 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
lklimek
approved these changes
Jun 19, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #814 +/- ##
==========================================
- Coverage 73.38% 72.81% -0.57%
==========================================
Files 323 323
Lines 72294 72000 -294
==========================================
- Hits 53052 52428 -624
- Misses 19242 19572 +330
|
4 of 5 tasks
ZocoLini
pushed a commit
that referenced
this pull request
Oct 3, 2026
* fix(rpc-json)!: re-land nested platform addresses for Core v24 Re-applies #808, which #814 reverted (this reverts commit ceee4a9). Masternode entries and diffs parse the nested `addresses` object (`MasternodeAddresses { core_p2p, platform_p2p, platform_https }`), the top-level platform ports become the deprecated `legacy_platform_{p2p,http}_port`, and `platform_{p2p,http}_address()` resolve the nested entry first and fall back to the legacy port paired with the service IP. Dash Core v24 reports an ExtAddr Evo's platform ports only in `addresses` (the scalar fields stay 0), so a port-only ProUpServTx is visible nowhere else. BREAKING CHANGE: DMNState/DMNStateDiff `platform_{p2p,http}_port` are renamed to `legacy_platform_{p2p,http}_port` and deprecated; both types gain an `addresses` field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(rpc-json): read Core v24 non-IP service and -1 platform ports Dash Core v24 prints `service` as the primary core P2P entry, which may be a Tor or I2P address, and prints both platform ports as -1 for an extended-address Evo with no addresses. A Tor/I2P service failed the whole `protx listdiff`, `masternodelist json`, `masternode status` or `protx diff` response; a -1 port failed `protx listdiff`, `masternodelist json` and `masternode status` (`protx diff` has no platform P2P port and its entries are not parsed for ports). - `service` (DMNState, DMNStateDiff, MasternodeStatus, QuorumMasternodeListItem) and `address` (Masternode) whose host is a Tor (`.onion`) or I2P (`.i2p`) address reads as `[::]:0`, the value Core prints for a masternode without an address. IP services, CJDNS included, are kept; any other value that is not an `ip:port` is still an error. The fields stay `SocketAddr`; where the type has an `addresses` object the entry remains readable there. Serialization is unchanged. - `platformP2PPort` / `platformHTTPPort` (DMNState, DMNStateDiff, Masternode) parse as signed: a negative value or `null` reads as `None`, a value above `u32::MAX` is an error instead of wrapping. Red -> green: the new tests fail on the parent commit with `invalid socket address syntax` (Tor/I2P service in DMNState, diff, MasternodeStatus, QuorumMasternodeListItem) and `invalid value: integer `-1`, expected u32` (DMNState, diff, masternodelist entry). With a catch-all fallback, a malformed service such as "192.0.2.1" silently read as `[::]:0`, a port of 2^32 + 26656 wrapped to 26656 and a `null` port failed with `invalid type: null, expected i64`; the tests pinning those fail against that version and pass after. The port-only extended-address diff test is a guard and already passes with the nested addresses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(rpc-json)!: model Core v24 shared and multi-payout masternodes Dash Core v24 omits `ownerAddress`, `payoutAddress` and `collateralAddress` for a shared masternode (it prints `shares` instead), and prints a `payouts` list instead of `payoutAddress` for every extended-address (version 3) masternode, including a legacy one raised to version 3. The required fields failed the whole listdiff. - DMNState.owner_address and payout_address become Option<[u8; 20]>. MasternodeListItem.collateral_address and ProTxInfo.collateral_address become Option<[u8; 20]>; ProTxInfo previously held the address string bytes and now holds the decoded hash, like MasternodeListItem. They are also None when Core runs without -txindex. - New DMNPayout { address, script, reward } and `payouts: Option<Vec<DMNPayout>>` on DMNState and DMNStateDiff. `address` is the key or script hash; `script` (P2PKH or P2SH) tells them apart. `shares`, `earlyPeriodBlocks`, `earlyPenalty` and `version` are not modelled. - apply_diff: a diff's `payouts` replaces `payout_address` and vice versa, so at most one of them is set, matching Core's output. compare_to_newer_dmn_state emits `payouts` changes and documents the Some -> None transitions a diff cannot express. Red -> green: on the parent commit the rc.1 fixtures fail with `missing field `ownerAddress`` (shared masternode in the listdiff and in protx info) and `missing field `payoutAddress`` (extended-address state in the listdiff and in masternode status), and the assertions on the new API (including DMNPayout.script) do not compile; all rpc-json tests pass after. The v23 `deserialize_mn_listdiff` fixture is unchanged and still passes. BREAKING CHANGE: DMNState.owner_address/payout_address, MasternodeListItem.collateral_address and ProTxInfo.collateral_address are now Option<[u8; 20]>; DMNState and DMNStateDiff gain a `payouts` field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(rpc-json): apply Core's partial addresses diffs per purpose Core's diff emitter (CDeterministicMNStateDiff::ToJson, unchanged since v23.0.0) prints `addresses` per changed field, not as a whole, but apply_diff replaced the stored object with it: - a changed core address prints `service` and `core_p2p` only, which dropped a legacy Evo's platform entries that the full state renders on the primary address (GetNetInfoWithLegacyFields); - a legacy Evo's port-only change prints `255.255.255.255:<port>`, which became the stored platform host; - emptied addresses (revocation, operator change) print `service` and no `addresses`, so the old addresses stayed. The resulting state depended on how Core batched changes into diffs. apply_diff now merges per purpose: a changed core address moves a legacy Evo's platform entries onto it, the placeholder host resolves to the primary core address (dropped when that is unknown, leaving the flat port the diff also prints), no core address means no platform entries, and `service` without `addresses` empties them. A state without nested addresses (older Core, or rebuilt from stored ports) keeps resolving through the legacy fallback. The platform address accessors fall back to the flat legacy port as-is, zero included, whenever the nested object has no valid entry for the purpose, `addresses: {}` included: that is how Core 23 and 24 print a revoked legacy Evo, and consumers that read the flat ports keep it. Flat ports keep what Core's diff prints, so an extended-address Evo's revocation leaves them at their last value. Which value that is can still depend on how Core batched the changes before the reset, since the diff doesn't say whether the Evo was extended-address when it was reset; a legacy Evo's resolved addresses no longer depend on batching. Nulling the flat ports on a reset would instead drop a revoked legacy Evo that consumers reading the flat ports (Dash Platform's validator sets) keep, so they are left as printed. Red -> green: on the parent commit, applying Core-shaped diffs to Core's full state of the same Evo diverges from the full state Core prints next. The core-address diff leaves `addresses` as {core_p2p} only. The port-only diff resolves the platform P2P host to "255.255.255.255". The two batchings give different states. Both revocation diffs leave the old addresses in place, and a zero legacy port resolves to None. All pass after. The reloaded-state test is a guard and passes before and after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(rpc-json): bracket an IPv6 host in the legacy platform address fallback The platform address accessors return the nested entry's host, which Core prints bracketed for IPv6 (`[2001:db8::4]`), or fall back to the legacy port paired with `service.ip()`, which formatted IPv6 without brackets (`2001:db8::4`). A consumer joining host and port then got an ambiguous `2001:db8::4:26656` that does not parse as a socket address, depending only on which source answered. The fallback now brackets an IPv6 node IP; IPv4 is unchanged. Test would have caught this: legacy_fallback_brackets_an_ipv6_host_like_the_nested_path fails before the fix (host "2001:db8::4"), and the revoked legacy Evo assertions expected "::"; all pass after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(rpc-json): keep the flat platform port fields without deprecation The flat `platformP2PPort` / `platformHTTPPort` fields are not on their way out yet: a legacy evonode first listed with an empty network address, and revived later with unchanged ports, has its platform ports nowhere else in Core's JSON. Consumers that must agree with readers of these fields (Dash Platform's validator sets) still fall back to them, so `#[deprecated]` only forced `#[allow(deprecated)]` onto every legitimate read. Drop the attribute and document the fields as Core's flat fields, printed with `-deprecatedrpc=service`, behind the `platform_{p2p,http}_address()` accessors that read `addresses` first. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * revert(rpc-json): keep the flat platform port fields deprecated This reverts commit 02554de. The flat platformP2PPort / platformHTTPPort fields are on their way out: consumers should read the nested `addresses` through the platform_{p2p,http}_address() accessors, and the flat fields remain only as a fallback until Core prints every evonode's platform ports in `addresses` and the -deprecatedrpc=service flag is dropped. The deprecation says exactly that; the few remaining reads carry `#[allow(deprecated)]`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #808